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
