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

[Simon Goldschmidt]

Marek Vasut <marex at denx.de> schrieb am Do., 3. Okt. 2019, 09:22:

> 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.
>

Are you aware that DM WDT is enabled in SPL by default for gen5 now? I get
a message that WDT is not found. I haven't sent a patch to fix that yet,
since the message is the only thing that happens, works normally otherwise.

But if you're working on that, you might check side effects of that setting
(which is new for this release).

Regards,
Simon


> [...]
>
> >> -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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>