[RFC PATCH 0/9] watchdog: don't print out if registering watchdog fails

[RFC PATCH 0/9] watchdog: don't print out if registering watchdog fails

[Wolfram Sang]

While working with the rza-driver, I noticed that the watchdog core does
not always print out errors when registering a watchdog fails. Only most
of the time. With a simple refactoring, it will always print out. We can
remove similar printouts then from drivers. This series does exactly
that. Not sure about the core change, thus still RFC. There are two more
drivers outside the 'watchdog' dir which could benefit from this change,
but I left them out for now until I know this path is acceptable.

Looking forward to comments...


Wolfram Sang (9):
  watchdog: always print when registering watchdog fails
  watchdog: da9055_wdt: don't print out if registering watchdog fails
  watchdog: hpe-wdt: don't print out if registering watchdog fails
  watchdog: iTCO_wdt: don't print out if registering watchdog fails
  watchdog: it87_wdt: don't print out if registering watchdog fails
  watchdog: octeon-wdt: don't print out if registering watchdog fails
  watchdog: rti_wdt: don't print out if registering watchdog fails
  watchdog: rza_wdt: don't print out if registering watchdog fails
  watchdog: sl28cpld_wdt: don't print out if registering watchdog fails

 drivers/watchdog/da9055_wdt.c      |  7 +------
 drivers/watchdog/gxp-wdt.c         |  4 +---
 drivers/watchdog/iTCO_wdt.c        |  4 +---
 drivers/watchdog/it87_wdt.c        |  4 +---
 drivers/watchdog/octeon-wdt-main.c |  4 +---
 drivers/watchdog/rti_wdt.c         |  4 +---
 drivers/watchdog/rza_wdt.c         |  7 +------
 drivers/watchdog/sl28cpld_wdt.c    |  4 +---
 drivers/watchdog/watchdog_core.c   | 26 +++++++++++++++++---------
 9 files changed, 25 insertions(+), 39 deletions(-)

-- 
2.45.2

[RFC PATCH 1/9] watchdog: always print when registering watchdog fails

[Wolfram Sang]

So far, only 'watchdog_register_device' prints an error if registering
the watchdog driver fails. '__watchdog_register_device' doesn't.
Refactor the code so that both print out. Drivers can then rely on that
and skip their own error messages.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/watchdog_core.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index aff2c3912ead..d46d8c8c01f2 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -237,7 +237,7 @@ void watchdog_set_restart_priority(struct watchdog_device *wdd, int priority)
 }
 EXPORT_SYMBOL_GPL(watchdog_set_restart_priority);
 
-static int __watchdog_register_device(struct watchdog_device *wdd)
+static int ___watchdog_register_device(struct watchdog_device *wdd)
 {
 	int ret, id = -1;
 
@@ -337,6 +337,22 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 	return 0;
 }
 
+static int __watchdog_register_device(struct watchdog_device *wdd)
+{
+	const char *dev_str;
+	int ret;
+
+	ret = ___watchdog_register_device(wdd);
+	if (ret) {
+		dev_str = wdd->parent ? dev_name(wdd->parent) :
+			  (const char *)wdd->info->identity;
+		pr_err("%s: failed to register watchdog device (err = %d)\n",
+			dev_str, ret);
+	}
+
+	return ret;
+}
+
 /**
  * watchdog_register_device() - register a watchdog device
  * @wdd: watchdog device
@@ -350,7 +366,6 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 
 int watchdog_register_device(struct watchdog_device *wdd)
 {
-	const char *dev_str;
 	int ret = 0;
 
 	mutex_lock(&wtd_deferred_reg_mutex);
@@ -360,13 +375,6 @@ int watchdog_register_device(struct watchdog_device *wdd)
 		watchdog_deferred_registration_add(wdd);
 	mutex_unlock(&wtd_deferred_reg_mutex);
 
-	if (ret) {
-		dev_str = wdd->parent ? dev_name(wdd->parent) :
-			  (const char *)wdd->info->identity;
-		pr_err("%s: failed to register watchdog device (err = %d)\n",
-			dev_str, ret);
-	}
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(watchdog_register_device);
-- 
2.45.2

Re: [RFC PATCH 1/9] watchdog: always print when registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:04PM +0200, Wolfram Sang wrote:
> So far, only 'watchdog_register_device' prints an error if registering
> the watchdog driver fails. '__watchdog_register_device' doesn't.
> Refactor the code so that both print out. Drivers can then rely on that
> and skip their own error messages.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

[RFC PATCH 2/9] watchdog: da9055_wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/da9055_wdt.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/watchdog/da9055_wdt.c b/drivers/watchdog/da9055_wdt.c
index 389a4bdd208c..9d5a2009466f 100644
--- a/drivers/watchdog/da9055_wdt.c
+++ b/drivers/watchdog/da9055_wdt.c
@@ -146,12 +146,7 @@ static int da9055_wdt_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = devm_watchdog_register_device(dev, &driver_data->wdt);
-	if (ret != 0)
-		dev_err(da9055->dev, "watchdog_register_device() failed: %d\n",
-			ret);
-
-	return ret;
+	return devm_watchdog_register_device(dev, &driver_data->wdt);
 }
 
 static struct platform_driver da9055_wdt_driver = {
-- 
2.45.2

Re: [RFC PATCH 2/9] watchdog: da9055_wdt: don't print out if registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:05PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

[RFC PATCH 3/9] watchdog: hpe-wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/gxp-wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/gxp-wdt.c b/drivers/watchdog/gxp-wdt.c
index 2fd85be88278..f2c236160266 100644
--- a/drivers/watchdog/gxp-wdt.c
+++ b/drivers/watchdog/gxp-wdt.c
@@ -151,10 +151,8 @@ static int gxp_wdt_probe(struct platform_device *pdev)
 
 	watchdog_stop_on_reboot(&drvdata->wdd);
 	err = devm_watchdog_register_device(dev, &drvdata->wdd);
-	if (err) {
-		dev_err(dev, "Failed to register watchdog device");
+	if (err)
 		return err;
-	}
 
 	dev_info(dev, "HPE GXP watchdog timer");
 
-- 
2.45.2

Re: [RFC PATCH 3/9] watchdog: hpe-wdt: don't print out if registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:06PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [RFC PATCH 3/9] watchdog: hpe-wdt: don't print out if registering watchdog fails

[Jerry Hoemann]

On Fri, Oct 04, 2024 at 10:03:06PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/watchdog/gxp-wdt.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Question:  should email have been titled
	"watchdog: gxp-wdt: ..."
instead of
	"watchdog: hpe-wdt: ..."

to match the module name as the email title gets put into
the git log for the file?

Thanks

Jerry


> 
> diff --git a/drivers/watchdog/gxp-wdt.c b/drivers/watchdog/gxp-wdt.c
> index 2fd85be88278..f2c236160266 100644
> --- a/drivers/watchdog/gxp-wdt.c
> +++ b/drivers/watchdog/gxp-wdt.c
> @@ -151,10 +151,8 @@ static int gxp_wdt_probe(struct platform_device *pdev)
>  
>  	watchdog_stop_on_reboot(&drvdata->wdd);
>  	err = devm_watchdog_register_device(dev, &drvdata->wdd);
> -	if (err) {
> -		dev_err(dev, "Failed to register watchdog device");
> +	if (err)
>  		return err;
> -	}
>  
>  	dev_info(dev, "HPE GXP watchdog timer");
>  
> -- 
> 2.45.2
> 

-- 

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

Re: [RFC PATCH 3/9] watchdog: hpe-wdt: don't print out if registering watchdog fails

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

On Fri, Oct 04, 2024 at 03:29:09PM -0600, Jerry Hoemann wrote:
> On Fri, Oct 04, 2024 at 10:03:06PM +0200, Wolfram Sang wrote:
> > The core will do this already.
> > 
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >  drivers/watchdog/gxp-wdt.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> Question:  should email have been titled
> 	"watchdog: gxp-wdt: ..."
> instead of
> 	"watchdog: hpe-wdt: ..."
> 
> to match the module name as the email title gets put into
> the git log for the file?

No objection, we can do that. I check git-log for the prefixes and found
there the following:

6b47441bed49 ("watchdog: hpe-wdt: Introduce HPE GXP Watchdog")

I am fine with both.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH 3/9] watchdog: hpe-wdt: don't print out if registering watchdog fails

[Jerry Hoemann]

On Sat, Oct 05, 2024 at 07:23:19AM +0200, Wolfram Sang wrote:
> On Fri, Oct 04, 2024 at 03:29:09PM -0600, Jerry Hoemann wrote:
> > On Fri, Oct 04, 2024 at 10:03:06PM +0200, Wolfram Sang wrote:
> > > The core will do this already.
> > > 
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > ---
> > >  drivers/watchdog/gxp-wdt.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > Question:  should email have been titled
> > 	"watchdog: gxp-wdt: ..."
> > instead of
> > 	"watchdog: hpe-wdt: ..."
> > 
> > to match the module name as the email title gets put into
> > the git log for the file?
> 
> No objection, we can do that. I check git-log for the prefixes and found
> there the following:
> 
> 6b47441bed49 ("watchdog: hpe-wdt: Introduce HPE GXP Watchdog")

That is somewhat unfortunate as it will can lead to confusion with
the long standing hpwdt.  These are different watchdogs.

Jerry
> 
> I am fine with both.
> 



-- 

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

[RFC PATCH 4/9] watchdog: iTCO_wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/iTCO_wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 35b358bcf94c..0d3c09a82f4f 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -592,10 +592,8 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
 	watchdog_stop_on_reboot(&p->wddev);
 	watchdog_stop_on_unregister(&p->wddev);
 	ret = devm_watchdog_register_device(dev, &p->wddev);
-	if (ret != 0) {
-		dev_err(dev, "cannot register watchdog device (err=%d)\n", ret);
+	if (ret != 0)
 		return ret;
-	}
 
 	dev_info(dev, "initialized. heartbeat=%d sec (nowayout=%d)\n",
 		heartbeat, nowayout);
-- 
2.45.2

Re: [RFC PATCH 4/9] watchdog: iTCO_wdt: don't print out if registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:07PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

[RFC PATCH 5/9] watchdog: it87_wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/it87_wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
index 3e8c15138edd..676cd134e677 100644
--- a/drivers/watchdog/it87_wdt.c
+++ b/drivers/watchdog/it87_wdt.c
@@ -349,10 +349,8 @@ static int __init it87_wdt_init(void)
 
 	watchdog_stop_on_reboot(&wdt_dev);
 	rc = watchdog_register_device(&wdt_dev);
-	if (rc) {
-		pr_err("Cannot register watchdog device (err=%d)\n", rc);
+	if (rc)
 		return rc;
-	}
 
 	pr_info("Chip IT%04x revision %d initialized. timeout=%d sec (nowayout=%d testmode=%d)\n",
 		chip_type, chip_rev, timeout, nowayout, testmode);
-- 
2.45.2

Re: [RFC PATCH 5/9] watchdog: it87_wdt: don't print out if registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:08PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

[RFC PATCH 6/9] watchdog: octeon-wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/octeon-wdt-main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/octeon-wdt-main.c b/drivers/watchdog/octeon-wdt-main.c
index 52d49e4e35a0..0615bb816082 100644
--- a/drivers/watchdog/octeon-wdt-main.c
+++ b/drivers/watchdog/octeon-wdt-main.c
@@ -559,10 +559,8 @@ static int __init octeon_wdt_init(void)
 	watchdog_set_nowayout(&octeon_wdt, nowayout);
 
 	ret = watchdog_register_device(&octeon_wdt);
-	if (ret) {
-		pr_err("watchdog_register_device() failed: %d\n", ret);
+	if (ret)
 		return ret;
-	}
 
 	if (disable) {
 		pr_notice("disabled\n");
-- 
2.45.2

Re: [RFC PATCH 6/9] watchdog: octeon-wdt: don't print out if registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:09PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

[RFC PATCH 7/9] watchdog: rti_wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/rti_wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 4895a69015a8..e319fa0787c2 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -336,10 +336,8 @@ static int rti_wdt_probe(struct platform_device *pdev)
 	watchdog_init_timeout(wdd, heartbeat, dev);
 
 	ret = watchdog_register_device(wdd);
-	if (ret) {
-		dev_err(dev, "cannot register watchdog device\n");
+	if (ret)
 		goto err_iomap;
-	}
 
 	if (last_ping)
 		watchdog_set_last_hw_keepalive(wdd, last_ping);
-- 
2.45.2

Re: [RFC PATCH 7/9] watchdog: rti_wdt: don't print out if registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:10PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

[RFC PATCH 8/9] watchdog: rza_wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/rza_wdt.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/watchdog/rza_wdt.c b/drivers/watchdog/rza_wdt.c
index cb4901b3f777..9334255a37e9 100644
--- a/drivers/watchdog/rza_wdt.c
+++ b/drivers/watchdog/rza_wdt.c
@@ -169,7 +169,6 @@ static int rza_wdt_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct rza_wdt *priv;
 	unsigned long rate;
-	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -218,11 +217,7 @@ static int rza_wdt_probe(struct platform_device *pdev)
 	watchdog_init_timeout(&priv->wdev, 0, dev);
 	watchdog_set_drvdata(&priv->wdev, priv);
 
-	ret = devm_watchdog_register_device(dev, &priv->wdev);
-	if (ret)
-		dev_err(dev, "Cannot register watchdog device\n");
-
-	return ret;
+	return devm_watchdog_register_device(dev, &priv->wdev);
 }
 
 static const struct of_device_id rza_wdt_of_match[] = {
-- 
2.45.2

[RFC PATCH 9/9] watchdog: sl28cpld_wdt: don't print out if registering watchdog fails

[Wolfram Sang]

The core will do this already.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/sl28cpld_wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/sl28cpld_wdt.c b/drivers/watchdog/sl28cpld_wdt.c
index 9ce456f09f73..8630c29818f2 100644
--- a/drivers/watchdog/sl28cpld_wdt.c
+++ b/drivers/watchdog/sl28cpld_wdt.c
@@ -198,10 +198,8 @@ static int sl28cpld_wdt_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_watchdog_register_device(&pdev->dev, wdd);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to register watchdog device\n");
+	if (ret < 0)
 		return ret;
-	}
 
 	dev_info(&pdev->dev, "initial timeout %d sec%s\n",
 		 wdd->timeout, nowayout ? ", nowayout" : "");
-- 
2.45.2

Re: [RFC PATCH 9/9] watchdog: sl28cpld_wdt: don't print out if registering watchdog fails

[Guenter Roeck]

On Fri, Oct 04, 2024 at 10:03:12PM +0200, Wolfram Sang wrote:
> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [RFC PATCH 9/9] watchdog: sl28cpld_wdt: don't print out if registering watchdog fails

[Michael Walle]

> The core will do this already.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Michael Walle <mwalle@kernel.org>

Thanks!
-michael