All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] iio: stm32 trigger: Add quadrature device and counter
@ 2017-03-27  9:43 Benjamin Gaignard
  2017-03-27  9:43 ` [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device Benjamin Gaignard
  2017-03-27  9:43 ` [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes Benjamin Gaignard
  0 siblings, 2 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-03-27  9:43 UTC (permalink / raw)
  To: linux-kernel, jic23, linux-iio, knaack.h, lars, pmeerw,
	vilhelm.gray, mwelling
  Cc: fabrice.gasnier, linaro-kernel, benjamin.gaignard, Benjamin Gaignard

A first encoder IIO driver (104-quad-8) with a count channel already exist.
STM32 driver have the same type of feature but with different counting modes.
Some parts, like counting direction (up/down) could be generalized so move the
corresponding documentation from driver file to sysfs-bus-iio.

version 3:
- fix typo in documentation
- change some functions names

version 2:
- since iio: Generalize counting direction modes commit of version has being
  pushed in iio.git testing branch I have remove it from this serie.
- split the original attribute in 2: one for quadrature modes and one for
  enable modes.
- add scale info
- change modes to more generic names to be able to merge them with 104-quad-8
  in the futur.

Benjamin Gaignard (2):
  iio: stm32 trigger: Add quadrature encoder device
  iio: stm32 trigger: Add counter enable modes

 .../ABI/testing/sysfs-bus-iio-timer-stm32          |  55 ++++
 drivers/iio/trigger/stm32-timer-trigger.c          | 314 ++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h                   |   2 +
 3 files changed, 365 insertions(+), 6 deletions(-)

-- 
1.9.1

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

* [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device
  2017-03-27  9:43 [PATCH v3 0/2] iio: stm32 trigger: Add quadrature device and counter Benjamin Gaignard
@ 2017-03-27  9:43 ` Benjamin Gaignard
  2017-03-27 13:47   ` William Breathitt Gray
  2017-04-02 11:08   ` Jonathan Cameron
  2017-03-27  9:43 ` [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes Benjamin Gaignard
  1 sibling, 2 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-03-27  9:43 UTC (permalink / raw)
  To: linux-kernel, jic23, linux-iio, knaack.h, lars, pmeerw,
	vilhelm.gray, mwelling
  Cc: fabrice.gasnier, linaro-kernel, benjamin.gaignard, Benjamin Gaignard

One of the features of STM32 trigger hardware block is a quadrature
encoder that can counts up/down depending of the levels and edges
of the selected external pins.

This patch allow to read/write the counter, get it direction,
set/get quadrature modes and get scale factor.

When counting up preset value is the limit of the counter.
When counting down the counter start from preset value down to 0.
This preset value could be set/get by using
/sys/bus/iio/devices/iio:deviceX/in_count0_preset attribute.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

version 3:
- fix typo in documentation
- change some functions names
---
 .../ABI/testing/sysfs-bus-iio-timer-stm32          |  32 +++
 drivers/iio/trigger/stm32-timer-trigger.c          | 244 ++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h                   |   2 +
 3 files changed, 272 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
index 6534a60..bf795ad 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
@@ -27,3 +27,35 @@ Description:
 		Reading returns the current sampling frequency.
 		Writing an value different of 0 set and start sampling.
 		Writing 0 stop sampling.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_count0_preset
+KernelVersion:	4.12
+Contact:	benjamin.gaignard@st.com
+Description:
+		Reading returns the current preset value.
+		Writing sets the preset value.
+		When counting up the counter starts from 0 and fires an event when reach preset value.
+		When counting down the counter start from preset value and fire event when reach 0.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
+KernelVersion:	4.12
+Contact:	benjamin.gaignard@st.com
+Description:
+		Reading returns the list possible quadrature modes.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
+KernelVersion:	4.12
+Contact:	benjamin.gaignard@st.com
+Description:
+		Configure the device counter quadrature modes:
+		channel_A:
+			Encoder A input servers as the count input and B as
+			the UP/DOWN direction control input.
+
+		channel_B:
+			Encoder B input serves as the count input and A as
+			the UP/DOWN direction control input.
+
+		quadrature:
+			Encoder A and B inputs are mixed to get direction
+			and count with a scale of 0.25.
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index 994b96d..7db904c 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 
 #define MAX_TRIGGERS 6
+#define MAX_VALIDS 5
 
 /* List the triggers created by each timer */
 static const void *triggers_table[][MAX_TRIGGERS] = {
@@ -32,12 +33,29 @@
 	{ TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
 };
 
+/* List the triggers accepted by each timer */
+static const void *valids_table[][MAX_VALIDS] = {
+	{ TIM5_TRGO, TIM2_TRGO, TIM3_TRGO, TIM4_TRGO,},
+	{ TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
+	{ TIM1_TRGO, TIM2_TRGO, TIM5_TRGO, TIM4_TRGO,},
+	{ TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
+	{ TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
+	{ }, /* timer 6 */
+	{ }, /* timer 7 */
+	{ TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
+	{ TIM2_TRGO, TIM3_TRGO,},
+	{ }, /* timer 10 */
+	{ }, /* timer 11 */
+	{ TIM4_TRGO, TIM5_TRGO,},
+};
+
 struct stm32_timer_trigger {
 	struct device *dev;
 	struct regmap *regmap;
 	struct clk *clk;
 	u32 max_arr;
 	const void *triggers;
+	const void *valids;
 };
 
 static int stm32_timer_start(struct stm32_timer_trigger *priv,
@@ -180,8 +198,7 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
 {
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
 	u32 cr2;
 
 	regmap_read(priv->regmap, TIM_CR2, &cr2);
@@ -194,8 +211,7 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
 					  struct device_attribute *attr,
 					  const char *buf, size_t len)
 {
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
@@ -275,6 +291,216 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
 	return 0;
 }
 
+static int stm32_counter_read_raw(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan,
+				  int *val, int *val2, long mask)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	{
+		u32 cnt;
+
+		regmap_read(priv->regmap, TIM_CNT, &cnt);
+		*val = cnt;
+
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_SCALE:
+	{
+		u32 smcr;
+
+		regmap_read(priv->regmap, TIM_SMCR, &smcr);
+		smcr &= TIM_SMCR_SMS;
+
+		*val = 1;
+		*val2 = 0;
+
+		/* in quadrature case scale = 0.25 */
+		if (smcr == 3)
+			*val2 = 2;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	}
+	}
+
+	return -EINVAL;
+}
+
+static int stm32_counter_write_raw(struct iio_dev *indio_dev,
+				   struct iio_chan_spec const *chan,
+				   int val, int val2, long mask)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		regmap_write(priv->regmap, TIM_CNT, val);
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/* fixed scale */
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info stm32_trigger_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = stm32_counter_read_raw,
+	.write_raw = stm32_counter_write_raw
+};
+
+static const char *const stm32_quadrature_modes[] = {
+	"channel_A",
+	"channel_B",
+	"quadrature",
+};
+
+static int stm32_set_quadrature_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int mode)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+
+	regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, mode + 1);
+
+	return 0;
+}
+
+static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	u32 smcr;
+
+	regmap_read(priv->regmap, TIM_SMCR, &smcr);
+	smcr &= TIM_SMCR_SMS;
+
+	return smcr - 1;
+}
+
+static const struct iio_enum stm32_quadrature_mode_enum = {
+	.items = stm32_quadrature_modes,
+	.num_items = ARRAY_SIZE(stm32_quadrature_modes),
+	.set = stm32_set_quadrature_mode,
+	.get = stm32_get_quadrature_mode
+};
+
+static const char *const stm32_count_direction_states[] = {
+	"up",
+	"down"
+};
+
+static int stm32_set_count_direction(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int mode)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
+
+	return 0;
+}
+
+static int stm32_get_count_direction(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	u32 cr1;
+
+	regmap_read(priv->regmap, TIM_CR1, &cr1);
+
+	return (cr1 & TIM_CR1_DIR);
+}
+
+static const struct iio_enum stm32_count_direction_enum = {
+	.items = stm32_count_direction_states,
+	.num_items = ARRAY_SIZE(stm32_count_direction_states),
+	.set = stm32_set_count_direction,
+	.get = stm32_get_count_direction
+};
+
+static ssize_t stm32_count_get_preset(struct iio_dev *indio_dev,
+				      uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      char *buf)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	u32 arr;
+
+	regmap_read(priv->regmap, TIM_ARR, &arr);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", arr);
+}
+
+static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
+				      uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      const char *buf, size_t len)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	unsigned int preset;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &preset);
+	if (ret)
+		return ret;
+
+	regmap_write(priv->regmap, TIM_ARR, preset);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
+
+	return len;
+}
+
+static const struct iio_chan_spec_ext_info stm32_trigger_count_info[] = {
+	{
+		.name = "preset",
+		.shared = IIO_SEPARATE,
+		.read = stm32_count_get_preset,
+		.write = stm32_count_set_preset
+	},
+	IIO_ENUM("count_direction", IIO_SEPARATE, &stm32_count_direction_enum),
+	IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
+	IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
+	IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
+	{}
+};
+
+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),
+	.ext_info = stm32_trigger_count_info,
+	.indexed = 1
+};
+
+static struct stm32_timer_trigger *stm32_setup_counter_device(struct device *dev)
+{
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev,
+					  sizeof(struct stm32_timer_trigger));
+	if (!indio_dev)
+		return NULL;
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &stm32_trigger_info;
+	indio_dev->num_channels = 1;
+	indio_dev->channels = &stm32_trigger_channel;
+	indio_dev->dev.of_node = dev->of_node;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return NULL;
+
+	return iio_priv(indio_dev);
+}
+
 /**
  * is_stm32_timer_trigger
  * @trig: trigger to be checked
@@ -299,10 +525,15 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
 	if (of_property_read_u32(dev->of_node, "reg", &index))
 		return -EINVAL;
 
-	if (index >= ARRAY_SIZE(triggers_table))
+	if (index >= ARRAY_SIZE(triggers_table) ||
+	    index >= ARRAY_SIZE(valids_table))
 		return -EINVAL;
 
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	/* Create an IIO device only if we have triggers to be validated */
+	if (*valids_table[index])
+		priv = stm32_setup_counter_device(dev);
+	else
+		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 
 	if (!priv)
 		return -ENOMEM;
@@ -312,6 +543,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
 	priv->clk = ddata->clk;
 	priv->max_arr = ddata->max_arr;
 	priv->triggers = triggers_table[index];
+	priv->valids = valids_table[index];
 
 	ret = stm32_setup_iio_triggers(priv);
 	if (ret)
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index d030004..4a0abbc 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -21,6 +21,7 @@
 #define TIM_CCMR1	0x18	/* Capt/Comp 1 Mode Reg    */
 #define TIM_CCMR2	0x1C	/* Capt/Comp 2 Mode Reg    */
 #define TIM_CCER	0x20	/* Capt/Comp Enable Reg    */
+#define TIM_CNT		0x24	/* Counter		   */
 #define TIM_PSC		0x28	/* Prescaler               */
 #define TIM_ARR		0x2c	/* Auto-Reload Register    */
 #define TIM_CCR1	0x34	/* Capt/Comp Register 1    */
@@ -30,6 +31,7 @@
 #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
 
 #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
+#define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
 #define TIM_CR1_ARPE	BIT(7)	/* Auto-reload Preload Ena */
 #define TIM_CR2_MMS	(BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
 #define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
-- 
1.9.1

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

* [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-03-27  9:43 [PATCH v3 0/2] iio: stm32 trigger: Add quadrature device and counter Benjamin Gaignard
  2017-03-27  9:43 ` [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device Benjamin Gaignard
@ 2017-03-27  9:43 ` Benjamin Gaignard
  2017-03-27 13:55   ` William Breathitt Gray
  2017-04-02 11:12   ` Jonathan Cameron
  1 sibling, 2 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-03-27  9:43 UTC (permalink / raw)
  To: linux-kernel, jic23, linux-iio, knaack.h, lars, pmeerw,
	vilhelm.gray, mwelling
  Cc: fabrice.gasnier, linaro-kernel, benjamin.gaignard, Benjamin Gaignard

Device counting could be controlled by the level or the edges of
a trigger.
in_count0_enable_mode attibute allow to set the control mode.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
 drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
index bf795ad..c0a1edc 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
@@ -59,3 +59,26 @@ Description:
 		quadrature:
 			Encoder A and B inputs are mixed to get direction
 			and count with a scale of 0.25.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
+KernelVersion:	4.12
+Contact:	benjamin.gaignard@st.com
+Description:
+		Reading returns the list possible enable modes.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
+KernelVersion:	4.12
+Contact:	benjamin.gaignard@st.com
+Description:
+		Configure the device counter enable modes, in all case
+		counting direction is set by in_count0_count_direction
+		attribute and the counter is clocked by the internal clock.
+		always:
+			Counter is always ON.
+
+		gated:
+			Counting is enabled when connected trigger signal
+			level is high else counting is disabled.
+
+		triggered:
+			Counting start on rising edge of the connected trigger.
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index 7db904c..0f1a2cf 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
 	.write_raw = stm32_counter_write_raw
 };
 
+static const char *const stm32_enable_modes[] = {
+	"always",
+	"gated",
+	"triggered",
+};
+
+static int stm32_enable_mode2sms(int mode)
+{
+	switch (mode) {
+	case 0:
+		return 0;
+	case 1:
+		return 5;
+	case 2:
+		return 6;
+	}
+
+	return -EINVAL;
+}
+
+static int stm32_set_enable_mode(struct iio_dev *indio_dev,
+				 const struct iio_chan_spec *chan,
+				 unsigned int mode)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	int sms = stm32_enable_mode2sms(mode);
+
+	if (sms < 0)
+		return sms;
+
+	regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
+
+	return 0;
+}
+
+static int stm32_sms2enable_mode(int mode)
+{
+	switch (mode) {
+	case 0:
+		return 0;
+	case 5:
+		return 1;
+	case 6:
+		return 2;
+	}
+
+	return -EINVAL;
+}
+
+static int stm32_get_enable_mode(struct iio_dev *indio_dev,
+				 const struct iio_chan_spec *chan)
+{
+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+	u32 smcr;
+
+	regmap_read(priv->regmap, TIM_SMCR, &smcr);
+	smcr &= TIM_SMCR_SMS;
+
+	return stm32_sms2enable_mode(smcr);
+}
+
+static const struct iio_enum stm32_enable_mode_enum = {
+	.items = stm32_enable_modes,
+	.num_items = ARRAY_SIZE(stm32_enable_modes),
+	.set = stm32_set_enable_mode,
+	.get = stm32_get_enable_mode
+};
+
 static const char *const stm32_quadrature_modes[] = {
 	"channel_A",
 	"channel_B",
@@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
 	IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
 	IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
 	IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
+	IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
+	IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
 	{}
 };
 
-- 
1.9.1

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

* Re: [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device
  2017-03-27  9:43 ` [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device Benjamin Gaignard
@ 2017-03-27 13:47   ` William Breathitt Gray
  2017-03-27 14:14       ` Benjamin Gaignard
  2017-04-02 11:08   ` Jonathan Cameron
  1 sibling, 1 reply; 19+ messages in thread
From: William Breathitt Gray @ 2017-03-27 13:47 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: linux-kernel, jic23, linux-iio, knaack.h, lars, pmeerw, mwelling,
	fabrice.gasnier, linaro-kernel, Benjamin Gaignard

On Mon, Mar 27, 2017 at 11:43:23AM +0200, Benjamin Gaignard wrote:
>One of the features of STM32 trigger hardware block is a quadrature
>encoder that can counts up/down depending of the levels and edges
>of the selected external pins.
>
>This patch allow to read/write the counter, get it direction,
>set/get quadrature modes and get scale factor.
>
>When counting up preset value is the limit of the counter.
>When counting down the counter start from preset value down to 0.
>This preset value could be set/get by using
>/sys/bus/iio/devices/iio:deviceX/in_count0_preset attribute.
>
>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

Hi Benjamin,

This is pretty close to the ABI I had in mind. I'm a bit confused about
how to select some of functionality we discussed in the previous version
so I'll write those comments inline. Overall nice work! :)

>
>version 3:
>- fix typo in documentation
>- change some functions names
>---
> .../ABI/testing/sysfs-bus-iio-timer-stm32          |  32 +++
> drivers/iio/trigger/stm32-timer-trigger.c          | 244 ++++++++++++++++++++-
> include/linux/mfd/stm32-timers.h                   |   2 +
> 3 files changed, 272 insertions(+), 6 deletions(-)
>
>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>index 6534a60..bf795ad 100644
>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>@@ -27,3 +27,35 @@ Description:
> 		Reading returns the current sampling frequency.
> 		Writing an value different of 0 set and start sampling.
> 		Writing 0 stop sampling.
>+
>+What:		/sys/bus/iio/devices/iio:deviceX/in_count0_preset
>+KernelVersion:	4.12
>+Contact:	benjamin.gaignard@st.com
>+Description:
>+		Reading returns the current preset value.
>+		Writing sets the preset value.
>+		When counting up the counter starts from 0 and fires an event when reach preset value.
>+		When counting down the counter start from preset value and fire event when reach 0.

In one of the previous versions we discussed a "reset" mode of operation
where the counter would be reset to the preset value on the rising edge
of pin 1. How would a user select this mode of operation using this ABI
(I assume the preset value would be the same value as in the
in_count0_preset attribute).

Also, would this "reset" mode be limited to count just the internal
clock, or may an external pin alternatively serve as a counter source?

>+
>+What:		/sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
>+KernelVersion:	4.12
>+Contact:	benjamin.gaignard@st.com
>+Description:
>+		Reading returns the list possible quadrature modes.
>+
>+What:		/sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
>+KernelVersion:	4.12
>+Contact:	benjamin.gaignard@st.com
>+Description:
>+		Configure the device counter quadrature modes:
>+		channel_A:
>+			Encoder A input servers as the count input and B as
>+			the UP/DOWN direction control input.
>+
>+		channel_B:
>+			Encoder B input serves as the count input and A as
>+			the UP/DOWN direction control input.
>+
>+		quadrature:
>+			Encoder A and B inputs are mixed to get direction
>+			and count with a scale of 0.25.

If I want to select the internal clock as the counter source, how would
I do so? Should there be an "internal_clock" mode available in the
quadrature_mode attribute?

Sincerely,

William Breathitt Gray

>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>index 994b96d..7db904c 100644
>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>@@ -15,6 +15,7 @@
> #include <linux/platform_device.h>
> 
> #define MAX_TRIGGERS 6
>+#define MAX_VALIDS 5
> 
> /* List the triggers created by each timer */
> static const void *triggers_table[][MAX_TRIGGERS] = {
>@@ -32,12 +33,29 @@
> 	{ TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
> };
> 
>+/* List the triggers accepted by each timer */
>+static const void *valids_table[][MAX_VALIDS] = {
>+	{ TIM5_TRGO, TIM2_TRGO, TIM3_TRGO, TIM4_TRGO,},
>+	{ TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
>+	{ TIM1_TRGO, TIM2_TRGO, TIM5_TRGO, TIM4_TRGO,},
>+	{ TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
>+	{ TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
>+	{ }, /* timer 6 */
>+	{ }, /* timer 7 */
>+	{ TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
>+	{ TIM2_TRGO, TIM3_TRGO,},
>+	{ }, /* timer 10 */
>+	{ }, /* timer 11 */
>+	{ TIM4_TRGO, TIM5_TRGO,},
>+};
>+
> struct stm32_timer_trigger {
> 	struct device *dev;
> 	struct regmap *regmap;
> 	struct clk *clk;
> 	u32 max_arr;
> 	const void *triggers;
>+	const void *valids;
> };
> 
> static int stm32_timer_start(struct stm32_timer_trigger *priv,
>@@ -180,8 +198,7 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
> 					 struct device_attribute *attr,
> 					 char *buf)
> {
>-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>-	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
> 	u32 cr2;
> 
> 	regmap_read(priv->regmap, TIM_CR2, &cr2);
>@@ -194,8 +211,7 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
> 					  struct device_attribute *attr,
> 					  const char *buf, size_t len)
> {
>-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>-	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
> 	int i;
> 
> 	for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
>@@ -275,6 +291,216 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
> 	return 0;
> }
> 
>+static int stm32_counter_read_raw(struct iio_dev *indio_dev,
>+				  struct iio_chan_spec const *chan,
>+				  int *val, int *val2, long mask)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+
>+	switch (mask) {
>+	case IIO_CHAN_INFO_RAW:
>+	{
>+		u32 cnt;
>+
>+		regmap_read(priv->regmap, TIM_CNT, &cnt);
>+		*val = cnt;
>+
>+		return IIO_VAL_INT;
>+	}
>+	case IIO_CHAN_INFO_SCALE:
>+	{
>+		u32 smcr;
>+
>+		regmap_read(priv->regmap, TIM_SMCR, &smcr);
>+		smcr &= TIM_SMCR_SMS;
>+
>+		*val = 1;
>+		*val2 = 0;
>+
>+		/* in quadrature case scale = 0.25 */
>+		if (smcr == 3)
>+			*val2 = 2;
>+
>+		return IIO_VAL_FRACTIONAL_LOG2;
>+	}
>+	}
>+
>+	return -EINVAL;
>+}
>+
>+static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>+				   struct iio_chan_spec const *chan,
>+				   int val, int val2, long mask)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+
>+	switch (mask) {
>+	case IIO_CHAN_INFO_RAW:
>+		regmap_write(priv->regmap, TIM_CNT, val);
>+
>+		return IIO_VAL_INT;
>+	case IIO_CHAN_INFO_SCALE:
>+		/* fixed scale */
>+		return -EINVAL;
>+	}
>+
>+	return -EINVAL;
>+}
>+
>+static const struct iio_info stm32_trigger_info = {
>+	.driver_module = THIS_MODULE,
>+	.read_raw = stm32_counter_read_raw,
>+	.write_raw = stm32_counter_write_raw
>+};
>+
>+static const char *const stm32_quadrature_modes[] = {
>+	"channel_A",
>+	"channel_B",
>+	"quadrature",
>+};
>+
>+static int stm32_set_quadrature_mode(struct iio_dev *indio_dev,
>+				     const struct iio_chan_spec *chan,
>+				     unsigned int mode)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+
>+	regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, mode + 1);
>+
>+	return 0;
>+}
>+
>+static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
>+				     const struct iio_chan_spec *chan)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	u32 smcr;
>+
>+	regmap_read(priv->regmap, TIM_SMCR, &smcr);
>+	smcr &= TIM_SMCR_SMS;
>+
>+	return smcr - 1;
>+}
>+
>+static const struct iio_enum stm32_quadrature_mode_enum = {
>+	.items = stm32_quadrature_modes,
>+	.num_items = ARRAY_SIZE(stm32_quadrature_modes),
>+	.set = stm32_set_quadrature_mode,
>+	.get = stm32_get_quadrature_mode
>+};
>+
>+static const char *const stm32_count_direction_states[] = {
>+	"up",
>+	"down"
>+};
>+
>+static int stm32_set_count_direction(struct iio_dev *indio_dev,
>+				     const struct iio_chan_spec *chan,
>+				     unsigned int mode)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+
>+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
>+
>+	return 0;
>+}
>+
>+static int stm32_get_count_direction(struct iio_dev *indio_dev,
>+				     const struct iio_chan_spec *chan)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	u32 cr1;
>+
>+	regmap_read(priv->regmap, TIM_CR1, &cr1);
>+
>+	return (cr1 & TIM_CR1_DIR);
>+}
>+
>+static const struct iio_enum stm32_count_direction_enum = {
>+	.items = stm32_count_direction_states,
>+	.num_items = ARRAY_SIZE(stm32_count_direction_states),
>+	.set = stm32_set_count_direction,
>+	.get = stm32_get_count_direction
>+};
>+
>+static ssize_t stm32_count_get_preset(struct iio_dev *indio_dev,
>+				      uintptr_t private,
>+				      const struct iio_chan_spec *chan,
>+				      char *buf)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	u32 arr;
>+
>+	regmap_read(priv->regmap, TIM_ARR, &arr);
>+
>+	return snprintf(buf, PAGE_SIZE, "%u\n", arr);
>+}
>+
>+static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>+				      uintptr_t private,
>+				      const struct iio_chan_spec *chan,
>+				      const char *buf, size_t len)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	unsigned int preset;
>+	int ret;
>+
>+	ret = kstrtouint(buf, 0, &preset);
>+	if (ret)
>+		return ret;
>+
>+	regmap_write(priv->regmap, TIM_ARR, preset);
>+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>+
>+	return len;
>+}
>+
>+static const struct iio_chan_spec_ext_info stm32_trigger_count_info[] = {
>+	{
>+		.name = "preset",
>+		.shared = IIO_SEPARATE,
>+		.read = stm32_count_get_preset,
>+		.write = stm32_count_set_preset
>+	},
>+	IIO_ENUM("count_direction", IIO_SEPARATE, &stm32_count_direction_enum),
>+	IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>+	IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>+	IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>+	{}
>+};
>+
>+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),
>+	.ext_info = stm32_trigger_count_info,
>+	.indexed = 1
>+};
>+
>+static struct stm32_timer_trigger *stm32_setup_counter_device(struct device *dev)
>+{
>+	struct iio_dev *indio_dev;
>+	int ret;
>+
>+	indio_dev = devm_iio_device_alloc(dev,
>+					  sizeof(struct stm32_timer_trigger));
>+	if (!indio_dev)
>+		return NULL;
>+
>+	indio_dev->name = dev_name(dev);
>+	indio_dev->dev.parent = dev;
>+	indio_dev->info = &stm32_trigger_info;
>+	indio_dev->num_channels = 1;
>+	indio_dev->channels = &stm32_trigger_channel;
>+	indio_dev->dev.of_node = dev->of_node;
>+
>+	ret = devm_iio_device_register(dev, indio_dev);
>+	if (ret)
>+		return NULL;
>+
>+	return iio_priv(indio_dev);
>+}
>+
> /**
>  * is_stm32_timer_trigger
>  * @trig: trigger to be checked
>@@ -299,10 +525,15 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
> 	if (of_property_read_u32(dev->of_node, "reg", &index))
> 		return -EINVAL;
> 
>-	if (index >= ARRAY_SIZE(triggers_table))
>+	if (index >= ARRAY_SIZE(triggers_table) ||
>+	    index >= ARRAY_SIZE(valids_table))
> 		return -EINVAL;
> 
>-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>+	/* Create an IIO device only if we have triggers to be validated */
>+	if (*valids_table[index])
>+		priv = stm32_setup_counter_device(dev);
>+	else
>+		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> 
> 	if (!priv)
> 		return -ENOMEM;
>@@ -312,6 +543,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
> 	priv->clk = ddata->clk;
> 	priv->max_arr = ddata->max_arr;
> 	priv->triggers = triggers_table[index];
>+	priv->valids = valids_table[index];
> 
> 	ret = stm32_setup_iio_triggers(priv);
> 	if (ret)
>diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>index d030004..4a0abbc 100644
>--- a/include/linux/mfd/stm32-timers.h
>+++ b/include/linux/mfd/stm32-timers.h
>@@ -21,6 +21,7 @@
> #define TIM_CCMR1	0x18	/* Capt/Comp 1 Mode Reg    */
> #define TIM_CCMR2	0x1C	/* Capt/Comp 2 Mode Reg    */
> #define TIM_CCER	0x20	/* Capt/Comp Enable Reg    */
>+#define TIM_CNT		0x24	/* Counter		   */
> #define TIM_PSC		0x28	/* Prescaler               */
> #define TIM_ARR		0x2c	/* Auto-Reload Register    */
> #define TIM_CCR1	0x34	/* Capt/Comp Register 1    */
>@@ -30,6 +31,7 @@
> #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
> 
> #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
>+#define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
> #define TIM_CR1_ARPE	BIT(7)	/* Auto-reload Preload Ena */
> #define TIM_CR2_MMS	(BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
> #define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
>-- 
>1.9.1
>

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-03-27  9:43 ` [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes Benjamin Gaignard
@ 2017-03-27 13:55   ` William Breathitt Gray
  2017-03-27 14:43       ` Benjamin Gaignard
  2017-04-02 11:12   ` Jonathan Cameron
  1 sibling, 1 reply; 19+ messages in thread
From: William Breathitt Gray @ 2017-03-27 13:55 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: linux-kernel, jic23, linux-iio, knaack.h, lars, pmeerw, mwelling,
	fabrice.gasnier, linaro-kernel, Benjamin Gaignard

On Mon, Mar 27, 2017 at 11:43:24AM +0200, Benjamin Gaignard wrote:
>Device counting could be controlled by the level or the edges of
>a trigger.
>in_count0_enable_mode attibute allow to set the control mode.
>
>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

This patch is pretty straight-forward and all right with me. A minor
nitpick inline regarding documentation.

Reviewed-by: William Breathitt Gray <vilhelm.gray@gmail.com>

>---
> .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
> drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
> 2 files changed, 93 insertions(+)
>
>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>index bf795ad..c0a1edc 100644
>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>@@ -59,3 +59,26 @@ Description:
> 		quadrature:
> 			Encoder A and B inputs are mixed to get direction
> 			and count with a scale of 0.25.
>+
>+What:		/sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
>+KernelVersion:	4.12
>+Contact:	benjamin.gaignard@st.com
>+Description:
>+		Reading returns the list possible enable modes.
>+
>+What:		/sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>+KernelVersion:	4.12
>+Contact:	benjamin.gaignard@st.com
>+Description:
>+		Configure the device counter enable modes, in all case
>+		counting direction is set by in_count0_count_direction
>+		attribute and the counter is clocked by the internal clock.
>+		always:
>+			Counter is always ON.
>+
>+		gated:
>+			Counting is enabled when connected trigger signal
>+			level is high else counting is disabled.
>+
>+		triggered:
>+			Counting start on rising edge of the connected trigger.

When I read the description of the "triggered" mode, I get the
impression that counting is restarted every time there is a rising edge
from the connected trigger signal. However, from our previous
discussions I believe this mode is simply a latched enable.

Perhaps it may be beneficial for clarity to rework the description to
say something along the lines of: "Counting is enabled on rising edge of
the connected trigger, and remains enabled for the duration of this
selected mode."

William Breathitt Gray

>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>index 7db904c..0f1a2cf 100644
>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>@@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> 	.write_raw = stm32_counter_write_raw
> };
> 
>+static const char *const stm32_enable_modes[] = {
>+	"always",
>+	"gated",
>+	"triggered",
>+};
>+
>+static int stm32_enable_mode2sms(int mode)
>+{
>+	switch (mode) {
>+	case 0:
>+		return 0;
>+	case 1:
>+		return 5;
>+	case 2:
>+		return 6;
>+	}
>+
>+	return -EINVAL;
>+}
>+
>+static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>+				 const struct iio_chan_spec *chan,
>+				 unsigned int mode)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	int sms = stm32_enable_mode2sms(mode);
>+
>+	if (sms < 0)
>+		return sms;
>+
>+	regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>+
>+	return 0;
>+}
>+
>+static int stm32_sms2enable_mode(int mode)
>+{
>+	switch (mode) {
>+	case 0:
>+		return 0;
>+	case 5:
>+		return 1;
>+	case 6:
>+		return 2;
>+	}
>+
>+	return -EINVAL;
>+}
>+
>+static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>+				 const struct iio_chan_spec *chan)
>+{
>+	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>+	u32 smcr;
>+
>+	regmap_read(priv->regmap, TIM_SMCR, &smcr);
>+	smcr &= TIM_SMCR_SMS;
>+
>+	return stm32_sms2enable_mode(smcr);
>+}
>+
>+static const struct iio_enum stm32_enable_mode_enum = {
>+	.items = stm32_enable_modes,
>+	.num_items = ARRAY_SIZE(stm32_enable_modes),
>+	.set = stm32_set_enable_mode,
>+	.get = stm32_get_enable_mode
>+};
>+
> static const char *const stm32_quadrature_modes[] = {
> 	"channel_A",
> 	"channel_B",
>@@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
> 	IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
> 	IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
> 	IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>+	IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>+	IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
> 	{}
> };
> 
>-- 
>1.9.1
>

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

* Re: [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device
  2017-03-27 13:47   ` William Breathitt Gray
@ 2017-03-27 14:14       ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-03-27 14:14 UTC (permalink / raw)
  To: William Breathitt Gray
  Cc: Linux Kernel Mailing List, Jonathan Cameron, linux-iio,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	mwelling, Fabrice Gasnier, Linaro Kernel Mailman List,
	Benjamin Gaignard

2017-03-27 15:47 GMT+02:00 William Breathitt Gray <vilhelm.gray@gmail.com>:
> On Mon, Mar 27, 2017 at 11:43:23AM +0200, Benjamin Gaignard wrote:
>>One of the features of STM32 trigger hardware block is a quadrature
>>encoder that can counts up/down depending of the levels and edges
>>of the selected external pins.
>>
>>This patch allow to read/write the counter, get it direction,
>>set/get quadrature modes and get scale factor.
>>
>>When counting up preset value is the limit of the counter.
>>When counting down the counter start from preset value down to 0.
>>This preset value could be set/get by using
>>/sys/bus/iio/devices/iio:deviceX/in_count0_preset attribute.
>>
>>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>
> Hi Benjamin,
>
> This is pretty close to the ABI I had in mind. I'm a bit confused about
> how to select some of functionality we discussed in the previous version
> so I'll write those comments inline. Overall nice work! :)
>
>>
>>version 3:
>>- fix typo in documentation
>>- change some functions names
>>---
>> .../ABI/testing/sysfs-bus-iio-timer-stm32          |  32 +++
>> drivers/iio/trigger/stm32-timer-trigger.c          | 244 ++++++++++++++++++++-
>> include/linux/mfd/stm32-timers.h                   |   2 +
>> 3 files changed, 272 insertions(+), 6 deletions(-)
>>
>>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>index 6534a60..bf795ad 100644
>>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>@@ -27,3 +27,35 @@ Description:
>>               Reading returns the current sampling frequency.
>>               Writing an value different of 0 set and start sampling.
>>               Writing 0 stop sampling.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_preset
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Reading returns the current preset value.
>>+              Writing sets the preset value.
>>+              When counting up the counter starts from 0 and fires an event when reach preset value.
>>+              When counting down the counter start from preset value and fire event when reach 0.
>
> In one of the previous versions we discussed a "reset" mode of operation
> where the counter would be reset to the preset value on the rising edge
> of pin 1. How would a user select this mode of operation using this ABI
> (I assume the preset value would be the same value as in the
> in_count0_preset attribute).
>
> Also, would this "reset" mode be limited to count just the internal
> clock, or may an external pin alternatively serve as a counter source?

In reset mode counter is clocked by the internal clock
Reset can only by drive by a rising edge of an internal signal
(represented by an IIO trigger).

>
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Reading returns the list possible quadrature modes.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Configure the device counter quadrature modes:
>>+              channel_A:
>>+                      Encoder A input servers as the count input and B as
>>+                      the UP/DOWN direction control input.
>>+
>>+              channel_B:
>>+                      Encoder B input serves as the count input and A as
>>+                      the UP/DOWN direction control input.
>>+
>>+              quadrature:
>>+                      Encoder A and B inputs are mixed to get direction
>>+                      and count with a scale of 0.25.
>
> If I want to select the internal clock as the counter source, how would
> I do so? Should there be an "internal_clock" mode available in the
> quadrature_mode attribute?

To use internal clock as counter source you must select one of the modes
in in_count0_enable_mode attribute (patch 2/2)

Counter could be drived by 3 "sources": internal clock, encoder input
and internal signal
I have in mind to keep this split in 3 attributes:enable_mode,
quadrature_mode and a futur
trigger_mode.


>
> Sincerely,
>
> William Breathitt Gray
>
>>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>index 994b96d..7db904c 100644
>>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>@@ -15,6 +15,7 @@
>> #include <linux/platform_device.h>
>>
>> #define MAX_TRIGGERS 6
>>+#define MAX_VALIDS 5
>>
>> /* List the triggers created by each timer */
>> static const void *triggers_table[][MAX_TRIGGERS] = {
>>@@ -32,12 +33,29 @@
>>       { TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
>> };
>>
>>+/* List the triggers accepted by each timer */
>>+static const void *valids_table[][MAX_VALIDS] = {
>>+      { TIM5_TRGO, TIM2_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>+      { TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>+      { TIM1_TRGO, TIM2_TRGO, TIM5_TRGO, TIM4_TRGO,},
>>+      { TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
>>+      { TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
>>+      { }, /* timer 6 */
>>+      { }, /* timer 7 */
>>+      { TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
>>+      { TIM2_TRGO, TIM3_TRGO,},
>>+      { }, /* timer 10 */
>>+      { }, /* timer 11 */
>>+      { TIM4_TRGO, TIM5_TRGO,},
>>+};
>>+
>> struct stm32_timer_trigger {
>>       struct device *dev;
>>       struct regmap *regmap;
>>       struct clk *clk;
>>       u32 max_arr;
>>       const void *triggers;
>>+      const void *valids;
>> };
>>
>> static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>@@ -180,8 +198,7 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
>>                                        struct device_attribute *attr,
>>                                        char *buf)
>> {
>>-      struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>-      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>       u32 cr2;
>>
>>       regmap_read(priv->regmap, TIM_CR2, &cr2);
>>@@ -194,8 +211,7 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
>>                                         struct device_attribute *attr,
>>                                         const char *buf, size_t len)
>> {
>>-      struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>-      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>       int i;
>>
>>       for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
>>@@ -275,6 +291,216 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
>>       return 0;
>> }
>>
>>+static int stm32_counter_read_raw(struct iio_dev *indio_dev,
>>+                                struct iio_chan_spec const *chan,
>>+                                int *val, int *val2, long mask)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+
>>+      switch (mask) {
>>+      case IIO_CHAN_INFO_RAW:
>>+      {
>>+              u32 cnt;
>>+
>>+              regmap_read(priv->regmap, TIM_CNT, &cnt);
>>+              *val = cnt;
>>+
>>+              return IIO_VAL_INT;
>>+      }
>>+      case IIO_CHAN_INFO_SCALE:
>>+      {
>>+              u32 smcr;
>>+
>>+              regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>+              smcr &= TIM_SMCR_SMS;
>>+
>>+              *val = 1;
>>+              *val2 = 0;
>>+
>>+              /* in quadrature case scale = 0.25 */
>>+              if (smcr == 3)
>>+                      *val2 = 2;
>>+
>>+              return IIO_VAL_FRACTIONAL_LOG2;
>>+      }
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>+                                 struct iio_chan_spec const *chan,
>>+                                 int val, int val2, long mask)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+
>>+      switch (mask) {
>>+      case IIO_CHAN_INFO_RAW:
>>+              regmap_write(priv->regmap, TIM_CNT, val);
>>+
>>+              return IIO_VAL_INT;
>>+      case IIO_CHAN_INFO_SCALE:
>>+              /* fixed scale */
>>+              return -EINVAL;
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static const struct iio_info stm32_trigger_info = {
>>+      .driver_module = THIS_MODULE,
>>+      .read_raw = stm32_counter_read_raw,
>>+      .write_raw = stm32_counter_write_raw
>>+};
>>+
>>+static const char *const stm32_quadrature_modes[] = {
>>+      "channel_A",
>>+      "channel_B",
>>+      "quadrature",
>>+};
>>+
>>+static int stm32_set_quadrature_mode(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan,
>>+                                   unsigned int mode)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+
>>+      regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, mode + 1);
>>+
>>+      return 0;
>>+}
>>+
>>+static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      u32 smcr;
>>+
>>+      regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>+      smcr &= TIM_SMCR_SMS;
>>+
>>+      return smcr - 1;
>>+}
>>+
>>+static const struct iio_enum stm32_quadrature_mode_enum = {
>>+      .items = stm32_quadrature_modes,
>>+      .num_items = ARRAY_SIZE(stm32_quadrature_modes),
>>+      .set = stm32_set_quadrature_mode,
>>+      .get = stm32_get_quadrature_mode
>>+};
>>+
>>+static const char *const stm32_count_direction_states[] = {
>>+      "up",
>>+      "down"
>>+};
>>+
>>+static int stm32_set_count_direction(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan,
>>+                                   unsigned int mode)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+
>>+      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
>>+
>>+      return 0;
>>+}
>>+
>>+static int stm32_get_count_direction(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      u32 cr1;
>>+
>>+      regmap_read(priv->regmap, TIM_CR1, &cr1);
>>+
>>+      return (cr1 & TIM_CR1_DIR);
>>+}
>>+
>>+static const struct iio_enum stm32_count_direction_enum = {
>>+      .items = stm32_count_direction_states,
>>+      .num_items = ARRAY_SIZE(stm32_count_direction_states),
>>+      .set = stm32_set_count_direction,
>>+      .get = stm32_get_count_direction
>>+};
>>+
>>+static ssize_t stm32_count_get_preset(struct iio_dev *indio_dev,
>>+                                    uintptr_t private,
>>+                                    const struct iio_chan_spec *chan,
>>+                                    char *buf)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      u32 arr;
>>+
>>+      regmap_read(priv->regmap, TIM_ARR, &arr);
>>+
>>+      return snprintf(buf, PAGE_SIZE, "%u\n", arr);
>>+}
>>+
>>+static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>+                                    uintptr_t private,
>>+                                    const struct iio_chan_spec *chan,
>>+                                    const char *buf, size_t len)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      unsigned int preset;
>>+      int ret;
>>+
>>+      ret = kstrtouint(buf, 0, &preset);
>>+      if (ret)
>>+              return ret;
>>+
>>+      regmap_write(priv->regmap, TIM_ARR, preset);
>>+      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>>+
>>+      return len;
>>+}
>>+
>>+static const struct iio_chan_spec_ext_info stm32_trigger_count_info[] = {
>>+      {
>>+              .name = "preset",
>>+              .shared = IIO_SEPARATE,
>>+              .read = stm32_count_get_preset,
>>+              .write = stm32_count_set_preset
>>+      },
>>+      IIO_ENUM("count_direction", IIO_SEPARATE, &stm32_count_direction_enum),
>>+      IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>+      IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>+      IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>+      {}
>>+};
>>+
>>+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),
>>+      .ext_info = stm32_trigger_count_info,
>>+      .indexed = 1
>>+};
>>+
>>+static struct stm32_timer_trigger *stm32_setup_counter_device(struct device *dev)
>>+{
>>+      struct iio_dev *indio_dev;
>>+      int ret;
>>+
>>+      indio_dev = devm_iio_device_alloc(dev,
>>+                                        sizeof(struct stm32_timer_trigger));
>>+      if (!indio_dev)
>>+              return NULL;
>>+
>>+      indio_dev->name = dev_name(dev);
>>+      indio_dev->dev.parent = dev;
>>+      indio_dev->info = &stm32_trigger_info;
>>+      indio_dev->num_channels = 1;
>>+      indio_dev->channels = &stm32_trigger_channel;
>>+      indio_dev->dev.of_node = dev->of_node;
>>+
>>+      ret = devm_iio_device_register(dev, indio_dev);
>>+      if (ret)
>>+              return NULL;
>>+
>>+      return iio_priv(indio_dev);
>>+}
>>+
>> /**
>>  * is_stm32_timer_trigger
>>  * @trig: trigger to be checked
>>@@ -299,10 +525,15 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>       if (of_property_read_u32(dev->of_node, "reg", &index))
>>               return -EINVAL;
>>
>>-      if (index >= ARRAY_SIZE(triggers_table))
>>+      if (index >= ARRAY_SIZE(triggers_table) ||
>>+          index >= ARRAY_SIZE(valids_table))
>>               return -EINVAL;
>>
>>-      priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>+      /* Create an IIO device only if we have triggers to be validated */
>>+      if (*valids_table[index])
>>+              priv = stm32_setup_counter_device(dev);
>>+      else
>>+              priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>
>>       if (!priv)
>>               return -ENOMEM;
>>@@ -312,6 +543,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>       priv->clk = ddata->clk;
>>       priv->max_arr = ddata->max_arr;
>>       priv->triggers = triggers_table[index];
>>+      priv->valids = valids_table[index];
>>
>>       ret = stm32_setup_iio_triggers(priv);
>>       if (ret)
>>diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>index d030004..4a0abbc 100644
>>--- a/include/linux/mfd/stm32-timers.h
>>+++ b/include/linux/mfd/stm32-timers.h
>>@@ -21,6 +21,7 @@
>> #define TIM_CCMR1     0x18    /* Capt/Comp 1 Mode Reg    */
>> #define TIM_CCMR2     0x1C    /* Capt/Comp 2 Mode Reg    */
>> #define TIM_CCER      0x20    /* Capt/Comp Enable Reg    */
>>+#define TIM_CNT               0x24    /* Counter                 */
>> #define TIM_PSC               0x28    /* Prescaler               */
>> #define TIM_ARR               0x2c    /* Auto-Reload Register    */
>> #define TIM_CCR1      0x34    /* Capt/Comp Register 1    */
>>@@ -30,6 +31,7 @@
>> #define TIM_BDTR      0x44    /* Break and Dead-Time Reg */
>>
>> #define TIM_CR1_CEN   BIT(0)  /* Counter Enable          */
>>+#define TIM_CR1_DIR   BIT(4)  /* Counter Direction       */
>> #define TIM_CR1_ARPE  BIT(7)  /* Auto-reload Preload Ena */
>> #define TIM_CR2_MMS   (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
>> #define TIM_SMCR_SMS  (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
>>--
>>1.9.1
>>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device
@ 2017-03-27 14:14       ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-03-27 14:14 UTC (permalink / raw)
  To: William Breathitt Gray
  Cc: Linux Kernel Mailing List, Jonathan Cameron, linux-iio,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	mwelling, Fabrice Gasnier, Linaro Kernel Mailman List,
	Benjamin Gaignard

2017-03-27 15:47 GMT+02:00 William Breathitt Gray <vilhelm.gray@gmail.com>:
> On Mon, Mar 27, 2017 at 11:43:23AM +0200, Benjamin Gaignard wrote:
>>One of the features of STM32 trigger hardware block is a quadrature
>>encoder that can counts up/down depending of the levels and edges
>>of the selected external pins.
>>
>>This patch allow to read/write the counter, get it direction,
>>set/get quadrature modes and get scale factor.
>>
>>When counting up preset value is the limit of the counter.
>>When counting down the counter start from preset value down to 0.
>>This preset value could be set/get by using
>>/sys/bus/iio/devices/iio:deviceX/in_count0_preset attribute.
>>
>>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>
> Hi Benjamin,
>
> This is pretty close to the ABI I had in mind. I'm a bit confused about
> how to select some of functionality we discussed in the previous version
> so I'll write those comments inline. Overall nice work! :)
>
>>
>>version 3:
>>- fix typo in documentation
>>- change some functions names
>>---
>> .../ABI/testing/sysfs-bus-iio-timer-stm32          |  32 +++
>> drivers/iio/trigger/stm32-timer-trigger.c          | 244 +++++++++++++++=
+++++-
>> include/linux/mfd/stm32-timers.h                   |   2 +
>> 3 files changed, 272 insertions(+), 6 deletions(-)
>>
>>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Docume=
ntation/ABI/testing/sysfs-bus-iio-timer-stm32
>>index 6534a60..bf795ad 100644
>>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>@@ -27,3 +27,35 @@ Description:
>>               Reading returns the current sampling frequency.
>>               Writing an value different of 0 set and start sampling.
>>               Writing 0 stop sampling.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_preset
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Reading returns the current preset value.
>>+              Writing sets the preset value.
>>+              When counting up the counter starts from 0 and fires an ev=
ent when reach preset value.
>>+              When counting down the counter start from preset value and=
 fire event when reach 0.
>
> In one of the previous versions we discussed a "reset" mode of operation
> where the counter would be reset to the preset value on the rising edge
> of pin 1. How would a user select this mode of operation using this ABI
> (I assume the preset value would be the same value as in the
> in_count0_preset attribute).
>
> Also, would this "reset" mode be limited to count just the internal
> clock, or may an external pin alternatively serve as a counter source?

In reset mode counter is clocked by the internal clock
Reset can only by drive by a rising edge of an internal signal
(represented by an IIO trigger).

>
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_=
available
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Reading returns the list possible quadrature modes.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Configure the device counter quadrature modes:
>>+              channel_A:
>>+                      Encoder A input servers as the count input and B a=
s
>>+                      the UP/DOWN direction control input.
>>+
>>+              channel_B:
>>+                      Encoder B input serves as the count input and A as
>>+                      the UP/DOWN direction control input.
>>+
>>+              quadrature:
>>+                      Encoder A and B inputs are mixed to get direction
>>+                      and count with a scale of 0.25.
>
> If I want to select the internal clock as the counter source, how would
> I do so? Should there be an "internal_clock" mode available in the
> quadrature_mode attribute?

To use internal clock as counter source you must select one of the modes
in in_count0_enable_mode attribute (patch 2/2)

Counter could be drived by 3 "sources": internal clock, encoder input
and internal signal
I have in mind to keep this split in 3 attributes:enable_mode,
quadrature_mode and a futur
trigger_mode.


>
> Sincerely,
>
> William Breathitt Gray
>
>>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trig=
ger/stm32-timer-trigger.c
>>index 994b96d..7db904c 100644
>>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>@@ -15,6 +15,7 @@
>> #include <linux/platform_device.h>
>>
>> #define MAX_TRIGGERS 6
>>+#define MAX_VALIDS 5
>>
>> /* List the triggers created by each timer */
>> static const void *triggers_table[][MAX_TRIGGERS] =3D {
>>@@ -32,12 +33,29 @@
>>       { TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
>> };
>>
>>+/* List the triggers accepted by each timer */
>>+static const void *valids_table[][MAX_VALIDS] =3D {
>>+      { TIM5_TRGO, TIM2_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>+      { TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>+      { TIM1_TRGO, TIM2_TRGO, TIM5_TRGO, TIM4_TRGO,},
>>+      { TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
>>+      { TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
>>+      { }, /* timer 6 */
>>+      { }, /* timer 7 */
>>+      { TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
>>+      { TIM2_TRGO, TIM3_TRGO,},
>>+      { }, /* timer 10 */
>>+      { }, /* timer 11 */
>>+      { TIM4_TRGO, TIM5_TRGO,},
>>+};
>>+
>> struct stm32_timer_trigger {
>>       struct device *dev;
>>       struct regmap *regmap;
>>       struct clk *clk;
>>       u32 max_arr;
>>       const void *triggers;
>>+      const void *valids;
>> };
>>
>> static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>@@ -180,8 +198,7 @@ static ssize_t stm32_tt_show_master_mode(struct devic=
e *dev,
>>                                        struct device_attribute *attr,
>>                                        char *buf)
>> {
>>-      struct iio_dev *indio_dev =3D dev_to_iio_dev(dev);
>>-      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      struct stm32_timer_trigger *priv =3D dev_get_drvdata(dev);
>>       u32 cr2;
>>
>>       regmap_read(priv->regmap, TIM_CR2, &cr2);
>>@@ -194,8 +211,7 @@ static ssize_t stm32_tt_store_master_mode(struct devi=
ce *dev,
>>                                         struct device_attribute *attr,
>>                                         const char *buf, size_t len)
>> {
>>-      struct iio_dev *indio_dev =3D dev_to_iio_dev(dev);
>>-      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      struct stm32_timer_trigger *priv =3D dev_get_drvdata(dev);
>>       int i;
>>
>>       for (i =3D 0; i < ARRAY_SIZE(master_mode_table); i++) {
>>@@ -275,6 +291,216 @@ static int stm32_setup_iio_triggers(struct stm32_ti=
mer_trigger *priv)
>>       return 0;
>> }
>>
>>+static int stm32_counter_read_raw(struct iio_dev *indio_dev,
>>+                                struct iio_chan_spec const *chan,
>>+                                int *val, int *val2, long mask)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+
>>+      switch (mask) {
>>+      case IIO_CHAN_INFO_RAW:
>>+      {
>>+              u32 cnt;
>>+
>>+              regmap_read(priv->regmap, TIM_CNT, &cnt);
>>+              *val =3D cnt;
>>+
>>+              return IIO_VAL_INT;
>>+      }
>>+      case IIO_CHAN_INFO_SCALE:
>>+      {
>>+              u32 smcr;
>>+
>>+              regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>+              smcr &=3D TIM_SMCR_SMS;
>>+
>>+              *val =3D 1;
>>+              *val2 =3D 0;
>>+
>>+              /* in quadrature case scale =3D 0.25 */
>>+              if (smcr =3D=3D 3)
>>+                      *val2 =3D 2;
>>+
>>+              return IIO_VAL_FRACTIONAL_LOG2;
>>+      }
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>+                                 struct iio_chan_spec const *chan,
>>+                                 int val, int val2, long mask)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+
>>+      switch (mask) {
>>+      case IIO_CHAN_INFO_RAW:
>>+              regmap_write(priv->regmap, TIM_CNT, val);
>>+
>>+              return IIO_VAL_INT;
>>+      case IIO_CHAN_INFO_SCALE:
>>+              /* fixed scale */
>>+              return -EINVAL;
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static const struct iio_info stm32_trigger_info =3D {
>>+      .driver_module =3D THIS_MODULE,
>>+      .read_raw =3D stm32_counter_read_raw,
>>+      .write_raw =3D stm32_counter_write_raw
>>+};
>>+
>>+static const char *const stm32_quadrature_modes[] =3D {
>>+      "channel_A",
>>+      "channel_B",
>>+      "quadrature",
>>+};
>>+
>>+static int stm32_set_quadrature_mode(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan,
>>+                                   unsigned int mode)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+
>>+      regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, mode + 1)=
;
>>+
>>+      return 0;
>>+}
>>+
>>+static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      u32 smcr;
>>+
>>+      regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>+      smcr &=3D TIM_SMCR_SMS;
>>+
>>+      return smcr - 1;
>>+}
>>+
>>+static const struct iio_enum stm32_quadrature_mode_enum =3D {
>>+      .items =3D stm32_quadrature_modes,
>>+      .num_items =3D ARRAY_SIZE(stm32_quadrature_modes),
>>+      .set =3D stm32_set_quadrature_mode,
>>+      .get =3D stm32_get_quadrature_mode
>>+};
>>+
>>+static const char *const stm32_count_direction_states[] =3D {
>>+      "up",
>>+      "down"
>>+};
>>+
>>+static int stm32_set_count_direction(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan,
>>+                                   unsigned int mode)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+
>>+      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
>>+
>>+      return 0;
>>+}
>>+
>>+static int stm32_get_count_direction(struct iio_dev *indio_dev,
>>+                                   const struct iio_chan_spec *chan)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      u32 cr1;
>>+
>>+      regmap_read(priv->regmap, TIM_CR1, &cr1);
>>+
>>+      return (cr1 & TIM_CR1_DIR);
>>+}
>>+
>>+static const struct iio_enum stm32_count_direction_enum =3D {
>>+      .items =3D stm32_count_direction_states,
>>+      .num_items =3D ARRAY_SIZE(stm32_count_direction_states),
>>+      .set =3D stm32_set_count_direction,
>>+      .get =3D stm32_get_count_direction
>>+};
>>+
>>+static ssize_t stm32_count_get_preset(struct iio_dev *indio_dev,
>>+                                    uintptr_t private,
>>+                                    const struct iio_chan_spec *chan,
>>+                                    char *buf)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      u32 arr;
>>+
>>+      regmap_read(priv->regmap, TIM_ARR, &arr);
>>+
>>+      return snprintf(buf, PAGE_SIZE, "%u\n", arr);
>>+}
>>+
>>+static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>+                                    uintptr_t private,
>>+                                    const struct iio_chan_spec *chan,
>>+                                    const char *buf, size_t len)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      unsigned int preset;
>>+      int ret;
>>+
>>+      ret =3D kstrtouint(buf, 0, &preset);
>>+      if (ret)
>>+              return ret;
>>+
>>+      regmap_write(priv->regmap, TIM_ARR, preset);
>>+      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_AR=
PE);
>>+
>>+      return len;
>>+}
>>+
>>+static const struct iio_chan_spec_ext_info stm32_trigger_count_info[] =
=3D {
>>+      {
>>+              .name =3D "preset",
>>+              .shared =3D IIO_SEPARATE,
>>+              .read =3D stm32_count_get_preset,
>>+              .write =3D stm32_count_set_preset
>>+      },
>>+      IIO_ENUM("count_direction", IIO_SEPARATE, &stm32_count_direction_e=
num),
>>+      IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum)=
,
>>+      IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_e=
num),
>>+      IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum)=
,
>>+      {}
>>+};
>>+
>>+static const struct iio_chan_spec stm32_trigger_channel =3D {
>>+      .type =3D IIO_COUNT,
>>+      .channel =3D 0,
>>+      .info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO=
_SCALE),
>>+      .ext_info =3D stm32_trigger_count_info,
>>+      .indexed =3D 1
>>+};
>>+
>>+static struct stm32_timer_trigger *stm32_setup_counter_device(struct dev=
ice *dev)
>>+{
>>+      struct iio_dev *indio_dev;
>>+      int ret;
>>+
>>+      indio_dev =3D devm_iio_device_alloc(dev,
>>+                                        sizeof(struct stm32_timer_trigge=
r));
>>+      if (!indio_dev)
>>+              return NULL;
>>+
>>+      indio_dev->name =3D dev_name(dev);
>>+      indio_dev->dev.parent =3D dev;
>>+      indio_dev->info =3D &stm32_trigger_info;
>>+      indio_dev->num_channels =3D 1;
>>+      indio_dev->channels =3D &stm32_trigger_channel;
>>+      indio_dev->dev.of_node =3D dev->of_node;
>>+
>>+      ret =3D devm_iio_device_register(dev, indio_dev);
>>+      if (ret)
>>+              return NULL;
>>+
>>+      return iio_priv(indio_dev);
>>+}
>>+
>> /**
>>  * is_stm32_timer_trigger
>>  * @trig: trigger to be checked
>>@@ -299,10 +525,15 @@ static int stm32_timer_trigger_probe(struct platfor=
m_device *pdev)
>>       if (of_property_read_u32(dev->of_node, "reg", &index))
>>               return -EINVAL;
>>
>>-      if (index >=3D ARRAY_SIZE(triggers_table))
>>+      if (index >=3D ARRAY_SIZE(triggers_table) ||
>>+          index >=3D ARRAY_SIZE(valids_table))
>>               return -EINVAL;
>>
>>-      priv =3D devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>+      /* Create an IIO device only if we have triggers to be validated *=
/
>>+      if (*valids_table[index])
>>+              priv =3D stm32_setup_counter_device(dev);
>>+      else
>>+              priv =3D devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>
>>       if (!priv)
>>               return -ENOMEM;
>>@@ -312,6 +543,7 @@ static int stm32_timer_trigger_probe(struct platform_=
device *pdev)
>>       priv->clk =3D ddata->clk;
>>       priv->max_arr =3D ddata->max_arr;
>>       priv->triggers =3D triggers_table[index];
>>+      priv->valids =3D valids_table[index];
>>
>>       ret =3D stm32_setup_iio_triggers(priv);
>>       if (ret)
>>diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-t=
imers.h
>>index d030004..4a0abbc 100644
>>--- a/include/linux/mfd/stm32-timers.h
>>+++ b/include/linux/mfd/stm32-timers.h
>>@@ -21,6 +21,7 @@
>> #define TIM_CCMR1     0x18    /* Capt/Comp 1 Mode Reg    */
>> #define TIM_CCMR2     0x1C    /* Capt/Comp 2 Mode Reg    */
>> #define TIM_CCER      0x20    /* Capt/Comp Enable Reg    */
>>+#define TIM_CNT               0x24    /* Counter                 */
>> #define TIM_PSC               0x28    /* Prescaler               */
>> #define TIM_ARR               0x2c    /* Auto-Reload Register    */
>> #define TIM_CCR1      0x34    /* Capt/Comp Register 1    */
>>@@ -30,6 +31,7 @@
>> #define TIM_BDTR      0x44    /* Break and Dead-Time Reg */
>>
>> #define TIM_CR1_CEN   BIT(0)  /* Counter Enable          */
>>+#define TIM_CR1_DIR   BIT(4)  /* Counter Direction       */
>> #define TIM_CR1_ARPE  BIT(7)  /* Auto-reload Preload Ena */
>> #define TIM_CR2_MMS   (BIT(4) | BIT(5) | BIT(6)) /* Master mode selectio=
n */
>> #define TIM_SMCR_SMS  (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection=
 */
>>--
>>1.9.1
>>



--=20
Benjamin Gaignard

Graphic Study Group

Linaro.org =E2=94=82 Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device
  2017-03-27 14:14       ` Benjamin Gaignard
  (?)
@ 2017-03-27 14:23       ` William Breathitt Gray
  2017-04-02 11:02         ` Jonathan Cameron
  -1 siblings, 1 reply; 19+ messages in thread
From: William Breathitt Gray @ 2017-03-27 14:23 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Linux Kernel Mailing List, Jonathan Cameron, linux-iio,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	mwelling, Fabrice Gasnier, Linaro Kernel Mailman List,
	Benjamin Gaignard

On Mon, Mar 27, 2017 at 04:14:25PM +0200, Benjamin Gaignard wrote:
>2017-03-27 15:47 GMT+02:00 William Breathitt Gray <vilhelm.gray@gmail.com>:
>> On Mon, Mar 27, 2017 at 11:43:23AM +0200, Benjamin Gaignard wrote:
>>>One of the features of STM32 trigger hardware block is a quadrature
>>>encoder that can counts up/down depending of the levels and edges
>>>of the selected external pins.
>>>
>>>This patch allow to read/write the counter, get it direction,
>>>set/get quadrature modes and get scale factor.
>>>
>>>When counting up preset value is the limit of the counter.
>>>When counting down the counter start from preset value down to 0.
>>>This preset value could be set/get by using
>>>/sys/bus/iio/devices/iio:deviceX/in_count0_preset attribute.
>>>
>>>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>
>> Hi Benjamin,
>>
>> This is pretty close to the ABI I had in mind. I'm a bit confused about
>> how to select some of functionality we discussed in the previous version
>> so I'll write those comments inline. Overall nice work! :)
>>
>>>
>>>version 3:
>>>- fix typo in documentation
>>>- change some functions names
>>>---
>>> .../ABI/testing/sysfs-bus-iio-timer-stm32          |  32 +++
>>> drivers/iio/trigger/stm32-timer-trigger.c          | 244 ++++++++++++++++++++-
>>> include/linux/mfd/stm32-timers.h                   |   2 +
>>> 3 files changed, 272 insertions(+), 6 deletions(-)
>>>
>>>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>index 6534a60..bf795ad 100644
>>>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>@@ -27,3 +27,35 @@ Description:
>>>               Reading returns the current sampling frequency.
>>>               Writing an value different of 0 set and start sampling.
>>>               Writing 0 stop sampling.
>>>+
>>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_preset
>>>+KernelVersion:        4.12
>>>+Contact:      benjamin.gaignard@st.com
>>>+Description:
>>>+              Reading returns the current preset value.
>>>+              Writing sets the preset value.
>>>+              When counting up the counter starts from 0 and fires an event when reach preset value.
>>>+              When counting down the counter start from preset value and fire event when reach 0.
>>
>> In one of the previous versions we discussed a "reset" mode of operation
>> where the counter would be reset to the preset value on the rising edge
>> of pin 1. How would a user select this mode of operation using this ABI
>> (I assume the preset value would be the same value as in the
>> in_count0_preset attribute).
>>
>> Also, would this "reset" mode be limited to count just the internal
>> clock, or may an external pin alternatively serve as a counter source?
>
>In reset mode counter is clocked by the internal clock
>Reset can only by drive by a rising edge of an internal signal
>(represented by an IIO trigger).

Ah, my apologies, I was under the incorrect impression that the reset
trigger was an external signal. I think understand how it works now.

>
>>
>>>+
>>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
>>>+KernelVersion:        4.12
>>>+Contact:      benjamin.gaignard@st.com
>>>+Description:
>>>+              Reading returns the list possible quadrature modes.
>>>+
>>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
>>>+KernelVersion:        4.12
>>>+Contact:      benjamin.gaignard@st.com
>>>+Description:
>>>+              Configure the device counter quadrature modes:
>>>+              channel_A:
>>>+                      Encoder A input servers as the count input and B as
>>>+                      the UP/DOWN direction control input.
>>>+
>>>+              channel_B:
>>>+                      Encoder B input serves as the count input and A as
>>>+                      the UP/DOWN direction control input.
>>>+
>>>+              quadrature:
>>>+                      Encoder A and B inputs are mixed to get direction
>>>+                      and count with a scale of 0.25.
>>
>> If I want to select the internal clock as the counter source, how would
>> I do so? Should there be an "internal_clock" mode available in the
>> quadrature_mode attribute?
>
>To use internal clock as counter source you must select one of the modes
>in in_count0_enable_mode attribute (patch 2/2)
>
>Counter could be drived by 3 "sources": internal clock, encoder input
>and internal signal
>I have in mind to keep this split in 3 attributes:enable_mode,
>quadrature_mode and a futur
>trigger_mode.

Many thanks for the further explanation. This clears up my confusion,
and I believe the interaction between these 3 attributes should work out
quite well for future drivers too.

William Breathitt Gray

>
>
>>
>> Sincerely,
>>
>> William Breathitt Gray
>>
>>>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>>index 994b96d..7db904c 100644
>>>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>>>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>>@@ -15,6 +15,7 @@
>>> #include <linux/platform_device.h>
>>>
>>> #define MAX_TRIGGERS 6
>>>+#define MAX_VALIDS 5
>>>
>>> /* List the triggers created by each timer */
>>> static const void *triggers_table[][MAX_TRIGGERS] = {
>>>@@ -32,12 +33,29 @@
>>>       { TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
>>> };
>>>
>>>+/* List the triggers accepted by each timer */
>>>+static const void *valids_table[][MAX_VALIDS] = {
>>>+      { TIM5_TRGO, TIM2_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>>+      { TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>>+      { TIM1_TRGO, TIM2_TRGO, TIM5_TRGO, TIM4_TRGO,},
>>>+      { TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
>>>+      { TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
>>>+      { }, /* timer 6 */
>>>+      { }, /* timer 7 */
>>>+      { TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
>>>+      { TIM2_TRGO, TIM3_TRGO,},
>>>+      { }, /* timer 10 */
>>>+      { }, /* timer 11 */
>>>+      { TIM4_TRGO, TIM5_TRGO,},
>>>+};
>>>+
>>> struct stm32_timer_trigger {
>>>       struct device *dev;
>>>       struct regmap *regmap;
>>>       struct clk *clk;
>>>       u32 max_arr;
>>>       const void *triggers;
>>>+      const void *valids;
>>> };
>>>
>>> static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>>@@ -180,8 +198,7 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
>>>                                        struct device_attribute *attr,
>>>                                        char *buf)
>>> {
>>>-      struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>-      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+      struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>>       u32 cr2;
>>>
>>>       regmap_read(priv->regmap, TIM_CR2, &cr2);
>>>@@ -194,8 +211,7 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
>>>                                         struct device_attribute *attr,
>>>                                         const char *buf, size_t len)
>>> {
>>>-      struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>-      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+      struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>>       int i;
>>>
>>>       for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
>>>@@ -275,6 +291,216 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
>>>       return 0;
>>> }
>>>
>>>+static int stm32_counter_read_raw(struct iio_dev *indio_dev,
>>>+                                struct iio_chan_spec const *chan,
>>>+                                int *val, int *val2, long mask)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+
>>>+      switch (mask) {
>>>+      case IIO_CHAN_INFO_RAW:
>>>+      {
>>>+              u32 cnt;
>>>+
>>>+              regmap_read(priv->regmap, TIM_CNT, &cnt);
>>>+              *val = cnt;
>>>+
>>>+              return IIO_VAL_INT;
>>>+      }
>>>+      case IIO_CHAN_INFO_SCALE:
>>>+      {
>>>+              u32 smcr;
>>>+
>>>+              regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>+              smcr &= TIM_SMCR_SMS;
>>>+
>>>+              *val = 1;
>>>+              *val2 = 0;
>>>+
>>>+              /* in quadrature case scale = 0.25 */
>>>+              if (smcr == 3)
>>>+                      *val2 = 2;
>>>+
>>>+              return IIO_VAL_FRACTIONAL_LOG2;
>>>+      }
>>>+      }
>>>+
>>>+      return -EINVAL;
>>>+}
>>>+
>>>+static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>>+                                 struct iio_chan_spec const *chan,
>>>+                                 int val, int val2, long mask)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+
>>>+      switch (mask) {
>>>+      case IIO_CHAN_INFO_RAW:
>>>+              regmap_write(priv->regmap, TIM_CNT, val);
>>>+
>>>+              return IIO_VAL_INT;
>>>+      case IIO_CHAN_INFO_SCALE:
>>>+              /* fixed scale */
>>>+              return -EINVAL;
>>>+      }
>>>+
>>>+      return -EINVAL;
>>>+}
>>>+
>>>+static const struct iio_info stm32_trigger_info = {
>>>+      .driver_module = THIS_MODULE,
>>>+      .read_raw = stm32_counter_read_raw,
>>>+      .write_raw = stm32_counter_write_raw
>>>+};
>>>+
>>>+static const char *const stm32_quadrature_modes[] = {
>>>+      "channel_A",
>>>+      "channel_B",
>>>+      "quadrature",
>>>+};
>>>+
>>>+static int stm32_set_quadrature_mode(struct iio_dev *indio_dev,
>>>+                                   const struct iio_chan_spec *chan,
>>>+                                   unsigned int mode)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+
>>>+      regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, mode + 1);
>>>+
>>>+      return 0;
>>>+}
>>>+
>>>+static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
>>>+                                   const struct iio_chan_spec *chan)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+      u32 smcr;
>>>+
>>>+      regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>+      smcr &= TIM_SMCR_SMS;
>>>+
>>>+      return smcr - 1;
>>>+}
>>>+
>>>+static const struct iio_enum stm32_quadrature_mode_enum = {
>>>+      .items = stm32_quadrature_modes,
>>>+      .num_items = ARRAY_SIZE(stm32_quadrature_modes),
>>>+      .set = stm32_set_quadrature_mode,
>>>+      .get = stm32_get_quadrature_mode
>>>+};
>>>+
>>>+static const char *const stm32_count_direction_states[] = {
>>>+      "up",
>>>+      "down"
>>>+};
>>>+
>>>+static int stm32_set_count_direction(struct iio_dev *indio_dev,
>>>+                                   const struct iio_chan_spec *chan,
>>>+                                   unsigned int mode)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+
>>>+      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
>>>+
>>>+      return 0;
>>>+}
>>>+
>>>+static int stm32_get_count_direction(struct iio_dev *indio_dev,
>>>+                                   const struct iio_chan_spec *chan)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+      u32 cr1;
>>>+
>>>+      regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>+
>>>+      return (cr1 & TIM_CR1_DIR);
>>>+}
>>>+
>>>+static const struct iio_enum stm32_count_direction_enum = {
>>>+      .items = stm32_count_direction_states,
>>>+      .num_items = ARRAY_SIZE(stm32_count_direction_states),
>>>+      .set = stm32_set_count_direction,
>>>+      .get = stm32_get_count_direction
>>>+};
>>>+
>>>+static ssize_t stm32_count_get_preset(struct iio_dev *indio_dev,
>>>+                                    uintptr_t private,
>>>+                                    const struct iio_chan_spec *chan,
>>>+                                    char *buf)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+      u32 arr;
>>>+
>>>+      regmap_read(priv->regmap, TIM_ARR, &arr);
>>>+
>>>+      return snprintf(buf, PAGE_SIZE, "%u\n", arr);
>>>+}
>>>+
>>>+static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>>+                                    uintptr_t private,
>>>+                                    const struct iio_chan_spec *chan,
>>>+                                    const char *buf, size_t len)
>>>+{
>>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>+      unsigned int preset;
>>>+      int ret;
>>>+
>>>+      ret = kstrtouint(buf, 0, &preset);
>>>+      if (ret)
>>>+              return ret;
>>>+
>>>+      regmap_write(priv->regmap, TIM_ARR, preset);
>>>+      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>>>+
>>>+      return len;
>>>+}
>>>+
>>>+static const struct iio_chan_spec_ext_info stm32_trigger_count_info[] = {
>>>+      {
>>>+              .name = "preset",
>>>+              .shared = IIO_SEPARATE,
>>>+              .read = stm32_count_get_preset,
>>>+              .write = stm32_count_set_preset
>>>+      },
>>>+      IIO_ENUM("count_direction", IIO_SEPARATE, &stm32_count_direction_enum),
>>>+      IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>>+      IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>>+      IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>>+      {}
>>>+};
>>>+
>>>+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),
>>>+      .ext_info = stm32_trigger_count_info,
>>>+      .indexed = 1
>>>+};
>>>+
>>>+static struct stm32_timer_trigger *stm32_setup_counter_device(struct device *dev)
>>>+{
>>>+      struct iio_dev *indio_dev;
>>>+      int ret;
>>>+
>>>+      indio_dev = devm_iio_device_alloc(dev,
>>>+                                        sizeof(struct stm32_timer_trigger));
>>>+      if (!indio_dev)
>>>+              return NULL;
>>>+
>>>+      indio_dev->name = dev_name(dev);
>>>+      indio_dev->dev.parent = dev;
>>>+      indio_dev->info = &stm32_trigger_info;
>>>+      indio_dev->num_channels = 1;
>>>+      indio_dev->channels = &stm32_trigger_channel;
>>>+      indio_dev->dev.of_node = dev->of_node;
>>>+
>>>+      ret = devm_iio_device_register(dev, indio_dev);
>>>+      if (ret)
>>>+              return NULL;
>>>+
>>>+      return iio_priv(indio_dev);
>>>+}
>>>+
>>> /**
>>>  * is_stm32_timer_trigger
>>>  * @trig: trigger to be checked
>>>@@ -299,10 +525,15 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>>       if (of_property_read_u32(dev->of_node, "reg", &index))
>>>               return -EINVAL;
>>>
>>>-      if (index >= ARRAY_SIZE(triggers_table))
>>>+      if (index >= ARRAY_SIZE(triggers_table) ||
>>>+          index >= ARRAY_SIZE(valids_table))
>>>               return -EINVAL;
>>>
>>>-      priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>+      /* Create an IIO device only if we have triggers to be validated */
>>>+      if (*valids_table[index])
>>>+              priv = stm32_setup_counter_device(dev);
>>>+      else
>>>+              priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>
>>>       if (!priv)
>>>               return -ENOMEM;
>>>@@ -312,6 +543,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>>       priv->clk = ddata->clk;
>>>       priv->max_arr = ddata->max_arr;
>>>       priv->triggers = triggers_table[index];
>>>+      priv->valids = valids_table[index];
>>>
>>>       ret = stm32_setup_iio_triggers(priv);
>>>       if (ret)
>>>diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>>index d030004..4a0abbc 100644
>>>--- a/include/linux/mfd/stm32-timers.h
>>>+++ b/include/linux/mfd/stm32-timers.h
>>>@@ -21,6 +21,7 @@
>>> #define TIM_CCMR1     0x18    /* Capt/Comp 1 Mode Reg    */
>>> #define TIM_CCMR2     0x1C    /* Capt/Comp 2 Mode Reg    */
>>> #define TIM_CCER      0x20    /* Capt/Comp Enable Reg    */
>>>+#define TIM_CNT               0x24    /* Counter                 */
>>> #define TIM_PSC               0x28    /* Prescaler               */
>>> #define TIM_ARR               0x2c    /* Auto-Reload Register    */
>>> #define TIM_CCR1      0x34    /* Capt/Comp Register 1    */
>>>@@ -30,6 +31,7 @@
>>> #define TIM_BDTR      0x44    /* Break and Dead-Time Reg */
>>>
>>> #define TIM_CR1_CEN   BIT(0)  /* Counter Enable          */
>>>+#define TIM_CR1_DIR   BIT(4)  /* Counter Direction       */
>>> #define TIM_CR1_ARPE  BIT(7)  /* Auto-reload Preload Ena */
>>> #define TIM_CR2_MMS   (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
>>> #define TIM_SMCR_SMS  (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
>>>--
>>>1.9.1
>>>
>
>
>
>-- 
>Benjamin Gaignard
>
>Graphic Study Group
>
>Linaro.org │ Open source software for ARM SoCs
>
>Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-03-27 13:55   ` William Breathitt Gray
@ 2017-03-27 14:43       ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-03-27 14:43 UTC (permalink / raw)
  To: William Breathitt Gray
  Cc: Linux Kernel Mailing List, Jonathan Cameron, linux-iio,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	mwelling, Fabrice Gasnier, Linaro Kernel Mailman List,
	Benjamin Gaignard

2017-03-27 15:55 GMT+02:00 William Breathitt Gray <vilhelm.gray@gmail.com>:
> On Mon, Mar 27, 2017 at 11:43:24AM +0200, Benjamin Gaignard wrote:
>>Device counting could be controlled by the level or the edges of
>>a trigger.
>>in_count0_enable_mode attibute allow to set the control mode.
>>
>>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>
> This patch is pretty straight-forward and all right with me. A minor
> nitpick inline regarding documentation.
>
> Reviewed-by: William Breathitt Gray <vilhelm.gray@gmail.com>
>
>>---
>> .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>> drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
>> 2 files changed, 93 insertions(+)
>>
>>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>index bf795ad..c0a1edc 100644
>>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>@@ -59,3 +59,26 @@ Description:
>>               quadrature:
>>                       Encoder A and B inputs are mixed to get direction
>>                       and count with a scale of 0.25.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Reading returns the list possible enable modes.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Configure the device counter enable modes, in all case
>>+              counting direction is set by in_count0_count_direction
>>+              attribute and the counter is clocked by the internal clock.
>>+              always:
>>+                      Counter is always ON.
>>+
>>+              gated:
>>+                      Counting is enabled when connected trigger signal
>>+                      level is high else counting is disabled.
>>+
>>+              triggered:
>>+                      Counting start on rising edge of the connected trigger.
>
> When I read the description of the "triggered" mode, I get the
> impression that counting is restarted every time there is a rising edge
> from the connected trigger signal. However, from our previous
> discussions I believe this mode is simply a latched enable.
>
> Perhaps it may be beneficial for clarity to rework the description to
> say something along the lines of: "Counting is enabled on rising edge of
> the connected trigger, and remains enabled for the duration of this
> selected mode."

I will update that in v4.

Thanks

>
> William Breathitt Gray
>
>>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>index 7db904c..0f1a2cf 100644
>>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>@@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>       .write_raw = stm32_counter_write_raw
>> };
>>
>>+static const char *const stm32_enable_modes[] = {
>>+      "always",
>>+      "gated",
>>+      "triggered",
>>+};
>>+
>>+static int stm32_enable_mode2sms(int mode)
>>+{
>>+      switch (mode) {
>>+      case 0:
>>+              return 0;
>>+      case 1:
>>+              return 5;
>>+      case 2:
>>+              return 6;
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>>+                               const struct iio_chan_spec *chan,
>>+                               unsigned int mode)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      int sms = stm32_enable_mode2sms(mode);
>>+
>>+      if (sms < 0)
>>+              return sms;
>>+
>>+      regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>>+
>>+      return 0;
>>+}
>>+
>>+static int stm32_sms2enable_mode(int mode)
>>+{
>>+      switch (mode) {
>>+      case 0:
>>+              return 0;
>>+      case 5:
>>+              return 1;
>>+      case 6:
>>+              return 2;
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>>+                               const struct iio_chan_spec *chan)
>>+{
>>+      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+      u32 smcr;
>>+
>>+      regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>+      smcr &= TIM_SMCR_SMS;
>>+
>>+      return stm32_sms2enable_mode(smcr);
>>+}
>>+
>>+static const struct iio_enum stm32_enable_mode_enum = {
>>+      .items = stm32_enable_modes,
>>+      .num_items = ARRAY_SIZE(stm32_enable_modes),
>>+      .set = stm32_set_enable_mode,
>>+      .get = stm32_get_enable_mode
>>+};
>>+
>> static const char *const stm32_quadrature_modes[] = {
>>       "channel_A",
>>       "channel_B",
>>@@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>+      IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>>+      IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>       {}
>> };
>>
>>--
>>1.9.1
>>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
@ 2017-03-27 14:43       ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-03-27 14:43 UTC (permalink / raw)
  To: William Breathitt Gray
  Cc: Linux Kernel Mailing List, Jonathan Cameron, linux-iio,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	mwelling, Fabrice Gasnier, Linaro Kernel Mailman List,
	Benjamin Gaignard

2017-03-27 15:55 GMT+02:00 William Breathitt Gray <vilhelm.gray@gmail.com>:
> On Mon, Mar 27, 2017 at 11:43:24AM +0200, Benjamin Gaignard wrote:
>>Device counting could be controlled by the level or the edges of
>>a trigger.
>>in_count0_enable_mode attibute allow to set the control mode.
>>
>>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>
> This patch is pretty straight-forward and all right with me. A minor
> nitpick inline regarding documentation.
>
> Reviewed-by: William Breathitt Gray <vilhelm.gray@gmail.com>
>
>>---
>> .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>> drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++=
++++++
>> 2 files changed, 93 insertions(+)
>>
>>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Docume=
ntation/ABI/testing/sysfs-bus-iio-timer-stm32
>>index bf795ad..c0a1edc 100644
>>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>@@ -59,3 +59,26 @@ Description:
>>               quadrature:
>>                       Encoder A and B inputs are mixed to get direction
>>                       and count with a scale of 0.25.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_avai=
lable
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Reading returns the list possible enable modes.
>>+
>>+What:         /sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>>+KernelVersion:        4.12
>>+Contact:      benjamin.gaignard@st.com
>>+Description:
>>+              Configure the device counter enable modes, in all case
>>+              counting direction is set by in_count0_count_direction
>>+              attribute and the counter is clocked by the internal clock=
.
>>+              always:
>>+                      Counter is always ON.
>>+
>>+              gated:
>>+                      Counting is enabled when connected trigger signal
>>+                      level is high else counting is disabled.
>>+
>>+              triggered:
>>+                      Counting start on rising edge of the connected tri=
gger.
>
> When I read the description of the "triggered" mode, I get the
> impression that counting is restarted every time there is a rising edge
> from the connected trigger signal. However, from our previous
> discussions I believe this mode is simply a latched enable.
>
> Perhaps it may be beneficial for clarity to rework the description to
> say something along the lines of: "Counting is enabled on rising edge of
> the connected trigger, and remains enabled for the duration of this
> selected mode."

I will update that in v4.

Thanks

>
> William Breathitt Gray
>
>>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trig=
ger/stm32-timer-trigger.c
>>index 7db904c..0f1a2cf 100644
>>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>@@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *i=
ndio_dev,
>>       .write_raw =3D stm32_counter_write_raw
>> };
>>
>>+static const char *const stm32_enable_modes[] =3D {
>>+      "always",
>>+      "gated",
>>+      "triggered",
>>+};
>>+
>>+static int stm32_enable_mode2sms(int mode)
>>+{
>>+      switch (mode) {
>>+      case 0:
>>+              return 0;
>>+      case 1:
>>+              return 5;
>>+      case 2:
>>+              return 6;
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>>+                               const struct iio_chan_spec *chan,
>>+                               unsigned int mode)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      int sms =3D stm32_enable_mode2sms(mode);
>>+
>>+      if (sms < 0)
>>+              return sms;
>>+
>>+      regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>>+
>>+      return 0;
>>+}
>>+
>>+static int stm32_sms2enable_mode(int mode)
>>+{
>>+      switch (mode) {
>>+      case 0:
>>+              return 0;
>>+      case 5:
>>+              return 1;
>>+      case 6:
>>+              return 2;
>>+      }
>>+
>>+      return -EINVAL;
>>+}
>>+
>>+static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>>+                               const struct iio_chan_spec *chan)
>>+{
>>+      struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>+      u32 smcr;
>>+
>>+      regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>+      smcr &=3D TIM_SMCR_SMS;
>>+
>>+      return stm32_sms2enable_mode(smcr);
>>+}
>>+
>>+static const struct iio_enum stm32_enable_mode_enum =3D {
>>+      .items =3D stm32_enable_modes,
>>+      .num_items =3D ARRAY_SIZE(stm32_enable_modes),
>>+      .set =3D stm32_set_enable_mode,
>>+      .get =3D stm32_get_enable_mode
>>+};
>>+
>> static const char *const stm32_quadrature_modes[] =3D {
>>       "channel_A",
>>       "channel_B",
>>@@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev =
*indio_dev,
>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum)=
,
>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_e=
num),
>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum)=
,
>>+      IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>>+      IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>       {}
>> };
>>
>>--
>>1.9.1
>>



--=20
Benjamin Gaignard

Graphic Study Group

Linaro.org =E2=94=82 Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device
  2017-03-27 14:23       ` William Breathitt Gray
@ 2017-04-02 11:02         ` Jonathan Cameron
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2017-04-02 11:02 UTC (permalink / raw)
  To: William Breathitt Gray, Benjamin Gaignard
  Cc: Linux Kernel Mailing List, linux-iio, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, mwelling,
	Fabrice Gasnier, Linaro Kernel Mailman List, Benjamin Gaignard

On 27/03/17 15:23, William Breathitt Gray wrote:
> On Mon, Mar 27, 2017 at 04:14:25PM +0200, Benjamin Gaignard wrote:
>> 2017-03-27 15:47 GMT+02:00 William Breathitt Gray <vilhelm.gray@gmail.com>:
>>> On Mon, Mar 27, 2017 at 11:43:23AM +0200, Benjamin Gaignard wrote:
>>>> One of the features of STM32 trigger hardware block is a quadrature
>>>> encoder that can counts up/down depending of the levels and edges
>>>> of the selected external pins.
>>>>
>>>> This patch allow to read/write the counter, get it direction,
>>>> set/get quadrature modes and get scale factor.
>>>>
>>>> When counting up preset value is the limit of the counter.
>>>> When counting down the counter start from preset value down to 0.
>>>> This preset value could be set/get by using
>>>> /sys/bus/iio/devices/iio:deviceX/in_count0_preset attribute.
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>
>>> Hi Benjamin,
>>>
>>> This is pretty close to the ABI I had in mind. I'm a bit confused about
>>> how to select some of functionality we discussed in the previous version
>>> so I'll write those comments inline. Overall nice work! :)
>>>
>>>>
>>>> version 3:
>>>> - fix typo in documentation
>>>> - change some functions names
>>>> ---
>>>> .../ABI/testing/sysfs-bus-iio-timer-stm32          |  32 +++
>>>> drivers/iio/trigger/stm32-timer-trigger.c          | 244 ++++++++++++++++++++-
>>>> include/linux/mfd/stm32-timers.h                   |   2 +
>>>> 3 files changed, 272 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> index 6534a60..bf795ad 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> @@ -27,3 +27,35 @@ Description:
>>>>               Reading returns the current sampling frequency.
>>>>               Writing an value different of 0 set and start sampling.
>>>>               Writing 0 stop sampling.
>>>> +
>>>> +What:         /sys/bus/iio/devices/iio:deviceX/in_count0_preset
>>>> +KernelVersion:        4.12
>>>> +Contact:      benjamin.gaignard@st.com
>>>> +Description:
>>>> +              Reading returns the current preset value.
>>>> +              Writing sets the preset value.
>>>> +              When counting up the counter starts from 0 and fires an event when reach preset value.
>>>> +              When counting down the counter start from preset value and fire event when reach 0.
>>>
>>> In one of the previous versions we discussed a "reset" mode of operation
>>> where the counter would be reset to the preset value on the rising edge
>>> of pin 1. How would a user select this mode of operation using this ABI
>>> (I assume the preset value would be the same value as in the
>>> in_count0_preset attribute).
>>>
>>> Also, would this "reset" mode be limited to count just the internal
>>> clock, or may an external pin alternatively serve as a counter source?
>>
>> In reset mode counter is clocked by the internal clock
>> Reset can only by drive by a rising edge of an internal signal
>> (represented by an IIO trigger).
> 
> Ah, my apologies, I was under the incorrect impression that the reset
> trigger was an external signal. I think understand how it works now.
That's certainly going to be an option on the 'general' form of this
ABI.  We can generalize at that point to cover it I think.
> 
>>
>>>
>>>> +
>>>> +What:         /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
>>>> +KernelVersion:        4.12
>>>> +Contact:      benjamin.gaignard@st.com
>>>> +Description:
>>>> +              Reading returns the list possible quadrature modes.
>>>> +
>>>> +What:         /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
>>>> +KernelVersion:        4.12
>>>> +Contact:      benjamin.gaignard@st.com
>>>> +Description:
>>>> +              Configure the device counter quadrature modes:
>>>> +              channel_A:
>>>> +                      Encoder A input servers as the count input and B as
>>>> +                      the UP/DOWN direction control input.
>>>> +
>>>> +              channel_B:
>>>> +                      Encoder B input serves as the count input and A as
>>>> +                      the UP/DOWN direction control input.
>>>> +
>>>> +              quadrature:
>>>> +                      Encoder A and B inputs are mixed to get direction
>>>> +                      and count with a scale of 0.25.
>>>
>>> If I want to select the internal clock as the counter source, how would
>>> I do so? Should there be an "internal_clock" mode available in the
>>> quadrature_mode attribute?
>>
>> To use internal clock as counter source you must select one of the modes
>> in in_count0_enable_mode attribute (patch 2/2)
>>
>> Counter could be drived by 3 "sources": internal clock, encoder input
>> and internal signal
>> I have in mind to keep this split in 3 attributes:enable_mode,
>> quadrature_mode and a futur
>> trigger_mode.
> 
> Many thanks for the further explanation. This clears up my confusion,
> and I believe the interaction between these 3 attributes should work out
> quite well for future drivers too.
> 
> William Breathitt Gray
> 
>>
>>
>>>
>>> Sincerely,
>>>
>>> William Breathitt Gray
>>>
>>>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>>> index 994b96d..7db904c 100644
>>>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>>>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>>> @@ -15,6 +15,7 @@
>>>> #include <linux/platform_device.h>
>>>>
>>>> #define MAX_TRIGGERS 6
>>>> +#define MAX_VALIDS 5
>>>>
>>>> /* List the triggers created by each timer */
>>>> static const void *triggers_table[][MAX_TRIGGERS] = {
>>>> @@ -32,12 +33,29 @@
>>>>       { TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
>>>> };
>>>>
>>>> +/* List the triggers accepted by each timer */
>>>> +static const void *valids_table[][MAX_VALIDS] = {
>>>> +      { TIM5_TRGO, TIM2_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>>> +      { TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>>> +      { TIM1_TRGO, TIM2_TRGO, TIM5_TRGO, TIM4_TRGO,},
>>>> +      { TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
>>>> +      { TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
>>>> +      { }, /* timer 6 */
>>>> +      { }, /* timer 7 */
>>>> +      { TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
>>>> +      { TIM2_TRGO, TIM3_TRGO,},
>>>> +      { }, /* timer 10 */
>>>> +      { }, /* timer 11 */
>>>> +      { TIM4_TRGO, TIM5_TRGO,},
>>>> +};
>>>> +
>>>> struct stm32_timer_trigger {
>>>>       struct device *dev;
>>>>       struct regmap *regmap;
>>>>       struct clk *clk;
>>>>       u32 max_arr;
>>>>       const void *triggers;
>>>> +      const void *valids;
>>>> };
>>>>
>>>> static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>>> @@ -180,8 +198,7 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
>>>>                                        struct device_attribute *attr,
>>>>                                        char *buf)
>>>> {
>>>> -      struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>> -      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +      struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>>>       u32 cr2;
>>>>
>>>>       regmap_read(priv->regmap, TIM_CR2, &cr2);
>>>> @@ -194,8 +211,7 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
>>>>                                         struct device_attribute *attr,
>>>>                                         const char *buf, size_t len)
>>>> {
>>>> -      struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>> -      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +      struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>>>       int i;
>>>>
>>>>       for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
>>>> @@ -275,6 +291,216 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
>>>>       return 0;
>>>> }
>>>>
>>>> +static int stm32_counter_read_raw(struct iio_dev *indio_dev,
>>>> +                                struct iio_chan_spec const *chan,
>>>> +                                int *val, int *val2, long mask)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +
>>>> +      switch (mask) {
>>>> +      case IIO_CHAN_INFO_RAW:
>>>> +      {
>>>> +              u32 cnt;
>>>> +
>>>> +              regmap_read(priv->regmap, TIM_CNT, &cnt);
>>>> +              *val = cnt;
>>>> +
>>>> +              return IIO_VAL_INT;
>>>> +      }
>>>> +      case IIO_CHAN_INFO_SCALE:
>>>> +      {
>>>> +              u32 smcr;
>>>> +
>>>> +              regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>> +              smcr &= TIM_SMCR_SMS;
>>>> +
>>>> +              *val = 1;
>>>> +              *val2 = 0;
>>>> +
>>>> +              /* in quadrature case scale = 0.25 */
>>>> +              if (smcr == 3)
>>>> +                      *val2 = 2;
>>>> +
>>>> +              return IIO_VAL_FRACTIONAL_LOG2;
>>>> +      }
>>>> +      }
>>>> +
>>>> +      return -EINVAL;
>>>> +}
>>>> +
>>>> +static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>>> +                                 struct iio_chan_spec const *chan,
>>>> +                                 int val, int val2, long mask)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +
>>>> +      switch (mask) {
>>>> +      case IIO_CHAN_INFO_RAW:
>>>> +              regmap_write(priv->regmap, TIM_CNT, val);
>>>> +
>>>> +              return IIO_VAL_INT;
>>>> +      case IIO_CHAN_INFO_SCALE:
>>>> +              /* fixed scale */
>>>> +              return -EINVAL;
>>>> +      }
>>>> +
>>>> +      return -EINVAL;
>>>> +}
>>>> +
>>>> +static const struct iio_info stm32_trigger_info = {
>>>> +      .driver_module = THIS_MODULE,
>>>> +      .read_raw = stm32_counter_read_raw,
>>>> +      .write_raw = stm32_counter_write_raw
>>>> +};
>>>> +
>>>> +static const char *const stm32_quadrature_modes[] = {
>>>> +      "channel_A",
>>>> +      "channel_B",
>>>> +      "quadrature",
>>>> +};
>>>> +
>>>> +static int stm32_set_quadrature_mode(struct iio_dev *indio_dev,
>>>> +                                   const struct iio_chan_spec *chan,
>>>> +                                   unsigned int mode)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +
>>>> +      regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, mode + 1);
>>>> +
>>>> +      return 0;
>>>> +}
>>>> +
>>>> +static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
>>>> +                                   const struct iio_chan_spec *chan)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +      u32 smcr;
>>>> +
>>>> +      regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>> +      smcr &= TIM_SMCR_SMS;
>>>> +
>>>> +      return smcr - 1;
>>>> +}
>>>> +
>>>> +static const struct iio_enum stm32_quadrature_mode_enum = {
>>>> +      .items = stm32_quadrature_modes,
>>>> +      .num_items = ARRAY_SIZE(stm32_quadrature_modes),
>>>> +      .set = stm32_set_quadrature_mode,
>>>> +      .get = stm32_get_quadrature_mode
>>>> +};
>>>> +
>>>> +static const char *const stm32_count_direction_states[] = {
>>>> +      "up",
>>>> +      "down"
>>>> +};
>>>> +
>>>> +static int stm32_set_count_direction(struct iio_dev *indio_dev,
>>>> +                                   const struct iio_chan_spec *chan,
>>>> +                                   unsigned int mode)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +
>>>> +      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
>>>> +
>>>> +      return 0;
>>>> +}
>>>> +
>>>> +static int stm32_get_count_direction(struct iio_dev *indio_dev,
>>>> +                                   const struct iio_chan_spec *chan)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +      u32 cr1;
>>>> +
>>>> +      regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>> +
>>>> +      return (cr1 & TIM_CR1_DIR);
>>>> +}
>>>> +
>>>> +static const struct iio_enum stm32_count_direction_enum = {
>>>> +      .items = stm32_count_direction_states,
>>>> +      .num_items = ARRAY_SIZE(stm32_count_direction_states),
>>>> +      .set = stm32_set_count_direction,
>>>> +      .get = stm32_get_count_direction
>>>> +};
>>>> +
>>>> +static ssize_t stm32_count_get_preset(struct iio_dev *indio_dev,
>>>> +                                    uintptr_t private,
>>>> +                                    const struct iio_chan_spec *chan,
>>>> +                                    char *buf)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +      u32 arr;
>>>> +
>>>> +      regmap_read(priv->regmap, TIM_ARR, &arr);
>>>> +
>>>> +      return snprintf(buf, PAGE_SIZE, "%u\n", arr);
>>>> +}
>>>> +
>>>> +static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>>> +                                    uintptr_t private,
>>>> +                                    const struct iio_chan_spec *chan,
>>>> +                                    const char *buf, size_t len)
>>>> +{
>>>> +      struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +      unsigned int preset;
>>>> +      int ret;
>>>> +
>>>> +      ret = kstrtouint(buf, 0, &preset);
>>>> +      if (ret)
>>>> +              return ret;
>>>> +
>>>> +      regmap_write(priv->regmap, TIM_ARR, preset);
>>>> +      regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>>>> +
>>>> +      return len;
>>>> +}
>>>> +
>>>> +static const struct iio_chan_spec_ext_info stm32_trigger_count_info[] = {
>>>> +      {
>>>> +              .name = "preset",
>>>> +              .shared = IIO_SEPARATE,
>>>> +              .read = stm32_count_get_preset,
>>>> +              .write = stm32_count_set_preset
>>>> +      },
>>>> +      IIO_ENUM("count_direction", IIO_SEPARATE, &stm32_count_direction_enum),
>>>> +      IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>>> +      IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>>> +      IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>>> +      {}
>>>> +};
>>>> +
>>>> +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),
>>>> +      .ext_info = stm32_trigger_count_info,
>>>> +      .indexed = 1
>>>> +};
>>>> +
>>>> +static struct stm32_timer_trigger *stm32_setup_counter_device(struct device *dev)
>>>> +{
>>>> +      struct iio_dev *indio_dev;
>>>> +      int ret;
>>>> +
>>>> +      indio_dev = devm_iio_device_alloc(dev,
>>>> +                                        sizeof(struct stm32_timer_trigger));
>>>> +      if (!indio_dev)
>>>> +              return NULL;
>>>> +
>>>> +      indio_dev->name = dev_name(dev);
>>>> +      indio_dev->dev.parent = dev;
>>>> +      indio_dev->info = &stm32_trigger_info;
>>>> +      indio_dev->num_channels = 1;
>>>> +      indio_dev->channels = &stm32_trigger_channel;
>>>> +      indio_dev->dev.of_node = dev->of_node;
>>>> +
>>>> +      ret = devm_iio_device_register(dev, indio_dev);
>>>> +      if (ret)
>>>> +              return NULL;
>>>> +
>>>> +      return iio_priv(indio_dev);
>>>> +}
>>>> +
>>>> /**
>>>>  * is_stm32_timer_trigger
>>>>  * @trig: trigger to be checked
>>>> @@ -299,10 +525,15 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>>>       if (of_property_read_u32(dev->of_node, "reg", &index))
>>>>               return -EINVAL;
>>>>
>>>> -      if (index >= ARRAY_SIZE(triggers_table))
>>>> +      if (index >= ARRAY_SIZE(triggers_table) ||
>>>> +          index >= ARRAY_SIZE(valids_table))
>>>>               return -EINVAL;
>>>>
>>>> -      priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>> +      /* Create an IIO device only if we have triggers to be validated */
>>>> +      if (*valids_table[index])
>>>> +              priv = stm32_setup_counter_device(dev);
>>>> +      else
>>>> +              priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>>
>>>>       if (!priv)
>>>>               return -ENOMEM;
>>>> @@ -312,6 +543,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>>>       priv->clk = ddata->clk;
>>>>       priv->max_arr = ddata->max_arr;
>>>>       priv->triggers = triggers_table[index];
>>>> +      priv->valids = valids_table[index];
>>>>
>>>>       ret = stm32_setup_iio_triggers(priv);
>>>>       if (ret)
>>>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>>> index d030004..4a0abbc 100644
>>>> --- a/include/linux/mfd/stm32-timers.h
>>>> +++ b/include/linux/mfd/stm32-timers.h
>>>> @@ -21,6 +21,7 @@
>>>> #define TIM_CCMR1     0x18    /* Capt/Comp 1 Mode Reg    */
>>>> #define TIM_CCMR2     0x1C    /* Capt/Comp 2 Mode Reg    */
>>>> #define TIM_CCER      0x20    /* Capt/Comp Enable Reg    */
>>>> +#define TIM_CNT               0x24    /* Counter                 */
>>>> #define TIM_PSC               0x28    /* Prescaler               */
>>>> #define TIM_ARR               0x2c    /* Auto-Reload Register    */
>>>> #define TIM_CCR1      0x34    /* Capt/Comp Register 1    */
>>>> @@ -30,6 +31,7 @@
>>>> #define TIM_BDTR      0x44    /* Break and Dead-Time Reg */
>>>>
>>>> #define TIM_CR1_CEN   BIT(0)  /* Counter Enable          */
>>>> +#define TIM_CR1_DIR   BIT(4)  /* Counter Direction       */
>>>> #define TIM_CR1_ARPE  BIT(7)  /* Auto-reload Preload Ena */
>>>> #define TIM_CR2_MMS   (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
>>>> #define TIM_SMCR_SMS  (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
>>>> --
>>>> 1.9.1
>>>>
>>
>>
>>
>> -- 
>> Benjamin Gaignard
>>
>> Graphic Study Group
>>
>> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro: Facebook | Twitter | Blog
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device
  2017-03-27  9:43 ` [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device Benjamin Gaignard
  2017-03-27 13:47   ` William Breathitt Gray
@ 2017-04-02 11:08   ` Jonathan Cameron
  1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2017-04-02 11:08 UTC (permalink / raw)
  To: Benjamin Gaignard, linux-kernel, linux-iio, knaack.h, lars,
	pmeerw, vilhelm.gray, mwelling
  Cc: fabrice.gasnier, linaro-kernel, Benjamin Gaignard

On 27/03/17 10:43, Benjamin Gaignard wrote:
> One of the features of STM32 trigger hardware block is a quadrature
> encoder that can counts up/down depending of the levels and edges
> of the selected external pins.
> 
> This patch allow to read/write the counter, get it direction,
> set/get quadrature modes and get scale factor.
> 
> When counting up preset value is the limit of the counter.
> When counting down the counter start from preset value down to 0.
> This preset value could be set/get by using
> /sys/bus/iio/devices/iio:deviceX/in_count0_preset attribute.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Just one trivial on line wrapping in the docs.

Otherwise, looks good to me.  Sounds like you are doing a V4 anyway,
otherwise I'd probably just have fixed this up.

Jonathan
> 
> version 3:
> - fix typo in documentation
> - change some functions names
> ---
>  .../ABI/testing/sysfs-bus-iio-timer-stm32          |  32 +++
>  drivers/iio/trigger/stm32-timer-trigger.c          | 244 ++++++++++++++++++++-
>  include/linux/mfd/stm32-timers.h                   |   2 +
>  3 files changed, 272 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> index 6534a60..bf795ad 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> @@ -27,3 +27,35 @@ Description:
>  		Reading returns the current sampling frequency.
>  		Writing an value different of 0 set and start sampling.
>  		Writing 0 stop sampling.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_count0_preset
> +KernelVersion:	4.12
> +Contact:	benjamin.gaignard@st.com
> +Description:
> +		Reading returns the current preset value.
> +		Writing sets the preset value.
> +		When counting up the counter starts from 0 and fires an event when reach preset value.

Please redo the wrapping as some long lines in here that aren't necessary.

> +		When counting down the counter start from preset value and fire event when reach 0.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
> +KernelVersion:	4.12
> +Contact:	benjamin.gaignard@st.com
> +Description:
> +		Reading returns the list possible quadrature modes.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
> +KernelVersion:	4.12
> +Contact:	benjamin.gaignard@st.com
> +Description:
> +		Configure the device counter quadrature modes:
> +		channel_A:
> +			Encoder A input servers as the count input and B as
> +			the UP/DOWN direction control input.
> +
> +		channel_B:
> +			Encoder B input serves as the count input and A as
> +			the UP/DOWN direction control input.
> +
> +		quadrature:
> +			Encoder A and B inputs are mixed to get direction
> +			and count with a scale of 0.25.

Good descriptions. I'm happy with this now.

> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index 994b96d..7db904c 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  
>  #define MAX_TRIGGERS 6
> +#define MAX_VALIDS 5
>  
>  /* List the triggers created by each timer */
>  static const void *triggers_table[][MAX_TRIGGERS] = {
> @@ -32,12 +33,29 @@
>  	{ TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
>  };
>  
> +/* List the triggers accepted by each timer */
> +static const void *valids_table[][MAX_VALIDS] = {
> +	{ TIM5_TRGO, TIM2_TRGO, TIM3_TRGO, TIM4_TRGO,},
> +	{ TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
> +	{ TIM1_TRGO, TIM2_TRGO, TIM5_TRGO, TIM4_TRGO,},
> +	{ TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
> +	{ TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
> +	{ }, /* timer 6 */
> +	{ }, /* timer 7 */
> +	{ TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
> +	{ TIM2_TRGO, TIM3_TRGO,},
> +	{ }, /* timer 10 */
> +	{ }, /* timer 11 */
> +	{ TIM4_TRGO, TIM5_TRGO,},
> +};
> +
>  struct stm32_timer_trigger {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct clk *clk;
>  	u32 max_arr;
>  	const void *triggers;
> +	const void *valids;
>  };
>  
>  static int stm32_timer_start(struct stm32_timer_trigger *priv,
> @@ -180,8 +198,7 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
>  					 struct device_attribute *attr,
>  					 char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>  	u32 cr2;
>  
>  	regmap_read(priv->regmap, TIM_CR2, &cr2);
> @@ -194,8 +211,7 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
>  					  struct device_attribute *attr,
>  					  const char *buf, size_t len)
>  {
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
> @@ -275,6 +291,216 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
>  	return 0;
>  }
>  
> +static int stm32_counter_read_raw(struct iio_dev *indio_dev,
> +				  struct iio_chan_spec const *chan,
> +				  int *val, int *val2, long mask)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +	{
> +		u32 cnt;
> +
> +		regmap_read(priv->regmap, TIM_CNT, &cnt);
> +		*val = cnt;
> +
> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_SCALE:
> +	{
> +		u32 smcr;
> +
> +		regmap_read(priv->regmap, TIM_SMCR, &smcr);
> +		smcr &= TIM_SMCR_SMS;
> +
> +		*val = 1;
> +		*val2 = 0;
> +
> +		/* in quadrature case scale = 0.25 */
> +		if (smcr == 3)
> +			*val2 = 2;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> +				   struct iio_chan_spec const *chan,
> +				   int val, int val2, long mask)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		regmap_write(priv->regmap, TIM_CNT, val);
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		/* fixed scale */
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info stm32_trigger_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = stm32_counter_read_raw,
> +	.write_raw = stm32_counter_write_raw
> +};
> +
> +static const char *const stm32_quadrature_modes[] = {
> +	"channel_A",
> +	"channel_B",
> +	"quadrature",
> +};
> +
> +static int stm32_set_quadrature_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     unsigned int mode)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +
> +	regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, mode + 1);
> +
> +	return 0;
> +}
> +
> +static int stm32_get_quadrature_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	u32 smcr;
> +
> +	regmap_read(priv->regmap, TIM_SMCR, &smcr);
> +	smcr &= TIM_SMCR_SMS;
> +
> +	return smcr - 1;
> +}
> +
> +static const struct iio_enum stm32_quadrature_mode_enum = {
> +	.items = stm32_quadrature_modes,
> +	.num_items = ARRAY_SIZE(stm32_quadrature_modes),
> +	.set = stm32_set_quadrature_mode,
> +	.get = stm32_get_quadrature_mode
> +};
> +
> +static const char *const stm32_count_direction_states[] = {
> +	"up",
> +	"down"
> +};
> +
> +static int stm32_set_count_direction(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     unsigned int mode)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +
> +	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_DIR, mode);
> +
> +	return 0;
> +}
> +
> +static int stm32_get_count_direction(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	u32 cr1;
> +
> +	regmap_read(priv->regmap, TIM_CR1, &cr1);
> +
> +	return (cr1 & TIM_CR1_DIR);
> +}
> +
> +static const struct iio_enum stm32_count_direction_enum = {
> +	.items = stm32_count_direction_states,
> +	.num_items = ARRAY_SIZE(stm32_count_direction_states),
> +	.set = stm32_set_count_direction,
> +	.get = stm32_get_count_direction
> +};
> +
> +static ssize_t stm32_count_get_preset(struct iio_dev *indio_dev,
> +				      uintptr_t private,
> +				      const struct iio_chan_spec *chan,
> +				      char *buf)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	u32 arr;
> +
> +	regmap_read(priv->regmap, TIM_ARR, &arr);
> +
> +	return snprintf(buf, PAGE_SIZE, "%u\n", arr);
> +}
> +
> +static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
> +				      uintptr_t private,
> +				      const struct iio_chan_spec *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	unsigned int preset;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 0, &preset);
> +	if (ret)
> +		return ret;
> +
> +	regmap_write(priv->regmap, TIM_ARR, preset);
> +	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
> +
> +	return len;
> +}
> +
> +static const struct iio_chan_spec_ext_info stm32_trigger_count_info[] = {
> +	{
> +		.name = "preset",
> +		.shared = IIO_SEPARATE,
> +		.read = stm32_count_get_preset,
> +		.write = stm32_count_set_preset
> +	},
> +	IIO_ENUM("count_direction", IIO_SEPARATE, &stm32_count_direction_enum),
> +	IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
> +	IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
> +	IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
> +	{}
> +};
> +
> +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),
> +	.ext_info = stm32_trigger_count_info,
> +	.indexed = 1
> +};
> +
> +static struct stm32_timer_trigger *stm32_setup_counter_device(struct device *dev)
> +{
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev,
> +					  sizeof(struct stm32_timer_trigger));
> +	if (!indio_dev)
> +		return NULL;
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->dev.parent = dev;
> +	indio_dev->info = &stm32_trigger_info;
> +	indio_dev->num_channels = 1;
> +	indio_dev->channels = &stm32_trigger_channel;
> +	indio_dev->dev.of_node = dev->of_node;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return NULL;
> +
> +	return iio_priv(indio_dev);
> +}
> +
>  /**
>   * is_stm32_timer_trigger
>   * @trig: trigger to be checked
> @@ -299,10 +525,15 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>  	if (of_property_read_u32(dev->of_node, "reg", &index))
>  		return -EINVAL;
>  
> -	if (index >= ARRAY_SIZE(triggers_table))
> +	if (index >= ARRAY_SIZE(triggers_table) ||
> +	    index >= ARRAY_SIZE(valids_table))
>  		return -EINVAL;
>  
> -	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	/* Create an IIO device only if we have triggers to be validated */
> +	if (*valids_table[index])
> +		priv = stm32_setup_counter_device(dev);
> +	else
> +		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  
>  	if (!priv)
>  		return -ENOMEM;
> @@ -312,6 +543,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>  	priv->clk = ddata->clk;
>  	priv->max_arr = ddata->max_arr;
>  	priv->triggers = triggers_table[index];
> +	priv->valids = valids_table[index];
>  
>  	ret = stm32_setup_iio_triggers(priv);
>  	if (ret)
> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> index d030004..4a0abbc 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -21,6 +21,7 @@
>  #define TIM_CCMR1	0x18	/* Capt/Comp 1 Mode Reg    */
>  #define TIM_CCMR2	0x1C	/* Capt/Comp 2 Mode Reg    */
>  #define TIM_CCER	0x20	/* Capt/Comp Enable Reg    */
> +#define TIM_CNT		0x24	/* Counter		   */
>  #define TIM_PSC		0x28	/* Prescaler               */
>  #define TIM_ARR		0x2c	/* Auto-Reload Register    */
>  #define TIM_CCR1	0x34	/* Capt/Comp Register 1    */
> @@ -30,6 +31,7 @@
>  #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
>  
>  #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
> +#define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
>  #define TIM_CR1_ARPE	BIT(7)	/* Auto-reload Preload Ena */
>  #define TIM_CR2_MMS	(BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
>  #define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
> 

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-03-27  9:43 ` [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes Benjamin Gaignard
  2017-03-27 13:55   ` William Breathitt Gray
@ 2017-04-02 11:12   ` Jonathan Cameron
  2017-04-03 10:17       ` Benjamin Gaignard
  1 sibling, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2017-04-02 11:12 UTC (permalink / raw)
  To: Benjamin Gaignard, linux-kernel, linux-iio, knaack.h, lars,
	pmeerw, vilhelm.gray, mwelling
  Cc: fabrice.gasnier, linaro-kernel, Benjamin Gaignard

On 27/03/17 10:43, Benjamin Gaignard wrote:
> Device counting could be controlled by the level or the edges of
> a trigger.
> in_count0_enable_mode attibute allow to set the control mode.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>  drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> index bf795ad..c0a1edc 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> @@ -59,3 +59,26 @@ Description:
>  		quadrature:
>  			Encoder A and B inputs are mixed to get direction
>  			and count with a scale of 0.25.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
> +KernelVersion:	4.12
> +Contact:	benjamin.gaignard@st.com
> +Description:
> +		Reading returns the list possible enable modes.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
> +KernelVersion:	4.12
> +Contact:	benjamin.gaignard@st.com
> +Description:
> +		Configure the device counter enable modes, in all case
> +		counting direction is set by in_count0_count_direction
> +		attribute and the counter is clocked by the internal clock.
Silly question, how does one use the encoder modes then?

> +		always:
> +			Counter is always ON.
> +
> +		gated:
> +			Counting is enabled when connected trigger signal
> +			level is high else counting is disabled.
> +
> +		triggered:
> +			Counting start on rising edge of the connected trigger.


> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index 7db904c..0f1a2cf 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>  	.write_raw = stm32_counter_write_raw
>  };
>  
> +static const char *const stm32_enable_modes[] = {
> +	"always",
> +	"gated",
> +	"triggered",
> +};
> +
> +static int stm32_enable_mode2sms(int mode)
> +{
> +	switch (mode) {
> +	case 0:
> +		return 0;
> +	case 1:
> +		return 5;
> +	case 2:
> +		return 6;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int stm32_set_enable_mode(struct iio_dev *indio_dev,
> +				 const struct iio_chan_spec *chan,
> +				 unsigned int mode)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	int sms = stm32_enable_mode2sms(mode);
> +
> +	if (sms < 0)
> +		return sms;
> +
> +	regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
> +
> +	return 0;
> +}
> +
> +static int stm32_sms2enable_mode(int mode)
> +{
> +	switch (mode) {
> +	case 0:
> +		return 0;
> +	case 5:
> +		return 1;
> +	case 6:
> +		return 2;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int stm32_get_enable_mode(struct iio_dev *indio_dev,
> +				 const struct iio_chan_spec *chan)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	u32 smcr;
> +
> +	regmap_read(priv->regmap, TIM_SMCR, &smcr);
> +	smcr &= TIM_SMCR_SMS;
> +
> +	return stm32_sms2enable_mode(smcr);
> +}
> +
> +static const struct iio_enum stm32_enable_mode_enum = {
> +	.items = stm32_enable_modes,
> +	.num_items = ARRAY_SIZE(stm32_enable_modes),
> +	.set = stm32_set_enable_mode,
> +	.get = stm32_get_enable_mode
> +};
> +
>  static const char *const stm32_quadrature_modes[] = {
>  	"channel_A",
>  	"channel_B",
> @@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>  	IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>  	IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>  	IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
> +	IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
> +	IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>  	{}
>  };
>  
> 

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-04-02 11:12   ` Jonathan Cameron
@ 2017-04-03 10:17       ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-04-03 10:17 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linux Kernel Mailing List, linux-iio, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	William Breathitt Gray, mwelling, Fabrice Gasnier,
	Linaro Kernel Mailman List, Benjamin Gaignard

2017-04-02 13:12 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
> On 27/03/17 10:43, Benjamin Gaignard wrote:
>> Device counting could be controlled by the level or the edges of
>> a trigger.
>> in_count0_enable_mode attibute allow to set the control mode.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>>  drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
>>  2 files changed, 93 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>> index bf795ad..c0a1edc 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>> @@ -59,3 +59,26 @@ Description:
>>               quadrature:
>>                       Encoder A and B inputs are mixed to get direction
>>                       and count with a scale of 0.25.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
>> +KernelVersion:       4.12
>> +Contact:     benjamin.gaignard@st.com
>> +Description:
>> +             Reading returns the list possible enable modes.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>> +KernelVersion:       4.12
>> +Contact:     benjamin.gaignard@st.com
>> +Description:
>> +             Configure the device counter enable modes, in all case
>> +             counting direction is set by in_count0_count_direction
>> +             attribute and the counter is clocked by the internal clock.
> Silly question, how does one use the encoder modes then?

When setting an encoder mode internal counter is enabled no need to touch to
in_count0_enable_mode in those cases

>
>> +             always:
>> +                     Counter is always ON.
>> +
>> +             gated:
>> +                     Counting is enabled when connected trigger signal
>> +                     level is high else counting is disabled.
>> +
>> +             triggered:
>> +                     Counting start on rising edge of the connected trigger.
>
>
>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>> index 7db904c..0f1a2cf 100644
>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>> @@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>       .write_raw = stm32_counter_write_raw
>>  };
>>
>> +static const char *const stm32_enable_modes[] = {
>> +     "always",
>> +     "gated",
>> +     "triggered",
>> +};
>> +
>> +static int stm32_enable_mode2sms(int mode)
>> +{
>> +     switch (mode) {
>> +     case 0:
>> +             return 0;
>> +     case 1:
>> +             return 5;
>> +     case 2:
>> +             return 6;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>> +                              const struct iio_chan_spec *chan,
>> +                              unsigned int mode)
>> +{
>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>> +     int sms = stm32_enable_mode2sms(mode);
>> +
>> +     if (sms < 0)
>> +             return sms;
>> +
>> +     regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_sms2enable_mode(int mode)
>> +{
>> +     switch (mode) {
>> +     case 0:
>> +             return 0;
>> +     case 5:
>> +             return 1;
>> +     case 6:
>> +             return 2;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>> +                              const struct iio_chan_spec *chan)
>> +{
>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>> +     u32 smcr;
>> +
>> +     regmap_read(priv->regmap, TIM_SMCR, &smcr);
>> +     smcr &= TIM_SMCR_SMS;
>> +
>> +     return stm32_sms2enable_mode(smcr);
>> +}
>> +
>> +static const struct iio_enum stm32_enable_mode_enum = {
>> +     .items = stm32_enable_modes,
>> +     .num_items = ARRAY_SIZE(stm32_enable_modes),
>> +     .set = stm32_set_enable_mode,
>> +     .get = stm32_get_enable_mode
>> +};
>> +
>>  static const char *const stm32_quadrature_modes[] = {
>>       "channel_A",
>>       "channel_B",
>> @@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>> +     IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>> +     IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>       {}
>>  };
>>
>>
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
@ 2017-04-03 10:17       ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-04-03 10:17 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linux Kernel Mailing List, linux-iio, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	William Breathitt Gray, mwelling, Fabrice Gasnier,
	Linaro Kernel Mailman List, Benjamin Gaignard

2017-04-02 13:12 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
> On 27/03/17 10:43, Benjamin Gaignard wrote:
>> Device counting could be controlled by the level or the edges of
>> a trigger.
>> in_count0_enable_mode attibute allow to set the control mode.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>>  drivers/iio/trigger/stm32-timer-trigger.c          | 70 +++++++++++++++=
+++++++
>>  2 files changed, 93 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Docum=
entation/ABI/testing/sysfs-bus-iio-timer-stm32
>> index bf795ad..c0a1edc 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>> @@ -59,3 +59,26 @@ Description:
>>               quadrature:
>>                       Encoder A and B inputs are mixed to get direction
>>                       and count with a scale of 0.25.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count_enable_m=
ode_available
>> +KernelVersion:       4.12
>> +Contact:     benjamin.gaignard@st.com
>> +Description:
>> +             Reading returns the list possible enable modes.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count0_enable_=
mode
>> +KernelVersion:       4.12
>> +Contact:     benjamin.gaignard@st.com
>> +Description:
>> +             Configure the device counter enable modes, in all case
>> +             counting direction is set by in_count0_count_direction
>> +             attribute and the counter is clocked by the internal clock=
.
> Silly question, how does one use the encoder modes then?

When setting an encoder mode internal counter is enabled no need to touch t=
o
in_count0_enable_mode in those cases

>
>> +             always:
>> +                     Counter is always ON.
>> +
>> +             gated:
>> +                     Counting is enabled when connected trigger signal
>> +                     level is high else counting is disabled.
>> +
>> +             triggered:
>> +                     Counting start on rising edge of the connected tri=
gger.
>
>
>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/tri=
gger/stm32-timer-trigger.c
>> index 7db904c..0f1a2cf 100644
>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>> @@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *=
indio_dev,
>>       .write_raw =3D stm32_counter_write_raw
>>  };
>>
>> +static const char *const stm32_enable_modes[] =3D {
>> +     "always",
>> +     "gated",
>> +     "triggered",
>> +};
>> +
>> +static int stm32_enable_mode2sms(int mode)
>> +{
>> +     switch (mode) {
>> +     case 0:
>> +             return 0;
>> +     case 1:
>> +             return 5;
>> +     case 2:
>> +             return 6;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>> +                              const struct iio_chan_spec *chan,
>> +                              unsigned int mode)
>> +{
>> +     struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>> +     int sms =3D stm32_enable_mode2sms(mode);
>> +
>> +     if (sms < 0)
>> +             return sms;
>> +
>> +     regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_sms2enable_mode(int mode)
>> +{
>> +     switch (mode) {
>> +     case 0:
>> +             return 0;
>> +     case 5:
>> +             return 1;
>> +     case 6:
>> +             return 2;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>> +                              const struct iio_chan_spec *chan)
>> +{
>> +     struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>> +     u32 smcr;
>> +
>> +     regmap_read(priv->regmap, TIM_SMCR, &smcr);
>> +     smcr &=3D TIM_SMCR_SMS;
>> +
>> +     return stm32_sms2enable_mode(smcr);
>> +}
>> +
>> +static const struct iio_enum stm32_enable_mode_enum =3D {
>> +     .items =3D stm32_enable_modes,
>> +     .num_items =3D ARRAY_SIZE(stm32_enable_modes),
>> +     .set =3D stm32_set_enable_mode,
>> +     .get =3D stm32_get_enable_mode
>> +};
>> +
>>  static const char *const stm32_quadrature_modes[] =3D {
>>       "channel_A",
>>       "channel_B",
>> @@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev=
 *indio_dev,
>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum)=
,
>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_e=
num),
>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum)=
,
>> +     IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>> +     IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>       {}
>>  };
>>
>>
>



--=20
Benjamin Gaignard

Graphic Study Group

Linaro.org =E2=94=82 Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-04-03 10:17       ` Benjamin Gaignard
  (?)
@ 2017-04-03 19:58       ` Jonathan Cameron
  2017-04-04  7:37           ` Benjamin Gaignard
  -1 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2017-04-03 19:58 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Linux Kernel Mailing List, linux-iio, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	William Breathitt Gray, mwelling, Fabrice Gasnier,
	Linaro Kernel Mailman List, Benjamin Gaignard

On 03/04/17 11:17, Benjamin Gaignard wrote:
> 2017-04-02 13:12 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
>> On 27/03/17 10:43, Benjamin Gaignard wrote:
>>> Device counting could be controlled by the level or the edges of
>>> a trigger.
>>> in_count0_enable_mode attibute allow to set the control mode.
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> ---
>>>  .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>>>  drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
>>>  2 files changed, 93 insertions(+)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>> index bf795ad..c0a1edc 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>> @@ -59,3 +59,26 @@ Description:
>>>               quadrature:
>>>                       Encoder A and B inputs are mixed to get direction
>>>                       and count with a scale of 0.25.
>>> +
>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
>>> +KernelVersion:       4.12
>>> +Contact:     benjamin.gaignard@st.com
>>> +Description:
>>> +             Reading returns the list possible enable modes.
>>> +
>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>>> +KernelVersion:       4.12
>>> +Contact:     benjamin.gaignard@st.com
>>> +Description:
>>> +             Configure the device counter enable modes, in all case
>>> +             counting direction is set by in_count0_count_direction
>>> +             attribute and the counter is clocked by the internal clock.
>> Silly question, how does one use the encoder modes then?
> 
> When setting an encoder mode internal counter is enabled no need to touch to
> in_count0_enable_mode in those cases
So in that case it's ignored? 
I'd be tempted to change the docs to drop the fact it is an internal clock.
However you'll also need to ensure that when an encoder mode is selected
this attribute will always read 'always'.   I think that ends up consistent..

Also there will be other devices that do these gating and triggering etc but
will do it even when fed by an encoder.

Jonathan
> 
>>
>>> +             always:
>>> +                     Counter is always ON.
>>> +
>>> +             gated:
>>> +                     Counting is enabled when connected trigger signal
>>> +                     level is high else counting is disabled.
>>> +
>>> +             triggered:
>>> +                     Counting start on rising edge of the connected trigger.
>>
>>
>>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>> index 7db904c..0f1a2cf 100644
>>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>> @@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>>       .write_raw = stm32_counter_write_raw
>>>  };
>>>
>>> +static const char *const stm32_enable_modes[] = {
>>> +     "always",
>>> +     "gated",
>>> +     "triggered",
>>> +};
>>> +
>>> +static int stm32_enable_mode2sms(int mode)
>>> +{
>>> +     switch (mode) {
>>> +     case 0:
>>> +             return 0;
>>> +     case 1:
>>> +             return 5;
>>> +     case 2:
>>> +             return 6;
>>> +     }
>>> +
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>>> +                              const struct iio_chan_spec *chan,
>>> +                              unsigned int mode)
>>> +{
>>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>> +     int sms = stm32_enable_mode2sms(mode);
>>> +
>>> +     if (sms < 0)
>>> +             return sms;
>>> +
>>> +     regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int stm32_sms2enable_mode(int mode)
>>> +{
>>> +     switch (mode) {
>>> +     case 0:
>>> +             return 0;
>>> +     case 5:
>>> +             return 1;
>>> +     case 6:
>>> +             return 2;
>>> +     }
>>> +
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>>> +                              const struct iio_chan_spec *chan)
>>> +{
>>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>> +     u32 smcr;
>>> +
>>> +     regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>> +     smcr &= TIM_SMCR_SMS;
>>> +
>>> +     return stm32_sms2enable_mode(smcr);
>>> +}
>>> +
>>> +static const struct iio_enum stm32_enable_mode_enum = {
>>> +     .items = stm32_enable_modes,
>>> +     .num_items = ARRAY_SIZE(stm32_enable_modes),
>>> +     .set = stm32_set_enable_mode,
>>> +     .get = stm32_get_enable_mode
>>> +};
>>> +
>>>  static const char *const stm32_quadrature_modes[] = {
>>>       "channel_A",
>>>       "channel_B",
>>> @@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>> +     IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>>> +     IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>>       {}
>>>  };
>>>
>>>
>>
> 
> 
> 

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-04-03 19:58       ` Jonathan Cameron
@ 2017-04-04  7:37           ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-04-04  7:37 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linux Kernel Mailing List, linux-iio, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	William Breathitt Gray, mwelling, Fabrice Gasnier,
	Linaro Kernel Mailman List, Benjamin Gaignard

2017-04-03 21:58 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
> On 03/04/17 11:17, Benjamin Gaignard wrote:
>> 2017-04-02 13:12 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
>>> On 27/03/17 10:43, Benjamin Gaignard wrote:
>>>> Device counting could be controlled by the level or the edges of
>>>> a trigger.
>>>> in_count0_enable_mode attibute allow to set the control mode.
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>> ---
>>>>  .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>>>>  drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
>>>>  2 files changed, 93 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> index bf795ad..c0a1edc 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> @@ -59,3 +59,26 @@ Description:
>>>>               quadrature:
>>>>                       Encoder A and B inputs are mixed to get direction
>>>>                       and count with a scale of 0.25.
>>>> +
>>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
>>>> +KernelVersion:       4.12
>>>> +Contact:     benjamin.gaignard@st.com
>>>> +Description:
>>>> +             Reading returns the list possible enable modes.
>>>> +
>>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>>>> +KernelVersion:       4.12
>>>> +Contact:     benjamin.gaignard@st.com
>>>> +Description:
>>>> +             Configure the device counter enable modes, in all case
>>>> +             counting direction is set by in_count0_count_direction
>>>> +             attribute and the counter is clocked by the internal clock.
>>> Silly question, how does one use the encoder modes then?
>>
>> When setting an encoder mode internal counter is enabled no need to touch to
>> in_count0_enable_mode in those cases
> So in that case it's ignored?
> I'd be tempted to change the docs to drop the fact it is an internal clock.
> However you'll also need to ensure that when an encoder mode is selected
> this attribute will always read 'always'.   I think that ends up consistent..

The counter could be drive (i.e counting up/down) by 3 type of source:
- the internal clock with 3 modes "always", "gated" and "triggered"
- the encoder input egdes and levels: 3 modes "channel_A", "channel_B"
and "quadrature"
- the rising egdes of a  connected trigger which is an internal signal
(for a later patch)

For each counter "source" I have an attribute but in the hardware they
are exclusive.
For example set "quadrature" means that the counter is counting on the edges of
encoder inputs and no more on the rising edges of the internal clock.

I would like to keep this representing the hardware status so if an
encoder mode is
selected in_count0_enable_mode will return -EINVAL.

>
> Also there will be other devices that do these gating and triggering etc but
> will do it even when fed by an encoder.
>
> Jonathan
>>
>>>
>>>> +             always:
>>>> +                     Counter is always ON.
>>>> +
>>>> +             gated:
>>>> +                     Counting is enabled when connected trigger signal
>>>> +                     level is high else counting is disabled.
>>>> +
>>>> +             triggered:
>>>> +                     Counting start on rising edge of the connected trigger.
>>>
>>>
>>>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>>> index 7db904c..0f1a2cf 100644
>>>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>>>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>>> @@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>>>       .write_raw = stm32_counter_write_raw
>>>>  };
>>>>
>>>> +static const char *const stm32_enable_modes[] = {
>>>> +     "always",
>>>> +     "gated",
>>>> +     "triggered",
>>>> +};
>>>> +
>>>> +static int stm32_enable_mode2sms(int mode)
>>>> +{
>>>> +     switch (mode) {
>>>> +     case 0:
>>>> +             return 0;
>>>> +     case 1:
>>>> +             return 5;
>>>> +     case 2:
>>>> +             return 6;
>>>> +     }
>>>> +
>>>> +     return -EINVAL;
>>>> +}
>>>> +
>>>> +static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>>>> +                              const struct iio_chan_spec *chan,
>>>> +                              unsigned int mode)
>>>> +{
>>>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +     int sms = stm32_enable_mode2sms(mode);
>>>> +
>>>> +     if (sms < 0)
>>>> +             return sms;
>>>> +
>>>> +     regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +
>>>> +static int stm32_sms2enable_mode(int mode)
>>>> +{
>>>> +     switch (mode) {
>>>> +     case 0:
>>>> +             return 0;
>>>> +     case 5:
>>>> +             return 1;
>>>> +     case 6:
>>>> +             return 2;
>>>> +     }
>>>> +
>>>> +     return -EINVAL;
>>>> +}
>>>> +
>>>> +static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>>>> +                              const struct iio_chan_spec *chan)
>>>> +{
>>>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>> +     u32 smcr;
>>>> +
>>>> +     regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>> +     smcr &= TIM_SMCR_SMS;
>>>> +
>>>> +     return stm32_sms2enable_mode(smcr);
>>>> +}
>>>> +
>>>> +static const struct iio_enum stm32_enable_mode_enum = {
>>>> +     .items = stm32_enable_modes,
>>>> +     .num_items = ARRAY_SIZE(stm32_enable_modes),
>>>> +     .set = stm32_set_enable_mode,
>>>> +     .get = stm32_get_enable_mode
>>>> +};
>>>> +
>>>>  static const char *const stm32_quadrature_modes[] = {
>>>>       "channel_A",
>>>>       "channel_B",
>>>> @@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>>> +     IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>>>> +     IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>>>       {}
>>>>  };
>>>>
>>>>
>>>
>>
>>
>>
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
@ 2017-04-04  7:37           ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2017-04-04  7:37 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linux Kernel Mailing List, linux-iio, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	William Breathitt Gray, mwelling, Fabrice Gasnier,
	Linaro Kernel Mailman List, Benjamin Gaignard

2017-04-03 21:58 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
> On 03/04/17 11:17, Benjamin Gaignard wrote:
>> 2017-04-02 13:12 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
>>> On 27/03/17 10:43, Benjamin Gaignard wrote:
>>>> Device counting could be controlled by the level or the edges of
>>>> a trigger.
>>>> in_count0_enable_mode attibute allow to set the control mode.
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>> ---
>>>>  .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>>>>  drivers/iio/trigger/stm32-timer-trigger.c          | 70 +++++++++++++=
+++++++++
>>>>  2 files changed, 93 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Doc=
umentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> index bf795ad..c0a1edc 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> @@ -59,3 +59,26 @@ Description:
>>>>               quadrature:
>>>>                       Encoder A and B inputs are mixed to get directio=
n
>>>>                       and count with a scale of 0.25.
>>>> +
>>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count_enable=
_mode_available
>>>> +KernelVersion:       4.12
>>>> +Contact:     benjamin.gaignard@st.com
>>>> +Description:
>>>> +             Reading returns the list possible enable modes.
>>>> +
>>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count0_enabl=
e_mode
>>>> +KernelVersion:       4.12
>>>> +Contact:     benjamin.gaignard@st.com
>>>> +Description:
>>>> +             Configure the device counter enable modes, in all case
>>>> +             counting direction is set by in_count0_count_direction
>>>> +             attribute and the counter is clocked by the internal clo=
ck.
>>> Silly question, how does one use the encoder modes then?
>>
>> When setting an encoder mode internal counter is enabled no need to touc=
h to
>> in_count0_enable_mode in those cases
> So in that case it's ignored?
> I'd be tempted to change the docs to drop the fact it is an internal cloc=
k.
> However you'll also need to ensure that when an encoder mode is selected
> this attribute will always read 'always'.   I think that ends up consiste=
nt..

The counter could be drive (i.e counting up/down) by 3 type of source:
- the internal clock with 3 modes "always", "gated" and "triggered"
- the encoder input egdes and levels: 3 modes "channel_A", "channel_B"
and "quadrature"
- the rising egdes of a  connected trigger which is an internal signal
(for a later patch)

For each counter "source" I have an attribute but in the hardware they
are exclusive.
For example set "quadrature" means that the counter is counting on the edge=
s of
encoder inputs and no more on the rising edges of the internal clock.

I would like to keep this representing the hardware status so if an
encoder mode is
selected in_count0_enable_mode will return -EINVAL.

>
> Also there will be other devices that do these gating and triggering etc =
but
> will do it even when fed by an encoder.
>
> Jonathan
>>
>>>
>>>> +             always:
>>>> +                     Counter is always ON.
>>>> +
>>>> +             gated:
>>>> +                     Counting is enabled when connected trigger signa=
l
>>>> +                     level is high else counting is disabled.
>>>> +
>>>> +             triggered:
>>>> +                     Counting start on rising edge of the connected t=
rigger.
>>>
>>>
>>>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/t=
rigger/stm32-timer-trigger.c
>>>> index 7db904c..0f1a2cf 100644
>>>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>>>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>>> @@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev=
 *indio_dev,
>>>>       .write_raw =3D stm32_counter_write_raw
>>>>  };
>>>>
>>>> +static const char *const stm32_enable_modes[] =3D {
>>>> +     "always",
>>>> +     "gated",
>>>> +     "triggered",
>>>> +};
>>>> +
>>>> +static int stm32_enable_mode2sms(int mode)
>>>> +{
>>>> +     switch (mode) {
>>>> +     case 0:
>>>> +             return 0;
>>>> +     case 1:
>>>> +             return 5;
>>>> +     case 2:
>>>> +             return 6;
>>>> +     }
>>>> +
>>>> +     return -EINVAL;
>>>> +}
>>>> +
>>>> +static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>>>> +                              const struct iio_chan_spec *chan,
>>>> +                              unsigned int mode)
>>>> +{
>>>> +     struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>>> +     int sms =3D stm32_enable_mode2sms(mode);
>>>> +
>>>> +     if (sms < 0)
>>>> +             return sms;
>>>> +
>>>> +     regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +
>>>> +static int stm32_sms2enable_mode(int mode)
>>>> +{
>>>> +     switch (mode) {
>>>> +     case 0:
>>>> +             return 0;
>>>> +     case 5:
>>>> +             return 1;
>>>> +     case 6:
>>>> +             return 2;
>>>> +     }
>>>> +
>>>> +     return -EINVAL;
>>>> +}
>>>> +
>>>> +static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>>>> +                              const struct iio_chan_spec *chan)
>>>> +{
>>>> +     struct stm32_timer_trigger *priv =3D iio_priv(indio_dev);
>>>> +     u32 smcr;
>>>> +
>>>> +     regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>> +     smcr &=3D TIM_SMCR_SMS;
>>>> +
>>>> +     return stm32_sms2enable_mode(smcr);
>>>> +}
>>>> +
>>>> +static const struct iio_enum stm32_enable_mode_enum =3D {
>>>> +     .items =3D stm32_enable_modes,
>>>> +     .num_items =3D ARRAY_SIZE(stm32_enable_modes),
>>>> +     .set =3D stm32_set_enable_mode,
>>>> +     .get =3D stm32_get_enable_mode
>>>> +};
>>>> +
>>>>  static const char *const stm32_quadrature_modes[] =3D {
>>>>       "channel_A",
>>>>       "channel_B",
>>>> @@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_d=
ev *indio_dev,
>>>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enu=
m),
>>>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode=
_enum),
>>>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enu=
m),
>>>> +     IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>>>> +     IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>>>       {}
>>>>  };
>>>>
>>>>
>>>
>>
>>
>>
>



--=20
Benjamin Gaignard

Graphic Study Group

Linaro.org =E2=94=82 Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
  2017-04-04  7:37           ` Benjamin Gaignard
  (?)
@ 2017-04-08 17:23           ` Jonathan Cameron
  -1 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2017-04-08 17:23 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Linux Kernel Mailing List, linux-iio, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	William Breathitt Gray, mwelling, Fabrice Gasnier,
	Linaro Kernel Mailman List, Benjamin Gaignard

On 04/04/17 08:37, Benjamin Gaignard wrote:
> 2017-04-03 21:58 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
>> On 03/04/17 11:17, Benjamin Gaignard wrote:
>>> 2017-04-02 13:12 GMT+02:00 Jonathan Cameron <jic23@kernel.org>:
>>>> On 27/03/17 10:43, Benjamin Gaignard wrote:
>>>>> Device counting could be controlled by the level or the edges of
>>>>> a trigger.
>>>>> in_count0_enable_mode attibute allow to set the control mode.
>>>>>
>>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>> ---
>>>>>  .../ABI/testing/sysfs-bus-iio-timer-stm32          | 23 +++++++
>>>>>  drivers/iio/trigger/stm32-timer-trigger.c          | 70 ++++++++++++++++++++++
>>>>>  2 files changed, 93 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>> index bf795ad..c0a1edc 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>> @@ -59,3 +59,26 @@ Description:
>>>>>               quadrature:
>>>>>                       Encoder A and B inputs are mixed to get direction
>>>>>                       and count with a scale of 0.25.
>>>>> +
>>>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
>>>>> +KernelVersion:       4.12
>>>>> +Contact:     benjamin.gaignard@st.com
>>>>> +Description:
>>>>> +             Reading returns the list possible enable modes.
>>>>> +
>>>>> +What:                /sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>>>>> +KernelVersion:       4.12
>>>>> +Contact:     benjamin.gaignard@st.com
>>>>> +Description:
>>>>> +             Configure the device counter enable modes, in all case
>>>>> +             counting direction is set by in_count0_count_direction
>>>>> +             attribute and the counter is clocked by the internal clock.
>>>> Silly question, how does one use the encoder modes then?
>>>
>>> When setting an encoder mode internal counter is enabled no need to touch to
>>> in_count0_enable_mode in those cases
>> So in that case it's ignored?
>> I'd be tempted to change the docs to drop the fact it is an internal clock.
>> However you'll also need to ensure that when an encoder mode is selected
>> this attribute will always read 'always'.   I think that ends up consistent..
> 
> The counter could be drive (i.e counting up/down) by 3 type of source:
> - the internal clock with 3 modes "always", "gated" and "triggered"
> - the encoder input egdes and levels: 3 modes "channel_A", "channel_B"
> and "quadrature"
> - the rising egdes of a  connected trigger which is an internal signal
> (for a later patch)
> 
> For each counter "source" I have an attribute but in the hardware they
> are exclusive.
> For example set "quadrature" means that the counter is counting on the edges of
> encoder inputs and no more on the rising edges of the internal clock.
> 
> I would like to keep this representing the hardware status so if an
> encoder mode is
> selected in_count0_enable_mode will return -EINVAL.
Cool. As long as there is a valid path between modes that's an acceptable
option.
> 
>>
>> Also there will be other devices that do these gating and triggering etc but
>> will do it even when fed by an encoder.
>>
>> Jonathan
>>>
>>>>
>>>>> +             always:
>>>>> +                     Counter is always ON.
>>>>> +
>>>>> +             gated:
>>>>> +                     Counting is enabled when connected trigger signal
>>>>> +                     level is high else counting is disabled.
>>>>> +
>>>>> +             triggered:
>>>>> +                     Counting start on rising edge of the connected trigger.
>>>>
>>>>
>>>>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>>>> index 7db904c..0f1a2cf 100644
>>>>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>>>>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>>>> @@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>>>>       .write_raw = stm32_counter_write_raw
>>>>>  };
>>>>>
>>>>> +static const char *const stm32_enable_modes[] = {
>>>>> +     "always",
>>>>> +     "gated",
>>>>> +     "triggered",
>>>>> +};
>>>>> +
>>>>> +static int stm32_enable_mode2sms(int mode)
>>>>> +{
>>>>> +     switch (mode) {
>>>>> +     case 0:
>>>>> +             return 0;
>>>>> +     case 1:
>>>>> +             return 5;
>>>>> +     case 2:
>>>>> +             return 6;
>>>>> +     }
>>>>> +
>>>>> +     return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>>>>> +                              const struct iio_chan_spec *chan,
>>>>> +                              unsigned int mode)
>>>>> +{
>>>>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>> +     int sms = stm32_enable_mode2sms(mode);
>>>>> +
>>>>> +     if (sms < 0)
>>>>> +             return sms;
>>>>> +
>>>>> +     regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>>>> +static int stm32_sms2enable_mode(int mode)
>>>>> +{
>>>>> +     switch (mode) {
>>>>> +     case 0:
>>>>> +             return 0;
>>>>> +     case 5:
>>>>> +             return 1;
>>>>> +     case 6:
>>>>> +             return 2;
>>>>> +     }
>>>>> +
>>>>> +     return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>>>>> +                              const struct iio_chan_spec *chan)
>>>>> +{
>>>>> +     struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>> +     u32 smcr;
>>>>> +
>>>>> +     regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>>> +     smcr &= TIM_SMCR_SMS;
>>>>> +
>>>>> +     return stm32_sms2enable_mode(smcr);
>>>>> +}
>>>>> +
>>>>> +static const struct iio_enum stm32_enable_mode_enum = {
>>>>> +     .items = stm32_enable_modes,
>>>>> +     .num_items = ARRAY_SIZE(stm32_enable_modes),
>>>>> +     .set = stm32_set_enable_mode,
>>>>> +     .get = stm32_get_enable_mode
>>>>> +};
>>>>> +
>>>>>  static const char *const stm32_quadrature_modes[] = {
>>>>>       "channel_A",
>>>>>       "channel_B",
>>>>> @@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>>>>>       IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>>>>>       IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>>>>>       IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>>>> +     IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>>>>> +     IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>>>>>       {}
>>>>>  };
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
> 
> 
> 

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

end of thread, other threads:[~2017-04-08 17:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27  9:43 [PATCH v3 0/2] iio: stm32 trigger: Add quadrature device and counter Benjamin Gaignard
2017-03-27  9:43 ` [PATCH v3 1/2] iio: stm32 trigger: Add quadrature encoder device Benjamin Gaignard
2017-03-27 13:47   ` William Breathitt Gray
2017-03-27 14:14     ` Benjamin Gaignard
2017-03-27 14:14       ` Benjamin Gaignard
2017-03-27 14:23       ` William Breathitt Gray
2017-04-02 11:02         ` Jonathan Cameron
2017-04-02 11:08   ` Jonathan Cameron
2017-03-27  9:43 ` [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes Benjamin Gaignard
2017-03-27 13:55   ` William Breathitt Gray
2017-03-27 14:43     ` Benjamin Gaignard
2017-03-27 14:43       ` Benjamin Gaignard
2017-04-02 11:12   ` Jonathan Cameron
2017-04-03 10:17     ` Benjamin Gaignard
2017-04-03 10:17       ` Benjamin Gaignard
2017-04-03 19:58       ` Jonathan Cameron
2017-04-04  7:37         ` Benjamin Gaignard
2017-04-04  7:37           ` Benjamin Gaignard
2017-04-08 17:23           ` Jonathan Cameron

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.