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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 Jan 2017 10:07:09 +0100
From:   Marc Gonzalez <marc_gonzalez@...madesigns.com>
To:     Guenter Roeck <linux@...ck-us.net>
CC:     Wim Van Sebroeck <wim@...ana.be>, <linux-watchdog@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        "Mans Rullgard" <mans@...sr.com>,
        Thibaud Cornic <thibaud_cornic@...madesigns.com>
Subject: Re: [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed
 functions

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@...madesigns.com>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
>  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;