[PATCH 0/5] watchdog: Do not use 'dev' from watchdog_device in watchdog drivers

[PATCH 0/5] watchdog: Do not use 'dev' from watchdog_device in watchdog drivers

[Guenter Roeck]

The 'dev' variable in watchdog drivers has a different lifetime than the
watchdog character device and should therefore not be used by watchdog
drivers.

Some of the drivers use the variable to print kernel messages. Those are
either dropped or converted to use pr_ functions. One driver sets the
variable during initialization to the watchdog driver's parent device,
which is wrong and also removed.

[PATCH 1/5] watchdog: bcm2835_wdt: Drop log message if watchdog is stopped

[Guenter Roeck]

Stopping a watchdog is a normal operation and does not warrant a log
message.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/bcm2835_wdt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
index 8a5ce5b5a0b6..2e6164c4abc0 100644
--- a/drivers/watchdog/bcm2835_wdt.c
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -79,7 +79,6 @@ static int bcm2835_wdt_stop(struct watchdog_device *wdog)
 	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
 
 	writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, wdt->base + PM_RSTC);
-	dev_info(wdog->dev, "Watchdog timer stopped");
 	return 0;
 }
 
-- 
2.1.4

[PATCH 2/5] watchdog: tangox: Print info message using pointer to platform device

[Guenter Roeck]

The device pointer in struct watchdog_device should not be used by drivers
and may be removed in the near future. Use the platform device pointer for
info messages instead.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/tangox_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/tangox_wdt.c b/drivers/watchdog/tangox_wdt.c
index b9ee6246e7c2..709c1ed6fd79 100644
--- a/drivers/watchdog/tangox_wdt.c
+++ b/drivers/watchdog/tangox_wdt.c
@@ -184,7 +184,7 @@ static int tangox_wdt_probe(struct platform_device *pdev)
 	if (err)
 		dev_warn(&pdev->dev, "failed to register restart handler\n");
 
-	dev_info(dev->wdt.dev, "SMP86xx/SMP87xx watchdog registered\n");
+	dev_info(&pdev->dev, "SMP86xx/SMP87xx watchdog registered\n");
 
 	return 0;
 }
-- 
2.1.4

[PATCH 3/5] watchdog: gpio: Do not use device pointer from struct watchdog_device

[Guenter Roeck]

The device pointer in struct watchdog_device has a different lifetime
than the driver code and should not be used in drivers. Use pr_crit
instead of dev_crit.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/gpio_wdt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
index 035c2387b846..44df983a1232 100644
--- a/drivers/watchdog/gpio_wdt.c
+++ b/drivers/watchdog/gpio_wdt.c
@@ -9,6 +9,8 @@
  * (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/module.h>
@@ -54,7 +56,8 @@ static void gpio_wdt_hwping(unsigned long data)
 
 	if (priv->armed && time_after(jiffies, priv->last_jiffies +
 				      msecs_to_jiffies(wdd->timeout * 1000))) {
-		dev_crit(wdd->dev, "Timer expired. System will reboot soon!\n");
+		pr_crit("watchdog%d: Timer expired. System will reboot soon!\n",
+			wdd->id);
 		return;
 	}
 
-- 
2.1.4

[PATCH 4/5] watchdog: mena21: Do not use device pointer from struct watchdog_device

[Guenter Roeck]

The device pointer in struct watchdog_device has a different lifetime
than the driver code and should not be used in drivers. Use pr_err
instead of dev_err.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/mena21_wdt.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
index 098fa9c34d6d..f0dc794d0fda 100644
--- a/drivers/watchdog/mena21_wdt.c
+++ b/drivers/watchdog/mena21_wdt.c
@@ -7,6 +7,9 @@
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/types.h>
@@ -100,13 +103,14 @@ static int a21_wdt_set_timeout(struct watchdog_device *wdt,
 	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
 
 	if (timeout != 1 && timeout != 30) {
-		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		pr_err("watchdog%d: Only 1 and 30 allowed as timeout\n",
+		       wdt->id);
 		return -EINVAL;
 	}
 
 	if (timeout == 30 && wdt->timeout == 1) {
-		dev_err(wdt->dev,
-			"Transition from fast to slow mode not allowed\n");
+		pr_err("watchdog%d: Transition from fast to slow mode not allowed\n",
+		       wdt->id);
 		return -EINVAL;
 	}
 
-- 
2.1.4

[PATCH 5/5] watchdog: qcom-wdt: Do not set 'dev' in struct watchdog_device

[Guenter Roeck]

The 'dev' pointer in struct watchdog_device is set by the watchdog core
when registering the watchdog device and not by the driver. It points to
the watchdog device, not its parent.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/qcom-wdt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index aa7105d32c02..424f9a952fee 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -164,7 +164,6 @@ static int qcom_wdt_probe(struct platform_device *pdev)
 		goto err_clk_unprepare;
 	}
 
-	wdt->wdd.dev = &pdev->dev;
 	wdt->wdd.info = &qcom_wdt_info;
 	wdt->wdd.ops = &qcom_wdt_ops;
 	wdt->wdd.min_timeout = 1;
-- 
2.1.4

Re: [PATCH 0/5] watchdog: Do not use 'dev' from watchdog_device in watchdog drivers

[Damien Riegel]

On Wed, Dec 23, 2015 at 09:11:28PM -0800, Guenter Roeck wrote:
> The 'dev' variable in watchdog drivers has a different lifetime than the
> watchdog character device and should therefore not be used by watchdog
> drivers.
> 
> Some of the drivers use the variable to print kernel messages. Those are
> either dropped or converted to use pr_ functions. One driver sets the
> variable during initialization to the watchdog driver's parent device,
> which is wrong and also removed.

Hi Guenter,

For gpio_wdt and mena21_wdt, wdd->parent is set and could be used for
dev_* printings. Do you prefer to keep this variable only for watchdog
core internal usage? Otherwise, the serie looks good.

Thanks,
Damien

Re: [PATCH 0/5] watchdog: Do not use 'dev' from watchdog_device in watchdog drivers

[Guenter Roeck]

On 12/24/2015 07:07 AM, Damien Riegel wrote:
> On Wed, Dec 23, 2015 at 09:11:28PM -0800, Guenter Roeck wrote:
>> The 'dev' variable in watchdog drivers has a different lifetime than the
>> watchdog character device and should therefore not be used by watchdog
>> drivers.
>>
>> Some of the drivers use the variable to print kernel messages. Those are
>> either dropped or converted to use pr_ functions. One driver sets the
>> variable during initialization to the watchdog driver's parent device,
>> which is wrong and also removed.
>
> Hi Guenter,
>
> For gpio_wdt and mena21_wdt, wdd->parent is set and could be used for
> dev_* printings. Do you prefer to keep this variable only for watchdog
> core internal usage? Otherwise, the serie looks good.
>
Good idea, I'll use ->parent for those. ->parent is set by the driver,
so it is safe to be used by the driver.

Thanks,
Guenter