RE: R: [PATCH v1] watchdog: wdat: add param. to start wdog on module insertion

From: Flavio Suligoi
Date: Mon Feb 22 2021 - 06:29:36 EST


Hi Guenter

> >>> const struct wdat_instruction *instr, u32 *value) { @@ -437,6
> >>> +443,8 @@ static int wdat_wdt_probe(struct platform_device
> >> *pdev)
> >>> }
> >>>
> >>> wdat_wdt_boot_status(wdat);
> >>> + if (start_enabled)
> >>> + wdat_wdt_start(&wdat->wdd);
> >>
> >> No objections to this if it is really needed. However, I think it is
> >> better start the watchdog after devm_watchdog_register_device() has
> >> been called so we have everything initialized.
> >
> > Yes, it is needed. We need this feature to enable the watchdog as soon
> > as possible and this is essential for unmanned applications, such as
> > routers, water pumping stations, climate data collections, etc.
> >
> FWIW, in your use case the watchdog should be enabled in the
> BIOS/ROMMON.

Yes, you are right, with the new BIOS version for the new boards
we'll implement this features, but for the old boards it is no more possible.

>
> > Right, ok for the correct positioning of the wdat_wdt_start function
> > at the end of the watchdog device initialization. Thanks!
> >
>
> No, it isn't, because it won't set WDOG_HW_RUNNING, and the watchdog
> core won't know that the watchdog is running.

Ok

> The watchdog has to be started before the call to wdat_wdt_set_running().
> If that isn't possible with the current location of wdat_wdt_set_running(),
> then
> wdat_wdt_set_running() has to be moved accordingly.
> Either case, both have to be called before calling
> devm_watchdog_register_device().

Ok

>
> Having said that, I'd prefer to have a module parameter in the watchdog
> core. We already have a number of similar module parameters in various
> drivers, all named differently, and I'd rather not have more.

Ok, I'll study how to introduce a this new parameter in the wdog core,
so that it can be available for all watchdog drivers.
Then we'll have to think what to do with the existent similar parameters.
I think we have to keep them for compatibility reasons.

>
> Guenter
>

Regards,
Flavio