On Mon, Oct 19, 2020 at 05:44:36AM +0000, Ayyathurai, Vijayakannan wrote:
> Hi Uwe,
>
> > From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > Sent: Friday, 16 October, 2020 1:04 PM
> > Subject: Re: [PATCH v12 1/2] pwm: Add PWM driver for Intel Keem Bay
> >
> > Hello Ayyathurai,
> > Note that we're both (Andy and I) are right. You must not disable the clocks
> > before pwmchip_remove() (otherwise for a short time the PWM looks ready
> > but isn't). And if you use devm-stuff to enable the clock it will be disabled only
> > after the remove callback completed and your .remove may look like:
> >
> > static int keembay_pwm_remove(struct platform_device *pdev)
> > {
> > struct keembay_pwm *priv = platform_get_drvdata(pdev);
> >
> > return pwmchip_remove(&priv->chip);
> > }
> >
> > because you won't have to care for the clock explicitly.
>
> Sure. I will incorporate the same in the next version.
>
> Also, Is it fine not to care clock during pwmchip_add like below,
>
> ret = pwmchip_add(&priv->chip);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to add PWM chip\n");
Yes, if you cared for disabling the clk using devm_* you don't need
(even: must not) disable the clock in the error path because the devm
release functions are also called on .probe() failing.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
