| Message ID | 20210623024429.1346349-1-art@khadas.com |
|---|---|
| Headers | show |
| Series | watchdog: meson_gxbb_wdt: improve | expand |
On 6/22/21 7:44 PM, Artem Lapkin wrote: > Remove watchdog_stop_on_reboot (still can be activated by > watchdog.stop_on_reboot=1) i think this driver configuration more useful > becouse we can get reboot waranty for abnormal situations on shutdown stage > This is personal opinion. Driver authors would have to agree. Guenter > Signed-off-by: Artem Lapkin <art@khadas.com> > --- > drivers/watchdog/meson_gxbb_wdt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c > index 5a9ca10fbcfa..3f86530c33b0 100644 > --- a/drivers/watchdog/meson_gxbb_wdt.c > +++ b/drivers/watchdog/meson_gxbb_wdt.c > @@ -186,7 +186,6 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev) > > meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout); > > - watchdog_stop_on_reboot(&data->wdt_dev); > return devm_watchdog_register_device(dev, &data->wdt_dev); > } > >
On 6/22/21 7:44 PM, Artem Lapkin wrote: > Added nowayout module param same as in other moduels Please use complete sentences and words, check your spelling (modules), and refrain from irrelevant statements such as "same as in other moduels". "Add nowayout module parameter" is sufficient here. > > Signed-off-by: Artem Lapkin <art@khadas.com> > --- > drivers/watchdog/meson_gxbb_wdt.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c > index ecd1fc6f48ba..0bf5dccf70b1 100644 > --- a/drivers/watchdog/meson_gxbb_wdt.c > +++ b/drivers/watchdog/meson_gxbb_wdt.c > @@ -34,6 +34,11 @@ module_param(timeout, uint, 0); > MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds=" > __MODULE_STRING(DEFAULT_TIMEOUT) ")"); > > +static bool nowayout = WATCHDOG_NOWAYOUT; > +module_param(nowayout, bool, 0); > +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default=" '(' missing before 'default' > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > + > struct meson_gxbb_wdt { > void __iomem *reg_base; > struct watchdog_device wdt_dev; > @@ -190,6 +195,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev) > data->reg_base + GXBB_WDT_CTRL_REG); > > meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout); > + watchdog_set_nowayout(&data->wdt_dev, nowayout); > > return devm_watchdog_register_device(dev, &data->wdt_dev); > } >
On 6/22/21 7:44 PM, Artem Lapkin wrote: > Print watchdog success driver start status notification > Another personal opinion: The driver author decided that the message is not needed (which matches my personal opinion). I'd rather see all those messages either go away or moved to the core. Either case, this one is misleading: The watchdog is _not_ enabled. The watchdog driver is loaded, which is different. Guenter > Signed-off-by: Artem Lapkin <art@khadas.com> > --- > drivers/watchdog/meson_gxbb_wdt.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c > index 2dbe254e5122..750b304b460d 100644 > --- a/drivers/watchdog/meson_gxbb_wdt.c > +++ b/drivers/watchdog/meson_gxbb_wdt.c > @@ -198,7 +198,14 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev) > watchdog_set_nowayout(&data->wdt_dev, nowayout); > watchdog_stop_on_unregister(&data->wdt_dev); > > - return devm_watchdog_register_device(dev, &data->wdt_dev); > + ret = devm_watchdog_register_device(dev, &data->wdt_dev); > + if (ret) > + return ret; > + > + dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)", > + data->wdt_dev.timeout, nowayout); > + > + return ret; > } > > static struct platform_driver meson_gxbb_wdt_driver = { >