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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux