Re: [PATCH] watchdog: add module parameter "force_no_reboot" for iTCO

[Guenter Roeck]

On 07/04/2018 11:28 PM, Tian, Baofeng wrote:
> Hi, Roeck
>
> Yes, this patch is for add a parameter to TCO to prevent reboot happen if you don't want to reboot system
> and want to stay here to check some HW status, logs, etc for debug purpose.
>
> Under some android related stability test, developer want to stay at the crash and use debug tools(LTB)
> to check more information, this is the purpose of this patch.
The watchdog doesn't even start with the module parameter set.
If the purpose is to disable the watchdog, just stop it by closing
the watchdog daemon, or don't run the watchdog daemon in the first
place, or blacklist the module.

Guenter

> Thanks
> Tim
>
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter Roeck
> Sent: Monday, July 2, 2018 9:21 PM
> To: Tian, Baofeng <baofeng.tian@xxxxxxxxx>; wim@xxxxxxxxxxxxxxxxxx; linux-watchdog@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] watchdog: add module parameter "force_no_reboot" for iTCO
>
> On 07/02/2018 12:30 AM, Tian, Baofeng wrote:
> > From: "Tian, Baofeng" <baofeng.tian@xxxxxxxxx
> > <mailto:baofeng.tian@xxxxxxxxx>>
> > Subject: [PATCH] watchdog: add module parameter "force_no_reboot" for
> > iTCO
> >
> > Setting "force_no_reboot" parameter to true (y/Y/1) will have the
> > effect to prevent to reset the NO_REBOOT flag thus preventing the iTCO
> > to reboot the platform, if not set or set to false, then system will
> > reboot after about 30s.
> >
> > Signed-off-by: Tian, Baofeng <baofeng.tian@xxxxxxxxx
> > <mailto:baofeng.tian@xxxxxxxxx>>
>
> Your e-mail address is messed up.
>
> > ---
> >   Âdrivers/watchdog/iTCO_wdt.c | 11 ++++++++++-
> >   Â1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> > index 347f038..255318b 100644
> > --- a/drivers/watchdog/iTCO_wdt.c
> > +++ b/drivers/watchdog/iTCO_wdt.c
> > @@ -131,6 +131,11 @@ module_param(turn_SMI_watchdog_clear_off, int,
> > 0);
> >   ÂMODULE_PARM_DESC(turn_SMI_watchdog_clear_off,
> >   ÂÂÂÂÂ "Turn off SMI clearing watchdog (depends on
> > TCO-version)(default=1)");
> >
> > +static bool force_no_reboot;
> > +module_param(force_no_reboot, bool, 0);
> > +MODULE_PARM_DESC(force_no_reboot,
> > +ÂÂÂÂÂÂÂÂÂÂ "Prevents the watchdog rebooting the platform
> > +(default=0)");
> > +
> >   Â/*
> >   Â * Some TCO specific functions
> >   Â */
> > @@ -243,6 +248,10 @@ static int iTCO_wdt_start(struct watchdog_device
> > *wd_dev)
> >   ÂÂÂÂÂ struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
> >   ÂÂÂÂÂ unsigned int val;
> >
> > +ÂÂÂÂ /* force_no_reboot will prevent to unset NO_REBOOT bit */
> > +ÂÂÂÂ if (force_no_reboot)
> > +ÂÂÂÂÂÂÂÂÂÂ return -EIO;
> > +
> It seems to me that this flag prevents the watchdog from being started, and on top it would return an unreasonable error (-EIO).
>
> I don't see the point of this patch, sorry.
>
> Guenter
>
> >   ÂÂÂÂÂ spin_lock(&p->io_lock);
> >
> >   ÂÂÂÂÂ iTCO_vendor_pre_start(p->smi_res, wd_dev->timeout); @@ -250,7
> > +259,7 @@ static int iTCO_wdt_start(struct watchdog_device *wd_dev)
> >   ÂÂÂÂÂ /* disable chipset's NO_REBOOT bit */
> >   ÂÂÂÂÂ if (p->update_no_reboot_bit(p->no_reboot_priv, false)) {
> >   ÂÂÂÂÂÂÂÂÂÂÂ spin_unlock(&p->io_lock);
> > -ÂÂÂÂÂÂÂÂÂÂ pr_err("failed to reset NO_REBOOT flag, reboot disabled by
> > hardware/BIOS\n");
> > +ÂÂÂÂÂÂÂÂÂÂ pr_err("failed to reset NO_REBOOT flag, reboot disabled by
> > +hardware/BIOS/rc_cmd\n");
> >   ÂÂÂÂÂÂÂÂÂÂÂ return -EIO;
> >   ÂÂÂÂÂ }
> >
> > --
> > 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html