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

[Marek Vasut]

On 10/4/19 11:26 AM, Ley Foon Tan wrote:
> On Thu, Oct 3, 2019 at 9:00 PM Marek Vasut wrote:
>>
>> Convert the designware watchdog timer driver to DM and add DT probing
>> support. Perform minor coding style clean up, like drop superfluous
>> braces. There ought to be no functional change.
> 
> All watchdog DT nodes with compatible "snps,dw-wdt" need to add
> "u-boot,dm-pre-reloc;".

Only those boards which actually use the WDT, right ? That is, these two
boards below.

>> ---
>> V2: - Support both DM and non-DM probing
>>     - Fix watchdog stop handling by setting CR bit
>> ---
>>  configs/socfpga_stratix10_defconfig       |   2 +
>>  configs/socfpga_vining_fpga_defconfig     |   1 +
> 
> Same here. All socfpga defconfig files need to add CONFIG_WDT.

Only those two, since only those two boards enable the WDT, no ?

[...]

>>  void hw_watchdog_init(void)
>> @@ -64,10 +70,80 @@ void hw_watchdog_init(void)
>>         /* reset to disable the watchdog */
>>         hw_watchdog_reset();
>>         /* set timer in miliseconds */
>> -       designware_wdt_settimeout(CONFIG_WATCHDOG_TIMEOUT_MSECS);
>> +       designware_wdt_settimeout((void __iomem *)CONFIG_DW_WDT_BASE,
>> +                                 CONFIG_DW_WDT_CLOCK_KHZ,
>> +                                 CONFIG_WATCHDOG_TIMEOUT_MSECS);
>>         /* enable the watchdog */
>> -       designware_wdt_enable();
>> +       designware_wdt_enable((void __iomem *)CONFIG_DW_WDT_BASE);
>>         /* reset the watchdog */
>>         hw_watchdog_reset();
> 
> In my "arm: socfpga: Convert drivers from struct to defines" patch
> series, I have moved spl_early_init() to the beginning of
> spl_board_f().
> So, DM framework is initialized in early stage, you should able to use
> DM for watchdog in SPL too.
> But, maybe need to add a wrapper function to probe watchdog device and
> start watchdog. Something like this:
> 
> uclass_get_device(UCLASS_WDT, 0, &dev);
> wdt_start(dev, CONFIG_WATCHDOG_TIMEOUT_MSECS, 0);

Except this driver is not used only on SoCFPGA.
So the question is, with your conversion series, will there be boards
left which use non-DM version of this WDT driver ?

>>  }
>> +#else
>> +static int designware_wdt_reset(struct udevice *dev)
>> +{
>> +       struct designware_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +       designware_wdt_reset_common(priv->base);
>> +
>> +       return 0;
>> +}
>> +
>> +static int designware_wdt_stop(struct udevice *dev)
>> +{
>> +       struct designware_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +       designware_wdt_reset(dev);
>> +       writel(DW_WDT_CR_RMOD_VAL << DW_WDT_CR_RMOD_OFFSET,
>> +               priv->base + DW_WDT_CR);
> 
> Still no fixing clear BIT(DW_WDT_CR_EN_OFFSET) in CR register to
> disable watchdog.

I guess we an just write 0 to the CR register and get rid of RMOD_VAL
altogether ?

>> +
>> +       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(priv->base, CONFIG_DW_WDT_CLOCK_KHZ, timeout);
>> +
>> +       designware_wdt_enable(priv->base);
>> +
>> +       /* reset the watchdog */
>> +       return designware_wdt_reset(dev);
>> +}
>> +
>> +static int designware_wdt_probe(struct udevice *dev)
>> +{
>> +       struct designware_wdt_priv *priv = dev_get_priv(dev);
>> +
> 
> Need de-assert watchdog reset using reset framework function, reset_get_bulk().

Ah yes, and make it presumably optional.

[...]