| Message ID | 20240709101806.52394-4-u.kleine-koenig@baylibre.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] pwm: atmel-tcb: Fix race condition and convert to guards | expand |
On 09/07/2024 at 12:18, Uwe Kleine-König wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > The two outputs provided by the supported hardware share some settings, > so access to the other PWM is required when one of them is configured. > > Instead of an explicit if to deterimine the other PWM just use > hwpwm ^ 1. Further atcbpwm is never NULL, so drop the corresponding > check. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> Thanks. Best regards, Nicolas > --- > drivers/pwm/pwm-atmel-tcb.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c > index aca11493239a..2feee3744b50 100644 > --- a/drivers/pwm/pwm-atmel-tcb.c > +++ b/drivers/pwm/pwm-atmel-tcb.c > @@ -260,7 +260,8 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > { > struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip); > struct atmel_tcb_pwm_device *tcbpwm = &tcbpwmc->pwms[pwm->hwpwm]; > - struct atmel_tcb_pwm_device *atcbpwm = NULL; > + /* companion PWM sharing register values period and div */ > + struct atmel_tcb_pwm_device *atcbpwm = &tcbpwmc->pwms[pwm->hwpwm ^ 1]; > int i = 0; > int slowclk = 0; > unsigned period; > @@ -305,11 +306,6 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > duty = div_u64(duty_ns, min); > period = div_u64(period_ns, min); > > - if (pwm->hwpwm == 0) > - atcbpwm = &tcbpwmc->pwms[1]; > - else > - atcbpwm = &tcbpwmc->pwms[0]; > - > /* > * PWM devices provided by the TCB driver are grouped by 2. > * PWM devices in a given group must be configured with the > @@ -318,8 +314,7 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > * We're checking the period value of the second PWM device > * in this group before applying the new config. > */ > - if ((atcbpwm && atcbpwm->duty > 0 && > - atcbpwm->duty != atcbpwm->period) && > + if ((atcbpwm->duty > 0 && atcbpwm->duty != atcbpwm->period) && > (atcbpwm->div != i || atcbpwm->period != period)) { > dev_err(pwmchip_parent(chip), > "failed to configure period_ns: PWM group already configured with a different value\n"); > -- > 2.43.0 >
On Wed, Jul 10, 2024 at 04:17:47PM +0200, Nicolas Ferre wrote: > On 09/07/2024 at 12:18, Uwe Kleine-König wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > The two outputs provided by the supported hardware share some settings, > > so access to the other PWM is required when one of them is configured. > > > > Instead of an explicit if to deterimine the other PWM just use > > hwpwm ^ 1. Further atcbpwm is never NULL, so drop the corresponding > > check. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> Thanks, I applied both patches with your Ack to my pwm/for-next branch. Best regards Uwe
diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c index aca11493239a..2feee3744b50 100644 --- a/drivers/pwm/pwm-atmel-tcb.c +++ b/drivers/pwm/pwm-atmel-tcb.c @@ -260,7 +260,8 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, { struct atmel_tcb_pwm_chip *tcbpwmc = to_tcb_chip(chip); struct atmel_tcb_pwm_device *tcbpwm = &tcbpwmc->pwms[pwm->hwpwm]; - struct atmel_tcb_pwm_device *atcbpwm = NULL; + /* companion PWM sharing register values period and div */ + struct atmel_tcb_pwm_device *atcbpwm = &tcbpwmc->pwms[pwm->hwpwm ^ 1]; int i = 0; int slowclk = 0; unsigned period; @@ -305,11 +306,6 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, duty = div_u64(duty_ns, min); period = div_u64(period_ns, min); - if (pwm->hwpwm == 0) - atcbpwm = &tcbpwmc->pwms[1]; - else - atcbpwm = &tcbpwmc->pwms[0]; - /* * PWM devices provided by the TCB driver are grouped by 2. * PWM devices in a given group must be configured with the @@ -318,8 +314,7 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, * We're checking the period value of the second PWM device * in this group before applying the new config. */ - if ((atcbpwm && atcbpwm->duty > 0 && - atcbpwm->duty != atcbpwm->period) && + if ((atcbpwm->duty > 0 && atcbpwm->duty != atcbpwm->period) && (atcbpwm->div != i || atcbpwm->period != period)) { dev_err(pwmchip_parent(chip), "failed to configure period_ns: PWM group already configured with a different value\n");
The two outputs provided by the supported hardware share some settings, so access to the other PWM is required when one of them is configured. Instead of an explicit if to deterimine the other PWM just use hwpwm ^ 1. Further atcbpwm is never NULL, so drop the corresponding check. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> --- drivers/pwm/pwm-atmel-tcb.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)