[PATCH v3 0/6] Add EXTI GPIO trigger support to STM32 ADC

[PATCH v3 0/6] Add EXTI GPIO trigger support to STM32 ADC

[Fabrice Gasnier]

STM32 ADC, can use EXTI line (external interrupt) as trigger source
for conversions.
This patchset introduces:
- IIO trigger providers / consumers that can be configured from DT
- DT support for IIO interrupt trigger
- STM32 ADC EXTI trigger selection, configured by DT.

---
Changes in V3:
- Rework most of it, to use generic iio-interrupt-trigger
- Drop GPIO trigger, use generic interrupt binding instead
- Comments from V2 and RFC: iio: trigger: Add OF support and GPIO based trigger
  https://lkml.org/lkml/2017/2/17/478

---
Changes in V2:
- Fixed Kconfig dependencies

Fabrice Gasnier (5):
  dt-bindings: iio: introduce trigger providers, consumers
  iio: trigger: add OF support
  dt-bindings: iio: document interrupt trigger support
  dt-bindings: iio: stm32-adc: add external interrupt trigger
  iio: adc: stm32: add support for EXTI trigger

Grégor Boirie (1):
  iio: iio-interrupt-trigger: device-tree support

 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |  21 +++++
 .../devicetree/bindings/iio/iio-bindings.txt       |  38 ++++++++
 .../bindings/iio/trigger/iio-trig-interrupt.txt    |  22 +++++
 drivers/iio/adc/stm32-adc.c                        |  27 +++++-
 drivers/iio/industrialio-trigger.c                 | 100 +++++++++++++++++++++
 drivers/iio/trigger/iio-trig-interrupt.c           |  10 +++
 include/linux/iio/trigger.h                        |   4 +
 7 files changed, 219 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt

-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 1/6] 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       | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
index 68d6f8c..01765e9 100644
--- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -95,3 +95,41 @@ 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:
+	trig0: interrupt-trigger0 {
+		#io-trigger-cells = <0>;
+		compatible = "interrupt-trigger";
+		interrupts = <11 0>;
+		interrupt-parent = <&gpioa>;
+	}
+
+==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 = <&trig0>;
+		io-trigger-names = "mytrig";
+	}
-- 
1.9.1

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Rob Herring]

On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
> Document iio provider and consumer bindings.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> index 68d6f8c..01765e9 100644
> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> @@ -95,3 +95,41 @@ 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:
> +	trig0: interrupt-trigger0 {
> +		#io-trigger-cells = <0>;
> +		compatible = "interrupt-trigger";
> +		interrupts = <11 0>;
> +		interrupt-parent = <&gpioa>;
> +	}
> +
> +==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 = <&trig0>;
> +		io-trigger-names = "mytrig";
> +	}

I have some reservations about this. We could just as easily add the 
interrupt directly to the consumer node and use "trigger" for a standard 
interrupt name. So the question is whether this extra level of 
indirection is needed? 

Rob

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Fabrice Gasnier]

On 03/03/2017 07:21 AM, Rob Herring wrote:
> On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
>> Document iio provider and consumer bindings.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>> index 68d6f8c..01765e9 100644
>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>> @@ -95,3 +95,41 @@ 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:
>> +	trig0: interrupt-trigger0 {
>> +		#io-trigger-cells = <0>;
>> +		compatible = "interrupt-trigger";
>> +		interrupts = <11 0>;
>> +		interrupt-parent = <&gpioa>;
>> +	}
>> +
>> +==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 = <&trig0>;
>> +		io-trigger-names = "mytrig";
>> +	}
>
> I have some reservations about this. We could just as easily add the
> interrupt directly to the consumer node and use "trigger" for a standard
Hi Rob,

Thanks for reviewing.

I hope I don't miss your point here... However, if I correctly
understand it:
Yes, this can be one way to get interrupt(s) directly from consumer 
node. Then, I understand consumer has to do exact same as what is being 
done in "iio_interrupt_trigger" for instance, basically:
- request irq, alloc and register trigger, do irq handling to call
trigger poll routine.

With current patchset, consumer is able to use standard trigger like
"interrupt-trigger" from DT. Please note I propose to add OF support
for it in current patchset (e.g. PATCHs 2 & 3). Currently only platform
data is supported.

-> And, please refer to PATCHs 5 & 6, I need to have some way to 
identify interrupt line (connected in HW to STM32 ADC IP). Currently,
this is best I came up with, trying to re-use, be generic, and to 
describe this HW in DT.

Of course, the other way is still valid. Also, I want to highlight,
STM32 has other IP, e.g. DAC, where same can be re-used then. This
will avoid having duplicates.

> interrupt name. So the question is whether this extra level of
> indirection is needed?

Purpose is to be able to get one or more named trigger(s) on consumer
side. Idea is to adopt similar 'philosophy' as in other bindings like
pinctrl, clk... where consumer has possibility to get them by name.
I hope this clarifies.

Please advise,
Best Regards,
Fabrice

>
> Rob
>

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Jonathan Cameron]

On 03/03/17 09:32, Fabrice Gasnier wrote:
> On 03/03/2017 07:21 AM, Rob Herring wrote:
>> On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
>>> Document iio provider and consumer bindings.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
>>>  1 file changed, 38 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> index 68d6f8c..01765e9 100644
>>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> @@ -95,3 +95,41 @@ 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:
>>> +    trig0: interrupt-trigger0 {
>>> +        #io-trigger-cells = <0>;
>>> +        compatible = "interrupt-trigger";
>>> +        interrupts = <11 0>;
>>> +        interrupt-parent = <&gpioa>;
>>> +    }
>>> +
>>> +==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 = <&trig0>;
>>> +        io-trigger-names = "mytrig";
>>> +    }
>>
>> I have some reservations about this. We could just as easily add the
>> interrupt directly to the consumer node and use "trigger" for a standard
> Hi Rob,
> 
> Thanks for reviewing.
> 
> I hope I don't miss your point here... However, if I correctly
> understand it:
> Yes, this can be one way to get interrupt(s) directly from consumer node. Then, I understand consumer has to do exact same as what is being done in "iio_interrupt_trigger" for instance, basically:
> - request irq, alloc and register trigger, do irq handling to call
> trigger poll routine.
> 
> With current patchset, consumer is able to use standard trigger like
> "interrupt-trigger" from DT. Please note I propose to add OF support
> for it in current patchset (e.g. PATCHs 2 & 3). Currently only platform
> data is supported.
> 
> -> And, please refer to PATCHs 5 & 6, I need to have some way to identify interrupt line (connected in HW to STM32 ADC IP). Currently,
> this is best I came up with, trying to re-use, be generic, and to describe this HW in DT.
> 
> Of course, the other way is still valid. Also, I want to highlight,
> STM32 has other IP, e.g. DAC, where same can be re-used then. This
> will avoid having duplicates.
Just to jump back a stage.  The binding here isn't stm32 specific
at all.  In general this binding allows for triggering anything
(currently IIO) from an interrupt. Nothing more - so that is the
level at which it should be considered.
> 
>> interrupt name. So the question is whether this extra level of
>> indirection is needed?
> 
> Purpose is to be able to get one or more named trigger(s) on consumer
> side. Idea is to adopt similar 'philosophy' as in other bindings like
> pinctrl, clk... where consumer has possibility to get them by name.
> I hope this clarifies.
Again, taking this in the general sense rather than on the stm32:
flexibility - if it makes sense to expose something to userspace we
do.  We could in theory list all the possible interrupt sources that
might drive each device in a system and then expose that to userspace
but that is hideous!
> 
> Please advise,
> Best Regards,
> Fabrice
> 
>>
>> Rob
>>
> -- 
> 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

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Jonathan Cameron]

On 03/03/17 06:21, Rob Herring wrote:
> On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
>> Document iio provider and consumer bindings.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
>> ---
>>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>> index 68d6f8c..01765e9 100644
>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>> @@ -95,3 +95,41 @@ 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:
>> +	trig0: interrupt-trigger0 {
>> +		#io-trigger-cells = <0>;
>> +		compatible = "interrupt-trigger";
>> +		interrupts = <11 0>;
>> +		interrupt-parent = <&gpioa>;
>> +	}
>> +
>> +==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 = <&trig0>;
>> +		io-trigger-names = "mytrig";
>> +	}
> 
> I have some reservations about this. We could just as easily add the 
> interrupt directly to the consumer node and use "trigger" for a standard 
> interrupt name. So the question is whether this extra level of 
> indirection is needed? 

First thing to note here, is that Fabrice's use of the generic interrupt
trigger is an extremely 'unusual' one! Normal use case is that we have
a random gpio pin providing interrupts to driver triggering on random
devices - there need be no association between the two whatsoever.
So what we are doing here is 'allowing' an interrupt to provide a trigger.
It's not necessarily the one going to be used by any particular device
driver.  The decision of which trigger to use is definitely one for
userspace, not something that should be configured in to the device tree.

For this particular case you could in theory just do it by using an interrupt
as you describe.  Ultimately though we should be able to play more complex
games with this device and having it able to handle any trigger - which 
includes those not using the direct hardware route.  It'll be up to the
driver to figure out when it can use the fast method and when it can't.

Conversely, even when we are using this hardware route to drive the
triggering it should be possible to hang off a device to be triggered
by the interrupt via the kernel rather than directly. 

So from a device tree point of view we are just describing the fact that
there is a pin, which may be used to trigger something.  What that something
is, is a question for userspace not the device tree.

Jonathan
> 
> Rob
> --
> 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
> 

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Jonathan Cameron]

On 05/03/17 11:43, Jonathan Cameron wrote:
> On 03/03/17 06:21, Rob Herring wrote:
>> On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
>>> Document iio provider and consumer bindings.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>> ---
>>>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
>>>  1 file changed, 38 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> index 68d6f8c..01765e9 100644
>>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> @@ -95,3 +95,41 @@ 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:
>>> +	trig0: interrupt-trigger0 {
>>> +		#io-trigger-cells = <0>;
>>> +		compatible = "interrupt-trigger";
>>> +		interrupts = <11 0>;
>>> +		interrupt-parent = <&gpioa>;
>>> +	}
>>> +
>>> +==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 = <&trig0>;
>>> +		io-trigger-names = "mytrig";
>>> +	}
>>
>> I have some reservations about this. We could just as easily add the 
>> interrupt directly to the consumer node and use "trigger" for a standard 
>> interrupt name. So the question is whether this extra level of 
>> indirection is needed? 
> 
> First thing to note here, is that Fabrice's use of the generic interrupt
> trigger is an extremely 'unusual' one! Normal use case is that we have
> a random gpio pin providing interrupts to driver triggering on random
> devices - there need be no association between the two whatsoever.
> So what we are doing here is 'allowing' an interrupt to provide a trigger.
> It's not necessarily the one going to be used by any particular device
> driver.  The decision of which trigger to use is definitely one for
> userspace, not something that should be configured in to the device tree.
> 
> For this particular case you could in theory just do it by using an interrupt
> as you describe.  Ultimately though we should be able to play more complex
> games with this device and having it able to handle any trigger - which 
> includes those not using the direct hardware route.  It'll be up to the
> driver to figure out when it can use the fast method and when it can't.
> 
> Conversely, even when we are using this hardware route to drive the
> triggering it should be possible to hang off a device to be triggered
> by the interrupt via the kernel rather than directly. 
> 
> So from a device tree point of view we are just describing the fact that
> there is a pin, which may be used to trigger something.  What that something
> is, is a question for userspace not the device tree.
> 
Ah, I'm half asleep this morning.  Clearly there is a more general follow
up question.  If we are arguing these are generic, why are we setting
up the mapping in device tree?

My gut feeling is we shouldn't be.  So I think we need the first chunk
above but the latter part should be a job for userspace not the devicetree.

Jonathan
> Jonathan
>>
>> Rob
>> --
>> 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
>>
> 
> --
> 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
> 

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Rob Herring]

On Sun, Mar 05, 2017 at 12:13:36PM +0000, Jonathan Cameron wrote:
> On 05/03/17 11:43, Jonathan Cameron wrote:
> > On 03/03/17 06:21, Rob Herring wrote:
> >> On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
> >>> Document iio provider and consumer bindings.
> >>>
> >>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> >>> ---
> >>>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
> >>>  1 file changed, 38 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>> index 68d6f8c..01765e9 100644
> >>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>> @@ -95,3 +95,41 @@ 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:
> >>> +	trig0: interrupt-trigger0 {
> >>> +		#io-trigger-cells = <0>;
> >>> +		compatible = "interrupt-trigger";
> >>> +		interrupts = <11 0>;
> >>> +		interrupt-parent = <&gpioa>;
> >>> +	}
> >>> +
> >>> +==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 = <&trig0>;
> >>> +		io-trigger-names = "mytrig";
> >>> +	}
> >>
> >> I have some reservations about this. We could just as easily add the 
> >> interrupt directly to the consumer node and use "trigger" for a standard 
> >> interrupt name. So the question is whether this extra level of 
> >> indirection is needed? 
> > 
> > First thing to note here, is that Fabrice's use of the generic interrupt
> > trigger is an extremely 'unusual' one! Normal use case is that we have
> > a random gpio pin providing interrupts to driver triggering on random
> > devices - there need be no association between the two whatsoever.
> > So what we are doing here is 'allowing' an interrupt to provide a trigger.
> > It's not necessarily the one going to be used by any particular device
> > driver.  The decision of which trigger to use is definitely one for
> > userspace, not something that should be configured in to the device tree.
> > 
> > For this particular case you could in theory just do it by using an interrupt
> > as you describe.  Ultimately though we should be able to play more complex
> > games with this device and having it able to handle any trigger - which 
> > includes those not using the direct hardware route.  It'll be up to the
> > driver to figure out when it can use the fast method and when it can't.
> > 
> > Conversely, even when we are using this hardware route to drive the
> > triggering it should be possible to hang off a device to be triggered
> > by the interrupt via the kernel rather than directly. 
> > 
> > So from a device tree point of view we are just describing the fact that
> > there is a pin, which may be used to trigger something.  What that something
> > is, is a question for userspace not the device tree.
> > 
> Ah, I'm half asleep this morning.  Clearly there is a more general follow
> up question.  If we are arguing these are generic, why are we setting
> up the mapping in device tree?
> 
> My gut feeling is we shouldn't be.  So I think we need the first chunk
> above but the latter part should be a job for userspace not the devicetree.

So you mean keep the provider side, but get rid of the consumer? That 
makes sense to me.

Rob

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Fabrice Gasnier]

On 03/15/2017 08:25 PM, Rob Herring wrote:
> On Sun, Mar 05, 2017 at 12:13:36PM +0000, Jonathan Cameron wrote:
>> On 05/03/17 11:43, Jonathan Cameron wrote:
>>> On 03/03/17 06:21, Rob Herring wrote:
>>>> On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
>>>>> Document iio provider and consumer bindings.
>>>>>
>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>>> ---
>>>>>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
>>>>>  1 file changed, 38 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>> index 68d6f8c..01765e9 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>> @@ -95,3 +95,41 @@ 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:
>>>>> +	trig0: interrupt-trigger0 {
>>>>> +		#io-trigger-cells = <0>;
>>>>> +		compatible = "interrupt-trigger";
>>>>> +		interrupts = <11 0>;
>>>>> +		interrupt-parent = <&gpioa>;
>>>>> +	}
>>>>> +
>>>>> +==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 = <&trig0>;
>>>>> +		io-trigger-names = "mytrig";
>>>>> +	}
>>>>
>>>> I have some reservations about this. We could just as easily add the 
>>>> interrupt directly to the consumer node and use "trigger" for a standard 
>>>> interrupt name. So the question is whether this extra level of 
>>>> indirection is needed? 
>>>
>>> First thing to note here, is that Fabrice's use of the generic interrupt
>>> trigger is an extremely 'unusual' one! Normal use case is that we have
Hi Rob, Jonathan,

Yes, I agree this is unusual.

>>> a random gpio pin providing interrupts to driver triggering on random
>>> devices - there need be no association between the two whatsoever.
>>> So what we are doing here is 'allowing' an interrupt to provide a trigger.
>>> It's not necessarily the one going to be used by any particular device
>>> driver.  The decision of which trigger to use is definitely one for
>>> userspace, not something that should be configured in to the device tree.
>>>
>>> For this particular case you could in theory just do it by using an interrupt
>>> as you describe.  Ultimately though we should be able to play more complex
>>> games with this device and having it able to handle any trigger - which 
>>> includes those not using the direct hardware route.  It'll be up to the
>>> driver to figure out when it can use the fast method and when it can't.
Agreed. Still, to benefit from hw capabilities, driver needs to have a
way to identify a particular trigger as a direct hardware route, or not
(and then default software handling, btw, that still needs to be
addressed in stm32-adc). DT Provider/consumer may help to achieve this.

>>>
>>> Conversely, even when we are using this hardware route to drive the
>>> triggering it should be possible to hang off a device to be triggered
>>> by the interrupt via the kernel rather than directly. 
>>>
>>> So from a device tree point of view we are just describing the fact that
>>> there is a pin, which may be used to trigger something.  What that something
>>> is, is a question for userspace not the device tree.
>>>
>> Ah, I'm half asleep this morning.  Clearly there is a more general follow
>> up question.  If we are arguing these are generic, why are we setting
>> up the mapping in device tree?
>>
>> My gut feeling is we shouldn't be.  So I think we need the first chunk
>> above but the latter part should be a job for userspace not the devicetree.
> 
> So you mean keep the provider side, but get rid of the consumer? That 
> makes sense to me.
In case getting rid of the consumer part, I still need one way, on
consumer side (stm32-adc) to specifically map EXTI signal in ADC
hardware, do some checks on trigger to validate hardware route.
I'm not sure how to handle this if I get rid of consumer part.
Shall I use something else not mentioned here? (put trigger dt node as
child node of stm32 adc, then use dev bus? ...)

Another way is as suggested by Rob in earlier comment: directly use this
interrupt in consumer dt node, e.g. in stm32-adc node. And register
relevant trigger and so on, from stm32-adc driver.
Please advise.

Thanks for reviewing
Best Regards,
Fabrice
> 
> Rob
> 

Re: [PATCH v3 1/6] dt-bindings: iio: introduce trigger providers, consumers

[Jonathan Cameron]

On 17/03/17 15:59, Fabrice Gasnier wrote:
> On 03/15/2017 08:25 PM, Rob Herring wrote:
>> On Sun, Mar 05, 2017 at 12:13:36PM +0000, Jonathan Cameron wrote:
>>> On 05/03/17 11:43, Jonathan Cameron wrote:
>>>> On 03/03/17 06:21, Rob Herring wrote:
>>>>> On Tue, Feb 28, 2017 at 05:51:14PM +0100, Fabrice Gasnier wrote:
>>>>>> Document iio provider and consumer bindings.
>>>>>>
>>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/iio/iio-bindings.txt       | 38 ++++++++++++++++++++++
>>>>>>  1 file changed, 38 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>>> index 68d6f8c..01765e9 100644
>>>>>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>>> @@ -95,3 +95,41 @@ 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:
>>>>>> +	trig0: interrupt-trigger0 {
>>>>>> +		#io-trigger-cells = <0>;
>>>>>> +		compatible = "interrupt-trigger";
>>>>>> +		interrupts = <11 0>;
>>>>>> +		interrupt-parent = <&gpioa>;
>>>>>> +	}
>>>>>> +
>>>>>> +==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 = <&trig0>;
>>>>>> +		io-trigger-names = "mytrig";
>>>>>> +	}
>>>>>
>>>>> I have some reservations about this. We could just as easily add the 
>>>>> interrupt directly to the consumer node and use "trigger" for a standard 
>>>>> interrupt name. So the question is whether this extra level of 
>>>>> indirection is needed? 
>>>>
>>>> First thing to note here, is that Fabrice's use of the generic interrupt
>>>> trigger is an extremely 'unusual' one! Normal use case is that we have
> Hi Rob, Jonathan,
> 
> Yes, I agree this is unusual.
> 
>>>> a random gpio pin providing interrupts to driver triggering on random
>>>> devices - there need be no association between the two whatsoever.
>>>> So what we are doing here is 'allowing' an interrupt to provide a trigger.
>>>> It's not necessarily the one going to be used by any particular device
>>>> driver.  The decision of which trigger to use is definitely one for
>>>> userspace, not something that should be configured in to the device tree.
>>>>
>>>> For this particular case you could in theory just do it by using an interrupt
>>>> as you describe.  Ultimately though we should be able to play more complex
>>>> games with this device and having it able to handle any trigger - which 
>>>> includes those not using the direct hardware route.  It'll be up to the
>>>> driver to figure out when it can use the fast method and when it can't.
> Agreed. Still, to benefit from hw capabilities, driver needs to have a
> way to identify a particular trigger as a direct hardware route, or not
> (and then default software handling, btw, that still needs to be
> addressed in stm32-adc). DT Provider/consumer may help to achieve this.
A simple additional callback that has access to both provider and consumer
should have sufficient data to figure this out somehow.
> 
>>>>
>>>> Conversely, even when we are using this hardware route to drive the
>>>> triggering it should be possible to hang off a device to be triggered
>>>> by the interrupt via the kernel rather than directly. 
>>>>
>>>> So from a device tree point of view we are just describing the fact that
>>>> there is a pin, which may be used to trigger something.  What that something
>>>> is, is a question for userspace not the device tree.
>>>>
>>> Ah, I'm half asleep this morning.  Clearly there is a more general follow
>>> up question.  If we are arguing these are generic, why are we setting
>>> up the mapping in device tree?
>>>
>>> My gut feeling is we shouldn't be.  So I think we need the first chunk
>>> above but the latter part should be a job for userspace not the devicetree.
>>
>> So you mean keep the provider side, but get rid of the consumer? That 
>> makes sense to me.
> In case getting rid of the consumer part, I still need one way, on
> consumer side (stm32-adc) to specifically map EXTI signal in ADC
> hardware, do some checks on trigger to validate hardware route.
> I'm not sure how to handle this if I get rid of consumer part.
> Shall I use something else not mentioned here? (put trigger dt node as
> child node of stm32 adc, then use dev bus? ...)
Something along those lines might work.
> 
> Another way is as suggested by Rob in earlier comment: directly use this
> interrupt in consumer dt node, e.g. in stm32-adc node. And register
> relevant trigger and so on, from stm32-adc driver.
> Please advise.
Also a possible option, be it a little restrictive given in theory at least
we could have a situation where this trigger had hardware routes to more
than one driver.

So I don't have a answer right now.  Would need to dig a little into the
actual drivers in question...  Run out of time this weekend, but might
get some time during the week.

Brief thoughts though: the interrupt trigger obviously knows the interrupt
in question.  Can we provide some access for the ADC to which interrupts
are valid and then an optional callback like validate_trigger that
is capable of discovering that there is a hardware route?  Some sort
of handle that the trigger provides would give the device scope to see
if it knows it can do 'magic' or not, return value could indicate to
the trigger whether it is going to be doing it with 'magic' or
really providing interrupts.

Jonathan

> 
> Thanks for reviewing
> Best Regards,
> Fabrice
>>
>> Rob
>>
> --
> 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
> 

[PATCH v3 2/6] 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..00ee3a7 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)
+		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

Re: [PATCH v3 2/6] iio: trigger: add OF support

[Jonathan Cameron]

On 28/02/17 16:51, Fabrice Gasnier wrote:
> 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>
Subject to binding discussion this looks fine to me.

Jonathan
> ---
>  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..00ee3a7 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)
> +		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;
> 

Re: [PATCH v3 2/6] iio: trigger: add OF support

[Linus Walleij]

On Tue, Feb 28, 2017 at 5:51 PM, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> 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>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

This is very elegant.

Yours,
Linus Walleij

[PATCH v3 3/6] dt-bindings: iio: document interrupt trigger support

[Fabrice Gasnier]

Document interrupt trigger that takes a generic interrupt, and
can be used as trigger source for sampling devices such as sensors,
ADCs...

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

diff --git a/Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt b/Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt
new file mode 100644
index 0000000..9de9856
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt
@@ -0,0 +1,22 @@
+Interrupt based trigger
+
+Any internal or external interrupt may be used as trigger source for
+devices like sensors, ADCs... As an example, external signal can be
+routed to a GPIO, and turned into an interrupt driven trigger.
+
+Required properties:
+- compatible: Should be "interrupt-trigger"
+- interrupts: The interrupt used as trigger. Generic interrupt client node as
+  described in ../../interrupt-controller/interrupts.txt
+
+Optional properties:
+- #io-trigger-cells = <0>; See the IIO bindings, IIO trigger providers
+  and consumers sections in ../iio-bindings.txt
+
+Example:
+	trig0: interrupt-trigger0 {
+		#io-trigger-cells = <0>;
+		compatible = "interrupt-trigger";
+		interrupts = <11 0>;
+		interrupt-parent = <&gpioa>;
+	}
-- 
1.9.1

Re: [PATCH v3 3/6] dt-bindings: iio: document interrupt trigger support

[Jonathan Cameron]

On 28/02/17 16:51, Fabrice Gasnier wrote:
> Document interrupt trigger that takes a generic interrupt, and
> can be used as trigger source for sampling devices such as sensors,
> ADCs...
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
I'm not convinced we want the earlier device tree binding doc at all.
In the vast majority of cases that chunk on providers is irrelevant.
What matters is to document the specific case as you have done here.

So this one I like, the earlier document less so, simply because
it repeats what this more specific document has described.

So Rob, what do you think of 'just' this bit?
> ---
>  .../bindings/iio/trigger/iio-trig-interrupt.txt    | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt b/Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt
> new file mode 100644
> index 0000000..9de9856
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/trigger/iio-trig-interrupt.txt
> @@ -0,0 +1,22 @@
> +Interrupt based trigger
> +
> +Any internal or external interrupt may be used as trigger source for
> +devices like sensors, ADCs... As an example, external signal can be
> +routed to a GPIO, and turned into an interrupt driven trigger.
> +
> +Required properties:
> +- compatible: Should be "interrupt-trigger"
> +- interrupts: The interrupt used as trigger. Generic interrupt client node as
> +  described in ../../interrupt-controller/interrupts.txt
> +
> +Optional properties:
> +- #io-trigger-cells = <0>; See the IIO bindings, IIO trigger providers
> +  and consumers sections in ../iio-bindings.txt
> +
> +Example:
> +	trig0: interrupt-trigger0 {
> +		#io-trigger-cells = <0>;
> +		compatible = "interrupt-trigger";
> +		interrupts = <11 0>;
> +		interrupt-parent = <&gpioa>;
> +	}
> 

--
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: [PATCH v3 3/6] dt-bindings: iio: document interrupt trigger support

[Rob Herring]

On Sun, Mar 05, 2017 at 12:16:28PM +0000, Jonathan Cameron wrote:
> On 28/02/17 16:51, Fabrice Gasnier wrote:
> > Document interrupt trigger that takes a generic interrupt, and
> > can be used as trigger source for sampling devices such as sensors,
> > ADCs...
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> I'm not convinced we want the earlier device tree binding doc at all.
> In the vast majority of cases that chunk on providers is irrelevant.
> What matters is to document the specific case as you have done here.
> 
> So this one I like, the earlier document less so, simply because
> it repeats what this more specific document has described.
> 
> So Rob, what do you think of 'just' this bit?

Yeah, it seems fine. Though, the #io-trigger-cells can be dropped.

Rob

[PATCH v3 4/6] iio: iio-interrupt-trigger: device-tree support

[Fabrice Gasnier]

From: Grégor Boirie <gregor.boirie@parrot.com>

Add compatible string that should be used in DT.
Also cascade of_node to newly allocated trigger device.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
Original PATCH from Gregor Boirie:
https://marc.info/?l=linux-iio&m=145590953607905&w=4

Updates:
- compatible string changed to "interrupt-trigger"
- cascade of_node down to iio trigger device
---
 drivers/iio/trigger/iio-trig-interrupt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/trigger/iio-trig-interrupt.c b/drivers/iio/trigger/iio-trig-interrupt.c
index e18f12b..6ee1f7e 100644
--- a/drivers/iio/trigger/iio-trig-interrupt.c
+++ b/drivers/iio/trigger/iio-trig-interrupt.c
@@ -62,6 +62,7 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 	}
 	iio_trigger_set_drvdata(trig, trig_info);
 	trig_info->irq = irq;
+	trig->dev.of_node = pdev->dev.of_node;
 	trig->ops = &iio_interrupt_trigger_ops;
 	ret = request_irq(irq, iio_interrupt_trigger_poll,
 			  irqflags, trig->name, trig);
@@ -104,11 +105,20 @@ static int iio_interrupt_trigger_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id iio_interrupt_trigger_of_match[] = {
+	{ .compatible = "interrupt-trigger" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, iio_interrupt_trigger_of_match);
+#endif
+
 static struct platform_driver iio_interrupt_trigger_driver = {
 	.probe = iio_interrupt_trigger_probe,
 	.remove = iio_interrupt_trigger_remove,
 	.driver = {
 		.name = "iio_interrupt_trigger",
+		.of_match_table = of_match_ptr(iio_interrupt_trigger_of_match),
 	},
 };
 
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 4/6] iio: iio-interrupt-trigger: device-tree support

[Jonathan Cameron]

On 28/02/17 16:51, Fabrice Gasnier wrote:
> From: Grégor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
> 
> Add compatible string that should be used in DT.
> Also cascade of_node to newly allocated trigger device.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
I'd normally not keep Gregor's sign off in a patch that has been
changed like this.  If Gregor is happy it's a different matter,
but I suspect he hasn't replied yet as he is always very busy!

> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Otherwise, all good, subject to convincing the device tree
guys about the binding.

Jonathan
> ---
> Original PATCH from Gregor Boirie:
> https://marc.info/?l=linux-iio&m=145590953607905&w=4
> 
> Updates:
> - compatible string changed to "interrupt-trigger"
> - cascade of_node down to iio trigger device
> ---
>  drivers/iio/trigger/iio-trig-interrupt.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/iio/trigger/iio-trig-interrupt.c b/drivers/iio/trigger/iio-trig-interrupt.c
> index e18f12b..6ee1f7e 100644
> --- a/drivers/iio/trigger/iio-trig-interrupt.c
> +++ b/drivers/iio/trigger/iio-trig-interrupt.c
> @@ -62,6 +62,7 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>  	}
>  	iio_trigger_set_drvdata(trig, trig_info);
>  	trig_info->irq = irq;
> +	trig->dev.of_node = pdev->dev.of_node;
>  	trig->ops = &iio_interrupt_trigger_ops;
>  	ret = request_irq(irq, iio_interrupt_trigger_poll,
>  			  irqflags, trig->name, trig);
> @@ -104,11 +105,20 @@ static int iio_interrupt_trigger_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id iio_interrupt_trigger_of_match[] = {
> +	{ .compatible = "interrupt-trigger" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, iio_interrupt_trigger_of_match);
> +#endif
> +
>  static struct platform_driver iio_interrupt_trigger_driver = {
>  	.probe = iio_interrupt_trigger_probe,
>  	.remove = iio_interrupt_trigger_remove,
>  	.driver = {
>  		.name = "iio_interrupt_trigger",
> +		.of_match_table = of_match_ptr(iio_interrupt_trigger_of_match),
>  	},
>  };
>  
> 

--
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

[PATCH v3 5/6] dt-bindings: iio: stm32-adc: add external interrupt trigger

[Fabrice Gasnier]

Document support for EXTI trigger. STM32 ADC can use external interrupt
line as trigger source for conversions.

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

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
index 5dfc88e..9e4af7f 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
@@ -57,6 +57,10 @@ Optional properties:
 - dmas: Phandle to dma channel for this ADC instance.
   See ../../dma/dma.txt for details.
 - dma-names: Must be "rx" when dmas property is being used.
+- io-triggers: Phandle to external interrupt trigger (e.g. EXTI). Must be
+  phandle to EXTI11 on stm32f4. See ../iio-bindings.txt for details on trigger
+  consumer.
+- io-trigger-names: Must be "exti" when io-triggers property is being used.
 
 Example:
 	adc: adc@40012000 {
@@ -88,3 +92,20 @@ Example:
 		...
 		other adc child nodes follow...
 	};
+
+Example with EXTI11 trigger:
+	trig: interrupt-trigger {
+		#io-trigger-cells = <0>;
+		compatible = "interrupt-trigger";
+		interrupts = <11 0>;
+		interrupt-parent = <&gpioa>;
+	}
+
+	adc: adc@40012000 {
+		...
+		adc@0 {
+			...
+			io-triggers = <&trig>;
+			io-trigger-names = "exti";
+		};
+	};
-- 
1.9.1

[PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

[Fabrice Gasnier]

EXTi (external interrupt) signal can be routed internally as trigger
source for ADC conversions: STM32F4 ADC can use EXTI11.

Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
via extsel.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 9b49a6ad..4d9040d 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -108,6 +108,9 @@ enum stm32_adc_extsel {
 	STM32_EXT15,
 };
 
+/* EXTI 11 trigger selection on STM32F4 */
+#define STM32F4_EXTI11_EXTSEL		STM32_EXT15
+
 /**
  * struct stm32_adc_trig_info - ADC trigger info
  * @name:		name of the trigger, corresponding to its source
@@ -146,6 +149,7 @@ struct stm32_adc_regs {
  * @rx_buf:		dma rx buffer cpu address
  * @rx_dma_buf:		dma rx buffer bus address
  * @rx_buf_sz:		dma rx buffer size
+ * @exti_trig		EXTI trigger
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -162,6 +166,7 @@ struct stm32_adc {
 	u8			*rx_buf;
 	dma_addr_t		rx_dma_buf;
 	unsigned int		rx_buf_sz;
+	struct iio_trigger	*exti_trig;
 };
 
 /**
@@ -395,10 +400,15 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
  *
  * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
  */
-static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
+static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
+				     struct iio_trigger *trig)
 {
+	struct stm32_adc *adc = iio_priv(indio_dev);
 	int i;
 
+	if (trig == adc->exti_trig)
+		return STM32F4_EXTI11_EXTSEL;
+
 	/* lookup triggers registered by stm32 timer trigger driver */
 	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
 		/**
@@ -432,7 +442,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
 	int ret;
 
 	if (trig) {
-		ret = stm32_adc_get_trig_extsel(trig);
+		ret = stm32_adc_get_trig_extsel(indio_dev, trig);
 		if (ret < 0)
 			return ret;
 
@@ -604,7 +614,7 @@ static irqreturn_t stm32_adc_isr(int irq, void *data)
 static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
 				      struct iio_trigger *trig)
 {
-	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
+	return stm32_adc_get_trig_extsel(indio_dev, trig) < 0 ? -EINVAL : 0;
 }
 
 static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
@@ -1030,6 +1040,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk_disable;
 
+	adc->exti_trig = devm_iio_trigger_get_by_name(&pdev->dev, "exti");
+	if (IS_ERR(adc->exti_trig)) {
+		ret = PTR_ERR(adc->exti_trig);
+		if (ret == -EPROBE_DEFER)
+			goto err_dma_disable;
+		dev_dbg(&pdev->dev, "No exti trigger found (%d)\n", ret);
+		adc->exti_trig = NULL;
+	} else {
+		dev_info(&pdev->dev, "Can use %s\n", adc->exti_trig->name);
+	}
+
 	ret = iio_triggered_buffer_setup(indio_dev,
 					 &iio_pollfunc_store_time,
 					 &stm32_adc_trigger_handler,
-- 
1.9.1

Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

[Lars-Peter Clausen]

On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
> EXTi (external interrupt) signal can be routed internally as trigger
> source for ADC conversions: STM32F4 ADC can use EXTI11.
> 
> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
> via extsel.

Hi,

Sorry, I have some trouble understanding the specifics of this. Is EXTI a
hardware input signal into the ADC that automatically triggers a conversion
when asserted? If yes how is it configured which external signal is used
here. Your bindings suggest that any GPIO can be used, but the driver only
differentiates between EXTI on or EXTI off.

Or is this just a software triggered conversion? The GPIO triggers a
software interrupt routine and the interrupt routine than triggers conversion?

Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

[Fabrice Gasnier]

On 03/03/2017 12:45 PM, Lars-Peter Clausen wrote:
> On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
>> EXTi (external interrupt) signal can be routed internally as trigger
>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>
>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>> via extsel.
> 
> Hi,
> 
> Sorry, I have some trouble understanding the specifics of this. Is EXTI a
> hardware input signal into the ADC that automatically triggers a conversion
> when asserted? If yes how is it configured which external signal is used
> here. Your bindings suggest that any GPIO can be used, but the driver only
> differentiates between EXTI on or EXTI off.
Hi Lars,

Yes, STM32 EXTI is external interrupt/event line. In case of STM32 ADC,
EXTI11 signal can be used to start a conversion. In this case, it must
be selected inside ADC IP using extsel bitfield. This EXTI11 line can
mapped from any GPIO bank A,B... line 11 (e.g. PA11 or PB11...) by using
interrupt binding.
This is why I expose this in DT.

> 
> Or is this just a software triggered conversion? The GPIO triggers a
> software interrupt routine and the interrupt routine than triggers conversion?
> 

Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

[Lars-Peter Clausen]

On 03/03/2017 02:00 PM, Fabrice Gasnier wrote:
> On 03/03/2017 12:45 PM, Lars-Peter Clausen wrote:
>> On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
>>> EXTi (external interrupt) signal can be routed internally as trigger
>>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>>
>>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>>> via extsel.
>>
>> Hi,
>>
>> Sorry, I have some trouble understanding the specifics of this. Is EXTI a
>> hardware input signal into the ADC that automatically triggers a conversion
>> when asserted? If yes how is it configured which external signal is used
>> here. Your bindings suggest that any GPIO can be used, but the driver only
>> differentiates between EXTI on or EXTI off.
> Hi Lars,
> 
> Yes, STM32 EXTI is external interrupt/event line. In case of STM32 ADC,
> EXTI11 signal can be used to start a conversion. In this case, it must
> be selected inside ADC IP using extsel bitfield. This EXTI11 line can
> mapped from any GPIO bank A,B... line 11 (e.g. PA11 or PB11...) by using
> interrupt binding.
> This is why I expose this in DT.

How is the mapping? That's the part I don't understand. How does requesting
the IRQ for the GPIO as a generic software IRQ establish the hardware
connection between the GPIO block and the ADC?

Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

[Lars-Peter Clausen]

On 03/03/2017 04:46 PM, Lars-Peter Clausen wrote:
> On 03/03/2017 02:00 PM, Fabrice Gasnier wrote:
>> On 03/03/2017 12:45 PM, Lars-Peter Clausen wrote:
>>> On 02/28/2017 05:51 PM, Fabrice Gasnier wrote:
>>>> EXTi (external interrupt) signal can be routed internally as trigger
>>>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>>>
>>>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>>>> via extsel.
>>>
>>> Hi,
>>>
>>> Sorry, I have some trouble understanding the specifics of this. Is EXTI a
>>> hardware input signal into the ADC that automatically triggers a conversion
>>> when asserted? If yes how is it configured which external signal is used
>>> here. Your bindings suggest that any GPIO can be used, but the driver only
>>> differentiates between EXTI on or EXTI off.
>> Hi Lars,
>>
>> Yes, STM32 EXTI is external interrupt/event line. In case of STM32 ADC,
>> EXTI11 signal can be used to start a conversion. In this case, it must
>> be selected inside ADC IP using extsel bitfield. This EXTI11 line can
>> mapped from any GPIO bank A,B... line 11 (e.g. PA11 or PB11...) by using
>> interrupt binding.
>> This is why I expose this in DT.
> 
> How is the mapping? That's the part I don't understand. How does requesting

How is the mapping done?

> the IRQ for the GPIO as a generic software IRQ establish the hardware
> connection between the GPIO block and the ADC?
> 

Re: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

[Jonathan Cameron]

On 28/02/17 16:51, Fabrice Gasnier wrote:
> EXTi (external interrupt) signal can be routed internally as trigger
> source for ADC conversions: STM32F4 ADC can use EXTI11.
> 
> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
> via extsel.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Minor question inline.
J
> ---
>  drivers/iio/adc/stm32-adc.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 9b49a6ad..4d9040d 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -108,6 +108,9 @@ enum stm32_adc_extsel {
>  	STM32_EXT15,
>  };
>  
> +/* EXTI 11 trigger selection on STM32F4 */
> +#define STM32F4_EXTI11_EXTSEL		STM32_EXT15
> +
>  /**
>   * struct stm32_adc_trig_info - ADC trigger info
>   * @name:		name of the trigger, corresponding to its source
> @@ -146,6 +149,7 @@ struct stm32_adc_regs {
>   * @rx_buf:		dma rx buffer cpu address
>   * @rx_dma_buf:		dma rx buffer bus address
>   * @rx_buf_sz:		dma rx buffer size
> + * @exti_trig		EXTI trigger
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -162,6 +166,7 @@ struct stm32_adc {
>  	u8			*rx_buf;
>  	dma_addr_t		rx_dma_buf;
>  	unsigned int		rx_buf_sz;
> +	struct iio_trigger	*exti_trig;
>  };
>  
>  /**
> @@ -395,10 +400,15 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>   *
>   * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
>   */
> -static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
> +static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
> +				     struct iio_trigger *trig)
>  {
> +	struct stm32_adc *adc = iio_priv(indio_dev);
>  	int i;
>  
> +	if (trig == adc->exti_trig)
> +		return STM32F4_EXTI11_EXTSEL;
> +
>  	/* lookup triggers registered by stm32 timer trigger driver */
>  	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
>  		/**
> @@ -432,7 +442,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>  	int ret;
>  
>  	if (trig) {
> -		ret = stm32_adc_get_trig_extsel(trig);
> +		ret = stm32_adc_get_trig_extsel(indio_dev, trig);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -604,7 +614,7 @@ static irqreturn_t stm32_adc_isr(int irq, void *data)
>  static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>  				      struct iio_trigger *trig)
>  {
> -	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
> +	return stm32_adc_get_trig_extsel(indio_dev, trig) < 0 ? -EINVAL : 0;
>  }
>  
>  static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> @@ -1030,6 +1040,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_clk_disable;
>  
> +	adc->exti_trig = devm_iio_trigger_get_by_name(&pdev->dev, "exti");
Can we tighten this in any way? If not I guess we'll have to rely on no
one messing up the devicetree to put the wrong trigger in there with this name.

> +	if (IS_ERR(adc->exti_trig)) {
> +		ret = PTR_ERR(adc->exti_trig);
> +		if (ret == -EPROBE_DEFER)
> +			goto err_dma_disable;
> +		dev_dbg(&pdev->dev, "No exti trigger found (%d)\n", ret);
> +		adc->exti_trig = NULL;
> +	} else {
> +		dev_info(&pdev->dev, "Can use %s\n", adc->exti_trig->name);
> +	}
> +
>  	ret = iio_triggered_buffer_setup(indio_dev,
>  					 &iio_pollfunc_store_time,
>  					 &stm32_adc_trigger_handler,
> 

--
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: [PATCH v3 6/6] iio: adc: stm32: add support for EXTI trigger

[Jonathan Cameron]

On 05/03/17 12:21, Jonathan Cameron wrote:
> On 28/02/17 16:51, Fabrice Gasnier wrote:
>> EXTi (external interrupt) signal can be routed internally as trigger
>> source for ADC conversions: STM32F4 ADC can use EXTI11.
>>
>> Retrieve interrupt trigger from DT, so it can be muxed into ADC IP,
>> via extsel.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Minor question inline.
and another.
> J
>> ---
>>  drivers/iio/adc/stm32-adc.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 9b49a6ad..4d9040d 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -108,6 +108,9 @@ enum stm32_adc_extsel {
>>  	STM32_EXT15,
>>  };
>>  
>> +/* EXTI 11 trigger selection on STM32F4 */
>> +#define STM32F4_EXTI11_EXTSEL		STM32_EXT15
>> +
>>  /**
>>   * struct stm32_adc_trig_info - ADC trigger info
>>   * @name:		name of the trigger, corresponding to its source
>> @@ -146,6 +149,7 @@ struct stm32_adc_regs {
>>   * @rx_buf:		dma rx buffer cpu address
>>   * @rx_dma_buf:		dma rx buffer bus address
>>   * @rx_buf_sz:		dma rx buffer size
>> + * @exti_trig		EXTI trigger
>>   */
>>  struct stm32_adc {
>>  	struct stm32_adc_common	*common;
>> @@ -162,6 +166,7 @@ struct stm32_adc {
>>  	u8			*rx_buf;
>>  	dma_addr_t		rx_dma_buf;
>>  	unsigned int		rx_buf_sz;
>> +	struct iio_trigger	*exti_trig;
>>  };
>>  
>>  /**
>> @@ -395,10 +400,15 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>>   *
>>   * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
>>   */
>> -static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
>> +static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
>> +				     struct iio_trigger *trig)
>>  {
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>>  	int i;
>>  
>> +	if (trig == adc->exti_trig)
>> +		return STM32F4_EXTI11_EXTSEL;
>> +
>>  	/* lookup triggers registered by stm32 timer trigger driver */
>>  	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
>>  		/**
>> @@ -432,7 +442,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>>  	int ret;
>>  
>>  	if (trig) {
>> -		ret = stm32_adc_get_trig_extsel(trig);
>> +		ret = stm32_adc_get_trig_extsel(indio_dev, trig);
>>  		if (ret < 0)
>>  			return ret;
>>  
>> @@ -604,7 +614,7 @@ static irqreturn_t stm32_adc_isr(int irq, void *data)
>>  static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>>  				      struct iio_trigger *trig)
>>  {
>> -	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
>> +	return stm32_adc_get_trig_extsel(indio_dev, trig) < 0 ? -EINVAL : 0;
>>  }
>>  
>>  static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
>> @@ -1030,6 +1040,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>  	if (ret < 0)
>>  		goto err_clk_disable;
>>  
>> +	adc->exti_trig = devm_iio_trigger_get_by_name(&pdev->dev, "exti");
> Can we tighten this in any way? If not I guess we'll have to rely on no
> one messing up the devicetree to put the wrong trigger in there with this name.

Also, given I'm arguing for this association to not be done in devicetree but
rather left to userspace, can we move this get to only occur during validate
trigger rather than here?  To my mind we shouldn't even know the trigger
is registered at during the ADC probe.  Also has the benefit of getting rid
of needing to handle the deferred element.
> 
>> +	if (IS_ERR(adc->exti_trig)) {
>> +		ret = PTR_ERR(adc->exti_trig);
>> +		if (ret == -EPROBE_DEFER)
>> +			goto err_dma_disable;
>> +		dev_dbg(&pdev->dev, "No exti trigger found (%d)\n", ret);
>> +		adc->exti_trig = NULL;
>> +	} else {
>> +		dev_info(&pdev->dev, "Can use %s\n", adc->exti_trig->name);
>> +	}
>> +
>>  	ret = iio_triggered_buffer_setup(indio_dev,
>>  					 &iio_pollfunc_store_time,
>>  					 &stm32_adc_trigger_handler,
>>
> 
> --
> 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
>