From: Flavio Suligoi <f.suligoi@asem.it>
To: Guenter Roeck <linux@roeck-us.net>,
Wim Van Sebroeck <wim@linux-watchdog.org>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3] watchdog: add new parameter to start the watchdog on module insertion
Date: Fri, 17 Sep 2021 15:21:44 +0000 [thread overview]
Message-ID: <DU2PR01MB8144F50D2FA05CDA18BC6E6AF9DD9@DU2PR01MB8144.eurprd01.prod.exchangelabs.com> (raw)
In-Reply-To: <36f5e976-e6d4-7e24-f971-0895ab6b1762@roeck-us.net>
Hi Guenter,
...
> > v3: - add check for the returned error of wdog start function
> > v2: - check WDOG_HW_RUNNING before starting watchdog;
> > - remove useless comments in commit text, watchdog-parameters.rst and
> > Kconfig;
> > v1: - first version;
> >
> > Documentation/watchdog/watchdog-parameters.rst | 3 +++
> > drivers/watchdog/Kconfig | 9 +++++++++
> > drivers/watchdog/watchdog_core.c | 16 ++++++++++++++++
> > 3 files changed, 28 insertions(+)
> >
> > diff --git a/Documentation/watchdog/watchdog-parameters.rst
> b/Documentation/watchdog/watchdog-parameters.rst
> > index 223c99361a30..7780d0c1fb4a 100644
> > --- a/Documentation/watchdog/watchdog-parameters.rst
> > +++ b/Documentation/watchdog/watchdog-parameters.rst
> > @@ -21,6 +21,9 @@ watchdog core:
> > timeout. Setting this to a non-zero value can be useful to ensure that
> > either userspace comes up properly, or the board gets reset and allows
> > fallback logic in the bootloader to try something else.
> > + start_enabled:
> > + Watchdog is started on module insertion. This option can be also
> > + selected by kernel config (default=kernel config parameter).
> >
> > -------------------------------------------------
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index b81fe4f7d434..19be709e01ac 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -47,6 +47,15 @@ config WATCHDOG_NOWAYOUT
> > get killed. If you say Y here, the watchdog cannot be stopped once
> > it has been started.
> >
> > +config WATCHDOG_START_ENABLED
> > + bool "Start watchdog on module insertion"
> > + help
> > + Say Y if you want to start the watchdog at the same time when the
> > + driver is loaded.
> > + This feature is very useful in embedded systems, to avoid cases where
> > + the system could hang before reaching userspace.
> > + This parameter applies to all watchdog drivers.
> > +
> > config WATCHDOG_HANDLE_BOOT_ENABLED
> > bool "Update boot-enabled watchdog until userspace takes over"
> > default y
> > diff --git a/drivers/watchdog/watchdog_core.c
> b/drivers/watchdog/watchdog_core.c
> > index 3fe8a7edc252..cb530b097223 100644
> > --- a/drivers/watchdog/watchdog_core.c
> > +++ b/drivers/watchdog/watchdog_core.c
> > @@ -44,6 +44,11 @@ static int stop_on_reboot = -1;
> > module_param(stop_on_reboot, int, 0444);
> > MODULE_PARM_DESC(stop_on_reboot, "Stop watchdogs on reboot
> (0=keep watching, 1=stop)");
> >
> > +static bool start_enabled =
> IS_ENABLED(CONFIG_WATCHDOG_START_ENABLED);
> > +module_param(start_enabled, bool, 0444);
> > +MODULE_PARM_DESC(start_enabled, "Start watchdog on module insertion
> (default="
> > +
> __MODULE_STRING(IS_ENABLED(CONFIG_WATCHDOG_START_ENABL
> ED)) ")");
> > +
> > /*
> > * Deferred Registration infrastructure.
> > *
> > @@ -252,6 +257,17 @@ static int __watchdog_register_device(struct
> watchdog_device *wdd)
> > * corrupted in a later stage then we expect a kernel panic!
> > */
> >
> > + /* If required, start the watchdog immediately */
> > + if (start_enabled && !watchdog_hw_running(wdd)) {
> > + set_bit(WDOG_HW_RUNNING, &wdd->status);
> > + ret = wdd->ops->start(wdd);
> > + if (ret == 0) {
> > + pr_info("Watchdog enabled\n");
> > + } else {
> > + return ret;
> > + }
>
> This should be
> if (ret)
> return ret;
> pr_info("Watchdog enabled\n");
>
Right, it is simpler!!! 😊
> Also, this needs an error handler to stop the watchdog if driver registration
> fails (if the driver supports stopping the watchdog).
Ok, I'll think how to do this.
>
> Thanks,
> Guenter
>
> > + }
> > +
> > /* Use alias for watchdog id if possible */
> > if (wdd->parent) {
> > ret = of_alias_get_id(wdd->parent->of_node, "watchdog");
> >
Flavio
prev parent reply other threads:[~2021-09-17 15:22 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-16 14:40 [PATCH v3] watchdog: add new parameter to start the watchdog on module insertion Flavio Suligoi
2021-09-17 14:45 ` Guenter Roeck
2021-09-17 15:21 ` Flavio Suligoi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DU2PR01MB8144F50D2FA05CDA18BC6E6AF9DD9@DU2PR01MB8144.eurprd01.prod.exchangelabs.com \
--to=f.suligoi@asem.it \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mika.westerberg@linux.intel.com \
--cc=wim@linux-watchdog.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).