On 10/07/2024 at 17:31, Uwe Kleine-König wrote:

Hello Nicolas,

On Wed, Jul 10, 2024 at 04:17:09PM +0200, Nicolas Ferre wrote:

quoted

On 09/07/2024 at 12:18, Uwe Kleine-König wrote:

quoted

The hardware only supports a single period length for both PWM outputs. So
atmel_tcb_pwm_config() checks the configuration of the other output if it's
compatible with the currently requested setting. The register values are
then actually updated in atmel_tcb_pwm_enable(). To make this race free
the lock must be held during the whole process, so grab the lock in
.apply() instead of individually in atmel_tcb_pwm_disable() and
atmel_tcb_pwm_enable() which then also covers atmel_tcb_pwm_config().

To simplify handling, use the guard helper to let the compiler care for
unlocking. Otherwise unlocking would be more difficult as there is more
than one exit path in atmel_tcb_pwm_apply().

Fixes: 9421bade0765 ("pwm: atmel: add Timer Counter Block PWM driver")
Signed-off-by: Uwe Kleine-König<redacted>

Acked-by: Nicolas Ferre<nicolas.ferre@microchip.com>

And I like the conversion to the "guard" lock helper.

I hesitated a bit to add it because it will make backporting to stable
harder. But I guess we will just not backport it, the problem doesn't
seem to matter in practise given that it was found by looking at code
and not hit in real life more more than 11 years after its introduction.

Fair indeed: +1.

Best regards and thanks for your Acks,
Uwe