Re: [PATCH 1/5] pwm: stm32: Replace write_ccrx with regmap_write

From: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Thierry Reding" <thierry.reding@gmail.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	<linux-pwm@vger.kernel.org>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	<linux-arm-kernel@lists.infradead.org>, <kernel@pengutronix.de>
Subject: Re: [PATCH 1/5] pwm: stm32: Replace write_ccrx with regmap_write
Date: Tue, 14 Nov 2023 14:26:26 +0100	[thread overview]
Message-ID: <77d75b3e-f9b7-4360-9e92-223befc9b57b@foss.st.com> (raw)
In-Reply-To: <20231019200658.1754190-8-u.kleine-koenig@pengutronix.de>

On 10/19/23 22:07, Uwe Kleine-König wrote:
> From: Philipp Zabel <p.zabel@pengutronix.de>
> 
> The TIM_CCR1...4 registers are consecutive, so replace the switch
> case with a simple calculation. Since ch is known to be in the 0...3
> range (it is set to hwpwm < npwm <= 4), drop the unnecessary error
> handling. The return value was not checked anyway. What remains does
> not warrant keeping the write_ccrx() function around, so instead call
> regmap_write() directly at the singular call site.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pwm/pwm-stm32.c | 17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)

Hi Uwe,

Sorry for the late reply,
You can add my:
Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Thanks,
Fabrice

> 
> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
> index 3303a754ea02..5e48584e3bd4 100644
> --- a/drivers/pwm/pwm-stm32.c
> +++ b/drivers/pwm/pwm-stm32.c
> @@ -52,21 +52,6 @@ static u32 active_channels(struct stm32_pwm *dev)
>  	return ccer & TIM_CCER_CCXE;
>  }
>  
> -static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)
> -{
> -	switch (ch) {
> -	case 0:
> -		return regmap_write(dev->regmap, TIM_CCR1, value);
> -	case 1:
> -		return regmap_write(dev->regmap, TIM_CCR2, value);
> -	case 2:
> -		return regmap_write(dev->regmap, TIM_CCR3, value);
> -	case 3:
> -		return regmap_write(dev->regmap, TIM_CCR4, value);
> -	}
> -	return -EINVAL;
> -}
> -
>  #define TIM_CCER_CC12P (TIM_CCER_CC1P | TIM_CCER_CC2P)
>  #define TIM_CCER_CC12E (TIM_CCER_CC1E | TIM_CCER_CC2E)
>  #define TIM_CCER_CC34P (TIM_CCER_CC3P | TIM_CCER_CC4P)
> @@ -369,7 +354,7 @@ static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
>  	dty = prd * duty_ns;
>  	do_div(dty, period_ns);
>  
> -	write_ccrx(priv, ch, dty);
> +	regmap_write(priv->regmap, TIM_CCR1 + 4 * ch, dty);
>  
>  	/* Configure output mode */
>  	shift = (ch & 0x1) * CCMR_CHANNEL_SHIFT;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel