[permalink] [raw]

Subject: [PATCH 19/23] watchdog: st_lpc_wdt: Convert to use device managed functions

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

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 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

Cc: Patrice Chotard <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/st_lpc_wdt.c | 47 ++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/watchdog/st_lpc_wdt.c b/drivers/watchdog/st_lpc_wdt.c
index 196fb4b72c5d..9a5ed95c3403 100644
--- a/drivers/watchdog/st_lpc_wdt.c
+++ b/drivers/watchdog/st_lpc_wdt.c
@@ -142,10 +142,16 @@ static struct watchdog_device st_wdog_dev = {
.ops = &st_wdog_ops,
};

+static void st_clk_disable_unprepare(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
static int st_wdog_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
const struct of_device_id *match;
- struct device_node *np = pdev->dev.of_node;
+ struct device_node *np = dev->of_node;
struct st_wdog *st_wdog;
struct regmap *regmap;
struct clk *clk;
@@ -155,7 +161,7 @@ static int st_wdog_probe(struct platform_device *pdev)

ret = of_property_read_u32(np, "st,lpc-mode", &mode);
if (ret) {
- dev_err(&pdev->dev, "An LPC mode must be provided\n");
+ dev_err(dev, "An LPC mode must be provided\n");
return -EINVAL;
}

@@ -163,13 +169,13 @@ static int st_wdog_probe(struct platform_device *pdev)
if (mode != ST_LPC_MODE_WDT)
return -ENODEV;

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

- match = of_match_device(st_wdog_match, &pdev->dev);
+ match = of_match_device(st_wdog_match, dev);
if (!match) {
- dev_err(&pdev->dev, "Couldn't match device\n");
+ dev_err(dev, "Couldn't match device\n");
return -ENODEV;
}
st_wdog->syscfg = (struct st_wdog_syscfg *)match->data;
@@ -180,17 +186,17 @@ static int st_wdog_probe(struct platform_device *pdev)

regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
if (IS_ERR(regmap)) {
- dev_err(&pdev->dev, "No syscfg phandle specified\n");
+ dev_err(dev, "No syscfg phandle specified\n");
return PTR_ERR(regmap);
}

- clk = devm_clk_get(&pdev->dev, NULL);
+ clk = devm_clk_get(dev, NULL);
if (IS_ERR(clk)) {
- dev_err(&pdev->dev, "Unable to request clock\n");
+ dev_err(dev, "Unable to request clock\n");
return PTR_ERR(clk);
}

- st_wdog->dev = &pdev->dev;
+ st_wdog->dev = dev;
st_wdog->base = base;
st_wdog->clk = clk;
st_wdog->regmap = regmap;
@@ -198,39 +204,40 @@ static int st_wdog_probe(struct platform_device *pdev)
st_wdog->clkrate = clk_get_rate(st_wdog->clk);

if (!st_wdog->clkrate) {
- dev_err(&pdev->dev, "Unable to fetch clock rate\n");
+ dev_err(dev, "Unable to fetch clock rate\n");
return -EINVAL;
}
st_wdog_dev.max_timeout = 0xFFFFFFFF / st_wdog->clkrate;
- st_wdog_dev.parent = &pdev->dev;
+ st_wdog_dev.parent = dev;

ret = clk_prepare_enable(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, st_clk_disable_unprepare, clk);
+ if (ret)
+ return ret;

watchdog_set_drvdata(&st_wdog_dev, st_wdog);
watchdog_set_nowayout(&st_wdog_dev, WATCHDOG_NOWAYOUT);

/* Init Watchdog timeout with value in DT */
- ret = watchdog_init_timeout(&st_wdog_dev, 0, &pdev->dev);
+ ret = watchdog_init_timeout(&st_wdog_dev, 0, dev);
if (ret) {
- dev_err(&pdev->dev, "Unable to initialise watchdog timeout\n");
- clk_disable_unprepare(clk);
+ dev_err(dev, "Unable to initialise watchdog timeout\n");
return ret;
}

- ret = watchdog_register_device(&st_wdog_dev);
+ ret = devm_watchdog_register_device(dev, &st_wdog_dev);
if (ret) {
- dev_err(&pdev->dev, "Unable to register watchdog\n");
- clk_disable_unprepare(clk);
+ dev_err(dev, "Unable to register watchdog\n");
return ret;
}

st_wdog_setup(st_wdog, true);

- dev_info(&pdev->dev, "LPC Watchdog driver registered, reset type is %s",
+ dev_info(dev, "LPC Watchdog driver registered, reset type is %s",
st_wdog->warm_reset ? "warm" : "cold");

return ret;
@@ -241,8 +248,6 @@ static int st_wdog_remove(struct platform_device *pdev)
struct st_wdog *st_wdog = watchdog_get_drvdata(&st_wdog_dev);

st_wdog_setup(st_wdog, false);
- watchdog_unregister_device(&st_wdog_dev);
- clk_disable_unprepare(st_wdog->clk);

return 0;
}
--
2.7.4

2019-04-09 17:27:36

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 16/23] watchdog: sama5d4_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
- Introduce local variable 'struct device *dev' and use it instead of
dereferencing it repeatedly
- Replace stop on remove with call to watchdog_stop_on_unregister()
- Use devm_watchdog_register_driver() to register watchdog device

Cc: Nicolas Ferre <[email protected]>
Cc: Alexandre Belloni <[email protected]>
Cc: Ludovic Desroches <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/sama5d4_wdt.c | 35 ++++++++++++-----------------------
1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index ea72fa0aa3ec..111695223aae 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -199,6 +199,7 @@ static int sama5d4_wdt_init(struct sama5d4_wdt *wdt)

static int sama5d4_wdt_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct watchdog_device *wdd;
struct sama5d4_wdt *wdt;
void __iomem *regs;
@@ -206,7 +207,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
u32 timeout;
int ret;

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

@@ -226,26 +227,25 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)

wdt->reg_base = regs;

- irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+ irq = irq_of_parse_and_map(dev->of_node, 0);
if (!irq)
- dev_warn(&pdev->dev, "failed to get IRQ from DT\n");
+ dev_warn(dev, "failed to get IRQ from DT\n");

- ret = of_sama5d4_wdt_init(pdev->dev.of_node, wdt);
+ ret = of_sama5d4_wdt_init(dev->of_node, wdt);
if (ret)
return ret;

if ((wdt->mr & AT91_WDT_WDFIEN) && irq) {
- ret = devm_request_irq(&pdev->dev, irq, sama5d4_wdt_irq_handler,
+ ret = devm_request_irq(dev, irq, sama5d4_wdt_irq_handler,
IRQF_SHARED | IRQF_IRQPOLL |
IRQF_NO_SUSPEND, pdev->name, pdev);
if (ret) {
- dev_err(&pdev->dev,
- "cannot register interrupt handler\n");
+ dev_err(dev, "cannot register interrupt handler\n");
return ret;
}
}

- watchdog_init_timeout(wdd, wdt_timeout, &pdev->dev);
+ watchdog_init_timeout(wdd, wdt_timeout, dev);

timeout = WDT_SEC2TICKS(wdd->timeout);

@@ -258,31 +258,21 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)

watchdog_set_nowayout(wdd, nowayout);

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

platform_set_drvdata(pdev, wdt);

- dev_info(&pdev->dev, "initialized (timeout = %d sec, nowayout = %d)\n",
+ dev_info(dev, "initialized (timeout = %d sec, nowayout = %d)\n",
wdd->timeout, nowayout);

return 0;
}

-static int sama5d4_wdt_remove(struct platform_device *pdev)
-{
- struct sama5d4_wdt *wdt = platform_get_drvdata(pdev);
-
- sama5d4_wdt_stop(&wdt->wdd);
-
- watchdog_unregister_device(&wdt->wdd);
-
- return 0;
-}
-
static const struct of_device_id sama5d4_wdt_of_match[] = {
{ .compatible = "atmel,sama5d4-wdt", },
{ }
@@ -310,7 +300,6 @@ static SIMPLE_DEV_PM_OPS(sama5d4_wdt_pm_ops, NULL,

static struct platform_driver sama5d4_wdt_driver = {
.probe = sama5d4_wdt_probe,
- .remove = sama5d4_wdt_remove,
.driver = {
.name = "sama5d4_wdt",
.pm = &sama5d4_wdt_pm_ops,
--
2.7.4

2019-04-09 17:27:38

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 09/23] watchdog: npcm_wdt: Use local variable 'dev' consistently

Use local variable 'struct device *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

Cc: Avi Fishman <[email protected]>
Cc: Tomer Maimon <[email protected]>
Cc: Patrick Venture <[email protected]>
Cc: Nancy Yuen <[email protected]>
Cc: Brendan Higgins <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/npcm_wdt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index 4fce10c145c2..9d6c1689b12c 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -184,7 +184,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
int irq;
int ret;

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

@@ -214,8 +214,8 @@ static int npcm_wdt_probe(struct platform_device *pdev)
set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
}

- ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0,
- "watchdog", wdt);
+ ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0, "watchdog",
+ wdt);
if (ret)
return ret;

--
2.7.4

2019-04-09 17:28:18

by Guenter Roeck

[permalink] [raw]

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

2019-04-09 17:28:21

by Guenter Roeck

[permalink] [raw]

Subject: [PATCH 15/23] watchdog: rza_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/rza_wdt.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/rza_wdt.c b/drivers/watchdog/rza_wdt.c
index b854f0aeb3ef..7b6c365f7cd3 100644
--- a/drivers/watchdog/rza_wdt.c
+++ b/drivers/watchdog/rza_wdt.c
@@ -166,11 +166,12 @@ static const struct watchdog_ops rza_wdt_ops = {

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(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;

@@ -178,21 +179,21 @@ static int rza_wdt_probe(struct platform_device *pdev)
if (IS_ERR(priv->base))
return PTR_ERR(priv->base);

- priv->clk = devm_clk_get(&pdev->dev, NULL);
+ priv->clk = devm_clk_get(dev, NULL);
if (IS_ERR(priv->clk))
return PTR_ERR(priv->clk);

rate = clk_get_rate(priv->clk);
if (rate < 16384) {
- dev_err(&pdev->dev, "invalid clock rate (%ld)\n", rate);
+ dev_err(dev, "invalid clock rate (%ld)\n", rate);
return -ENOENT;
}

priv->wdev.info = &rza_wdt_ident,
priv->wdev.ops = &rza_wdt_ops,
- priv->wdev.parent = &pdev->dev;
+ priv->wdev.parent = dev;

- priv->cks = (u8)(uintptr_t)of_device_get_match_data(&pdev->dev);
+ priv->cks = (u8)(uintptr_t) of_device_get_match_data(dev);
if (priv->cks == CKS_4BIT) {
/* Assume slowest clock rate possible (CKS=0xF) */
priv->wdev.max_timeout = (DIVIDER_4BIT * U8_MAX) / rate;
@@ -207,19 +208,19 @@ static int rza_wdt_probe(struct platform_device *pdev)
* max_hw_heartbeat_ms.
*/
priv->wdev.max_hw_heartbeat_ms = (1000 * U8_MAX) / rate;
- dev_dbg(&pdev->dev, "max hw timeout of %dms\n",
- priv->wdev.max_hw_heartbeat_ms);
+ dev_dbg(dev, "max hw timeout of %dms\n",
+ priv->wdev.max_hw_heartbeat_ms);
}

priv->wdev.min_timeout = 1;
priv->wdev.timeout = DEFAULT_TIMEOUT;

- watchdog_init_timeout(&priv->wdev, 0, &pdev->dev);
+ watchdog_init_timeout(&priv->wdev, 0, dev);
watchdog_set_drvdata(&priv->wdev, priv);

- ret = devm_watchdog_register_device(&pdev->dev, &priv->wdev);
+ ret = devm_watchdog_register_device(dev, &priv->wdev);
if (ret)
- dev_err(&pdev->dev, "Cannot register watchdog device\n");
+ dev_err(dev, "Cannot register watchdog device\n");

return ret;
}
--
2.7.4

2019-04-09 17:28:35

by Guenter Roeck

[permalink] [raw]

[permalink] [raw]

Subject: Re: [PATCH 23/23] watchdog: tangox_wdt: Convert to use device managed functions and other improvements

On 10/04/2019 15:37, Guenter Roeck wrote:

> On 4/10/19 6:04 AM, Marc Gonzalez wrote:
>
>> On 09/04/2019 19:24, Guenter Roeck wrote:
>>
>>> Use device managed functions to simplify error handling, reduce
>>> source code size, improve readability, and reduce the likelihood 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 unnecessary braces around conditional return statements
>>> - Drop empty remove function
>>> - Use devm_add_action_or_reset() for calls to clk_disable_unprepare
>>> - Replace stop on remove with call to watchdog_stop_on_unregister()
>>> - Use devm_watchdog_register_driver() to register watchdog device
>>
>> No devm_clk_prepare() in mainline? :-(
>>
>> https://lore.kernel.org/patchwork/patch/755487/
>
> We went through that several times and never succeeded. This was the major
> reason why I didn't submit this series earlier since I was hoping for it
> to appear at some point. Unfortunately, someone always objected, typically
> with comments along the line that it could be misused, or citing individual
> examples where the current code in some driver is wrong and should be fixed
> instead.
>
> This isn't really a technical argument: Everything can be misused, and all
> code has bugs. Neither is a reason to reject a new useful API. As such, one
> has to assume that after refuting such arguments, and even after fixing all
> bugs in existing code, the opponents of the new API will come up with other
> reasons to reject it.
>
> At the end, I gave up trying. Feel free to try yourself; I most definitely
> won't try it anymore. Using devm_add_action_or_reset() is a bit more clumsy,
> but works just as well.

Hello Guenter,

I am saddened to read about your frustration. I might give it a try, if Dmitry
doesn't feel like giving it another shot.

Regards.

2019-04-11 07:20:36

by Johannes Thumshirn

[permalink] [raw]

Subject: Re: [PATCH 02/23] watchdog: mena21_wdt: Use 'dev' instead of dereferencing it repeatedly

On 09/04/2019 19:23, 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: Johannes Thumshirn <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---

FWIW,
Reviewed-by: Johannes Thumshirn <[email protected]>