Commit 37f77070 authored by Uwe Kleine-König's avatar Uwe Kleine-König Committed by Uwe Kleine-König
Browse files

pwm: atmel-tcb: Fix race condition and convert to guards 



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: 9421bade ("pwm: atmel: add Timer Counter Block PWM driver")
Signed-off-by: default avatarUwe Kleine-König <u.kleine-koenig@baylibre.com>
Acked-by: default avatarNicolas Ferre <nicolas.ferre@microchip.com>
Link: https://lore.kernel.org/r/20240709101806.52394-3-u.kleine-koenig@baylibre.com


Signed-off-by: default avatarUwe Kleine-König <ukleinek@kernel.org>
parent 32207e9d

drivers/pwm/pwm-atmel-tcb.c

+5 −7
Original line number Diff line number Diff line
@@ -81,7 +81,8 @@ static int atmel_tcb_pwm_request(struct pwm_chip *chip, 
	tcbpwm->period = 0;

	tcbpwm->div = 0;



	spin_lock(&tcbpwmc->lock);

	guard(spinlock)(&tcbpwmc->lock);



	regmap_read(tcbpwmc->regmap, ATMEL_TC_REG(tcbpwmc->channel, CMR), &cmr);

	/*

	 * Get init config from Timer Counter registers if
@@ -107,7 +108,6 @@ static int atmel_tcb_pwm_request(struct pwm_chip *chip, 


	cmr |= ATMEL_TC_WAVE | ATMEL_TC_WAVESEL_UP_AUTO | ATMEL_TC_EEVT_XC0;

	regmap_write(tcbpwmc->regmap, ATMEL_TC_REG(tcbpwmc->channel, CMR), cmr);

	spin_unlock(&tcbpwmc->lock);



	return 0;

}
@@ -137,7 +137,6 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm, 
	if (tcbpwm->duty == 0)

		polarity = !polarity;



	spin_lock(&tcbpwmc->lock);

	regmap_read(tcbpwmc->regmap, ATMEL_TC_REG(tcbpwmc->channel, CMR), &cmr);



	/* flush old setting and set the new one */
@@ -172,8 +171,6 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm, 
			     ATMEL_TC_SWTRG);

		tcbpwmc->bkup.enabled = 0;

	}



	spin_unlock(&tcbpwmc->lock);

}



static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -194,7 +191,6 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm, 
	if (tcbpwm->duty == 0)

		polarity = !polarity;



	spin_lock(&tcbpwmc->lock);

	regmap_read(tcbpwmc->regmap, ATMEL_TC_REG(tcbpwmc->channel, CMR), &cmr);



	/* flush old setting and set the new one */
@@ -256,7 +252,6 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm, 
	regmap_write(tcbpwmc->regmap, ATMEL_TC_REG(tcbpwmc->channel, CCR),

		     ATMEL_TC_SWTRG | ATMEL_TC_CLKEN);

	tcbpwmc->bkup.enabled = 1;

	spin_unlock(&tcbpwmc->lock);

	return 0;

}
@@ -341,9 +336,12 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, 
static int atmel_tcb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,

			       const struct pwm_state *state)

{

	struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip);

	int duty_cycle, period;

	int ret;



	guard(spinlock)(&tcbpwmc->lock);



	if (!state->enabled) {

		atmel_tcb_pwm_disable(chip, pwm, state->polarity);

		return 0;