Hello Ayyathurai,
Can you please fix your MUA to properly quote when replying, this is
really annoying.
On Fri, Oct 16, 2020 at 03:18:08AM +0000, Ayyathurai, Vijayakannan wrote:
> > On Thu, Oct 15, 2020 at 03:36:09AM +0800, vijayakannan.ayyathurai@xxxxxxxxx wrote:
> > > +static int keembay_pwm_remove(struct platform_device *pdev) {
> > > + struct keembay_pwm *priv = platform_get_drvdata(pdev);
> > > + int ret;
> > > +
> > > + ret = pwmchip_remove(&priv->chip);
> > > + clk_disable_unprepare(priv->clk);
> > > +
> > > + return ret;
> >
> > ...and this will be simplified to
> >
> > return pwmchip_remove(&priv->chip);
>
> Until v10, It is as per your suggestion. But I have changed it in v11
> to overcome the issue mentioned by Uwe. I have kept the snip of v10
> FYR below.
>
> //Start snip from v10 review mailing list
> //> +static int keembay_pwm_remove(struct platform_device *pdev) {
> //> + struct keembay_pwm *priv = platform_get_drvdata(pdev);
> //> +
> //> + clk_disable_unprepare(priv->clk);
> //> +
> //> + return pwmchip_remove(&priv->chip);
> //
> //You have to call pwmchip_remove first. Otherwise you're stopping the PWM while the framework still believes everything to be fine.
> //
> //> +}
> //End snip from v10 review mailing review
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.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
