[U-Boot] [PATCH 1/4] ARM: dts: dh-imx6: add wdt-reboot node for sysreset driver

[Claudius Heine]

On 28/11/2019 14.55, Fabio Estevam wrote:
> On Thu, Nov 28, 2019 at 10:42 AM Claudius Heine <ch at denx.de> wrote:
>>
>> On 28/11/2019 14.18, Fabio Estevam wrote:
>>> Hi Claudius,
>>>
>>> On Thu, Nov 28, 2019 at 10:15 AM Claudius Heine <ch at denx.de> wrote:
>>>
>>>> That is the sysreset device node, not the imx2_wdt one. (I will move
>>>> that into a '*-u-boot.dtsi' in v2)
>>>>
>>>> Or am I misunderstanding you?
>>>
>>> What I am asking is: why do we need a specific sysreset node for U-Boot?
>>
>> Good question. But I don't know a better answer to that than just saying
>> that this is currently the way the reset is implemented in u-boot with
>> DM AFAIK.
>>
>>> Can't we just add the wdog node the same way we do in the kernel (from
>>> the imx2_wdt binding) and get it to work?
>>
>> Probably. But I currently don't know a way to do it that doesn't involve
>> implementing new code.
> 
> Could you please try passing the wdog node the same we do in the kernel?
> 
> I prefer we have a single way to deal with watchdog instead of having
> a specific U-Boot method.

Sorry, but we are probably misunderstanding each other here. So I try to
go step by step to show how I think about this. Maybe that way we figure
out where our understanding differs. Please bear with me, its a bit
verbose :)

My patch adds the 'wdt-reboot' device node which is needed by u-boot to
reset the device but doesn't exist in the linux kernel (since there is
no 'wdt-reboot' driver):

+       wdt-reboot {
+               compatible = "wdt-reboot";
+               wdt = <&wdog1>;
+       };

This 'wdt-reboot' node instruments its driver
(drivers/sysreset/sysreset_watchdog.c [1]) to use the watchdog node
(arch/arm/dts/imx6qdl.dtsi [2]) and its driver
(drivers/watchdog/imx_watchdog.c [3]).

Neither the watchdog node in imx6qdl.dtsi [2], that is nearly identical
to the one in the linux kernel [4], nor its driver [3] was touched by
the changes in this patchset. The thing that has changed however is that
'CONFIG_SYSRESET_WATCHDOG' was enabled, which in turn enabled the
'CONFIG_WDT' switch. 'CONFIG_WDT' in turn disabled the 'hw_watchdog*'
function definitions in the imx-watchdog driver and enabled
'CONFIG_WATCHDOG' and disabled 'CONFIG_HW_WATCHDOG'.

The 'CONFIG_WATCHDOG' and 'CONFIG_HW_WATCHDOG' changes cause the
'WATCHDOG_RESET*' defines in [5] to no longer refer to the
'hw_watchdog*' but instead to the 'watchdog*' functions. Those functions
are handled by the watchdog DM class driver [6].


[1]
https://gitlab.denx.de/u-boot/u-boot/blob/4b19b89ca4a866b7baa642533e6dbd67cd832d27/drivers/sysreset/sysreset_watchdog.c#L48
[2]
https://gitlab.denx.de/u-boot/u-boot/blob/4b19b89ca4a866b7baa642533e6dbd67cd832d27/arch/arm/dts/imx6qdl.dtsi#L659
[3]
https://gitlab.denx.de/u-boot/u-boot/blob/4b19b89ca4a866b7baa642533e6dbd67cd832d27/drivers/watchdog/imx_watchdog.c
[4]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/imx6qdl.dtsi?h=v5.4#n672
[5]
https://gitlab.denx.de/u-boot/u-boot/blob/4b19b89ca4a866b7baa642533e6dbd67cd832d27/include/watchdog.h#L38
[6]
https://gitlab.denx.de/u-boot/u-boot/blob/4b19b89ca4a866b7baa642533e6dbd67cd832d27/drivers/watchdog/wdt-uclass.c#L74