LinuxLists.cc - [PATCH 00/22] watchdog: Convert to use device managed functions and other improvements

2019-04-08 19:43:59

Subject: [PATCH 00/22] watchdog: Convert to use device managed functions and other improvements

Use device managed functions and other changes to simplify error handling,
reduce source code size, improve readability, and reduce the likelyhood
of bugs.

The changes made in this series can be summarized to

- Use devm_add_action_or_reset() for calls to clk_disable_unprepare
- Use devm_watchdog_register_driver() to register watchdog device
- Replace 'of_clk_get(np, 0)' with 'devm_clk_get(dev, NULL)'
- Drop assignments to otherwise unused variables
- Drop unnecessary braces around conditional return statements
- Drop empty remove function
- Replace shutdown function with call to watchdog_stop_on_reboot()
- Replace 'goto l; ... l: return e;' with 'return e;'
- Replace 'ret = e; return ret;' with 'return e;'.
- Use local variable 'struct device *dev' consistently
- Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly
- Drop unnecessary calls to platform_set_drvdata()

Conversions were performed automatically with coccinelle using a number
of semantic patches. The semantic patches and the scripts used to generate
commit logs are available at https://github.com/groeck/coccinelle-patches.

This is the first of (at least) three series of similar patches for watchdog
drivers.

2019-04-08 19:43:53

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 01/22] watchdog: armada_37xx_wdt: Convert to use device managed functions and other improvements

2019-04-08 19:44:36

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 03/22] watchdog: aspeed_wdt: Use 'dev' instead of dereferencing it repeatedly

Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

Cc: Joel Stanley <[email protected]>
Cc: Andrew Jeffery <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/aspeed_wdt.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index f09333fd54b4..34117745c65f 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -187,6 +187,7 @@ static const struct watchdog_info aspeed_wdt_info = {

static int aspeed_wdt_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
const struct aspeed_wdt_config *config;
const struct of_device_id *ofdid;
struct aspeed_wdt *wdt;
@@ -196,7 +197,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
u32 status;
int ret;

- wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+ wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
return -ENOMEM;

@@ -212,12 +213,12 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
wdt->wdd.info = &aspeed_wdt_info;
wdt->wdd.ops = &aspeed_wdt_ops;
wdt->wdd.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT_MS;
- wdt->wdd.parent = &pdev->dev;
+ wdt->wdd.parent = dev;

wdt->wdd.timeout = WDT_DEFAULT_TIMEOUT;
- watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
+ watchdog_init_timeout(&wdt->wdd, 0, dev);

- np = pdev->dev.of_node;
+ np = dev->of_node;

ofdid = of_match_node(aspeed_wdt_of_table, np);
if (!ofdid)
@@ -286,11 +287,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
u32 max_duration = config->ext_pulse_width_mask + 1;

if (duration == 0 || duration > max_duration) {
- dev_err(&pdev->dev, "Invalid pulse duration: %uus\n",
- duration);
+ dev_err(dev, "Invalid pulse duration: %uus\n",
+ duration);
duration = max(1U, min(max_duration, duration));
- dev_info(&pdev->dev, "Pulse duration set to %uus\n",
- duration);
+ dev_info(dev, "Pulse duration set to %uus\n",
+ duration);
}

/*
@@ -312,9 +313,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
wdt->wdd.bootstatus = WDIOF_CARDRESET;

- ret = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
+ ret = devm_watchdog_register_device(dev, &wdt->wdd);
if (ret) {
- dev_err(&pdev->dev, "failed to register\n");
+ dev_err(dev, "failed to register\n");
return ret;
}

--
2.7.4

2019-04-08 19:44:54

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 05/22] watchdog: bcm2835_wdt: drop platform_set_drvdata

There is no call to platform_get_drvdata() in the driver,
so platform_set_drvdata() is unnecessary and can be dropped.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

Cc: Florian Fainelli <[email protected]>
Cc: Ray Jui <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: [email protected]
Cc: Eric Anholt <[email protected]>
Cc: Stefan Wahren <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
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 1834524ae373..560c1c54c177 100644
--- a/drivers/watchdog/bcm2835_wdt.c
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -177,7 +177,6 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
if (!wdt)
return -ENOMEM;
- platform_set_drvdata(pdev, wdt);

spin_lock_init(&wdt->lock);

--
2.7.4

2019-04-08 19:45:47

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 20/22] watchdog: kempld_wdt: Convert to use device managed functions and other improvements

2019-04-08 19:46:16

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 19/22] watchdog: intel-mid_wdt: Use 'dev' instead of dereferencing it repeatedly

Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/intel-mid_wdt.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
index 72c108a12c19..6cf7cc1ff615 100644
--- a/drivers/watchdog/intel-mid_wdt.c
+++ b/drivers/watchdog/intel-mid_wdt.c
@@ -110,12 +110,13 @@ static const struct watchdog_ops mid_wdt_ops = {

static int mid_wdt_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct watchdog_device *wdt_dev;
- struct intel_mid_wdt_pdata *pdata = pdev->dev.platform_data;
+ struct intel_mid_wdt_pdata *pdata = dev->platform_data;
int ret;

if (!pdata) {
- dev_err(&pdev->dev, "missing platform data\n");
+ dev_err(dev, "missing platform data\n");
return -EINVAL;
}

@@ -125,7 +126,7 @@ static int mid_wdt_probe(struct platform_device *pdev)
return ret;
}

- wdt_dev = devm_kzalloc(&pdev->dev, sizeof(*wdt_dev), GFP_KERNEL);
+ wdt_dev = devm_kzalloc(dev, sizeof(*wdt_dev), GFP_KERNEL);
if (!wdt_dev)
return -ENOMEM;

@@ -134,16 +135,15 @@ static int mid_wdt_probe(struct platform_device *pdev)
wdt_dev->min_timeout = MID_WDT_TIMEOUT_MIN;
wdt_dev->max_timeout = MID_WDT_TIMEOUT_MAX;
wdt_dev->timeout = MID_WDT_DEFAULT_TIMEOUT;
- wdt_dev->parent = &pdev->dev;
+ wdt_dev->parent = dev;

- watchdog_set_drvdata(wdt_dev, &pdev->dev);
+ watchdog_set_drvdata(wdt_dev, dev);

- ret = devm_request_irq(&pdev->dev, pdata->irq, mid_wdt_irq,
+ ret = devm_request_irq(dev, pdata->irq, mid_wdt_irq,
IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
wdt_dev);
if (ret) {
- dev_err(&pdev->dev, "error requesting warning irq %d\n",
- pdata->irq);
+ dev_err(dev, "error requesting warning irq %d\n", pdata->irq);
return ret;
}

@@ -163,13 +163,13 @@ static int mid_wdt_probe(struct platform_device *pdev)
/* Make sure the watchdog is serviced */
set_bit(WDOG_HW_RUNNING, &wdt_dev->status);

- ret = devm_watchdog_register_device(&pdev->dev, wdt_dev);
+ ret = devm_watchdog_register_device(dev, wdt_dev);
if (ret) {
- dev_err(&pdev->dev, "error registering watchdog device\n");
+ dev_err(dev, "error registering watchdog device\n");
return ret;
}

- dev_info(&pdev->dev, "Intel MID watchdog device probed\n");
+ dev_info(dev, "Intel MID watchdog device probed\n");

return 0;
}
--
2.7.4

2019-04-08 19:46:42

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 13/22] watchdog: davinci_wdt: Convert to use device managed functions and other improvements

2019-04-08 19:47:16

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 21/22] watchdog: lpc18xx_wdt: Convert to use device managed functions and other improvements

2019-04-08 19:47:24

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 18/22] watchdog: imgpdc_wdt: Convert to use device managed functions and other improvements

Use device managed functions to simplify error handling, reduce
source code size, improve readability, and reduce the likelyhood of bugs.
Other improvements as listed below.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

- Drop assignments to otherwise unused variables
- Drop empty remove function
- Use devm_add_action_or_reset() for calls to clk_disable_unprepare
- Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly
- Use devm_watchdog_register_driver() to register watchdog device
- Replace shutdown function with call to watchdog_stop_on_reboot()

Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/imgpdc_wdt.c | 91 +++++++++++++++++--------------------------
1 file changed, 36 insertions(+), 55 deletions(-)

diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
index 84c9fb905072..0fc31aadeee3 100644
--- a/drivers/watchdog/imgpdc_wdt.c
+++ b/drivers/watchdog/imgpdc_wdt.c
@@ -178,14 +178,20 @@ static const struct watchdog_ops pdc_wdt_ops = {
.restart = pdc_wdt_restart,
};

+static void pdc_clk_disable_unprepare(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
static int pdc_wdt_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
u64 div;
int ret, val;
unsigned long clk_rate;
struct pdc_wdt_dev *pdc_wdt;

- pdc_wdt = devm_kzalloc(&pdev->dev, sizeof(*pdc_wdt), GFP_KERNEL);
+ pdc_wdt = devm_kzalloc(dev, sizeof(*pdc_wdt), GFP_KERNEL);
if (!pdc_wdt)
return -ENOMEM;

@@ -193,42 +199,48 @@ static int pdc_wdt_probe(struct platform_device *pdev)
if (IS_ERR(pdc_wdt->base))
return PTR_ERR(pdc_wdt->base);

- pdc_wdt->sys_clk = devm_clk_get(&pdev->dev, "sys");
+ pdc_wdt->sys_clk = devm_clk_get(dev, "sys");
if (IS_ERR(pdc_wdt->sys_clk)) {
- dev_err(&pdev->dev, "failed to get the sys clock\n");
+ dev_err(dev, "failed to get the sys clock\n");
return PTR_ERR(pdc_wdt->sys_clk);
}

- pdc_wdt->wdt_clk = devm_clk_get(&pdev->dev, "wdt");
+ pdc_wdt->wdt_clk = devm_clk_get(dev, "wdt");
if (IS_ERR(pdc_wdt->wdt_clk)) {
- dev_err(&pdev->dev, "failed to get the wdt clock\n");
+ dev_err(dev, "failed to get the wdt clock\n");
return PTR_ERR(pdc_wdt->wdt_clk);
}

ret = clk_prepare_enable(pdc_wdt->sys_clk);
if (ret) {
- dev_err(&pdev->dev, "could not prepare or enable sys clock\n");
+ dev_err(dev, "could not prepare or enable sys clock\n");
return ret;
}
+ ret = devm_add_action_or_reset(dev, pdc_clk_disable_unprepare,
+ pdc_wdt->sys_clk);
+ if (ret)
+ return ret;

ret = clk_prepare_enable(pdc_wdt->wdt_clk);
if (ret) {
- dev_err(&pdev->dev, "could not prepare or enable wdt clock\n");
- goto disable_sys_clk;
+ dev_err(dev, "could not prepare or enable wdt clock\n");
+ return ret;
}
+ ret = devm_add_action_or_reset(dev, pdc_clk_disable_unprepare,
+ pdc_wdt->wdt_clk);
+ if (ret)
+ return ret;

/* We use the clock rate to calculate the max timeout */
clk_rate = clk_get_rate(pdc_wdt->wdt_clk);
if (clk_rate == 0) {
- dev_err(&pdev->dev, "failed to get clock rate\n");
- ret = -EINVAL;
- goto disable_wdt_clk;
+ dev_err(dev, "failed to get clock rate\n");
+ return -EINVAL;
}

if (order_base_2(clk_rate) > PDC_WDT_CONFIG_DELAY_MASK + 1) {
- dev_err(&pdev->dev, "invalid clock rate\n");
- ret = -EINVAL;
- goto disable_wdt_clk;
+ dev_err(dev, "invalid clock rate\n");
+ return -EINVAL;
}

if (order_base_2(clk_rate) == 0)
@@ -243,10 +255,10 @@ static int pdc_wdt_probe(struct platform_device *pdev)
do_div(div, clk_rate);
pdc_wdt->wdt_dev.max_timeout = div;
pdc_wdt->wdt_dev.timeout = PDC_WDT_DEF_TIMEOUT;
- pdc_wdt->wdt_dev.parent = &pdev->dev;
+ pdc_wdt->wdt_dev.parent = dev;
watchdog_set_drvdata(&pdc_wdt->wdt_dev, pdc_wdt);

- watchdog_init_timeout(&pdc_wdt->wdt_dev, heartbeat, &pdev->dev);
+ watchdog_init_timeout(&pdc_wdt->wdt_dev, heartbeat, dev);

pdc_wdt_stop(&pdc_wdt->wdt_dev);

@@ -257,24 +269,22 @@ static int pdc_wdt_probe(struct platform_device *pdev)
case PDC_WDT_TICKLE_STATUS_TICKLE:
case PDC_WDT_TICKLE_STATUS_TIMEOUT:
pdc_wdt->wdt_dev.bootstatus |= WDIOF_CARDRESET;
- dev_info(&pdev->dev,
- "watchdog module last reset due to timeout\n");
+ dev_info(dev, "watchdog module last reset due to timeout\n");
break;
case PDC_WDT_TICKLE_STATUS_HRESET:
- dev_info(&pdev->dev,
+ dev_info(dev,
"watchdog module last reset due to hard reset\n");
break;
case PDC_WDT_TICKLE_STATUS_SRESET:
- dev_info(&pdev->dev,
+ dev_info(dev,
"watchdog module last reset due to soft reset\n");
break;
case PDC_WDT_TICKLE_STATUS_USER:
- dev_info(&pdev->dev,
+ dev_info(dev,
"watchdog module last reset due to user reset\n");
break;
default:
- dev_info(&pdev->dev,
- "contains an illegal status code (%08x)\n", val);
+ dev_info(dev, "contains an illegal status code (%08x)\n", val);
break;
}

@@ -283,36 +293,9 @@ static int pdc_wdt_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, pdc_wdt);

- ret = watchdog_register_device(&pdc_wdt->wdt_dev);
- if (ret)
- goto disable_wdt_clk;
-
- return 0;
-
-disable_wdt_clk:
- clk_disable_unprepare(pdc_wdt->wdt_clk);
-disable_sys_clk:
- clk_disable_unprepare(pdc_wdt->sys_clk);
- return ret;
-}
-
-static void pdc_wdt_shutdown(struct platform_device *pdev)
-{
- struct pdc_wdt_dev *pdc_wdt = platform_get_drvdata(pdev);
-
- pdc_wdt_stop(&pdc_wdt->wdt_dev);
-}
-
-static int pdc_wdt_remove(struct platform_device *pdev)
-{
- struct pdc_wdt_dev *pdc_wdt = platform_get_drvdata(pdev);
-
- pdc_wdt_stop(&pdc_wdt->wdt_dev);
- watchdog_unregister_device(&pdc_wdt->wdt_dev);
- clk_disable_unprepare(pdc_wdt->wdt_clk);
- clk_disable_unprepare(pdc_wdt->sys_clk);
-
- return 0;
+ watchdog_stop_on_reboot(&pdc_wdt->wdt_dev);
+ watchdog_stop_on_unregister(&pdc_wdt->wdt_dev);
+ return devm_watchdog_register_device(dev, &pdc_wdt->wdt_dev);
}

static const struct of_device_id pdc_wdt_match[] = {
@@ -327,8 +310,6 @@ static struct platform_driver pdc_wdt_driver = {
.of_match_table = pdc_wdt_match,
},
.probe = pdc_wdt_probe,
- .remove = pdc_wdt_remove,
- .shutdown = pdc_wdt_shutdown,
};
module_platform_driver(pdc_wdt_driver);

--
2.7.4

2019-04-08 19:47:29

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 14/22] watchdog: ep93xx_wdt: Use 'dev' instead of dereferencing it repeatedly

2019-04-08 19:48:10

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 15/22] watchdog: ftwdt010_wdt: Use 'dev' consistently

Use local variable 'dev' consistently.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/ftwdt010_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/ftwdt010_wdt.c b/drivers/watchdog/ftwdt010_wdt.c
index ecb32c42e839..9ea0e56fa7ee 100644
--- a/drivers/watchdog/ftwdt010_wdt.c
+++ b/drivers/watchdog/ftwdt010_wdt.c
@@ -169,7 +169,7 @@ static int ftwdt010_wdt_probe(struct platform_device *pdev)

ret = devm_watchdog_register_device(dev, &gwdt->wdd);
if (ret) {
- dev_err(&pdev->dev, "failed to register watchdog\n");
+ dev_err(dev, "failed to register watchdog\n");
return ret;
}

--
2.7.4

2019-04-08 19:48:29

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 09/22] watchdog: da9052_wdt: Use 'dev' instead of dereferencing it repeatedly

Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

Cc: Support Opensource <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/da9052_wdt.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/da9052_wdt.c b/drivers/watchdog/da9052_wdt.c
index e263bad99574..a2feef1ff307 100644
--- a/drivers/watchdog/da9052_wdt.c
+++ b/drivers/watchdog/da9052_wdt.c
@@ -150,13 +150,13 @@ static const struct watchdog_ops da9052_wdt_ops = {

static int da9052_wdt_probe(struct platform_device *pdev)
{
- struct da9052 *da9052 = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct da9052 *da9052 = dev_get_drvdata(dev->parent);
struct da9052_wdt_data *driver_data;
struct watchdog_device *da9052_wdt;
int ret;

- driver_data = devm_kzalloc(&pdev->dev, sizeof(*driver_data),
- GFP_KERNEL);
+ driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
if (!driver_data)
return -ENOMEM;
driver_data->da9052 = da9052;
@@ -166,18 +166,17 @@ static int da9052_wdt_probe(struct platform_device *pdev)
da9052_wdt->timeout = DA9052_DEF_TIMEOUT;
da9052_wdt->info = &da9052_wdt_info;
da9052_wdt->ops = &da9052_wdt_ops;
- da9052_wdt->parent = &pdev->dev;
+ da9052_wdt->parent = dev;
watchdog_set_drvdata(da9052_wdt, driver_data);

ret = da9052_reg_update(da9052, DA9052_CONTROL_D_REG,
DA9052_CONTROLD_TWDSCALE, 0);
if (ret < 0) {
- dev_err(&pdev->dev, "Failed to disable watchdog bits, %d\n",
- ret);
+ dev_err(dev, "Failed to disable watchdog bits, %d\n", ret);
return ret;
}

- ret = devm_watchdog_register_device(&pdev->dev, &driver_data->wdt);
+ ret = devm_watchdog_register_device(dev, &driver_data->wdt);
if (ret != 0) {
dev_err(da9052->dev, "watchdog_register_device() failed: %d\n",
ret);
--
2.7.4

2019-04-08 19:48:37

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 12/22] watchdog: da9063_wdt: Use 'dev' instead of dereferencing it repeatedly

2019-04-08 19:49:08

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 10/22] watchdog: da9055_wdt: Use 'dev' instead of dereferencing it repeatedly

Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

Cc: Support Opensource <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/da9055_wdt.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/da9055_wdt.c b/drivers/watchdog/da9055_wdt.c
index 26a5b2984094..389a4bdd208c 100644
--- a/drivers/watchdog/da9055_wdt.c
+++ b/drivers/watchdog/da9055_wdt.c
@@ -119,13 +119,13 @@ static const struct watchdog_ops da9055_wdt_ops = {

static int da9055_wdt_probe(struct platform_device *pdev)
{
- struct da9055 *da9055 = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct da9055 *da9055 = dev_get_drvdata(dev->parent);
struct da9055_wdt_data *driver_data;
struct watchdog_device *da9055_wdt;
int ret;

- driver_data = devm_kzalloc(&pdev->dev, sizeof(*driver_data),
- GFP_KERNEL);
+ driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
if (!driver_data)
return -ENOMEM;

@@ -136,17 +136,17 @@ static int da9055_wdt_probe(struct platform_device *pdev)
da9055_wdt->timeout = DA9055_DEF_TIMEOUT;
da9055_wdt->info = &da9055_wdt_info;
da9055_wdt->ops = &da9055_wdt_ops;
- da9055_wdt->parent = &pdev->dev;
+ da9055_wdt->parent = dev;
watchdog_set_nowayout(da9055_wdt, nowayout);
watchdog_set_drvdata(da9055_wdt, driver_data);

ret = da9055_wdt_stop(da9055_wdt);
if (ret < 0) {
- dev_err(&pdev->dev, "Failed to stop watchdog, %d\n", ret);
+ dev_err(dev, "Failed to stop watchdog, %d\n", ret);
return ret;
}

- ret = devm_watchdog_register_device(&pdev->dev, &driver_data->wdt);
+ ret = devm_watchdog_register_device(dev, &driver_data->wdt);
if (ret != 0)
dev_err(da9055->dev, "watchdog_register_device() failed: %d\n",
ret);
--
2.7.4

2019-04-08 19:50:12

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 06/22] watchdog: bcm7038_wdt: Convert to use device managed functions and other improvements

2019-04-08 19:51:12

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 04/22] watchdog: atlas7_wdt: Convert to use device managed functions and other improvements

2019-04-08 19:51:17

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 02/22] watchdog: asm9260_wdt: Convert to use device managed functions and other improvements

Use device managed functions to simplify error handling, reduce
source code size, improve readability, and reduce the likelyhood of bugs.
Other improvements as listed below.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

- Drop assignments to otherwise unused variables
- Drop empty remove function
- Use devm_add_action_or_reset() for calls to clk_disable_unprepare
- Replace 'goto l; ... l: return e;' with 'return e;'
- Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly
- Use devm_watchdog_register_driver() to register watchdog device
- Replace shutdown function with call to watchdog_stop_on_reboot()

Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/asm9260_wdt.c | 73 +++++++++++++++++-------------------------
1 file changed, 29 insertions(+), 44 deletions(-)

diff --git a/drivers/watchdog/asm9260_wdt.c b/drivers/watchdog/asm9260_wdt.c
index 1de272bb2070..c5b9aae544dd 100644
--- a/drivers/watchdog/asm9260_wdt.c
+++ b/drivers/watchdog/asm9260_wdt.c
@@ -196,6 +196,11 @@ static const struct watchdog_ops asm9260_wdt_ops = {
.restart = asm9260_restart,
};

+static void asm9260_clk_disable_unprepare(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
static int asm9260_wdt_get_dt_clks(struct asm9260_wdt_priv *priv)
{
int err;
@@ -219,26 +224,32 @@ static int asm9260_wdt_get_dt_clks(struct asm9260_wdt_priv *priv)
dev_err(priv->dev, "Failed to enable ahb_clk!\n");
return err;
}
+ err = devm_add_action_or_reset(priv->dev,
+ asm9260_clk_disable_unprepare,
+ priv->clk_ahb);
+ if (err)
+ return err;

err = clk_set_rate(priv->clk, CLOCK_FREQ);
if (err) {
- clk_disable_unprepare(priv->clk_ahb);
dev_err(priv->dev, "Failed to set rate!\n");
return err;
}

err = clk_prepare_enable(priv->clk);
if (err) {
- clk_disable_unprepare(priv->clk_ahb);
dev_err(priv->dev, "Failed to enable clk!\n");
return err;
}
+ err = devm_add_action_or_reset(priv->dev,
+ asm9260_clk_disable_unprepare,
+ priv->clk);
+ if (err)
+ return err;

/* wdt has internal divider */
clk = clk_get_rate(priv->clk);
if (!clk) {
- clk_disable_unprepare(priv->clk);
- clk_disable_unprepare(priv->clk_ahb);
dev_err(priv->dev, "Failed, clk is 0!\n");
return -EINVAL;
}
@@ -274,23 +285,23 @@ static void asm9260_wdt_get_dt_mode(struct asm9260_wdt_priv *priv)

static int asm9260_wdt_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct asm9260_wdt_priv *priv;
struct watchdog_device *wdd;
int ret;
static const char * const mode_name[] = { "hw", "sw", "debug", };

- priv = devm_kzalloc(&pdev->dev, sizeof(struct asm9260_wdt_priv),
- GFP_KERNEL);
+ priv = devm_kzalloc(dev, sizeof(struct asm9260_wdt_priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;

- priv->dev = &pdev->dev;
+ priv->dev = dev;

priv->iobase = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(priv->iobase))
return PTR_ERR(priv->iobase);

- priv->rst = devm_reset_control_get_exclusive(&pdev->dev, "wdt_rst");
+ priv->rst = devm_reset_control_get_exclusive(dev, "wdt_rst");
if (IS_ERR(priv->rst))
return PTR_ERR(priv->rst);

@@ -303,7 +314,7 @@ static int asm9260_wdt_probe(struct platform_device *pdev)
wdd->ops = &asm9260_wdt_ops;
wdd->min_timeout = 1;
wdd->max_timeout = BM_WDTC_MAX(priv->wdt_freq);
- wdd->parent = &pdev->dev;
+ wdd->parent = dev;

watchdog_set_drvdata(wdd, priv);

@@ -313,7 +324,7 @@ static int asm9260_wdt_probe(struct platform_device *pdev)
* the max instead.
*/
wdd->timeout = ASM9260_WDT_DEFAULT_TIMEOUT;
- watchdog_init_timeout(wdd, 0, &pdev->dev);
+ watchdog_init_timeout(wdd, 0, dev);

asm9260_wdt_get_dt_mode(priv);

@@ -325,49 +336,25 @@ static int asm9260_wdt_probe(struct platform_device *pdev)
* Not all supported platforms specify an interrupt for the
* watchdog, so let's make it optional.
*/
- ret = devm_request_irq(&pdev->dev, priv->irq,
- asm9260_wdt_irq, 0, pdev->name, priv);
+ ret = devm_request_irq(dev, priv->irq, asm9260_wdt_irq, 0,
+ pdev->name, priv);
if (ret < 0)
- dev_warn(&pdev->dev, "failed to request IRQ\n");
+ dev_warn(dev, "failed to request IRQ\n");
}

watchdog_set_restart_priority(wdd, 128);

- ret = watchdog_register_device(wdd);
+ watchdog_stop_on_reboot(wdd);
+ watchdog_stop_on_unregister(wdd);
+ ret = devm_watchdog_register_device(dev, wdd);
if (ret)
- goto clk_off;
+ return ret;

platform_set_drvdata(pdev, priv);

- dev_info(&pdev->dev, "Watchdog enabled (timeout: %d sec, mode: %s)\n",
+ dev_info(dev, "Watchdog enabled (timeout: %d sec, mode: %s)\n",
wdd->timeout, mode_name[priv->mode]);
return 0;
-
-clk_off:
- clk_disable_unprepare(priv->clk);
- clk_disable_unprepare(priv->clk_ahb);
- return ret;
-}
-
-static void asm9260_wdt_shutdown(struct platform_device *pdev)
-{
- struct asm9260_wdt_priv *priv = platform_get_drvdata(pdev);
-
- asm9260_wdt_disable(&priv->wdd);
-}
-
-static int asm9260_wdt_remove(struct platform_device *pdev)
-{
- struct asm9260_wdt_priv *priv = platform_get_drvdata(pdev);
-
- asm9260_wdt_disable(&priv->wdd);
-
- watchdog_unregister_device(&priv->wdd);
-
- clk_disable_unprepare(priv->clk);
- clk_disable_unprepare(priv->clk_ahb);
-
- return 0;
}

static const struct of_device_id asm9260_wdt_of_match[] = {
@@ -382,8 +369,6 @@ static struct platform_driver asm9260_wdt_driver = {
.of_match_table = asm9260_wdt_of_match,
},
.probe = asm9260_wdt_probe,
- .remove = asm9260_wdt_remove,
- .shutdown = asm9260_wdt_shutdown,
};
module_platform_driver(asm9260_wdt_driver);

--
2.7.4

2019-04-08 21:01:26

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 11/22] watchdog: da9062_wdt: Use 'dev' instead of dereferencing it repeatedly

Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly. Also replace 'ret = func(); return ret;'
with 'return func();'.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

Cc: Support Opensource <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/da9062_wdt.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
index fe169d8e1fb2..aac749cfaccb 100644
--- a/drivers/watchdog/da9062_wdt.c
+++ b/drivers/watchdog/da9062_wdt.c
@@ -46,14 +46,9 @@ static unsigned int da9062_wdt_timeout_to_sel(unsigned int secs)

static int da9062_reset_watchdog_timer(struct da9062_watchdog *wdt)
{
- int ret;
-
- ret = regmap_update_bits(wdt->hw->regmap,
- DA9062AA_CONTROL_F,
- DA9062AA_WATCHDOG_MASK,
- DA9062AA_WATCHDOG_MASK);
-
- return ret;
+ return regmap_update_bits(wdt->hw->regmap, DA9062AA_CONTROL_F,
+ DA9062AA_WATCHDOG_MASK,
+ DA9062AA_WATCHDOG_MASK);
}

static int da9062_wdt_update_timeout_register(struct da9062_watchdog *wdt,
@@ -190,15 +185,16 @@ MODULE_DEVICE_TABLE(of, da9062_compatible_id_table);

static int da9062_wdt_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
int ret;
struct da9062 *chip;
struct da9062_watchdog *wdt;

- chip = dev_get_drvdata(pdev->dev.parent);
+ chip = dev_get_drvdata(dev->parent);
if (!chip)
return -EINVAL;

- wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+ wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
return -ENOMEM;

@@ -211,13 +207,13 @@ static int da9062_wdt_probe(struct platform_device *pdev)
wdt->wdtdev.min_hw_heartbeat_ms = DA9062_RESET_PROTECTION_MS;
wdt->wdtdev.timeout = DA9062_WDG_DEFAULT_TIMEOUT;
wdt->wdtdev.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
- wdt->wdtdev.parent = &pdev->dev;
+ wdt->wdtdev.parent = dev;

watchdog_set_restart_priority(&wdt->wdtdev, 128);

watchdog_set_drvdata(&wdt->wdtdev, wdt);

- ret = devm_watchdog_register_device(&pdev->dev, &wdt->wdtdev);
+ ret = devm_watchdog_register_device(dev, &wdt->wdtdev);
if (ret < 0) {
dev_err(wdt->hw->dev,
"watchdog registration failed (%d)\n", ret);
--
2.7.4

2019-04-08 21:04:29

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 17/22] watchdog: iTCO_wdt: Various improvements

Various coccinelle driven transformations as detailed below.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

- Use watchdog_stop_on_unregister to stop the watchdog on remove
- Drop assignments to otherwise unused variables
- Drop empty remove function

Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/iTCO_wdt.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 0a5318b7865e..89cea6ce9a08 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -545,6 +545,7 @@ 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) {
pr_err("cannot register watchdog device (err=%d)\n", ret);
@@ -557,17 +558,6 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
return 0;
}

-static int iTCO_wdt_remove(struct platform_device *pdev)
-{
- struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
-
- /* Stop the timer before we leave */
- if (!nowayout)
- iTCO_wdt_stop(&p->wddev);
-
- return 0;
-}
-
#ifdef CONFIG_PM_SLEEP
/*
* Suspend-to-idle requires this, because it stops the ticks and timekeeping, so
@@ -620,7 +610,6 @@ static const struct dev_pm_ops iTCO_wdt_pm = {

static struct platform_driver iTCO_wdt_driver = {
.probe = iTCO_wdt_probe,
- .remove = iTCO_wdt_remove,
.driver = {
.name = DRV_NAME,
.pm = ITCO_WDT_PM_OPS,
--
2.7.4

2019-04-08 21:05:31

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 16/22] watchdog: gpio_wdt: Convert to use device managed functions and other improvements

2019-04-08 21:21:27

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 07/22] watchdog: bcm_kona_wdt: Convert to use device managed functions and other improvements

2019-04-08 21:21:27

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 22/22] watchdog: max63xx_wdt: Convert to use device managed functions and other improvements

2019-04-08 21:22:01

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 08/22] watchdog: cadence_wdt: Convert to use device managed functions and other improvements

Use device managed functions to simplify error handling, reduce
source code size, improve readability, and reduce the likelyhood of bugs.
Other improvements as listed below.

The conversion was done automatically with coccinelle using the
following semantic patches. The semantic patches and the scripts
used to generate this commit log are available at
https://github.com/groeck/coccinelle-patches

- Drop assignments to otherwise unused variables
- Drop empty remove function
- Use devm_add_action_or_reset() for calls to clk_disable_unprepare
- Replace 'goto l; ... l: return e;' with 'return e;'
- Replace 'val = e; return val;' with 'return e;'
- Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly
- Use devm_watchdog_register_driver() to register watchdog device
- Replace shutdown function with call to watchdog_stop_on_reboot()

Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/cadence_wdt.c | 83 ++++++++++++++----------------------------
1 file changed, 27 insertions(+), 56 deletions(-)

diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c
index 5986f18f97e2..c5051907df00 100644
--- a/drivers/watchdog/cadence_wdt.c
+++ b/drivers/watchdog/cadence_wdt.c
@@ -274,6 +274,11 @@ static const struct watchdog_ops cdns_wdt_ops = {
.set_timeout = cdns_wdt_settimeout,
};

+static void cdns_clk_disable_unprepare(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
/************************Platform Operations*****************************/
/**
* cdns_wdt_probe - Probe call for the device.
@@ -285,12 +290,13 @@ static const struct watchdog_ops cdns_wdt_ops = {
*/
static int cdns_wdt_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
int ret, irq;
unsigned long clock_f;
struct cdns_wdt *wdt;
struct watchdog_device *cdns_wdt_device;

- wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+ wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
return -ENOMEM;

@@ -306,13 +312,13 @@ static int cdns_wdt_probe(struct platform_device *pdev)
return PTR_ERR(wdt->regs);

/* Register the interrupt */
- wdt->rst = of_property_read_bool(pdev->dev.of_node, "reset-on-timeout");
+ wdt->rst = of_property_read_bool(dev->of_node, "reset-on-timeout");
irq = platform_get_irq(pdev, 0);
if (!wdt->rst && irq >= 0) {
- ret = devm_request_irq(&pdev->dev, irq, cdns_wdt_irq_handler, 0,
+ ret = devm_request_irq(dev, irq, cdns_wdt_irq_handler, 0,
pdev->name, pdev);
if (ret) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"cannot register interrupt handler err=%d\n",
ret);
return ret;
@@ -320,11 +326,11 @@ static int cdns_wdt_probe(struct platform_device *pdev)
}

/* Initialize the members of cdns_wdt structure */
- cdns_wdt_device->parent = &pdev->dev;
+ cdns_wdt_device->parent = dev;

- ret = watchdog_init_timeout(cdns_wdt_device, wdt_timeout, &pdev->dev);
+ ret = watchdog_init_timeout(cdns_wdt_device, wdt_timeout, dev);
if (ret) {
- dev_err(&pdev->dev, "unable to set timeout value\n");
+ dev_err(dev, "unable to set timeout value\n");
return ret;
}

@@ -332,18 +338,21 @@ static int cdns_wdt_probe(struct platform_device *pdev)
watchdog_stop_on_reboot(cdns_wdt_device);
watchdog_set_drvdata(cdns_wdt_device, wdt);

- wdt->clk = devm_clk_get(&pdev->dev, NULL);
+ wdt->clk = devm_clk_get(dev, NULL);
if (IS_ERR(wdt->clk)) {
- dev_err(&pdev->dev, "input clock not found\n");
- ret = PTR_ERR(wdt->clk);
- return ret;
+ dev_err(dev, "input clock not found\n");
+ return PTR_ERR(wdt->clk);
}

ret = clk_prepare_enable(wdt->clk);
if (ret) {
- dev_err(&pdev->dev, "unable to enable clock\n");
+ dev_err(dev, "unable to enable clock\n");
return ret;
}
+ ret = devm_add_action_or_reset(dev, cdns_clk_disable_unprepare,
+ wdt->clk);
+ if (ret)
+ return ret;

clock_f = clk_get_rate(wdt->clk);
if (clock_f <= CDNS_WDT_CLK_75MHZ) {
@@ -356,56 +365,20 @@ static int cdns_wdt_probe(struct platform_device *pdev)

spin_lock_init(&wdt->io_lock);

- ret = watchdog_register_device(cdns_wdt_device);
+ watchdog_stop_on_reboot(cdns_wdt_device);
+ watchdog_stop_on_unregister(cdns_wdt_device);
+ ret = devm_watchdog_register_device(dev, cdns_wdt_device);
if (ret) {
- dev_err(&pdev->dev, "Failed to register wdt device\n");
- goto err_clk_disable;
+ dev_err(dev, "Failed to register wdt device\n");
+ return ret;
}
platform_set_drvdata(pdev, wdt);

- dev_info(&pdev->dev, "Xilinx Watchdog Timer at %p with timeout %ds%s\n",
+ dev_info(dev, "Xilinx Watchdog Timer at %p with timeout %ds%s\n",
wdt->regs, cdns_wdt_device->timeout,
nowayout ? ", nowayout" : "");

return 0;
-
-err_clk_disable:
- clk_disable_unprepare(wdt->clk);
-
- return ret;
-}
-
-/**
- * cdns_wdt_remove - Probe call for the device.
- *
- * @pdev: handle to the platform device structure.
- * Return: 0 on success, otherwise negative error.
- *
- * Unregister the device after releasing the resources.
- */
-static int cdns_wdt_remove(struct platform_device *pdev)
-{
- struct cdns_wdt *wdt = platform_get_drvdata(pdev);
-
- cdns_wdt_stop(&wdt->cdns_wdt_device);
- watchdog_unregister_device(&wdt->cdns_wdt_device);
- clk_disable_unprepare(wdt->clk);
-
- return 0;
-}
-
-/**
- * cdns_wdt_shutdown - Stop the device.
- *
- * @pdev: handle to the platform structure.
- *
- */
-static void cdns_wdt_shutdown(struct platform_device *pdev)
-{
- struct cdns_wdt *wdt = platform_get_drvdata(pdev);
-
- cdns_wdt_stop(&wdt->cdns_wdt_device);
- clk_disable_unprepare(wdt->clk);
}

/**
@@ -460,8 +433,6 @@ MODULE_DEVICE_TABLE(of, cdns_wdt_of_match);
/* Driver Structure */
static struct platform_driver cdns_wdt_driver = {
.probe = cdns_wdt_probe,
- .remove = cdns_wdt_remove,
- .shutdown = cdns_wdt_shutdown,
.driver = {
.name = "cdns-wdt",
.of_match_table = cdns_wdt_of_match,
--
2.7.4

2019-04-09 00:45:41

by Andrew Jeffery

[permalink] [raw]

Subject: Re: [PATCH 03/22] watchdog: aspeed_wdt: Use 'dev' instead of dereferencing it repeatedly

On Tue, 9 Apr 2019, at 05:09, Guenter Roeck wrote:
> Introduce local variable 'struct device *dev' and use it instead of
> dereferencing it repeatedly.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts
> used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> Cc: Joel Stanley <[email protected]>
> Cc: Andrew Jeffery <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/aspeed_wdt.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index f09333fd54b4..34117745c65f 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -187,6 +187,7 @@ static const struct watchdog_info aspeed_wdt_info = {
>
> static int aspeed_wdt_probe(struct platform_device *pdev)
> {
> + struct device *dev = &pdev->dev;
> const struct aspeed_wdt_config *config;
> const struct of_device_id *ofdid;
> struct aspeed_wdt *wdt;
> @@ -196,7 +197,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> u32 status;
> int ret;
>
> - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> + wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
> if (!wdt)
> return -ENOMEM;

Looks like it's missed this one somehow?

wdt->base = devm_ioremap_resource(&pdev->dev, res);

Otherwise,

Reviewed-by: Andrew Jeffery <[email protected]>

>
> @@ -212,12 +213,12 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> wdt->wdd.info = &aspeed_wdt_info;
> wdt->wdd.ops = &aspeed_wdt_ops;
> wdt->wdd.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT_MS;
> - wdt->wdd.parent = &pdev->dev;
> + wdt->wdd.parent = dev;
>
> wdt->wdd.timeout = WDT_DEFAULT_TIMEOUT;
> - watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
> + watchdog_init_timeout(&wdt->wdd, 0, dev);
>
> - np = pdev->dev.of_node;
> + np = dev->of_node;
>
> ofdid = of_match_node(aspeed_wdt_of_table, np);
> if (!ofdid)
> @@ -286,11 +287,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> u32 max_duration = config->ext_pulse_width_mask + 1;
>
> if (duration == 0 || duration > max_duration) {
> - dev_err(&pdev->dev, "Invalid pulse duration: %uus\n",
> - duration);
> + dev_err(dev, "Invalid pulse duration: %uus\n",
> + duration);
> duration = max(1U, min(max_duration, duration));
> - dev_info(&pdev->dev, "Pulse duration set to %uus\n",
> - duration);
> + dev_info(dev, "Pulse duration set to %uus\n",
> + duration);
> }
>
> /*
> @@ -312,9 +313,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
> wdt->wdd.bootstatus = WDIOF_CARDRESET;
>
> - ret = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
> + ret = devm_watchdog_register_device(dev, &wdt->wdd);
> if (ret) {
> - dev_err(&pdev->dev, "failed to register\n");
> + dev_err(dev, "failed to register\n");
> return ret;
> }
>
> --
> 2.7.4
>
>

2019-04-09 01:07:11

by Guenter Roeck

[permalink] [raw]

Subject: Re: [PATCH 03/22] watchdog: aspeed_wdt: Use 'dev' instead of dereferencing it repeatedly

Hi Andrew,

On 4/8/19 5:37 PM, Andrew Jeffery wrote:
>
>
> On Tue, 9 Apr 2019, at 05:09, Guenter Roeck wrote:
>> Introduce local variable 'struct device *dev' and use it instead of
>> dereferencing it repeatedly.
>>
>> The conversion was done automatically with coccinelle using the
>> following semantic patches. The semantic patches and the scripts
>> used to generate this commit log are available at
>> https://github.com/groeck/coccinelle-patches
>>
>> Cc: Joel Stanley <[email protected]>
>> Cc: Andrew Jeffery <[email protected]>
>> Signed-off-by: Guenter Roeck <[email protected]>
>> ---
>> drivers/watchdog/aspeed_wdt.c | 21 +++++++++++----------
>> 1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
>> index f09333fd54b4..34117745c65f 100644
>> --- a/drivers/watchdog/aspeed_wdt.c
>> +++ b/drivers/watchdog/aspeed_wdt.c
>> @@ -187,6 +187,7 @@ static const struct watchdog_info aspeed_wdt_info = {
>>
>> static int aspeed_wdt_probe(struct platform_device *pdev)
>> {
>> + struct device *dev = &pdev->dev;
>> const struct aspeed_wdt_config *config;
>> const struct of_device_id *ofdid;
>> struct aspeed_wdt *wdt;
>> @@ -196,7 +197,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>> u32 status;
>> int ret;
>>
>> - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>> + wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
>> if (!wdt)
>> return -ENOMEM;
>
> Looks like it's missed this one somehow?
>
> wdt->base = devm_ioremap_resource(&pdev->dev, res);
>
> Otherwise,
>
> Reviewed-by: Andrew Jeffery <[email protected]>
>

Assuming you mean the conversion to use devm_platform_ioremap_resource(),
I had this already addressed with a single patch for all watchdog drivers.

https://patchwork.kernel.org/patch/10882207/

Sorry, I didn't Cc: you on that one - I limited the number of Cc:s to avoid being
tagged as spammer.

Guenter

2019-04-09 01:08:33

by Andrew Jeffery

[permalink] [raw]

Subject: Re: [PATCH 03/22] watchdog: aspeed_wdt: Use 'dev' instead of dereferencing it repeatedly

On Tue, 9 Apr 2019, at 10:33, Guenter Roeck wrote:
> Hi Andrew,
>
> On 4/8/19 5:37 PM, Andrew Jeffery wrote:
> >
> >
> > On Tue, 9 Apr 2019, at 05:09, Guenter Roeck wrote:
> >> Introduce local variable 'struct device *dev' and use it instead of
> >> dereferencing it repeatedly.
> >>
> >> The conversion was done automatically with coccinelle using the
> >> following semantic patches. The semantic patches and the scripts
> >> used to generate this commit log are available at
> >> https://github.com/groeck/coccinelle-patches
> >>
> >> Cc: Joel Stanley <[email protected]>
> >> Cc: Andrew Jeffery <[email protected]>
> >> Signed-off-by: Guenter Roeck <[email protected]>
> >> ---
> >> drivers/watchdog/aspeed_wdt.c | 21 +++++++++++----------
> >> 1 file changed, 11 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> >> index f09333fd54b4..34117745c65f 100644
> >> --- a/drivers/watchdog/aspeed_wdt.c
> >> +++ b/drivers/watchdog/aspeed_wdt.c
> >> @@ -187,6 +187,7 @@ static const struct watchdog_info aspeed_wdt_info = {
> >>
> >> static int aspeed_wdt_probe(struct platform_device *pdev)
> >> {
> >> + struct device *dev = &pdev->dev;
> >> const struct aspeed_wdt_config *config;
> >> const struct of_device_id *ofdid;
> >> struct aspeed_wdt *wdt;
> >> @@ -196,7 +197,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> >> u32 status;
> >> int ret;
> >>
> >> - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> >> + wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
> >> if (!wdt)
> >> return -ENOMEM;
> >
> > Looks like it's missed this one somehow?
> >
> > wdt->base = devm_ioremap_resource(&pdev->dev, res);
> >
> > Otherwise,
> >
> > Reviewed-by: Andrew Jeffery <[email protected]>
> >
>
> Assuming you mean the conversion to use devm_platform_ioremap_resource(),
> I had this already addressed with a single patch for all watchdog drivers.
>
> https://patchwork.kernel.org/patch/10882207/
>
> Sorry, I didn't Cc: you on that one - I limited the number of Cc:s to
> avoid being
> tagged as spammer.

That's the piece I was missing.

Thanks!

Andrew

2019-04-09 06:55:44

by Stefan Wahren

[permalink] [raw]

Subject: Re: [PATCH 05/22] watchdog: bcm2835_wdt: drop platform_set_drvdata

Am 08.04.19 um 21:38 schrieb Guenter Roeck:
> There is no call to platform_get_drvdata() in the driver,
> so platform_set_drvdata() is unnecessary and can be dropped.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts
> used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> Cc: Florian Fainelli <[email protected]>
> Cc: Ray Jui <[email protected]>
> Cc: Scott Branden <[email protected]>
> Cc: [email protected]
> Cc: Eric Anholt <[email protected]>
> Cc: Stefan Wahren <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>

Acked-by: Stefan Wahren <[email protected]>

Thanks

2019-04-09 16:37:19

by Ray Jui

[permalink] [raw]

Subject: Re: [PATCH 07/22] watchdog: bcm_kona_wdt: Convert to use device managed functions and other improvements

On 4/8/2019 12:38 PM, Guenter Roeck wrote:
> Use device managed functions to simplify error handling, reduce
> source code size, improve readability, and reduce the likelyhood of bugs.
> Other improvements as listed below.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts
> used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> - Use local variable 'struct device *dev' consistently
> - Use devm_watchdog_register_driver() to register watchdog device
> - Replace shutdown function with call to watchdog_stop_on_reboot()
>
> Cc: Florian Fainelli <[email protected]>
> Cc: Ray Jui <[email protected]>
> Cc: Scott Branden <[email protected]>
> Cc: [email protected]
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/bcm_kona_wdt.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/watchdog/bcm_kona_wdt.c b/drivers/watchdog/bcm_kona_wdt.c
> index d52334ab0805..e2ad44816359 100644
> --- a/drivers/watchdog/bcm_kona_wdt.c
> +++ b/drivers/watchdog/bcm_kona_wdt.c
> @@ -271,11 +271,6 @@ static struct watchdog_device bcm_kona_wdt_wdd = {
> .timeout = SECWDOG_MAX_COUNT >> SECWDOG_DEFAULT_RESOLUTION,
> };
>
> -static void bcm_kona_wdt_shutdown(struct platform_device *pdev)
> -{
> - bcm_kona_wdt_stop(&bcm_kona_wdt_wdd);
> -}
> -
> static int bcm_kona_wdt_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -301,7 +296,7 @@ static int bcm_kona_wdt_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, wdt);
> watchdog_set_drvdata(&bcm_kona_wdt_wdd, wdt);
> - bcm_kona_wdt_wdd.parent = &pdev->dev;
> + bcm_kona_wdt_wdd.parent = dev;
>
> ret = bcm_kona_wdt_set_timeout_reg(&bcm_kona_wdt_wdd, 0);
> if (ret) {
> @@ -309,7 +304,9 @@ static int bcm_kona_wdt_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = watchdog_register_device(&bcm_kona_wdt_wdd);
> + watchdog_stop_on_reboot(&bcm_kona_wdt_wdd);
> + watchdog_stop_on_unregister(&bcm_kona_wdt_wdd);
> + ret = devm_watchdog_register_device(dev, &bcm_kona_wdt_wdd);
> if (ret) {
> dev_err(dev, "Failed to register watchdog device");
> return ret;
> @@ -324,8 +321,6 @@ static int bcm_kona_wdt_probe(struct platform_device *pdev)
> static int bcm_kona_wdt_remove(struct platform_device *pdev)
> {
> bcm_kona_wdt_debug_exit(pdev);
> - bcm_kona_wdt_shutdown(pdev);
> - watchdog_unregister_device(&bcm_kona_wdt_wdd);
> dev_dbg(&pdev->dev, "Watchdog driver disabled");
>
> return 0;
> @@ -344,7 +339,6 @@ static struct platform_driver bcm_kona_wdt_driver = {
> },
> .probe = bcm_kona_wdt_probe,
> .remove = bcm_kona_wdt_remove,
> - .shutdown = bcm_kona_wdt_shutdown,
> };
>
> module_platform_driver(bcm_kona_wdt_driver);
>

Change looks good to me. Thanks!

Reviewed-by: Ray Jui <[email protected]>

2019-04-10 13:14:25

by Steve Twiss

[permalink] [raw]

Subject: RE: [PATCH 09/22] watchdog: da9052_wdt: Use 'dev' instead of dereferencing it repeatedly

On 08 April 2019 20:39, Guenter Roeck wrote:

> Subject: [PATCH 09/22] watchdog: da9052_wdt: Use 'dev' instead of
> dereferencing it repeatedly
>
> Introduce local variable 'struct device *dev' and use it instead of
> dereferencing it repeatedly.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts
> used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> Cc: Support Opensource <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>

Acked-by: Steve Twiss <[email protected]>

Thanks!
Regards,
Steve

2019-04-10 13:18:23

by Steve Twiss

[permalink] [raw]

Subject: RE: [PATCH 11/22] watchdog: da9062_wdt: Use 'dev' instead of dereferencing it repeatedly

Hi Guenter,

On 08 April 2019 20:39, Guenter Roeck wrote:

> Subject: [PATCH 11/22] watchdog: da9062_wdt: Use 'dev' instead of
> dereferencing it repeatedly
>
> Introduce local variable 'struct device *dev' and use it instead of
> dereferencing it repeatedly. Also replace 'ret = func(); return ret;'
> with 'return func();'.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts
> used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> Cc: Support Opensource <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>

Acked-by: Steve Twiss <[email protected]>

Thanks for that,
Regards,
Steve

2019-04-10 13:40:12

by Steve Twiss

[permalink] [raw]

Subject: RE: [PATCH 12/22] watchdog: da9063_wdt: Use 'dev' instead of dereferencing it repeatedly

Hi Guenter,

On 08 April 2019 20:39, Guenter Roeck:

> Subject: [PATCH 12/22] watchdog: da9063_wdt: Use 'dev' instead of
> dereferencing it repeatedly
>
> Introduce local variable 'struct device *dev' and use it instead of
> dereferencing it repeatedly.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts
> used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> Cc: Support Opensource <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/da9063_wdt.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
> index 384dca16af8b..06eb9070203c 100644
> --- a/drivers/watchdog/da9063_wdt.c
> +++ b/drivers/watchdog/da9063_wdt.c
> @@ -188,17 +188,18 @@ static const struct watchdog_ops
> da9063_watchdog_ops = {
>
> static int da9063_wdt_probe(struct platform_device *pdev)
> {
> + struct device *dev = &pdev->dev;
> struct da9063 *da9063;
> struct watchdog_device *wdd;
>
> - if (!pdev->dev.parent)
> + if (!dev->parent)
> return -EINVAL;

None of my previous Acked e-mails in this patch set considered whether the
dev->parent was NULL. But this DA9063 driver does.

Logically, this is correct to check, but ... any thoughts?
Otherwise,

Acked-by: Steve Twiss <[email protected]>

Regards,
Steve

2019-04-10 15:40:36

by Steve Twiss

[permalink] [raw]

Subject: RE: [PATCH 10/22] watchdog: da9055_wdt: Use 'dev' instead of dereferencing it repeatedly

On 08 April 2019 20:39, Guenter Roeck wrote:

> Subject: [PATCH 10/22] watchdog: da9055_wdt: Use 'dev' instead of
> dereferencing it repeatedly
>
> Introduce local variable 'struct device *dev' and use it instead of
> dereferencing it repeatedly.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts
> used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> Cc: Support Opensource <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>

Acked-by: Steve Twiss <[email protected]>

Regards,
Steve

2019-04-10 16:19:26

by Guenter Roeck

[permalink] [raw]

Subject: Re: [PATCH 12/22] watchdog: da9063_wdt: Use 'dev' instead of dereferencing it repeatedly

Hi Steve,

On 4/10/19 5:50 AM, Steve Twiss wrote:
> Hi Guenter,
>
> On 08 April 2019 20:39, Guenter Roeck:
>
>> Subject: [PATCH 12/22] watchdog: da9063_wdt: Use 'dev' instead of
>> dereferencing it repeatedly
>>
>> Introduce local variable 'struct device *dev' and use it instead of
>> dereferencing it repeatedly.
>>
>> The conversion was done automatically with coccinelle using the
>> following semantic patches. The semantic patches and the scripts
>> used to generate this commit log are available at
>> https://github.com/groeck/coccinelle-patches
>>
>> Cc: Support Opensource <[email protected]>
>> Signed-off-by: Guenter Roeck <[email protected]>
>> ---
>> drivers/watchdog/da9063_wdt.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
>> index 384dca16af8b..06eb9070203c 100644
>> --- a/drivers/watchdog/da9063_wdt.c
>> +++ b/drivers/watchdog/da9063_wdt.c
>> @@ -188,17 +188,18 @@ static const struct watchdog_ops
>> da9063_watchdog_ops = {
>>
>> static int da9063_wdt_probe(struct platform_device *pdev)
>> {
>> + struct device *dev = &pdev->dev;
>> struct da9063 *da9063;
>> struct watchdog_device *wdd;
>>
>> - if (!pdev->dev.parent)
>> + if (!dev->parent)
>> return -EINVAL;
>
> None of my previous Acked e-mails in this patch set considered whether the
> dev->parent was NULL. But this DA9063 driver does.
>
> Logically, this is correct to check, but ... any thoughts?

The check is not really necessary. All da90xx drivers are instantiated from
mfd drivers and do provide a parent. Anyone changing that code or trying
to instantiate the drivers from some other place without providing a parent
really deserves the resulting crash (it would be a bug).

Either case, I don't think this warrants changing this driver to drop
the check, or changing the other drivers to add unnecessary checks
just to make the code consistent.

> Otherwise,
>
> Acked-by: Steve Twiss <[email protected]>
>

Thanks,
Guenter