Re: [PATCH v12 1/2] pwm: Add PWM driver for Intel Keem Bay

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

On Fri, Oct 16, 2020 at 08:24:39AM +0200, Uwe Kleine-König wrote:
> On Thu, Oct 15, 2020 at 01:42:17PM +0300, Andy Shevchenko wrote:
> > On Thu, Oct 15, 2020 at 03:36:09AM +0800, vijayakannan.ayyathurai@intel.com wrote:
> > > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@intel.com>

...

> > > +	ret = pwmchip_add(&priv->chip);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to add PWM chip: %pe\n", ERR_PTR(ret));
> > 
> > > +		clk_disable_unprepare(priv->clk);
> > 
> > This messes up with ordering of things.
> > 
> > That's why devm golden rule is either all or none. You may fix this by
> > switching to devm_add_action_or_reset().
> > 
> > One of possible way is like in below drivers:
> > 
> > 	% git grep -n devm_add_action_or_reset.*disable_unprepare -- drivers/
> > 
> > But it may be fixed in follow up change. Depends on maintainers' wishes.
> 
> I recently sent a patch to address this in a still more comfortable way:
>
> 	https://lore.kernel.org/r/20201013082132.661993-1-u.kleine-koenig@pengutronix.de

Good to know, thanks!

> Other than that I don't think there is a real issue to fix here, yes,
> the unroll order doesn't match the init order, but here this is about
> ioremap vs clk_prepare_enable, so I'm fine with the status quo.
> 
> (I assume I didn't miss a real issue here. Please tell if so.)

No, as you said and as I answered in previous reply that the more serious issue
(which you pointed out) has been replaced with less serious which can even be
addressed separately, but it's up to you and Thierry.

-- 
With Best Regards,
Andy Shevchenko