Re: [PATCH] Watchdog: sp5100_tco: Lower verbosity of disabled watchdog hardware

[Mario Limonciello]

On 5/9/22 19:33, Jerry Hoemann wrote:
> On Mon, May 09, 2022 at 03:55:54PM -0700, Guenter Roeck wrote:
> > On 5/9/22 09:33, Mario Limonciello wrote:
> > > If watchdog hardware has been disabled, currently the kernel driver
> > > will show at err level during probe:
> > >
> > > "Watchdog hardware is disabled"
> > >
> > > This is unnecessarily verbose as there is already a -ENODEV returned.
> > > Lower the level to debug.
> >
> > Is it ? Without this message, a user may try to load the driver,
> > get an error message, and have no idea why the driver was not
> > enabled even though the hardware exists. If anything , -ENODEV
> > is less than perfect. Unfortunately there does not seem to be
> > a better error code, or at least I don't see one.
> >
> > Guenter
>
> Coincidentally, I was looking at this code on Friday.
>
> Some HPE Proliant servers are disabling the AMD WDT in BIOS.  However,
> sp5100_tco was still getting configured.  It was the lack of
> "Watchdog hardware is disabled" message that helped clue us into
> what was going on (Linux is enabling the WDT anyway.)
>
> So, I liked that this message exists.
>
> I'll send an RFC patch for this other issue as it orthogonal.
> But just wanted to point out the message is useful.

I personally don't have a problem blacklisting on a system I encounter 
this. I take anything at "err" level as there is a firmware problem or a 
hardware problem that should be looked at.

As the message is genuinely useful as Jerry points out how about meeting 
in the middle at info or notice?

> > > Reported-by: ionut_n2001@xxxxxxxxx
> > > Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D215762&data=05%7C01%7Cmario.limonciello%40amd.com%7C5eb5f65caaf241d1515308da321cbd10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637877396427792688%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tqQYyfwbGK%2BTvTxlkef0I2iKyLe8WsOjxskA0SQPlcw%3D&reserved=0
> > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> > > ---
> > >    drivers/watchdog/sp5100_tco.c | 2 +-
> > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
> > > index 86ffb58fbc85..e51ecbd5c8b7 100644
> > > --- a/drivers/watchdog/sp5100_tco.c
> > > +++ b/drivers/watchdog/sp5100_tco.c
> > > @@ -286,7 +286,7 @@ static int sp5100_tco_timer_init(struct sp5100_tco *tco)
> > >    	val = readl(SP5100_WDT_CONTROL(tco->tcobase));
> > >    	if (val & SP5100_WDT_DISABLED) {
> > > -		dev_err(dev, "Watchdog hardware is disabled\n");
> > > +		dev_dbg(dev, "Watchdog hardware is disabled\n");
> > >    		return -ENODEV;
> > >    	}