Re: [RFC PATCH 07/15] pwm: move the enabled/disabled info to pwm_state struct

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]

On Wed, Jul 01, 2015 at 10:21:53AM +0200, Boris Brezillon wrote:
> Prepare the transition to PWM atomic update by moving the enabled/disabled
> state into the pwm_state struct. This way we can easily update the whole
> PWM state by copying the new state in the ->state field.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/pwm/core.c  | 15 ++++++++++++---
>  include/linux/pwm.h |  6 +++---
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index a6bc8e6..3e830ce 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -474,8 +474,15 @@ EXPORT_SYMBOL_GPL(pwm_set_polarity);
>   */
>  int pwm_enable(struct pwm_device *pwm)
>  {
> -	if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags))
> -		return pwm->chip->ops->enable(pwm->chip, pwm);
> +	if (pwm && !pwm_is_enabled(pwm)) {
> +		int err;
> +
> +		err = pwm->chip->ops->enable(pwm->chip, pwm);
> +		if (!err)
> +			pwm->state.enabled = true;
> +
> +		return err;
> +	}

Technically there's now a race between the pwm_is_enabled() and
pwm->state.enabled = true; statements, but as discussed in the cover
letter I think that's fine because of the assumptions about concurrent
usage of PWMs.

The most important check (PWMF_REQUESTED) is still atomic, so it is
still up to drivers to properly lock concurrent access to a PWM device
and the core will make sure that a device can only be requested once.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]