Miscellaneous fixes for stm32-timer-trigger driver.
Last patch adds enable attribute, which unlocks use of
encoder mode.
Fabrice Gasnier (4):
iio: trigger: stm32-timer: fix quadrature mode get routine
iio: trigger: stm32-timer: fix write_raw return value
iio: trigger: stm32-timer: fix get/set down count direction
iio: trigger: stm32-timer: add enable attribute
drivers/iio/trigger/stm32-timer-trigger.c | 80 ++++++++++++++++++++++---------
1 file changed, 58 insertions(+), 22 deletions(-)
--
1.9.1
Fixes: 4adec7da0536 ("iio: stm32 trigger: Add quadrature encoder device")
SMS bitfiled is mode + 1. After reset, upon boot, SMS = 0. When reading
from sysfs, stm32_get_quadrature_mode() returns -1 (e.g. -EPERM) which is
wrong error code here. So, check SMS bitfiled matches valid encoder mode,
or return -EINVAL.
Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/trigger/stm32-timer-trigger.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index d22bc56..6aa73d6 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -571,11 +571,14 @@ static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
u32 smcr;
+ int mode;
regmap_read(priv->regmap, TIM_SMCR, &smcr);
- smcr &= TIM_SMCR_SMS;
+ mode = (smcr & TIM_SMCR_SMS) - 1;
+ if ((mode < 0) || (mode > ARRAY_SIZE(stm32_quadrature_modes)))
+ return -EINVAL;
- return smcr - 1;
+ return mode;
}
static const struct iio_enum stm32_quadrature_mode_enum = {
--
1.9.1
In order to use encoder mode, timers needs to be enabled (e.g. CEN bit)
along with peripheral clock.
Add IIO_CHAN_INFO_ENABLE attribute to handle this.
Also, in triggered mode, CEN bit is set automatically in hardware.
Then clock must be enabled before starting triggered mode.
Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/trigger/stm32-timer-trigger.c | 55 +++++++++++++++++++++++--------
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index d28aa02..14e6eb0 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -366,34 +366,32 @@ static int stm32_counter_read_raw(struct iio_dev *indio_dev,
int *val, int *val2, long mask)
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+ u32 dat;
switch (mask) {
case IIO_CHAN_INFO_RAW:
- {
- u32 cnt;
-
- regmap_read(priv->regmap, TIM_CNT, &cnt);
- *val = cnt;
+ regmap_read(priv->regmap, TIM_CNT, &dat);
+ *val = dat;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_ENABLE:
+ regmap_read(priv->regmap, TIM_CR1, &dat);
+ *val = (dat & TIM_CR1_CEN) ? 1 : 0;
return IIO_VAL_INT;
- }
- case IIO_CHAN_INFO_SCALE:
- {
- u32 smcr;
- regmap_read(priv->regmap, TIM_SMCR, &smcr);
- smcr &= TIM_SMCR_SMS;
+ case IIO_CHAN_INFO_SCALE:
+ regmap_read(priv->regmap, TIM_SMCR, &dat);
+ dat &= TIM_SMCR_SMS;
*val = 1;
*val2 = 0;
/* in quadrature case scale = 0.25 */
- if (smcr == 3)
+ if (dat == 3)
*val2 = 2;
return IIO_VAL_FRACTIONAL_LOG2;
}
- }
return -EINVAL;
}
@@ -403,6 +401,7 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
int val, int val2, long mask)
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+ u32 dat;
switch (mask) {
case IIO_CHAN_INFO_RAW:
@@ -411,6 +410,22 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SCALE:
/* fixed scale */
return -EINVAL;
+
+ case IIO_CHAN_INFO_ENABLE:
+ if (val) {
+ regmap_read(priv->regmap, TIM_CR1, &dat);
+ if (!(dat & TIM_CR1_CEN))
+ clk_enable(priv->clk);
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
+ TIM_CR1_CEN);
+ } else {
+ regmap_read(priv->regmap, TIM_CR1, &dat);
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
+ 0);
+ if (dat & TIM_CR1_CEN)
+ clk_disable(priv->clk);
+ }
+ return 0;
}
return -EINVAL;
@@ -506,9 +521,19 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
int sms = stm32_enable_mode2sms(mode);
+ u32 val;
if (sms < 0)
return sms;
+ /*
+ * Triggered mode sets CEN bit automatically by hardware. So, first
+ * enable counter clock, so it can use it. Keeps it in sync with CEN.
+ */
+ if (sms == 6) {
+ regmap_read(priv->regmap, TIM_CR1, &val);
+ if (!(val & TIM_CR1_CEN))
+ clk_enable(priv->clk);
+ }
regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
@@ -681,7 +706,9 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
static const struct iio_chan_spec stm32_trigger_channel = {
.type = IIO_COUNT,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_ENABLE) |
+ BIT(IIO_CHAN_INFO_SCALE),
.ext_info = stm32_trigger_count_info,
.indexed = 1
};
--
1.9.1
Fixes: 4adec7da0536 ("iio: stm32 trigger: Add quadrature encoder device")
IIO core expects zero as return value for write_raw() callback
in case of success.
Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/trigger/stm32-timer-trigger.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index 6aa73d6..107918b 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -406,9 +406,8 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
- regmap_write(priv->regmap, TIM_CNT, val);
+ return regmap_write(priv->regmap, TIM_CNT, val);
- return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
/* fixed scale */
return -EINVAL;
--
1.9.1
Fixes: 4adec7da0536 ("iio: stm32 trigger: Add quadrature encoder device")
This fixes two issues:
- stm32_set_count_direction: to set down direction
- stm32_get_count_direction: to get down direction
IIO core provides/expects value to be an index of iio_enum items array.
This needs to be turned by these routines into TIM_CR1_DIR (e.g. BIT(4))
value.
Also, report error when attempting to write direction, when in encoder
mode: in this case, direction is read only (given by encoder inputs).
Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/trigger/stm32-timer-trigger.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index 107918b..d28aa02 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -594,13 +594,20 @@ static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
static int stm32_set_count_direction(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
- unsigned int mode)
+ unsigned int dir)
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+ u32 val;
+ int mode;
- regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
+ /* In encoder mode, direction is RO (given by TI1/TI2 signals) */
+ regmap_read(priv->regmap, TIM_SMCR, &val);
+ mode = (val & TIM_SMCR_SMS) - 1;
+ if ((mode >= 0) || (mode < ARRAY_SIZE(stm32_quadrature_modes)))
+ return -EBUSY;
- return 0;
+ return regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR,
+ dir ? TIM_CR1_DIR : 0);
}
static int stm32_get_count_direction(struct iio_dev *indio_dev,
@@ -611,7 +618,7 @@ static int stm32_get_count_direction(struct iio_dev *indio_dev,
regmap_read(priv->regmap, TIM_CR1, &cr1);
- return (cr1 & TIM_CR1_DIR);
+ return ((cr1 & TIM_CR1_DIR) ? 1 : 0);
}
static const struct iio_enum stm32_count_direction_enum = {
--
1.9.1
2017-07-27 18:18 GMT+02:00 Fabrice Gasnier <[email protected]>:
> Miscellaneous fixes for stm32-timer-trigger driver.
> Last patch adds enable attribute, which unlocks use of
> encoder mode.
>
> Fabrice Gasnier (4):
> iio: trigger: stm32-timer: fix quadrature mode get routine
> iio: trigger: stm32-timer: fix write_raw return value
> iio: trigger: stm32-timer: fix get/set down count direction
> iio: trigger: stm32-timer: add enable attribute
>
For the series:
Acked-by: Benjamin Gaignard <[email protected]>
> drivers/iio/trigger/stm32-timer-trigger.c | 80 ++++++++++++++++++++++---------
> 1 file changed, 58 insertions(+), 22 deletions(-)
>
> --
> 1.9.1
>
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, 27 Jul 2017 18:18:57 +0200
Fabrice Gasnier <[email protected]> wrote:
> Fixes: 4adec7da0536 ("iio: stm32 trigger: Add quadrature encoder device")
>
> SMS bitfiled is mode + 1. After reset, upon boot, SMS = 0. When reading
> from sysfs, stm32_get_quadrature_mode() returns -1 (e.g. -EPERM) which is
> wrong error code here. So, check SMS bitfiled matches valid encoder mode,
> or return -EINVAL.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
> ---
> drivers/iio/trigger/stm32-timer-trigger.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index d22bc56..6aa73d6 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -571,11 +571,14 @@ static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> u32 smcr;
> + int mode;
>
> regmap_read(priv->regmap, TIM_SMCR, &smcr);
> - smcr &= TIM_SMCR_SMS;
> + mode = (smcr & TIM_SMCR_SMS) - 1;
> + if ((mode < 0) || (mode > ARRAY_SIZE(stm32_quadrature_modes)))
> + return -EINVAL;
>
> - return smcr - 1;
> + return mode;
> }
>
> static const struct iio_enum stm32_quadrature_mode_enum = {
On Thu, 27 Jul 2017 18:18:58 +0200
Fabrice Gasnier <[email protected]> wrote:
> Fixes: 4adec7da0536 ("iio: stm32 trigger: Add quadrature encoder device")
>
> IIO core expects zero as return value for write_raw() callback
> in case of success.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the fixes-togreg branch of iio.git.
Thanks
Jonathan
> ---
> drivers/iio/trigger/stm32-timer-trigger.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index 6aa73d6..107918b 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -406,9 +406,8 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - regmap_write(priv->regmap, TIM_CNT, val);
> + return regmap_write(priv->regmap, TIM_CNT, val);
>
> - return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> /* fixed scale */
> return -EINVAL;
On Thu, 27 Jul 2017 18:18:59 +0200
Fabrice Gasnier <[email protected]> wrote:
> Fixes: 4adec7da0536 ("iio: stm32 trigger: Add quadrature encoder device")
>
> This fixes two issues:
> - stm32_set_count_direction: to set down direction
> - stm32_get_count_direction: to get down direction
>
> IIO core provides/expects value to be an index of iio_enum items array.
> This needs to be turned by these routines into TIM_CR1_DIR (e.g. BIT(4))
> value.
> Also, report error when attempting to write direction, when in encoder
> mode: in this case, direction is read only (given by encoder inputs).
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
> ---
> drivers/iio/trigger/stm32-timer-trigger.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index 107918b..d28aa02 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -594,13 +594,20 @@ static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
>
> static int stm32_set_count_direction(struct iio_dev *indio_dev,
> const struct iio_chan_spec *chan,
> - unsigned int mode)
> + unsigned int dir)
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> + u32 val;
> + int mode;
>
> - regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
> + /* In encoder mode, direction is RO (given by TI1/TI2 signals) */
> + regmap_read(priv->regmap, TIM_SMCR, &val);
> + mode = (val & TIM_SMCR_SMS) - 1;
> + if ((mode >= 0) || (mode < ARRAY_SIZE(stm32_quadrature_modes)))
> + return -EBUSY;
>
> - return 0;
> + return regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR,
> + dir ? TIM_CR1_DIR : 0);
> }
>
> static int stm32_get_count_direction(struct iio_dev *indio_dev,
> @@ -611,7 +618,7 @@ static int stm32_get_count_direction(struct iio_dev *indio_dev,
>
> regmap_read(priv->regmap, TIM_CR1, &cr1);
>
> - return (cr1 & TIM_CR1_DIR);
> + return ((cr1 & TIM_CR1_DIR) ? 1 : 0);
> }
>
> static const struct iio_enum stm32_count_direction_enum = {
On Thu, 27 Jul 2017 18:19:00 +0200
Fabrice Gasnier <[email protected]> wrote:
> In order to use encoder mode, timers needs to be enabled (e.g. CEN bit)
> along with peripheral clock.
> Add IIO_CHAN_INFO_ENABLE attribute to handle this.
> Also, in triggered mode, CEN bit is set automatically in hardware.
> Then clock must be enabled before starting triggered mode.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
This one could perhaps have been a little more compact given
it is a fix, but it's still not huge so we'll let it go.
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
> ---
> drivers/iio/trigger/stm32-timer-trigger.c | 55 +++++++++++++++++++++++--------
> 1 file changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index d28aa02..14e6eb0 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -366,34 +366,32 @@ static int stm32_counter_read_raw(struct iio_dev *indio_dev,
> int *val, int *val2, long mask)
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> + u32 dat;
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - {
> - u32 cnt;
> -
> - regmap_read(priv->regmap, TIM_CNT, &cnt);
> - *val = cnt;
> + regmap_read(priv->regmap, TIM_CNT, &dat);
> + *val = dat;
> + return IIO_VAL_INT;
>
> + case IIO_CHAN_INFO_ENABLE:
> + regmap_read(priv->regmap, TIM_CR1, &dat);
> + *val = (dat & TIM_CR1_CEN) ? 1 : 0;
> return IIO_VAL_INT;
> - }
> - case IIO_CHAN_INFO_SCALE:
> - {
> - u32 smcr;
>
> - regmap_read(priv->regmap, TIM_SMCR, &smcr);
> - smcr &= TIM_SMCR_SMS;
> + case IIO_CHAN_INFO_SCALE:
> + regmap_read(priv->regmap, TIM_SMCR, &dat);
> + dat &= TIM_SMCR_SMS;
>
> *val = 1;
> *val2 = 0;
>
> /* in quadrature case scale = 0.25 */
> - if (smcr == 3)
> + if (dat == 3)
> *val2 = 2;
>
> return IIO_VAL_FRACTIONAL_LOG2;
> }
> - }
>
> return -EINVAL;
> }
> @@ -403,6 +401,7 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> int val, int val2, long mask)
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> + u32 dat;
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> @@ -411,6 +410,22 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_SCALE:
> /* fixed scale */
> return -EINVAL;
> +
> + case IIO_CHAN_INFO_ENABLE:
> + if (val) {
> + regmap_read(priv->regmap, TIM_CR1, &dat);
> + if (!(dat & TIM_CR1_CEN))
> + clk_enable(priv->clk);
> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
> + TIM_CR1_CEN);
> + } else {
> + regmap_read(priv->regmap, TIM_CR1, &dat);
> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
> + 0);
> + if (dat & TIM_CR1_CEN)
> + clk_disable(priv->clk);
> + }
> + return 0;
> }
>
> return -EINVAL;
> @@ -506,9 +521,19 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> int sms = stm32_enable_mode2sms(mode);
> + u32 val;
>
> if (sms < 0)
> return sms;
> + /*
> + * Triggered mode sets CEN bit automatically by hardware. So, first
> + * enable counter clock, so it can use it. Keeps it in sync with CEN.
> + */
> + if (sms == 6) {
> + regmap_read(priv->regmap, TIM_CR1, &val);
> + if (!(val & TIM_CR1_CEN))
> + clk_enable(priv->clk);
> + }
>
> regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>
> @@ -681,7 +706,9 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
> static const struct iio_chan_spec stm32_trigger_channel = {
> .type = IIO_COUNT,
> .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_ENABLE) |
> + BIT(IIO_CHAN_INFO_SCALE),
> .ext_info = stm32_trigger_count_info,
> .indexed = 1
> };