Re: [PATCH 5/5] pwm: stm32: Use regmap_clear_bits and regmap_set_bits where applicable

[Fabrice Gasnier <fabrice.gasnier@foss.st.com>]

On 11/15/22 12:13, Uwe Kleine-König wrote:
> Found using coccinelle and the following semantic patch:
> 
> @@
> expression map, reg, bits;
> @@
> 
> - regmap_update_bits(map, reg, bits, bits)
> + regmap_set_bits(map, reg, bits)
> 
> @@
> expression map, reg, bits;
> @@
> 
> - regmap_update_bits(map, reg, bits, 0)
> + regmap_clear_bits(map, reg, bits)
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-stm32.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)

Hi Uwe,

You can add my:
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Best Regards,
Thanks
Fabrice

> 
> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
> index 794ca5b02968..21e4a34dfff3 100644
> --- a/drivers/pwm/pwm-stm32.c
> +++ b/drivers/pwm/pwm-stm32.c
> @@ -115,14 +115,14 @@ static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
>  	int ret;
>  
>  	/* Ensure registers have been updated, enable counter and capture */
> -	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
> -	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
> +	regmap_set_bits(priv->regmap, TIM_EGR, TIM_EGR_UG);
> +	regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
>  
>  	/* Use cc1 or cc3 DMA resp for PWM input channels 1 & 2 or 3 & 4 */
>  	dma_id = pwm->hwpwm < 2 ? STM32_TIMERS_DMA_CH1 : STM32_TIMERS_DMA_CH3;
>  	ccen = pwm->hwpwm < 2 ? TIM_CCER_CC12E : TIM_CCER_CC34E;
>  	ccr = pwm->hwpwm < 2 ? TIM_CCR1 : TIM_CCR3;
> -	regmap_update_bits(priv->regmap, TIM_CCER, ccen, ccen);
> +	regmap_set_bits(priv->regmap, TIM_CCER, ccen);
>  
>  	/*
>  	 * Timer DMA burst mode. Request 2 registers, 2 bursts, to get both
> @@ -160,8 +160,8 @@ static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
>  	}
>  
>  stop:
> -	regmap_update_bits(priv->regmap, TIM_CCER, ccen, 0);
> -	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
> +	regmap_clear_bits(priv->regmap, TIM_CCER, ccen);
> +	regmap_clear_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
>  
>  	return ret;
>  }
> @@ -359,7 +359,7 @@ static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
>  
>  	regmap_write(priv->regmap, TIM_PSC, prescaler);
>  	regmap_write(priv->regmap, TIM_ARR, prd - 1);
> -	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
> +	regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE);
>  
>  	/* Calculate the duty cycles */
>  	dty = prd * duty_ns;
> @@ -377,7 +377,7 @@ static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
>  	else
>  		regmap_update_bits(priv->regmap, TIM_CCMR2, mask, ccmr);
>  
> -	regmap_update_bits(priv->regmap, TIM_BDTR, TIM_BDTR_MOE, TIM_BDTR_MOE);
> +	regmap_set_bits(priv->regmap, TIM_BDTR, TIM_BDTR_MOE);
>  
>  	return 0;
>  }
> @@ -411,13 +411,13 @@ static int stm32_pwm_enable(struct stm32_pwm *priv, int ch)
>  	if (priv->have_complementary_output)
>  		mask |= TIM_CCER_CC1NE << (ch * 4);
>  
> -	regmap_update_bits(priv->regmap, TIM_CCER, mask, mask);
> +	regmap_set_bits(priv->regmap, TIM_CCER, mask);
>  
>  	/* Make sure that registers are updated */
> -	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
> +	regmap_set_bits(priv->regmap, TIM_EGR, TIM_EGR_UG);
>  
>  	/* Enable controller */
> -	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
> +	regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
>  
>  	return 0;
>  }
> @@ -431,11 +431,11 @@ static void stm32_pwm_disable(struct stm32_pwm *priv, int ch)
>  	if (priv->have_complementary_output)
>  		mask |= TIM_CCER_CC1NE << (ch * 4);
>  
> -	regmap_update_bits(priv->regmap, TIM_CCER, mask, 0);
> +	regmap_clear_bits(priv->regmap, TIM_CCER, mask);
>  
>  	/* When all channels are disabled, we can disable the controller */
>  	if (!active_channels(priv))
> -		regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
> +		regmap_clear_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
>  
>  	clk_disable(priv->clk);
>  }
> @@ -568,10 +568,9 @@ static void stm32_pwm_detect_complementary(struct stm32_pwm *priv)
>  	 * If complementary bit doesn't exist writing 1 will have no
>  	 * effect so we can detect it.
>  	 */
> -	regmap_update_bits(priv->regmap,
> -			   TIM_CCER, TIM_CCER_CC1NE, TIM_CCER_CC1NE);
> +	regmap_set_bits(priv->regmap, TIM_CCER, TIM_CCER_CC1NE);
>  	regmap_read(priv->regmap, TIM_CCER, &ccer);
> -	regmap_update_bits(priv->regmap, TIM_CCER, TIM_CCER_CC1NE, 0);
> +	regmap_clear_bits(priv->regmap, TIM_CCER, TIM_CCER_CC1NE);
>  
>  	priv->have_complementary_output = (ccer != 0);
>  }
> @@ -585,10 +584,9 @@ static int stm32_pwm_detect_channels(struct stm32_pwm *priv)
>  	 * If channels enable bits don't exist writing 1 will have no
>  	 * effect so we can detect and count them.
>  	 */
> -	regmap_update_bits(priv->regmap,
> -			   TIM_CCER, TIM_CCER_CCXE, TIM_CCER_CCXE);
> +	regmap_set_bits(priv->regmap, TIM_CCER, TIM_CCER_CCXE);
>  	regmap_read(priv->regmap, TIM_CCER, &ccer);
> -	regmap_update_bits(priv->regmap, TIM_CCER, TIM_CCER_CCXE, 0);
> +	regmap_clear_bits(priv->regmap, TIM_CCER, TIM_CCER_CCXE);
>  
>  	if (ccer & TIM_CCER_CC1E)
>  		npwm++;