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 <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Signed-off-by: William Breathitt Gray <[email protected]>
---
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(-)
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 = {
--
2.30.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 <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: Alexandre Torgue <[email protected]>
> Signed-off-by: William Breathitt Gray <[email protected]>
> ---
> 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 <[email protected]>
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 = {
>
On Fri, 26 Feb 2021 16:24:32 +0100
Fabrice Gasnier <[email protected]> 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 <[email protected]>
> > Cc: Maxime Coquelin <[email protected]>
> > Cc: Alexandre Torgue <[email protected]>
> > Signed-off-by: William Breathitt Gray <[email protected]>
> > ---
> > 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 <[email protected]>
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 = {
> >