[RFC 0/4] iio: trigger: Add OF support and GPIO based trigger

[RFC 0/4] iio: trigger: Add OF support and GPIO based trigger

[Fabrice Gasnier]

Hi,

I send this as an RFC, as I'm digging to try to get as generic
as possible GPIO based trigger support, as discussed earlier in
other patchset "Add EXTI GPIO trigger support to STM32 ADC":
https://www.spinics.net/lists/arm-kernel/msg559091.html

Following various comments on this, I sent this RFC to address two
topics, that I see as possible prerequisite:
- Add trigger OF support
- GPIO based trigger

Please feel free to advise.
Best Regards.

Fabrice Gasnier (4):
  dt-bindings: iio: introduce trigger providers, consumers
  iio: trigger: add OF support
  dt-bindings: iio: add support for GPIO triggers
  iio: trigger: add GPIO trigger

 .../devicetree/bindings/iio/iio-bindings.txt       |  37 +++++++
 .../bindings/iio/trigger/iio-trig-gpio.txt         |  26 +++++
 drivers/iio/industrialio-trigger.c                 | 100 ++++++++++++++++++
 drivers/iio/trigger/Kconfig                        |   9 ++
 drivers/iio/trigger/Makefile                       |   1 +
 drivers/iio/trigger/iio-trig-gpio.c                | 112 +++++++++++++++++++++
 include/linux/iio/trigger.h                        |   4 +
 7 files changed, 289 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt
 create mode 100644 drivers/iio/trigger/iio-trig-gpio.c

-- 
1.9.1

[RFC 1/4] dt-bindings: iio: introduce trigger providers, consumers

[Fabrice Gasnier]

Document iio provider and consumer bindings.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 .../devicetree/bindings/iio/iio-bindings.txt       | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
index 68d6f8c..3125390 100644
--- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -95,3 +95,40 @@ vdd channel is connected to output 0 of the &ref device.
 		io-channels = <&adc 10>, <&adc 11>;
 		io-channel-names = "adc1", "adc2";
 	};
+
+==IIO trigger providers==
+Sources of IIO triggers can be represented by any node in the device
+tree. Those nodes are designated as IIO trigger providers. IIO trigger
+consumer uses a phandle and an IIO trigger specifier to connect to an
+IIO trigger provider.
+An IIO trigger specifier is an array of one or more cells identifying
+the IIO trigger output on a device. The length of an IIO trigger
+specifier is defined by the value of a #io-trigger-cells property in
+the IIO trigger provider node.
+
+Required properties:
+#io-trigger-cells:
+		Number of cells in an IIO trigger specifier; Typically
+		0 for nodes with a simple IIO trigger output.
+
+Example:
+	gpiotrig0: iio-gpio-trigger0 {
+		#io-trigger-cells = <0>;
+		compatible = "iio-gpio-trigger";
+		gpios = <&gpioa 11 0>;
+	}
+
+==IIO trigger consumers==
+Required properties:
+- io-triggers:	List of phandle representing the IIO trigger specifier.
+
+Optional properties:
+- io-trigger-names :
+		List of IIO trigger name strings that matches elements
+		in 'io-triggers' list property.
+
+Example:
+	some_trigger_consumer {
+		io-triggers = <&gpiotrig0>;
+		io-trigger-names = "mytrig";
+	}
-- 
1.9.1

[RFC 2/4] iio: trigger: add OF support

[Fabrice Gasnier]

Provide OF support. Device drivers can get IIO triggers from dt.
Introduce IIO trigger specifiers, so there are:
- IIO trigger providers, e.g. dt nodes designated with
  #io-trigger-cells=<num of cells>
- IIO trigger consumers, e.g. phandles listed in io-triggers = <...>.
  Those can be identified by names by using 'io-trigger-names'.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/industrialio-trigger.c | 100 +++++++++++++++++++++++++++++++++++++
 include/linux/iio/trigger.h        |   4 ++
 2 files changed, 104 insertions(+)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 978e1592..d3ac33c 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -768,3 +768,103 @@ int iio_triggered_buffer_predisable(struct iio_dev *indio_dev)
 					     indio_dev->pollfunc);
 }
 EXPORT_SYMBOL(iio_triggered_buffer_predisable);
+
+#ifdef CONFIG_OF
+static int iio_trig_node_match(struct device *dev, void *data)
+{
+	return dev->of_node == data && dev->type == &iio_trig_type;
+}
+
+static struct iio_trigger *__of_iio_trig_get(struct device_node *np, int index)
+{
+	struct of_phandle_args trigspec;
+	struct device *dev;
+	int err;
+
+	err = of_parse_phandle_with_args(np, "io-triggers",
+					 "#io-trigger-cells",
+					 index, &trigspec);
+	if (err)
+		return ERR_PTR(err);
+
+	dev = bus_find_device(&iio_bus_type, NULL, trigspec.np,
+			      iio_trig_node_match);
+	of_node_put(trigspec.np);
+	if (dev == NULL)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return to_iio_trigger(dev);
+}
+
+static struct iio_trigger *__of_iio_trig_get_by_name(struct device_node *np,
+						     const char *name)
+{
+	struct iio_trigger *trig;
+	int index = 0;
+
+	if (!np)
+		return ERR_PTR(-EINVAL);
+	if (name)
+		index = of_property_match_string(np, "io-trigger-names", name);
+	if (index < 0)
+		return ERR_PTR(index);
+	trig = __of_iio_trig_get(np, index);
+	if (!IS_ERR(trig) || PTR_ERR(trig) == -EPROBE_DEFER)
+		return trig;
+
+	if (name && index >= 0)
+		pr_err("ERROR: could not get IIO trigger %s:%s(%i)\n",
+			np->full_name, name ? name : "", index);
+
+	return ERR_PTR(-ENOENT);
+}
+#else /* CONFIG_OF */
+static inline struct iio_trigger *
+__of_iio_trig_get_by_name(struct device_node *np, const char *name)
+{
+	return ERR_PTR(-ENOENT);
+}
+#endif
+
+struct iio_trigger *iio_trigger_get_by_name(struct device *dev,
+					    const char *name)
+{
+	struct iio_trigger *trig;
+
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	trig = __of_iio_trig_get_by_name(dev->of_node, name);
+	if (!IS_ERR(trig))
+		return iio_trigger_get(trig);
+
+	return trig;
+}
+EXPORT_SYMBOL_GPL(iio_trigger_get_by_name);
+
+static void devm_iio_trigger_put(struct device *dev, void *res)
+{
+	iio_trigger_put(*(struct iio_trigger **)res);
+}
+
+struct iio_trigger *devm_iio_trigger_get_by_name(struct device *dev,
+						 const char *name)
+{
+	struct iio_trigger **ptr, *trig;
+
+	ptr = devres_alloc(devm_iio_trigger_put, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	trig = iio_trigger_get_by_name(dev, name);
+	if (IS_ERR(trig)) {
+		devres_free(ptr);
+		return trig;
+	}
+
+	*ptr = trig;
+	devres_add(dev, ptr);
+
+	return trig;
+}
+EXPORT_SYMBOL_GPL(devm_iio_trigger_get_by_name)
diff --git a/include/linux/iio/trigger.h b/include/linux/iio/trigger.h
index ea08302..1f1ec20 100644
--- a/include/linux/iio/trigger.h
+++ b/include/linux/iio/trigger.h
@@ -173,6 +173,10 @@ void devm_iio_trigger_unregister(struct device *dev,
 int iio_trigger_validate_own_device(struct iio_trigger *trig,
 				     struct iio_dev *indio_dev);
 
+struct iio_trigger *iio_trigger_get_by_name(struct device *dev,
+					    const char *name);
+struct iio_trigger *devm_iio_trigger_get_by_name(struct device *dev,
+						 const char *name);
 #else
 struct iio_trigger;
 struct iio_trigger_ops;
-- 
1.9.1

[RFC 3/4] dt-bindings: iio: add support for GPIO triggers

[Fabrice Gasnier]

Document Industrial I/O GPIO trigger support.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 .../bindings/iio/trigger/iio-trig-gpio.txt         | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt

diff --git a/Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt b/Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt
new file mode 100644
index 0000000..a8dbf5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt
@@ -0,0 +1,26 @@
+Industrial I/O - GPIO based trigger
+
+GPIOs may be used as IIO trigger provider, when trigger is connected to
+GPIO lines.
+
+Required properties:
+- compatible: Should be "iio-gpio-trigger"
+- gpios: Should specify one GPIO line used as trigger source.
+  See Documentation/devicetree/bindings/gpio/gpio.txt
+- #io-trigger-cells: Should be 0, as simple trigger provider.
+  See Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+Optional properties:
+- label: String to specifiy trigger name.
+- gpio-trigger-rising-edge: A rising edge on GPIO will trigger.
+- gpio-trigger-falling-edge: A falling edge on GPIO will trigger.
+  Note: Both rising and falling edge may be used. In case none of
+  rising or falling edge is selected, rising edge is selected by
+  default.
+
+Example:
+	gpiotrig0: iio-gpio-trigger0 {
+		#io-trigger-cells = <0>;
+		compatible = "iio-gpio-trigger";
+		gpios = <&gpioa 11 0>;
+	}
-- 
1.9.1

Re: [RFC 3/4] dt-bindings: iio: add support for GPIO triggers

[Jonathan Cameron]

On 17/02/17 16:03, Fabrice Gasnier wrote:
> Document Industrial I/O GPIO trigger support.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Whilst I think this needs redoing for the interrupt trigger driver,
please be aware that bindings must be OS agnostic i.e. you can't
name things after linux specific subsystem.  We get a lot
of stick for the iio-hwmon bindings specifically for doing this!

Which makes this 'interesting' to describe.

Once we have it updated feedback from Rob and Mark will definitely be needed on
this one.

Jonathan
> ---
>  .../bindings/iio/trigger/iio-trig-gpio.txt         | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt b/Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt
> new file mode 100644
> index 0000000..a8dbf5c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/trigger/iio-trig-gpio.txt
> @@ -0,0 +1,26 @@
> +Industrial I/O - GPIO based trigger
> +
> +GPIOs may be used as IIO trigger provider, when trigger is connected to
> +GPIO lines.
> +
> +Required properties:
> +- compatible: Should be "iio-gpio-trigger"
> +- gpios: Should specify one GPIO line used as trigger source.
> +  See Documentation/devicetree/bindings/gpio/gpio.txt
> +- #io-trigger-cells: Should be 0, as simple trigger provider.
> +  See Documentation/devicetree/bindings/iio/iio-bindings.txt
> +
> +Optional properties:
> +- label: String to specifiy trigger name.
> +- gpio-trigger-rising-edge: A rising edge on GPIO will trigger.
> +- gpio-trigger-falling-edge: A falling edge on GPIO will trigger.
> +  Note: Both rising and falling edge may be used. In case none of
> +  rising or falling edge is selected, rising edge is selected by
> +  default.
> +
> +Example:
> +	gpiotrig0: iio-gpio-trigger0 {
> +		#io-trigger-cells = <0>;
> +		compatible = "iio-gpio-trigger";
> +		gpios = <&gpioa 11 0>;
> +	}
> 

Re: [RFC 3/4] dt-bindings: iio: add support for GPIO triggers

[Linus Walleij]

On Fri, Feb 17, 2017 at 5:03 PM, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> Document Industrial I/O GPIO trigger support.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
(...)
> +- gpio-trigger-rising-edge: A rising edge on GPIO will trigger.
> +- gpio-trigger-falling-edge: A falling edge on GPIO will trigger.
> +  Note: Both rising and falling edge may be used. In case none of
> +  rising or falling edge is selected, rising edge is selected by
> +  default.

We have GPIO_ACTIVE*_HIGH and GPIO_ACTIVE_LOW in the
gpios second cell already.

Can't we simply use that and specify that for this usecase, reading
that cell as GPIO_ACTIVE_HIGH == trigger on rising edge and
GPIO_ACTIVE_LOW == trigger on falling edge?

Yours,
Linus Walleij

Re: [RFC 3/4] dt-bindings: iio: add support for GPIO triggers

[Fabrice Gasnier]

On 02/23/2017 04:03 PM, Linus Walleij wrote:
> On Fri, Feb 17, 2017 at 5:03 PM, Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:
>
>> Document Industrial I/O GPIO trigger support.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> (...)
>> +- gpio-trigger-rising-edge: A rising edge on GPIO will trigger.
>> +- gpio-trigger-falling-edge: A falling edge on GPIO will trigger.
>> +  Note: Both rising and falling edge may be used. In case none of
>> +  rising or falling edge is selected, rising edge is selected by
>> +  default.
>
> We have GPIO_ACTIVE*_HIGH and GPIO_ACTIVE_LOW in the
> gpios second cell already.
>
> Can't we simply use that and specify that for this usecase, reading
> that cell as GPIO_ACTIVE_HIGH == trigger on rising edge and
> GPIO_ACTIVE_LOW == trigger on falling edge?

Hi Linus,

Thanks for reviewing,

I think I'll simply drop patch 3 & 4, as suggested by Lars and Jonathan,
on patch 4, to use interrupt binding directly. Then standard interrupt
binding can be used instead.
Only thing is to add OF support for iio_interrupt_trigger.

BR,
Fabrice
>
> Yours,
> Linus Walleij
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 4/4] iio: trigger: add GPIO trigger

[Fabrice Gasnier]

GPIOs can be used to generate triggers for any IIO device.
Introduce generic GPIO trigger driver. It offers some options
to tune polarity (e.g. rising/falling edge), and label so trigger
name may be customized.
By default, gpio triggers will be named gpiotrigX, where X is gpio
number as returned by desc_to_gpio() call.
Rising edge is selected if polarity isn't set.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/trigger/Kconfig         |   9 +++
 drivers/iio/trigger/Makefile        |   1 +
 drivers/iio/trigger/iio-trig-gpio.c | 112 ++++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+)
 create mode 100644 drivers/iio/trigger/iio-trig-gpio.c

diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
index e4d4e63..54b6706 100644
--- a/drivers/iio/trigger/Kconfig
+++ b/drivers/iio/trigger/Kconfig
@@ -5,6 +5,15 @@
 
 menu "Triggers - standalone"
 
+config IIO_GPIO_TRIGGER
+	tristate "Generic GPIO trigger"
+	help
+	  Provides support for a GPIO based IIO trigger, when trigger
+	  source has been connected to a GPIO line.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called iio-gpio-trigger.
+
 config IIO_HRTIMER_TRIGGER
 	tristate "High resolution timer trigger"
 	depends on IIO_SW_TRIGGER
diff --git a/drivers/iio/trigger/Makefile b/drivers/iio/trigger/Makefile
index 5c4ecd3..a984668 100644
--- a/drivers/iio/trigger/Makefile
+++ b/drivers/iio/trigger/Makefile
@@ -4,6 +4,7 @@
 
 # When adding new entries keep the list in alphabetical order
 
+obj-$(CONFIG_IIO_GPIO_TRIGGER) += iio-trig-gpio.o
 obj-$(CONFIG_IIO_HRTIMER_TRIGGER) += iio-trig-hrtimer.o
 obj-$(CONFIG_IIO_INTERRUPT_TRIGGER) += iio-trig-interrupt.o
 obj-$(CONFIG_IIO_STM32_TIMER_TRIGGER) += stm32-timer-trigger.o
diff --git a/drivers/iio/trigger/iio-trig-gpio.c b/drivers/iio/trigger/iio-trig-gpio.c
new file mode 100644
index 0000000..4b48df0
--- /dev/null
+++ b/drivers/iio/trigger/iio-trig-gpio.c
@@ -0,0 +1,112 @@
+/*
+ * Industrial I/O - GPIO based trigger support
+ *
+ * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
+ *
+ * License type: GPLv2
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+static const struct iio_trigger_ops iio_gpio_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static int iio_gpio_trigger_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct gpio_desc *gpio;
+	char name[16];
+	const char *label = NULL;
+	struct iio_trigger *trig;
+	unsigned long irqflags = IRQF_SHARED;
+	int irq, ret;
+
+	gpio = devm_gpiod_get(&pdev->dev, NULL, GPIOD_IN);
+	if (IS_ERR(gpio)) {
+		if (PTR_ERR(gpio) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "gpio get failed (%ld)\n",
+				PTR_ERR(gpio));
+		return PTR_ERR(gpio);
+	}
+
+	if (of_property_read_string(np, "label", &label))
+		snprintf(name, sizeof(name), "gpiotrig%d", desc_to_gpio(gpio));
+
+	if (of_property_read_bool(np, "gpio-trigger-rising-edge"))
+		irqflags |= IRQF_TRIGGER_RISING;
+
+	if (of_property_read_bool(np, "gpio-trigger-falling-edge"))
+		irqflags |= IRQF_TRIGGER_FALLING;
+
+	/* Default to rising edge */
+	if (!(irqflags & IRQF_TRIGGER_MASK))
+		irqflags |= IRQF_TRIGGER_RISING;
+
+	trig = devm_iio_trigger_alloc(&pdev->dev, "%s", label ? label : name);
+	if (!trig)
+		return -ENOMEM;
+	trig->dev.parent = &pdev->dev;
+	trig->dev.of_node = pdev->dev.of_node;
+	trig->ops = &iio_gpio_trigger_ops;
+
+	irq = gpiod_to_irq(gpio);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "gpio %d to irq failed (%d)\n",
+			desc_to_gpio(gpio), irq);
+		return irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq,
+			       iio_trigger_generic_data_rdy_poll, irqflags,
+			       trig->name, trig);
+	if (ret) {
+		dev_err(&pdev->dev, "request IRQ %d failed\n", irq);
+		return ret;
+	}
+
+	ret = devm_iio_trigger_register(&pdev->dev, trig);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id iio_gpio_trigger_of_match[] = {
+	{ .compatible = "iio-gpio-trigger" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, iio_gpio_trigger_of_match);
+#endif
+
+static struct platform_driver iio_gpio_trigger_driver = {
+	.probe = iio_gpio_trigger_probe,
+	.driver = {
+		.name = "iio-gpio-trigger",
+		.of_match_table = of_match_ptr(iio_gpio_trigger_of_match),
+	},
+};
+module_platform_driver(iio_gpio_trigger_driver);
+
+MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
+MODULE_DESCRIPTION("GPIO trigger for iio subsystem");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [RFC 4/4] iio: trigger: add GPIO trigger

[Lars-Peter Clausen]

On 02/17/2017 05:03 PM, Fabrice Gasnier wrote:
> GPIOs can be used to generate triggers for any IIO device.
> Introduce generic GPIO trigger driver. It offers some options
> to tune polarity (e.g. rising/falling edge), and label so trigger
> name may be customized.
> By default, gpio triggers will be named gpiotrigX, where X is gpio
> number as returned by desc_to_gpio() call.
> Rising edge is selected if polarity isn't set.

If you want to use the GPIO only as an interrupt, just use it as an
interrupt directly, no need to go the route via the GPIO.

Re: [RFC 4/4] iio: trigger: add GPIO trigger

[Jonathan Cameron]

On 17/02/17 16:16, Lars-Peter Clausen wrote:
> On 02/17/2017 05:03 PM, Fabrice Gasnier wrote:
>> GPIOs can be used to generate triggers for any IIO device.
>> Introduce generic GPIO trigger driver. It offers some options
>> to tune polarity (e.g. rising/falling edge), and label so trigger
>> name may be customized.
>> By default, gpio triggers will be named gpiotrigX, where X is gpio
>> number as returned by desc_to_gpio() call.
>> Rising edge is selected if polarity isn't set.
> 
> If you want to use the GPIO only as an interrupt, just use it as an
> interrupt directly, no need to go the route via the GPIO.
Absolutely.  The majority of the bindings will then just become standard
interrupt bindings and will be effectively handled for you.

Please work to extend the existing interrupt trigger driver rather than adding
this new one.  That one actually started as a gpio trigger then got
generalised into a interrupt trigger later so lets not do that again ;)

Jonathan

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 4/4] iio: trigger: add GPIO trigger

[Fabrice Gasnier]

On 02/18/2017 08:19 PM, Jonathan Cameron wrote:
> On 17/02/17 16:16, Lars-Peter Clausen wrote:
>> On 02/17/2017 05:03 PM, Fabrice Gasnier wrote:
>>> GPIOs can be used to generate triggers for any IIO device.
>>> Introduce generic GPIO trigger driver. It offers some options
>>> to tune polarity (e.g. rising/falling edge), and label so trigger
>>> name may be customized.
>>> By default, gpio triggers will be named gpiotrigX, where X is gpio
>>> number as returned by desc_to_gpio() call.
>>> Rising edge is selected if polarity isn't set.
>>
>> If you want to use the GPIO only as an interrupt, just use it as an
>> interrupt directly, no need to go the route via the GPIO.
> Absolutely.  The majority of the bindings will then just become standard
> interrupt bindings and will be effectively handled for you.
>
> Please work to extend the existing interrupt trigger driver rather than adding
> this new one.  That one actually started as a gpio trigger then got
> generalised into a interrupt trigger later so lets not do that again ;)

Hi Lars, Jonathan

Ok, got it.
I'll drop this and focus on interrupt trigger, if this is worth having 
DT for it (e.g. patch 1 & 2 of this RFC).

BTW, I figured out there has been this patch a year ago, on interrupt 
trigger, to add device tree support:
http://www.spinics.net/lists/linux-iio/msg23126.html
Any reason why it hasn't found its way in ?

Thanks for reviewing,
Best Regards,
Fabrice

>
> Jonathan
>

Re: [RFC 4/4] iio: trigger: add GPIO trigger

[Jonathan Cameron]

On 20/02/17 16:24, Fabrice Gasnier wrote:
> On 02/18/2017 08:19 PM, Jonathan Cameron wrote:
>> On 17/02/17 16:16, Lars-Peter Clausen wrote:
>>> On 02/17/2017 05:03 PM, Fabrice Gasnier wrote:
>>>> GPIOs can be used to generate triggers for any IIO device.
>>>> Introduce generic GPIO trigger driver. It offers some options
>>>> to tune polarity (e.g. rising/falling edge), and label so trigger
>>>> name may be customized.
>>>> By default, gpio triggers will be named gpiotrigX, where X is gpio
>>>> number as returned by desc_to_gpio() call.
>>>> Rising edge is selected if polarity isn't set.
>>>
>>> If you want to use the GPIO only as an interrupt, just use it as an
>>> interrupt directly, no need to go the route via the GPIO.
>> Absolutely.  The majority of the bindings will then just become standard
>> interrupt bindings and will be effectively handled for you.
>>
>> Please work to extend the existing interrupt trigger driver rather than adding
>> this new one.  That one actually started as a gpio trigger then got
>> generalised into a interrupt trigger later so lets not do that again ;)
> 
> Hi Lars, Jonathan
> 
> Ok, got it.
> I'll drop this and focus on interrupt trigger, if this is worth having DT for it (e.g. patch 1 & 2 of this RFC).
> 
> BTW, I figured out there has been this patch a year ago, on interrupt trigger, to add device tree support:
> http://www.spinics.net/lists/linux-iio/msg23126.html
> Any reason why it hasn't found its way in ?
Gregor being very busy I suspect and not having time to address the reviews.

I 'think' the conclusion then was we needed some representative device that
could exist in devicetree to allow us to specify that a particular interrupt
could be used to trigger sensor capture.

Not a big issue, but probably still needs implementing unless Gregor has had
a chance to look at this.

Jonathan


> 
> Thanks for reviewing,
> Best Regards,
> Fabrice
> 
>>
>> Jonathan
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 0/4] iio: trigger: Add OF support and GPIO based trigger

[Daniel Baluta]

On Fri, Feb 17, 2017 at 6:03 PM, Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org> wrote:
> Hi,
>
> I send this as an RFC, as I'm digging to try to get as generic
> as possible GPIO based trigger support, as discussed earlier in
> other patchset "Add EXTI GPIO trigger support to STM32 ADC":
> https://www.spinics.net/lists/arm-kernel/msg559091.html
>
> Following various comments on this, I sent this RFC to address two
> topics, that I see as possible prerequisite:
> - Add trigger OF support
> - GPIO based trigger

I wonder if we could use configfs to create and configure
a GPIO based trigger.

You should just create another trigger type (look into
drivers/iio/trigger/iio-trig-hrtimer.c)
the userspace API will look like this:

# cd /config/iio/triggers
# insert your iio-trig-gpio
# setup the global parameters for the GPIO (the local params can be
set via dts/acpi)
# cd /config/iio/trigers/gpios/

#mkdir t1

And this will create an io-gpio-trigger named t1.

Not sure this fits your usecase but would be nice to consider.

thanks,
Daniel.

Re: [RFC 0/4] iio: trigger: Add OF support and GPIO based trigger

[Jonathan Cameron]

On 17/02/17 16:23, Daniel Baluta wrote:
> On Fri, Feb 17, 2017 at 6:03 PM, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>> Hi,
>>
>> I send this as an RFC, as I'm digging to try to get as generic
>> as possible GPIO based trigger support, as discussed earlier in
>> other patchset "Add EXTI GPIO trigger support to STM32 ADC":
>> https://www.spinics.net/lists/arm-kernel/msg559091.html
>>
>> Following various comments on this, I sent this RFC to address two
>> topics, that I see as possible prerequisite:
>> - Add trigger OF support
>> - GPIO based trigger
> 
> I wonder if we could use configfs to create and configure
> a GPIO based trigger.
> 
> You should just create another trigger type (look into
> drivers/iio/trigger/iio-trig-hrtimer.c)
> the userspace API will look like this:
> 
> # cd /config/iio/triggers
> # insert your iio-trig-gpio
> # setup the global parameters for the GPIO (the local params can be
> set via dts/acpi)
> # cd /config/iio/trigers/gpios/
> 
> #mkdir t1
> 
> And this will create an io-gpio-trigger named t1.
> 
> Not sure this fits your usecase but would be nice to consider.
> 
It might potentially make sense to support both DT or similar instantiation and
instantiation from userspace. Primary route seems to me to always be DT based
though as we are dealing with wiring...

So I'm unconvinced on the configfs option, but might be talked round.
> thanks,
> Daniel.
>