Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 9 May 2022 18:33:39 -0600
From:   Jerry Hoemann <jerry.hoemann@hpe.com>
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     Mario Limonciello <mario.limonciello@amd.com>,
        Wim Van Sebroeck <wim@linux-watchdog.org>,
        "open list:WATCHDOG DEVICE DRIVERS" <linux-watchdog@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>, ionut_n2001@yahoo.com
Subject: Re: [PATCH] Watchdog: sp5100_tco: Lower verbosity of disabled
 watchdog hardware
Message-ID: <20220510003339.GA2788@perchik.americas.hpqcorp.net>
Reply-To: Jerry.Hoemann@hpe.com
References: <20220509163304.86-1-mario.limonciello@amd.com>
 <b394bfcf-9f0d-b27c-5bc0-3d628de98755@roeck-us.net>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b394bfcf-9f0d-b27c-5bc0-3d628de98755@roeck-us.net>
User-Agent: Mutt/1.10.1 (2018-07-13)
MIME-Version: 1.0
Precedence: bulk

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.


> 
> > 
> > Reported-by: ionut_n2001@yahoo.com
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215762
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> >   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;
> >   	}

-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------