| Message ID | 20210329064111.148508-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | pwm: lpc3200: Don't modify HW state in .remove callback | expand |
Hi Uwe, On 3/29/21 9:41 AM, Uwe Kleine-König wrote: > A consumer is expected to disable a PWM before calling pwm_put(). And if > they didn't there is hopefully a good reason (or the consumer needs > fixing). Also if disabling an enabled PWM was the right thing to do, > this should better be done in the framework instead of in each low level > driver. > > So drop the hardware modification from the .remove() callback. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/pwm/pwm-lpc32xx.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c > index c42cc314c170..2834a0f001d3 100644 > --- a/drivers/pwm/pwm-lpc32xx.c > +++ b/drivers/pwm/pwm-lpc32xx.c > @@ -136,10 +136,6 @@ static int lpc32xx_pwm_probe(struct platform_device *pdev) > static int lpc32xx_pwm_remove(struct platform_device *pdev) > { > struct lpc32xx_pwm_chip *lpc32xx = platform_get_drvdata(pdev); > - unsigned int i; > - > - for (i = 0; i < lpc32xx->chip.npwm; i++) > - pwm_disable(&lpc32xx->chip.pwms[i]); > > return pwmchip_remove(&lpc32xx->chip); > } > Acked-by: Vladimir Zapolskiy <vz@mleia.com> I've briefly checked other PWM drivers, some of them do contain the same logic, for instance please check pwm-stm32.c, pwm-stm32-lp.c Many other drivers disable at least the controller clock, I believe there should be a clear agreed policy, it makes no sense to vary the behaviour among the drivers, thus likely all the drivers should be checked and fixed at one series, thanks. -- Best wishes, Vladimir
On Mon, Mar 29, 2021 at 08:41:11AM +0200, Uwe Kleine-König wrote: > A consumer is expected to disable a PWM before calling pwm_put(). And if > they didn't there is hopefully a good reason (or the consumer needs > fixing). Also if disabling an enabled PWM was the right thing to do, > this should better be done in the framework instead of in each low level > driver. > > So drop the hardware modification from the .remove() callback. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/pwm/pwm-lpc32xx.c | 4 ---- > 1 file changed, 4 deletions(-) Applied, thanks. Thierry
diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c index c42cc314c170..2834a0f001d3 100644 --- a/drivers/pwm/pwm-lpc32xx.c +++ b/drivers/pwm/pwm-lpc32xx.c @@ -136,10 +136,6 @@ static int lpc32xx_pwm_probe(struct platform_device *pdev) static int lpc32xx_pwm_remove(struct platform_device *pdev) { struct lpc32xx_pwm_chip *lpc32xx = platform_get_drvdata(pdev); - unsigned int i; - - for (i = 0; i < lpc32xx->chip.npwm; i++) - pwm_disable(&lpc32xx->chip.pwms[i]); return pwmchip_remove(&lpc32xx->chip); }
A consumer is expected to disable a PWM before calling pwm_put(). And if they didn't there is hopefully a good reason (or the consumer needs fixing). Also if disabling an enabled PWM was the right thing to do, this should better be done in the framework instead of in each low level driver. So drop the hardware modification from the .remove() callback. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pwm/pwm-lpc32xx.c | 4 ---- 1 file changed, 4 deletions(-)