diff mbox series

[v2] counter: stm32-timer-cnt: Report count function when SLAVE_MODE_DISABLED

Message ID 20210226012931.161429-1-vilhelm.gray@gmail.com (mailing list archive)
State New
Headers show
Series [v2] counter: stm32-timer-cnt: Report count function when SLAVE_MODE_DISABLED | expand

Commit Message

William Breathitt Gray Feb. 26, 2021, 1:29 a.m. UTC
When in SLAVE_MODE_DISABLED mode, the count still increases if the
counter is enabled because an internal clock is used. This patch fixes
the stm32_count_function_get() and stm32_count_function_set() functions
to properly handle this behavior.

Fixes: ad29937e206f ("counter: Add STM32 Timer quadrature encoder")
Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
Changes in v2:
 - Support an explicit 0 case for function_get()/function_set()

 drivers/counter/stm32-timer-cnt.c | 39 ++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 14 deletions(-)

Comments

Fabrice Gasnier Feb. 26, 2021, 3:24 p.m. UTC | #1
On 2/26/21 2:29 AM, William Breathitt Gray wrote:
> When in SLAVE_MODE_DISABLED mode, the count still increases if the
> counter is enabled because an internal clock is used. This patch fixes
> the stm32_count_function_get() and stm32_count_function_set() functions
> to properly handle this behavior.
> 
> Fixes: ad29937e206f ("counter: Add STM32 Timer quadrature encoder")
> Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
> Changes in v2:
>  - Support an explicit 0 case for function_get()/function_set()
> 
>  drivers/counter/stm32-timer-cnt.c | 39 ++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 14 deletions(-)

Hi William,

Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Many thanks for this fix.
Best Regards,
Fabrice


> 
> diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
> index ef2a974a2f10..cd50dc12bd02 100644
> --- a/drivers/counter/stm32-timer-cnt.c
> +++ b/drivers/counter/stm32-timer-cnt.c
> @@ -44,13 +44,14 @@ struct stm32_timer_cnt {
>   * @STM32_COUNT_ENCODER_MODE_3: counts on both TI1FP1 and TI2FP2 edges
>   */
>  enum stm32_count_function {
> -	STM32_COUNT_SLAVE_MODE_DISABLED = -1,
> +	STM32_COUNT_SLAVE_MODE_DISABLED,
>  	STM32_COUNT_ENCODER_MODE_1,
>  	STM32_COUNT_ENCODER_MODE_2,
>  	STM32_COUNT_ENCODER_MODE_3,
>  };
>  
>  static enum counter_count_function stm32_count_functions[] = {
> +	[STM32_COUNT_SLAVE_MODE_DISABLED] = COUNTER_COUNT_FUNCTION_INCREASE,
>  	[STM32_COUNT_ENCODER_MODE_1] = COUNTER_COUNT_FUNCTION_QUADRATURE_X2_A,
>  	[STM32_COUNT_ENCODER_MODE_2] = COUNTER_COUNT_FUNCTION_QUADRATURE_X2_B,
>  	[STM32_COUNT_ENCODER_MODE_3] = COUNTER_COUNT_FUNCTION_QUADRATURE_X4,
> @@ -90,6 +91,9 @@ static int stm32_count_function_get(struct counter_device *counter,
>  	regmap_read(priv->regmap, TIM_SMCR, &smcr);
>  
>  	switch (smcr & TIM_SMCR_SMS) {
> +	case 0:
> +		*function = STM32_COUNT_SLAVE_MODE_DISABLED;
> +		return 0;
>  	case 1:
>  		*function = STM32_COUNT_ENCODER_MODE_1;
>  		return 0;
> @@ -99,9 +103,9 @@ static int stm32_count_function_get(struct counter_device *counter,
>  	case 3:
>  		*function = STM32_COUNT_ENCODER_MODE_3;
>  		return 0;
> +	default:
> +		return -EINVAL;
>  	}
> -
> -	return -EINVAL;
>  }
>  
>  static int stm32_count_function_set(struct counter_device *counter,
> @@ -112,6 +116,9 @@ static int stm32_count_function_set(struct counter_device *counter,
>  	u32 cr1, sms;
>  
>  	switch (function) {
> +	case STM32_COUNT_SLAVE_MODE_DISABLED:
> +		sms = 0;
> +		break;
>  	case STM32_COUNT_ENCODER_MODE_1:
>  		sms = 1;
>  		break;
> @@ -122,8 +129,7 @@ static int stm32_count_function_set(struct counter_device *counter,
>  		sms = 3;
>  		break;
>  	default:
> -		sms = 0;
> -		break;
> +		return -EINVAL;
>  	}
>  
>  	/* Store enable status */
> @@ -274,31 +280,36 @@ static int stm32_action_get(struct counter_device *counter,
>  	size_t function;
>  	int err;
>  
> -	/* Default action mode (e.g. STM32_COUNT_SLAVE_MODE_DISABLED) */
> -	*action = STM32_SYNAPSE_ACTION_NONE;
> -
>  	err = stm32_count_function_get(counter, count, &function);
>  	if (err)
> -		return 0;
> +		return err;
>  
>  	switch (function) {
> +	case STM32_COUNT_SLAVE_MODE_DISABLED:
> +		/* counts on internal clock when CEN=1 */
> +		*action = STM32_SYNAPSE_ACTION_NONE;
> +		return 0;
>  	case STM32_COUNT_ENCODER_MODE_1:
>  		/* counts up/down on TI1FP1 edge depending on TI2FP2 level */
>  		if (synapse->signal->id == count->synapses[0].signal->id)
>  			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
> -		break;
> +		else
> +			*action = STM32_SYNAPSE_ACTION_NONE;
> +		return 0;
>  	case STM32_COUNT_ENCODER_MODE_2:
>  		/* counts up/down on TI2FP2 edge depending on TI1FP1 level */
>  		if (synapse->signal->id == count->synapses[1].signal->id)
>  			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
> -		break;
> +		else
> +			*action = STM32_SYNAPSE_ACTION_NONE;
> +		return 0;
>  	case STM32_COUNT_ENCODER_MODE_3:
>  		/* counts up/down on both TI1FP1 and TI2FP2 edges */
>  		*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
> -		break;
> +		return 0;
> +	default:
> +		return -EINVAL;
>  	}
> -
> -	return 0;
>  }
>  
>  static const struct counter_ops stm32_timer_cnt_ops = {
>
Jonathan Cameron Feb. 27, 2021, 4:11 p.m. UTC | #2
On Fri, 26 Feb 2021 16:24:32 +0100
Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote:

> On 2/26/21 2:29 AM, William Breathitt Gray wrote:
> > When in SLAVE_MODE_DISABLED mode, the count still increases if the
> > counter is enabled because an internal clock is used. This patch fixes
> > the stm32_count_function_get() and stm32_count_function_set() functions
> > to properly handle this behavior.
> > 
> > Fixes: ad29937e206f ("counter: Add STM32 Timer quadrature encoder")
> > Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> > Cc: Alexandre Torgue <alexandre.torgue@st.com>
> > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> > ---
> > Changes in v2:
> >  - Support an explicit 0 case for function_get()/function_set()
> > 
> >  drivers/counter/stm32-timer-cnt.c | 39 ++++++++++++++++++++-----------
> >  1 file changed, 25 insertions(+), 14 deletions(-)  
> 
> Hi William,
> 
> Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

> 
> Many thanks for this fix.
> Best Regards,
> Fabrice
> 
> 
> > 
> > diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
> > index ef2a974a2f10..cd50dc12bd02 100644
> > --- a/drivers/counter/stm32-timer-cnt.c
> > +++ b/drivers/counter/stm32-timer-cnt.c
> > @@ -44,13 +44,14 @@ struct stm32_timer_cnt {
> >   * @STM32_COUNT_ENCODER_MODE_3: counts on both TI1FP1 and TI2FP2 edges
> >   */
> >  enum stm32_count_function {
> > -	STM32_COUNT_SLAVE_MODE_DISABLED = -1,
> > +	STM32_COUNT_SLAVE_MODE_DISABLED,
> >  	STM32_COUNT_ENCODER_MODE_1,
> >  	STM32_COUNT_ENCODER_MODE_2,
> >  	STM32_COUNT_ENCODER_MODE_3,
> >  };
> >  
> >  static enum counter_count_function stm32_count_functions[] = {
> > +	[STM32_COUNT_SLAVE_MODE_DISABLED] = COUNTER_COUNT_FUNCTION_INCREASE,
> >  	[STM32_COUNT_ENCODER_MODE_1] = COUNTER_COUNT_FUNCTION_QUADRATURE_X2_A,
> >  	[STM32_COUNT_ENCODER_MODE_2] = COUNTER_COUNT_FUNCTION_QUADRATURE_X2_B,
> >  	[STM32_COUNT_ENCODER_MODE_3] = COUNTER_COUNT_FUNCTION_QUADRATURE_X4,
> > @@ -90,6 +91,9 @@ static int stm32_count_function_get(struct counter_device *counter,
> >  	regmap_read(priv->regmap, TIM_SMCR, &smcr);
> >  
> >  	switch (smcr & TIM_SMCR_SMS) {
> > +	case 0:
> > +		*function = STM32_COUNT_SLAVE_MODE_DISABLED;
> > +		return 0;
> >  	case 1:
> >  		*function = STM32_COUNT_ENCODER_MODE_1;
> >  		return 0;
> > @@ -99,9 +103,9 @@ static int stm32_count_function_get(struct counter_device *counter,
> >  	case 3:
> >  		*function = STM32_COUNT_ENCODER_MODE_3;
> >  		return 0;
> > +	default:
> > +		return -EINVAL;
> >  	}
> > -
> > -	return -EINVAL;
> >  }
> >  
> >  static int stm32_count_function_set(struct counter_device *counter,
> > @@ -112,6 +116,9 @@ static int stm32_count_function_set(struct counter_device *counter,
> >  	u32 cr1, sms;
> >  
> >  	switch (function) {
> > +	case STM32_COUNT_SLAVE_MODE_DISABLED:
> > +		sms = 0;
> > +		break;
> >  	case STM32_COUNT_ENCODER_MODE_1:
> >  		sms = 1;
> >  		break;
> > @@ -122,8 +129,7 @@ static int stm32_count_function_set(struct counter_device *counter,
> >  		sms = 3;
> >  		break;
> >  	default:
> > -		sms = 0;
> > -		break;
> > +		return -EINVAL;
> >  	}
> >  
> >  	/* Store enable status */
> > @@ -274,31 +280,36 @@ static int stm32_action_get(struct counter_device *counter,
> >  	size_t function;
> >  	int err;
> >  
> > -	/* Default action mode (e.g. STM32_COUNT_SLAVE_MODE_DISABLED) */
> > -	*action = STM32_SYNAPSE_ACTION_NONE;
> > -
> >  	err = stm32_count_function_get(counter, count, &function);
> >  	if (err)
> > -		return 0;
> > +		return err;
> >  
> >  	switch (function) {
> > +	case STM32_COUNT_SLAVE_MODE_DISABLED:
> > +		/* counts on internal clock when CEN=1 */
> > +		*action = STM32_SYNAPSE_ACTION_NONE;
> > +		return 0;
> >  	case STM32_COUNT_ENCODER_MODE_1:
> >  		/* counts up/down on TI1FP1 edge depending on TI2FP2 level */
> >  		if (synapse->signal->id == count->synapses[0].signal->id)
> >  			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
> > -		break;
> > +		else
> > +			*action = STM32_SYNAPSE_ACTION_NONE;
> > +		return 0;
> >  	case STM32_COUNT_ENCODER_MODE_2:
> >  		/* counts up/down on TI2FP2 edge depending on TI1FP1 level */
> >  		if (synapse->signal->id == count->synapses[1].signal->id)
> >  			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
> > -		break;
> > +		else
> > +			*action = STM32_SYNAPSE_ACTION_NONE;
> > +		return 0;
> >  	case STM32_COUNT_ENCODER_MODE_3:
> >  		/* counts up/down on both TI1FP1 and TI2FP2 edges */
> >  		*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
> > -		break;
> > +		return 0;
> > +	default:
> > +		return -EINVAL;
> >  	}
> > -
> > -	return 0;
> >  }
> >  
> >  static const struct counter_ops stm32_timer_cnt_ops = {
> >
diff mbox series

Patch

diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index ef2a974a2f10..cd50dc12bd02 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -44,13 +44,14 @@  struct stm32_timer_cnt {
  * @STM32_COUNT_ENCODER_MODE_3: counts on both TI1FP1 and TI2FP2 edges
  */
 enum stm32_count_function {
-	STM32_COUNT_SLAVE_MODE_DISABLED = -1,
+	STM32_COUNT_SLAVE_MODE_DISABLED,
 	STM32_COUNT_ENCODER_MODE_1,
 	STM32_COUNT_ENCODER_MODE_2,
 	STM32_COUNT_ENCODER_MODE_3,
 };
 
 static enum counter_count_function stm32_count_functions[] = {
+	[STM32_COUNT_SLAVE_MODE_DISABLED] = COUNTER_COUNT_FUNCTION_INCREASE,
 	[STM32_COUNT_ENCODER_MODE_1] = COUNTER_COUNT_FUNCTION_QUADRATURE_X2_A,
 	[STM32_COUNT_ENCODER_MODE_2] = COUNTER_COUNT_FUNCTION_QUADRATURE_X2_B,
 	[STM32_COUNT_ENCODER_MODE_3] = COUNTER_COUNT_FUNCTION_QUADRATURE_X4,
@@ -90,6 +91,9 @@  static int stm32_count_function_get(struct counter_device *counter,
 	regmap_read(priv->regmap, TIM_SMCR, &smcr);
 
 	switch (smcr & TIM_SMCR_SMS) {
+	case 0:
+		*function = STM32_COUNT_SLAVE_MODE_DISABLED;
+		return 0;
 	case 1:
 		*function = STM32_COUNT_ENCODER_MODE_1;
 		return 0;
@@ -99,9 +103,9 @@  static int stm32_count_function_get(struct counter_device *counter,
 	case 3:
 		*function = STM32_COUNT_ENCODER_MODE_3;
 		return 0;
+	default:
+		return -EINVAL;
 	}
-
-	return -EINVAL;
 }
 
 static int stm32_count_function_set(struct counter_device *counter,
@@ -112,6 +116,9 @@  static int stm32_count_function_set(struct counter_device *counter,
 	u32 cr1, sms;
 
 	switch (function) {
+	case STM32_COUNT_SLAVE_MODE_DISABLED:
+		sms = 0;
+		break;
 	case STM32_COUNT_ENCODER_MODE_1:
 		sms = 1;
 		break;
@@ -122,8 +129,7 @@  static int stm32_count_function_set(struct counter_device *counter,
 		sms = 3;
 		break;
 	default:
-		sms = 0;
-		break;
+		return -EINVAL;
 	}
 
 	/* Store enable status */
@@ -274,31 +280,36 @@  static int stm32_action_get(struct counter_device *counter,
 	size_t function;
 	int err;
 
-	/* Default action mode (e.g. STM32_COUNT_SLAVE_MODE_DISABLED) */
-	*action = STM32_SYNAPSE_ACTION_NONE;
-
 	err = stm32_count_function_get(counter, count, &function);
 	if (err)
-		return 0;
+		return err;
 
 	switch (function) {
+	case STM32_COUNT_SLAVE_MODE_DISABLED:
+		/* counts on internal clock when CEN=1 */
+		*action = STM32_SYNAPSE_ACTION_NONE;
+		return 0;
 	case STM32_COUNT_ENCODER_MODE_1:
 		/* counts up/down on TI1FP1 edge depending on TI2FP2 level */
 		if (synapse->signal->id == count->synapses[0].signal->id)
 			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
-		break;
+		else
+			*action = STM32_SYNAPSE_ACTION_NONE;
+		return 0;
 	case STM32_COUNT_ENCODER_MODE_2:
 		/* counts up/down on TI2FP2 edge depending on TI1FP1 level */
 		if (synapse->signal->id == count->synapses[1].signal->id)
 			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
-		break;
+		else
+			*action = STM32_SYNAPSE_ACTION_NONE;
+		return 0;
 	case STM32_COUNT_ENCODER_MODE_3:
 		/* counts up/down on both TI1FP1 and TI2FP2 edges */
 		*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
-		break;
+		return 0;
+	default:
+		return -EINVAL;
 	}
-
-	return 0;
 }
 
 static const struct counter_ops stm32_timer_cnt_ops = {