| Message ID | 20210324152058.69022-2-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] pwm: Drop unused error path from pwmchip_remove() | expand |
On Wed, Mar 24, 2021 at 04:20:57PM +0100, Uwe Kleine-König wrote: > Since the pwm core uses device links (commit b2c200e3f2fd ("pwm: Add > consumer device link")) it cannot happen any more that there is still a > consumer when a pwmchip goes away. So drop this check. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/pwm/core.c | 16 ++-------------- > 1 file changed, 2 insertions(+), 14 deletions(-) Can't this still happen when a consumer forgets to pwm_put() the PWM? Thierry
On Fri, Apr 09, 2021 at 01:57:46PM +0200, Thierry Reding wrote: > On Wed, Mar 24, 2021 at 04:20:57PM +0100, Uwe Kleine-König wrote: > > Since the pwm core uses device links (commit b2c200e3f2fd ("pwm: Add > > consumer device link")) it cannot happen any more that there is still a > > consumer when a pwmchip goes away. So drop this check. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > drivers/pwm/core.c | 16 ++-------------- > > 1 file changed, 2 insertions(+), 14 deletions(-) > > Can't this still happen when a consumer forgets to pwm_put() the PWM? The change is still good, and a more correct change log would be: Since the pwm core uses device links (commit b2c200e3f2fd ("pwm: Add consumer device link")) each consumer driver that requested the PWMs is already gone. If they called pwm_put() (as they should) the PWMF_REQUESTED bit is not set. If they failed (which is a bug) the PWMF_REQUESTED bit is still set, but the driver that cared is still gone, so nothing bad happens if the pwmchip goes away anyhow. So the check can be dropped. Does this sound better? Best regards Uwe
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 7d0266bc5fcb..57b90469a7ad 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -324,22 +324,10 @@ EXPORT_SYMBOL_GPL(pwmchip_add); */ int pwmchip_remove(struct pwm_chip *chip) { - unsigned int i; - int ret = 0; - pwmchip_sysfs_unexport(chip); mutex_lock(&pwm_lock); - for (i = 0; i < chip->npwm; i++) { - struct pwm_device *pwm = &chip->pwms[i]; - - if (test_bit(PWMF_REQUESTED, &pwm->flags)) { - ret = -EBUSY; - goto out; - } - } - list_del_init(&chip->list); if (IS_ENABLED(CONFIG_OF)) @@ -347,9 +335,9 @@ int pwmchip_remove(struct pwm_chip *chip) free_pwms(chip); -out: mutex_unlock(&pwm_lock); - return ret; + + return 0; } EXPORT_SYMBOL_GPL(pwmchip_remove);
Since the pwm core uses device links (commit b2c200e3f2fd ("pwm: Add consumer device link")) it cannot happen any more that there is still a consumer when a pwmchip goes away. So drop this check. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pwm/core.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)