Re: [RFC PATCH 0/3] watchdog: arm_smc_wdt: allow a list of SMC function ids

[Guenter Roeck <linux@roeck-us.net>]

On 11/4/26 10:10, mateusz.nowicki@posteo.net wrote:
> On 11/4/26 Guenter Roeck wrote:
>> On 11/4/26 08:16, Mateusz Nowicki wrote:
>>> My earlier patch, commit ae4d08bbe3f4 ("arm64: dts: st: fix arm,smc-id
>>> of the SMC watchdog on stm32mp2"), switched the dtsi to 0xbc000000. That
>>> made the watchdog work with current OP-TEE, but it broke boards that run
>>> the older firmware. So today 6.18.y uses one id and 7.4 the other. When
>>> the kernel and the firmware disagree, nobody pets the watchdog that the
>>> bootloader started, and the board resets after 32 seconds. Marek hit
>>> this even with a recovery kernel, which is about the worst place for it.
>>>
>>
>> So the DT on the board is updated to use the new ID but the firmware itself
>> isn't ? That seems a bit odd; I would have assumed that the DT on a board
>> is kept in sync with its firmware. Is that not the case here ?
> 
> Not necessarily. On these boards the DTB is built from the kernel tree
> and installed with the kernel, U-Boot just loads it from the boot
> partition. TF-A and OP-TEE sit in the FIP and are updated separately, so
> updating the kernel alone changes the ID. That's how Marek got hit: he
> updated OP-TEE, but his recovery kernel still had the old DTB.
> 

If the DT does not really reflect the HW, is installed as part of the kernel
image, and the ID can change across FW versions, it seems to me that it does
not even make sense to store the ID in devicetree.

>>> The idea from that thread: let arm,smc-id be a list. The driver tries
>>> the ids one by one and keeps the first one the firmware accepts. Older
>>> kernels only read the first cell, so they behave exactly as they do
>>> now. For stm32mp2 the list is <0xbc000000 0xb200005a>. Calling an id
>>> that belongs to some other firmware service would be bad, so the
>>> binding says the list may only hold ids known to be this watchdog.
>>>
>>
>> Would it possibly make sense to check keep a list of known IDs in the
>> driver ? It already checks 0x82003D06, so it might as well check a sequence.
> 
> Fine with me, and it avoids the binding issue. I'd just rather not try
> the ST IDs on every platform, so how about making the list per
> compatible:
> 
>    compatible = "st,stm32mp25-smc-wdt", "arm,smc-wdt";
> 
> with { 0xbc000000, 0xb200005a } as match data for it. arm,smc-id stays a
> single u32 and, if present, is tried first. Older kernels just match
> "arm,smc-wdt" and behave as today.
> 
> If that works for you and the DT folks, I'll send v2.
> 
Works for me.

Thanks,
Guenter