[PATCH] counter: stm32-timer-cnt: Reset TIM_TISEL and TIM_SMCR to their default value

[PATCH] counter: stm32-timer-cnt: Reset TIM_TISEL and TIM_SMCR to their default value

[Uwe Kleine-König]

The driver assumes that the input selection register (TIM_TISEL) is at
its reset default value. Usually this is the case, but the bootloader
might have modified it. Also reset the SMCR register while at it.

This bases on the effectively same patch submitted by Olivier Moysan for
pwm-stm32.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

note that the patch by Olivier Moysan[1] for pwm-stm32 is expected to
appear in Thierry's tree soon. It added the definition of TIM_TISEL in
the same way, so the two patches should merge just fine. Alternatively
you can commit it to a tree that already has the pwm change (and then
drop the change to include/linux/mfd/stm32-timers.h from this one).

Best regards
Uwe

[1] https://lore.kernel.org/linux-pwm/20221213102707.1096345-1-olivier.moysan@foss.st.com

 drivers/counter/stm32-timer-cnt.c | 4 ++++
 include/linux/mfd/stm32-timers.h  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index 9bf20a5d6bda..d001d77f17ac 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -342,6 +342,10 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
+	/* Reset input selector to its default input and disable slave mode */
+	regmap_write(priv->regmap, TIM_TISEL, 0x0);
+	regmap_write(priv->regmap, TIM_SMCR, 0x0);
+
 	/* Register Counter device */
 	ret = devm_counter_add(dev, counter);
 	if (ret < 0)
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 5f5c43fd69dd..1b94325febb3 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -31,6 +31,7 @@
 #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
 #define TIM_DCR		0x48	/* DMA control register    */
 #define TIM_DMAR	0x4C	/* DMA register for transfer */
+#define TIM_TISEL	0x68	/* Input Selection         */
 
 #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
 #define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */

base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
-- 
2.39.2


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

Re: [PATCH] counter: stm32-timer-cnt: Reset TIM_TISEL and TIM_SMCR to their default value

[Fabrice Gasnier]

On 4/12/23 17:37, Uwe Kleine-König wrote:
> The driver assumes that the input selection register (TIM_TISEL) is at
> its reset default value. Usually this is the case, but the bootloader
> might have modified it. Also reset the SMCR register while at it.
> 
> This bases on the effectively same patch submitted by Olivier Moysan for
> pwm-stm32.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> note that the patch by Olivier Moysan[1] for pwm-stm32 is expected to
> appear in Thierry's tree soon. It added the definition of TIM_TISEL in
> the same way, so the two patches should merge just fine. Alternatively
> you can commit it to a tree that already has the pwm change (and then
> drop the change to include/linux/mfd/stm32-timers.h from this one).
> 
> Best regards
> Uwe
> 
> [1] https://lore.kernel.org/linux-pwm/20221213102707.1096345-1-olivier.moysan@foss.st.com
> 
>  drivers/counter/stm32-timer-cnt.c | 4 ++++
>  include/linux/mfd/stm32-timers.h  | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
> index 9bf20a5d6bda..d001d77f17ac 100644
> --- a/drivers/counter/stm32-timer-cnt.c
> +++ b/drivers/counter/stm32-timer-cnt.c
> @@ -342,6 +342,10 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, priv);
>  
> +	/* Reset input selector to its default input and disable slave mode */
> +	regmap_write(priv->regmap, TIM_TISEL, 0x0);
> +	regmap_write(priv->regmap, TIM_SMCR, 0x0);

Hi Uwe,

Just on SMCR: the function_write routine will take care of setting it
depending on the count function (increase, encoder mode...). So it looks
like useless to disable slave mode during probe.
Could it be removed ?

With that, you can add my:
Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com

Best Regards,
Fabrice

> +
>  	/* Register Counter device */
>  	ret = devm_counter_add(dev, counter);
>  	if (ret < 0)
> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> index 5f5c43fd69dd..1b94325febb3 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -31,6 +31,7 @@
>  #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
>  #define TIM_DCR		0x48	/* DMA control register    */
>  #define TIM_DMAR	0x4C	/* DMA register for transfer */
> +#define TIM_TISEL	0x68	/* Input Selection         */
>  
>  #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
>  #define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
> 
> base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6

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

Re: [PATCH] counter: stm32-timer-cnt: Reset TIM_TISEL and TIM_SMCR to their default value

[Lee Jones]

On Wed, 12 Apr 2023, Uwe Kleine-König wrote:

> The driver assumes that the input selection register (TIM_TISEL) is at
> its reset default value. Usually this is the case, but the bootloader
> might have modified it. Also reset the SMCR register while at it.
> 
> This bases on the effectively same patch submitted by Olivier Moysan for
> pwm-stm32.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> note that the patch by Olivier Moysan[1] for pwm-stm32 is expected to
> appear in Thierry's tree soon. It added the definition of TIM_TISEL in
> the same way, so the two patches should merge just fine. Alternatively
> you can commit it to a tree that already has the pwm change (and then
> drop the change to include/linux/mfd/stm32-timers.h from this one).
> 
> Best regards
> Uwe
> 
> [1] https://lore.kernel.org/linux-pwm/20221213102707.1096345-1-olivier.moysan@foss.st.com
> 
>  drivers/counter/stm32-timer-cnt.c | 4 ++++

>  include/linux/mfd/stm32-timers.h  | 1 +

Acked-by: Lee Jones <lee@kernel.org>

>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
> index 9bf20a5d6bda..d001d77f17ac 100644
> --- a/drivers/counter/stm32-timer-cnt.c
> +++ b/drivers/counter/stm32-timer-cnt.c
> @@ -342,6 +342,10 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, priv);
>  
> +	/* Reset input selector to its default input and disable slave mode */
> +	regmap_write(priv->regmap, TIM_TISEL, 0x0);
> +	regmap_write(priv->regmap, TIM_SMCR, 0x0);
> +
>  	/* Register Counter device */
>  	ret = devm_counter_add(dev, counter);
>  	if (ret < 0)
> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> index 5f5c43fd69dd..1b94325febb3 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -31,6 +31,7 @@
>  #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
>  #define TIM_DCR		0x48	/* DMA control register    */
>  #define TIM_DMAR	0x4C	/* DMA register for transfer */
> +#define TIM_TISEL	0x68	/* Input Selection         */
>  
>  #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
>  #define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
> 
> base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
> -- 
> 2.39.2
> 

-- 
Lee Jones [李琼斯]

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

Re: [PATCH] counter: stm32-timer-cnt: Reset TIM_TISEL and TIM_SMCR to their default value

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 1618 bytes --]

Hello Lee,

On Thu, Apr 20, 2023 at 12:01:11PM +0100, Lee Jones wrote:
> On Wed, 12 Apr 2023, Uwe Kleine-König wrote:
> 
> > The driver assumes that the input selection register (TIM_TISEL) is at
> > its reset default value. Usually this is the case, but the bootloader
> > might have modified it. Also reset the SMCR register while at it.
> > 
> > This bases on the effectively same patch submitted by Olivier Moysan for
> > pwm-stm32.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > note that the patch by Olivier Moysan[1] for pwm-stm32 is expected to
> > appear in Thierry's tree soon. It added the definition of TIM_TISEL in
> > the same way, so the two patches should merge just fine. Alternatively
> > you can commit it to a tree that already has the pwm change (and then
> > drop the change to include/linux/mfd/stm32-timers.h from this one).
> > 
> > Best regards
> > Uwe
> > 
> > [1] https://lore.kernel.org/linux-pwm/20221213102707.1096345-1-olivier.moysan@foss.st.com
> > 
> >  drivers/counter/stm32-timer-cnt.c | 4 ++++
> 
> >  include/linux/mfd/stm32-timers.h  | 1 +
> 
> Acked-by: Lee Jones <lee@kernel.org>

Note there is a v2 at
https://lore.kernel.org/r/20230413212339.3611722-1-u.kleine-koenig@pengutronix.de,
William wrote that he already applied it, so replying with tag in that
thread might be already to late, too.

(nevertheless) Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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