Re: [PATCH v4 1/7] pwm: Add more locking

[David Lechner <dlechner@baylibre.com>]

On 9/6/24 10:42 AM, Uwe Kleine-König wrote:
> This ensures that a pwm_chip that has no corresponding driver isn't used
> and that a driver doesn't go away while a callback is still running.
> 
> In the presence of device links this isn't necessary yet (so this is no
> fix) but for pwm character device support this is needed.
> 
> To not serialize all pwm_apply_state() calls, this introduces a per chip
> lock. An additional complication is that for atomic chips a mutex cannot
> be used (as pwm_apply_atomic() must not sleep) and a spinlock cannot be
> held while calling an operation for a sleeping chip. So depending on the
> chip being atomic or not a spinlock or a mutex is used.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---

...

> @@ -336,6 +367,11 @@ static int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result,
>  
>  	guard(mutex)(&pwm_lock);

Do we still need to hold pwm_lock here? Maybe we can remove it?
Or add a comment about why it is still necessary even though we
also hold pwmchip lock.

>  
> +	guard(pwmchip)(chip);
> +
> +	if (!chip->operational)
> +		return -ENODEV;
> +
>  	return ops->capture(chip, pwm, result, timeout);
>  }
>