[U-Boot] [PATCH 2/2] watchdog: designware: Convert to DM and DT probing

[Marek Vasut]

On 10/3/19 3:57 AM, Ley Foon Tan wrote:

[...]

>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 6fd9b0a177..ec34993664 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -36,13 +36,6 @@ config ULP_WATCHDOG
>>         help
>>           Say Y here to enable i.MX7ULP watchdog driver.
>>
>> -config DESIGNWARE_WATCHDOG
>> -       bool "Designware watchdog timer support"
>> -       select HW_WATCHDOG
> CONFIG_HW_WATCHDOG is disabled now. Few areas of code in
> arm/mach-socfpga/ still using this CONFIG and call to
> hw_watchdog_init(). They need to remove too.
> Do we need call to uclass_get_device(UCLASS_WDT, 0, &dev) in SPL to
> probe watchdog and call to wdt_start() to start watchdog? Can't find
> place that start watchdog.

You're right, and the WDT is enabled early on, before the DM structures
are available.

I wonder whether we should do what we did on iMX -- have a non-DM WDT
driver for SPL and DM-one for U-Boot proper.

[...]

>> -void hw_watchdog_init(void)
>> +static int designware_wdt_stop(struct udevice *dev)
>>  {
>>         /* reset to disable the watchdog */
>> -       hw_watchdog_reset();
>> +       designware_wdt_reset(dev);
> Need to clear BIT(DW_WDT_CR_EN_OFFSET) in CR register to disable watchdog.

Fixed

>> +       return 0;
>> +}
>> +
>> +static int designware_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>> +{
>> +       struct designware_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +       designware_wdt_stop(dev);
>> +
>>         /* set timer in miliseconds */
>> -       designware_wdt_settimeout(CONFIG_WATCHDOG_TIMEOUT_MSECS);
>> -       /* enable the watchdog */
>> -       designware_wdt_enable();
>> +       designware_wdt_settimeout(dev, timeout);
>> +
>> +       writel((DW_WDT_CR_RMOD_VAL << DW_WDT_CR_RMOD_OFFSET) |
>> +             BIT(DW_WDT_CR_EN_OFFSET),
>> +             priv->base + DW_WDT_CR);
>> +
>>         /* reset the watchdog */
>> -       hw_watchdog_reset();
>> +       designware_wdt_reset(dev);
> Need move to before enable CR_EN bit if we add clear CR_EN bit in
> designware_wdt_reset().

I think if someone needs to clear CR_EN, they should call
designware_wdt_stop(), which should clear the CR_EN.

>> +       return 0;
>> +}
>> +
>> +static int designware_wdt_probe(struct udevice *dev)
>> +{
> Need to de-assert reset for watchdog in probe using reset framework.
>> +       /* reset to disable the watchdog */
>> +       designware_wdt_reset(dev);
> designware_wdt_reset() only reset watchdog counter, but doesn't
> disable the watchdog.
> Can change call to designware_wdt_stop() if _stop() add clear CR_EN bit.

Yep, fixed, thanks.

[...]

-- 
Best regards,
Marek Vasut