Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions

From: Marc Gonzalez
Date: Wed Jan 11 2017 - 04:07:24 EST


On 11/01/2017 03:09, Guenter Roeck wrote:

> 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
> - Replace 'goto l; ... l: return e;' with 'return e;'
> - Replace 'val = e; return val;' with 'return e;'
> - Replace 'if (e) { return expr; }' with 'if (e) return expr;'
> - Use devm_watchdog_register_driver() to register watchdog device
>
> Cc: Marc Gonzalez <marc_gonzalez@xxxxxxxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
> drivers/watchdog/tangox_wdt.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/watchdog/tangox_wdt.c b/drivers/watchdog/tangox_wdt.c
> index d5fcce062920..7688e1b35867 100644
> --- a/drivers/watchdog/tangox_wdt.c
> +++ b/drivers/watchdog/tangox_wdt.c
> @@ -134,12 +134,15 @@ static int tangox_wdt_probe(struct platform_device *pdev)
> err = clk_prepare_enable(dev->clk);
> if (err)
> return err;
> + err = devm_add_action_or_reset(&pdev->dev,
> + (void(*)(void *))clk_disable_unprepare,
> + dev->clk);
> + if (err)
> + return err;

Hello Guenter,

I would rather avoid the function pointer cast.
How about defining an auxiliary function for the cleanup action?

clk_disable_unprepare() is static inline, so gcc will have to
define an auxiliary function either way. What do you think?

Regards.


diff --git a/drivers/watchdog/tangox_wdt.c b/drivers/watchdog/tangox_wdt.c
index 202c4b9cc921..1a4f6d245a83 100644
--- a/drivers/watchdog/tangox_wdt.c
+++ b/drivers/watchdog/tangox_wdt.c
@@ -114,6 +114,11 @@ static int tangox_wdt_restart(struct notifier_block *nb, unsigned long action,
return NOTIFY_DONE;
}

+static void cleanup(void *clk)
+{
+ clk_disable_unprepare(clk);
+}
+
static int tangox_wdt_probe(struct platform_device *pdev)
{
struct tangox_wdt_device *dev;
@@ -138,6 +143,10 @@ static int tangox_wdt_probe(struct platform_device *pdev)
if (err)
return err;

+ err = devm_add_action_or_reset(&pdev->dev, cleanup, dev->clk);
+ if (err)
+ return err;
+
dev->clk_rate = clk_get_rate(dev->clk);
if (!dev->clk_rate) {
err = -EINVAL;