linux-kernel - Re: [PATCH v8 2/2] Add PWM fan controller driver for LGM SoC

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:   Fri, 21 Aug 2020 11:23:19 +0300
From:   Andy Shevchenko <andriy.shevchenko@...el.com>
To:     "Tanwar, Rahul" <rahul.tanwar@...ux.intel.com>
Cc:     u.kleine-koenig@...gutronix.de, linux-pwm@...r.kernel.org,
        lee.jones@...aro.org, thierry.reding@...il.com,
        p.zabel@...gutronix.de, robh+dt@...nel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        songjun.Wu@...el.com, cheol.yong.kim@...el.com,
        qi-ming.wu@...el.com, rahul.tanwar.linux@...il.com,
        rtanwar@...linear.com
Subject: Re: [PATCH v8 2/2] Add PWM fan controller driver for LGM SoC

On Fri, Aug 21, 2020 at 02:07:49PM +0800, Tanwar, Rahul wrote:
> On 20/8/2020 6:52 pm, Andy Shevchenko wrote:
> > On Thu, Aug 20, 2020 at 12:50:46PM +0800, Rahul Tanwar wrote:

...

> >> +	ret = clk_prepare_enable(pc->clk);
> > Wrap it with devm_add_action_or_reset(). Same for reset_control_deassert().
> > You probably can even put them under one function.
> 
> I did some study and research for using devm_add_action_or_reset(). But
> still i have some doubts. Below steps is what i intend to do in order to
> switch to using this API. Please do review and let me know it is ok and
> i am not missing anything else. Thanks.
> 
> 1. Call reset_control_assert()
> 2. Call clk_prepare_enable()

> 3. Call pwmchip_add()

First of all, I haven't told anything about this.

> 4. Call devm_add_action_or_reset(dev, my_action, pc)
> 5. Remove explicit calls to unprepare/reset_control_assert from probe in
> failure cases.
> 6. static void my_action(void *pc)
>    {
>       pwmchip_remove(&pc->chip);
>       clk_disable_upprepare(pc->clk);
>       reset_control_assert(pc->rst);
>    }
> 7. Remove platform_driver.remove() entirely.

Nope, pwmchip_add() and pwmchip_remove() stay as is now.

Only wrap clock and reset resources.

> >> +	if (ret) {
> >> +		dev_err(dev, "failed to enable clock\n");
> >> +		reset_control_assert(pc->rst);
> >> +		return ret;
> >> +	}

-- 
With Best Regards,
Andy Shevchenko