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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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