[U-Boot] [PATCH] watchdog: Implement generic watchdog_reset() version

[Michal Simek]

On 05. 04. 19 15:24, Stefan Roese wrote:
> On 05.04.19 15:00, Stefan Roese wrote:
> 
> <snip>
> 
>>>> +        debug("Only one watchdog device used!\n");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * Init watchdog: This will call the probe function of the
>>>> +     * watchdog driver, enabling the use of the device
>>>> +     */
>>>> +    if (uclass_get_device(UCLASS_WDT, 0,
>>>> +                  (struct udevice **)&gd->watchdog_dev)) {
>>>> +        debug("Watchdog: Not found!\n");
>>>> +        return;
>>>> +    }
>>>
>>> I think you should c&p what I have done in zynq/zynqmp. It means check
>>> watchdog0 alias first because this is supposed to be the first primary
>>> watchdog.
>>>
>>> zynq/zynqmp/mb code
>>>          if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) {
>>>                  debug("Watchdog: Not found by seq!\n");
>>>                  if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) {
>>>                          puts("Watchdog: Not found!\n");
>>>                          return 0;
>>>                  }
>>>          }
>>
>> In my (internal) first version, I had exactly this code included. I
>> removed the first part, as it makes no real sense to call it at this
>> stage (wdt_post_bind() for the first WDT found in the system). My
>> understanding is, that wdt_post_bind() will get called for all WDT
>> found in the system. My current logic here is, to only register the
>> first WDT found to the global watchdog_dev in GD. To prevent that
>> multiple WDT are used to service the U-Boot watchdog via WATCHDOG_RESET.
>>
>> Or does the DM subsystem automatically bind the watchdog0 alias first
>> in its binding sequence?
>>
>> I do not have a system with multiple watchdogs here, so its not that
>> easy to test this. I could simulate this though somehow...
> 
> I just did some testing on this. One solution might be, to only use
> uclass_get_device_by_seq() (instead of uclass_get_device) and return
> if the alias is not found. This way, all watchdog devices will get
> probed (bound) and only the one referenced by the alias will get used.
> 
> This means on the other hand, that an watchdog0 alias is needed in the
> DT to enable the WATCHDOG_RESET U-Boot servicing, which might be not
> backward compatible.
> 
> Do you have any other thought on this?

As you see above this is what I used and strictly speaking using
different logic will break compatibility for xilinx platforms.

I forget details about logic seq/req_seq but maybe watchdog core should
adopt dev_read_alias_highest_id() which I added for i2c some time ago.

For example if you have two watchdogs and only one alias.
aliases {
	watchdog1 = &wdt1;
}

wdt2: {};

wdt1: {};

Which one should be used by u-boot?
wdt1 because it has alias?
or wdt2 because it is listed first?
or none because there is no watchdog0 alias?

Maybe even solution with taking first alias is wrong and xlnx,watchdog
property in chosen node is a reasonable solution.

Thanks,
Michal