All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pwm: stm32: enforce settings for pwm capture
@ 2022-12-13 10:27 ` Olivier Moysan
  0 siblings, 0 replies; 12+ messages in thread
From: Olivier Moysan @ 2022-12-13 10:27 UTC (permalink / raw)
  To: Fabrice Gasnier, Thierry Reding, Uwe Kleine-König,
	Maxime Coquelin, Alexandre Torgue, Lee Jones, Benjamin Gaignard
  Cc: Olivier Moysan, linux-pwm, linux-stm32, linux-arm-kernel

The PWM capture assumes that the input selector is set to default
input and that the slave mode is disabled. Force reset state for
TISEL and SMCR registers to match this requirement.

Note that slave mode disabling is not a pre-requisite by itself
for capture mode, as hardware supports it for PWM capture.
However, the current implementation of the driver does not
allow slave mode for PWM capture. Setting slave mode for PWM
capture results in wrong capture values.

Fixes: 53e38fe73f94 ("pwm: stm32: Add capture support")
Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 drivers/pwm/pwm-stm32.c          | 4 ++++
 include/linux/mfd/stm32-timers.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 794ca5b02968..24aab0450c78 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -207,6 +207,10 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	regmap_write(priv->regmap, TIM_ARR, priv->max_arr);
 	regmap_write(priv->regmap, TIM_PSC, psc);
 
+	/* 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);
+
 	/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */
 	regmap_update_bits(priv->regmap,
 			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
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	   */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] pwm: stm32: enforce settings for pwm capture
@ 2022-12-13 10:27 ` Olivier Moysan
  0 siblings, 0 replies; 12+ messages in thread
From: Olivier Moysan @ 2022-12-13 10:27 UTC (permalink / raw)
  To: Fabrice Gasnier, Thierry Reding, Uwe Kleine-König,
	Maxime Coquelin, Alexandre Torgue, Lee Jones, Benjamin Gaignard
  Cc: Olivier Moysan, linux-pwm, linux-stm32, linux-arm-kernel

The PWM capture assumes that the input selector is set to default
input and that the slave mode is disabled. Force reset state for
TISEL and SMCR registers to match this requirement.

Note that slave mode disabling is not a pre-requisite by itself
for capture mode, as hardware supports it for PWM capture.
However, the current implementation of the driver does not
allow slave mode for PWM capture. Setting slave mode for PWM
capture results in wrong capture values.

Fixes: 53e38fe73f94 ("pwm: stm32: Add capture support")
Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 drivers/pwm/pwm-stm32.c          | 4 ++++
 include/linux/mfd/stm32-timers.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 794ca5b02968..24aab0450c78 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -207,6 +207,10 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	regmap_write(priv->regmap, TIM_ARR, priv->max_arr);
 	regmap_write(priv->regmap, TIM_PSC, psc);
 
+	/* 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);
+
 	/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */
 	regmap_update_bits(priv->regmap,
 			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
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	   */
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
  2022-12-13 10:27 ` Olivier Moysan
@ 2022-12-13 10:51   ` Uwe Kleine-König
  -1 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2022-12-13 10:51 UTC (permalink / raw)
  To: Olivier Moysan
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, Benjamin Gaignard,
	William Breathitt Gray, linux-pwm, linux-stm32, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

Hello Olivier,

[Cc: += William Breathitt Gray, linux-iio@v.k.o]

On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote:
> The PWM capture assumes that the input selector is set to default
> input and that the slave mode is disabled. Force reset state for
> TISEL and SMCR registers to match this requirement.

When does the problem occur? Only if the bootloader changed that
setting? Regarding the urgency: With the current knowledge I'd say this
patch is material for the next merge window. Do you recommend
backporting to stable?

> Note that slave mode disabling is not a pre-requisite by itself
> for capture mode, as hardware supports it for PWM capture.
> However, the current implementation of the driver does not
> allow slave mode for PWM capture. Setting slave mode for PWM
> capture results in wrong capture values.

What is your usecase for PWM capture support? I didn't double check, but
I think you're the first contributor to PWM capture since 2018 (i.e. the
commit you're fixing).

Did you check if the counter subsystem would solve your problems? If it
doesn't I assume William would like to hear about that.

Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
slave mode, TISEL isn't touched though. So maybe this driver needs a
similar fix?

Apart from that the change looks reasonable:

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
@ 2022-12-13 10:51   ` Uwe Kleine-König
  0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2022-12-13 10:51 UTC (permalink / raw)
  To: Olivier Moysan
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, Benjamin Gaignard,
	William Breathitt Gray, linux-pwm, linux-stm32, linux-arm-kernel


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

Hello Olivier,

[Cc: += William Breathitt Gray, linux-iio@v.k.o]

On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote:
> The PWM capture assumes that the input selector is set to default
> input and that the slave mode is disabled. Force reset state for
> TISEL and SMCR registers to match this requirement.

When does the problem occur? Only if the bootloader changed that
setting? Regarding the urgency: With the current knowledge I'd say this
patch is material for the next merge window. Do you recommend
backporting to stable?

> Note that slave mode disabling is not a pre-requisite by itself
> for capture mode, as hardware supports it for PWM capture.
> However, the current implementation of the driver does not
> allow slave mode for PWM capture. Setting slave mode for PWM
> capture results in wrong capture values.

What is your usecase for PWM capture support? I didn't double check, but
I think you're the first contributor to PWM capture since 2018 (i.e. the
commit you're fixing).

Did you check if the counter subsystem would solve your problems? If it
doesn't I assume William would like to hear about that.

Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
slave mode, TISEL isn't touched though. So maybe this driver needs a
similar fix?

Apart from that the change looks reasonable:

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
  2022-12-13 10:51   ` Uwe Kleine-König
@ 2022-12-14 15:09     ` Olivier MOYSAN
  -1 siblings, 0 replies; 12+ messages in thread
From: Olivier MOYSAN @ 2022-12-14 15:09 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, Benjamin Gaignard,
	William Breathitt Gray, linux-pwm, linux-stm32, linux-arm-kernel

Hello Uwe,

On 12/13/22 11:51, Uwe Kleine-König wrote:
> Hello Olivier,
> 
> [Cc: += William Breathitt Gray, linux-iio@v.k.o]
> 
> On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote:
>> The PWM capture assumes that the input selector is set to default
>> input and that the slave mode is disabled. Force reset state for
>> TISEL and SMCR registers to match this requirement.
> 
> When does the problem occur? Only if the bootloader changed that
> setting? Regarding the urgency: With the current knowledge I'd say this
> patch is material for the next merge window. Do you recommend
> backporting to stable?
> 

Yes, the PWM may not be in the default expected state, if the 
configuration has been changed in the bootloader. This is not an actual 
case today, so this patch can wait the next merge window and there is no
urgency to have it in stable.

BRs
Olivier

>> Note that slave mode disabling is not a pre-requisite by itself
>> for capture mode, as hardware supports it for PWM capture.
>> However, the current implementation of the driver does not
>> allow slave mode for PWM capture. Setting slave mode for PWM
>> capture results in wrong capture values.
> 
> What is your usecase for PWM capture support? I didn't double check, but
> I think you're the first contributor to PWM capture since 2018 (i.e. the
> commit you're fixing).
> 
> Did you check if the counter subsystem would solve your problems? If it
> doesn't I assume William would like to hear about that.
> 
> Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
> slave mode, TISEL isn't touched though. So maybe this driver needs a
> similar fix?
> 
> Apart from that the change looks reasonable:
> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Best regards
> Uwe
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
@ 2022-12-14 15:09     ` Olivier MOYSAN
  0 siblings, 0 replies; 12+ messages in thread
From: Olivier MOYSAN @ 2022-12-14 15:09 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, Benjamin Gaignard,
	William Breathitt Gray, linux-pwm, linux-stm32, linux-arm-kernel

Hello Uwe,

On 12/13/22 11:51, Uwe Kleine-König wrote:
> Hello Olivier,
> 
> [Cc: += William Breathitt Gray, linux-iio@v.k.o]
> 
> On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote:
>> The PWM capture assumes that the input selector is set to default
>> input and that the slave mode is disabled. Force reset state for
>> TISEL and SMCR registers to match this requirement.
> 
> When does the problem occur? Only if the bootloader changed that
> setting? Regarding the urgency: With the current knowledge I'd say this
> patch is material for the next merge window. Do you recommend
> backporting to stable?
> 

Yes, the PWM may not be in the default expected state, if the 
configuration has been changed in the bootloader. This is not an actual 
case today, so this patch can wait the next merge window and there is no
urgency to have it in stable.

BRs
Olivier

>> Note that slave mode disabling is not a pre-requisite by itself
>> for capture mode, as hardware supports it for PWM capture.
>> However, the current implementation of the driver does not
>> allow slave mode for PWM capture. Setting slave mode for PWM
>> capture results in wrong capture values.
> 
> What is your usecase for PWM capture support? I didn't double check, but
> I think you're the first contributor to PWM capture since 2018 (i.e. the
> commit you're fixing).
> 
> Did you check if the counter subsystem would solve your problems? If it
> doesn't I assume William would like to hear about that.
> 
> Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
> slave mode, TISEL isn't touched though. So maybe this driver needs a
> similar fix?
> 
> Apart from that the change looks reasonable:
> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Best regards
> Uwe
> 

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
  2022-12-13 10:27 ` Olivier Moysan
@ 2023-01-03 12:34   ` Lee Jones
  -1 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2023-01-03 12:34 UTC (permalink / raw)
  To: Olivier Moysan
  Cc: Fabrice Gasnier, Thierry Reding, Uwe Kleine-König,
	Maxime Coquelin, Alexandre Torgue, Benjamin Gaignard, linux-pwm,
	linux-stm32, linux-arm-kernel

On Tue, 13 Dec 2022, Olivier Moysan wrote:

> The PWM capture assumes that the input selector is set to default
> input and that the slave mode is disabled. Force reset state for
> TISEL and SMCR registers to match this requirement.
> 
> Note that slave mode disabling is not a pre-requisite by itself
> for capture mode, as hardware supports it for PWM capture.
> However, the current implementation of the driver does not
> allow slave mode for PWM capture. Setting slave mode for PWM
> capture results in wrong capture values.
> 
> Fixes: 53e38fe73f94 ("pwm: stm32: Add capture support")
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> ---
>  drivers/pwm/pwm-stm32.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/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
> index 794ca5b02968..24aab0450c78 100644
> --- a/drivers/pwm/pwm-stm32.c
> +++ b/drivers/pwm/pwm-stm32.c
> @@ -207,6 +207,10 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
>  	regmap_write(priv->regmap, TIM_ARR, priv->max_arr);
>  	regmap_write(priv->regmap, TIM_PSC, psc);
>  
> +	/* 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);
> +
>  	/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */
>  	regmap_update_bits(priv->regmap,
>  			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
> 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	   */
> -- 
> 2.25.1
> 

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
@ 2023-01-03 12:34   ` Lee Jones
  0 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2023-01-03 12:34 UTC (permalink / raw)
  To: Olivier Moysan
  Cc: Fabrice Gasnier, Thierry Reding, Uwe Kleine-König,
	Maxime Coquelin, Alexandre Torgue, Benjamin Gaignard, linux-pwm,
	linux-stm32, linux-arm-kernel

On Tue, 13 Dec 2022, Olivier Moysan wrote:

> The PWM capture assumes that the input selector is set to default
> input and that the slave mode is disabled. Force reset state for
> TISEL and SMCR registers to match this requirement.
> 
> Note that slave mode disabling is not a pre-requisite by itself
> for capture mode, as hardware supports it for PWM capture.
> However, the current implementation of the driver does not
> allow slave mode for PWM capture. Setting slave mode for PWM
> capture results in wrong capture values.
> 
> Fixes: 53e38fe73f94 ("pwm: stm32: Add capture support")
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> ---
>  drivers/pwm/pwm-stm32.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/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
> index 794ca5b02968..24aab0450c78 100644
> --- a/drivers/pwm/pwm-stm32.c
> +++ b/drivers/pwm/pwm-stm32.c
> @@ -207,6 +207,10 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
>  	regmap_write(priv->regmap, TIM_ARR, priv->max_arr);
>  	regmap_write(priv->regmap, TIM_PSC, psc);
>  
> +	/* 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);
> +
>  	/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */
>  	regmap_update_bits(priv->regmap,
>  			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
> 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	   */
> -- 
> 2.25.1
> 

-- 
Lee Jones [李琼斯]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
  2022-12-14 15:09     ` Olivier MOYSAN
@ 2023-01-17 21:43       ` Uwe Kleine-König
  -1 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2023-01-17 21:43 UTC (permalink / raw)
  To: Olivier MOYSAN
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, Benjamin Gaignard,
	William Breathitt Gray, linux-pwm, linux-stm32, linux-arm-kernel


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

Hello,

On Wed, Dec 14, 2022 at 04:09:08PM +0100, Olivier MOYSAN wrote:
> Hello Uwe,
> 
> On 12/13/22 11:51, Uwe Kleine-König wrote:
> > Hello Olivier,
> > 
> > [Cc: += William Breathitt Gray, linux-iio@v.k.o]
> > 
> > On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote:
> > > The PWM capture assumes that the input selector is set to default
> > > input and that the slave mode is disabled. Force reset state for
> > > TISEL and SMCR registers to match this requirement.
> > 
> > When does the problem occur? Only if the bootloader changed that
> > setting? Regarding the urgency: With the current knowledge I'd say this
> > patch is material for the next merge window. Do you recommend
> > backporting to stable?
> > 
> 
> Yes, the PWM may not be in the default expected state, if the configuration
> has been changed in the bootloader. This is not an actual case today, so
> this patch can wait the next merge window and there is no
> urgency to have it in stable.

Then I'd drop the fixes line.

> > > Note that slave mode disabling is not a pre-requisite by itself
> > > for capture mode, as hardware supports it for PWM capture.
> > > However, the current implementation of the driver does not
> > > allow slave mode for PWM capture. Setting slave mode for PWM
> > > capture results in wrong capture values.
> > 
> > What is your usecase for PWM capture support? I didn't double check, but
> > I think you're the first contributor to PWM capture since 2018 (i.e. the
> > commit you're fixing).
> > 
> > Did you check if the counter subsystem would solve your problems? If it
> > doesn't I assume William would like to hear about that.
> > 
> > Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
> > slave mode, TISEL isn't touched though. So maybe this driver needs a
> > similar fix?

I want to come back to this question. I only checked lightly, but I
guess the counter patch needs the same patch.

Best regards
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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
@ 2023-01-17 21:43       ` Uwe Kleine-König
  0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2023-01-17 21:43 UTC (permalink / raw)
  To: Olivier MOYSAN
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, Benjamin Gaignard,
	William Breathitt Gray, linux-pwm, linux-stm32, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]

Hello,

On Wed, Dec 14, 2022 at 04:09:08PM +0100, Olivier MOYSAN wrote:
> Hello Uwe,
> 
> On 12/13/22 11:51, Uwe Kleine-König wrote:
> > Hello Olivier,
> > 
> > [Cc: += William Breathitt Gray, linux-iio@v.k.o]
> > 
> > On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote:
> > > The PWM capture assumes that the input selector is set to default
> > > input and that the slave mode is disabled. Force reset state for
> > > TISEL and SMCR registers to match this requirement.
> > 
> > When does the problem occur? Only if the bootloader changed that
> > setting? Regarding the urgency: With the current knowledge I'd say this
> > patch is material for the next merge window. Do you recommend
> > backporting to stable?
> > 
> 
> Yes, the PWM may not be in the default expected state, if the configuration
> has been changed in the bootloader. This is not an actual case today, so
> this patch can wait the next merge window and there is no
> urgency to have it in stable.

Then I'd drop the fixes line.

> > > Note that slave mode disabling is not a pre-requisite by itself
> > > for capture mode, as hardware supports it for PWM capture.
> > > However, the current implementation of the driver does not
> > > allow slave mode for PWM capture. Setting slave mode for PWM
> > > capture results in wrong capture values.
> > 
> > What is your usecase for PWM capture support? I didn't double check, but
> > I think you're the first contributor to PWM capture since 2018 (i.e. the
> > commit you're fixing).
> > 
> > Did you check if the counter subsystem would solve your problems? If it
> > doesn't I assume William would like to hear about that.
> > 
> > Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
> > slave mode, TISEL isn't touched though. So maybe this driver needs a
> > similar fix?

I want to come back to this question. I only checked lightly, but I
guess the counter patch needs the same patch.

Best regards
Uwe

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
  2023-01-17 21:43       ` Uwe Kleine-König
@ 2023-04-12 15:41         ` Uwe Kleine-König
  -1 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2023-04-12 15:41 UTC (permalink / raw)
  To: Olivier MOYSAN
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, William Breathitt Gray, linux-pwm,
	linux-stm32, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

Hello,

[Cc -= Benjamin Gaignard, their email address bounced]

On Tue, Jan 17, 2023 at 10:43:37PM +0100, Uwe Kleine-König wrote:
> On Wed, Dec 14, 2022 at 04:09:08PM +0100, Olivier MOYSAN wrote:
> > On 12/13/22 11:51, Uwe Kleine-König wrote:
> > > Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
> > > slave mode, TISEL isn't touched though. So maybe this driver needs a
> > > similar fix?
> 
> I want to come back to this question. I only checked lightly, but I
> guess the counter patch needs the same patch.

I cared for that one, now. See
https://lore.kernel.org/linux-iio/20230412153709.3557323-1-u.kleine-koenig@pengutronix.de

Best regards
Uwe

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] pwm: stm32: enforce settings for pwm capture
@ 2023-04-12 15:41         ` Uwe Kleine-König
  0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2023-04-12 15:41 UTC (permalink / raw)
  To: Olivier MOYSAN
  Cc: Fabrice Gasnier, Thierry Reding, Maxime Coquelin,
	Alexandre Torgue, Lee Jones, William Breathitt Gray, linux-pwm,
	linux-stm32, linux-arm-kernel


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

Hello,

[Cc -= Benjamin Gaignard, their email address bounced]

On Tue, Jan 17, 2023 at 10:43:37PM +0100, Uwe Kleine-König wrote:
> On Wed, Dec 14, 2022 at 04:09:08PM +0100, Olivier MOYSAN wrote:
> > On 12/13/22 11:51, Uwe Kleine-König wrote:
> > > Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in
> > > slave mode, TISEL isn't touched though. So maybe this driver needs a
> > > similar fix?
> 
> I want to come back to this question. I only checked lightly, but I
> guess the counter patch needs the same patch.

I cared for that one, now. See
https://lore.kernel.org/linux-iio/20230412153709.3557323-1-u.kleine-koenig@pengutronix.de

Best regards
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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-04-12 15:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-13 10:27 [PATCH] pwm: stm32: enforce settings for pwm capture Olivier Moysan
2022-12-13 10:27 ` Olivier Moysan
2022-12-13 10:51 ` Uwe Kleine-König
2022-12-13 10:51   ` Uwe Kleine-König
2022-12-14 15:09   ` Olivier MOYSAN
2022-12-14 15:09     ` Olivier MOYSAN
2023-01-17 21:43     ` Uwe Kleine-König
2023-01-17 21:43       ` Uwe Kleine-König
2023-04-12 15:41       ` Uwe Kleine-König
2023-04-12 15:41         ` Uwe Kleine-König
2023-01-03 12:34 ` Lee Jones
2023-01-03 12:34   ` Lee Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.