Re: [PATCH 3/5] Documentation: dt: iio: document stm32 exti trigger

From: Rob Herring
Date: Thu Feb 02 2017 - 10:45:38 EST


+Linus W

On Thu, Feb 2, 2017 at 3:19 AM, Fabrice Gasnier <fabrice.gasnier@xxxxxx> wrote:
> On 02/01/2017 05:35 PM, Rob Herring wrote:
>>
>> On Mon, Jan 30, 2017 at 02:57:41PM +0100, Fabrice Gasnier wrote:
>>>
>>> Add dt documentation for st,stm32-exti-trigger.
>>> EXTi gpio signal can be routed internally as trigger source for various
>>
>>
>> s/gpio/GPIO/
>>
>>> IPs (e.g. for ADC or DAC conversions).
>>
>>
>> Please use "dt-bindings: iio:" for the subject prefix.
>
>
> Hi Rob,
>
> I'll fix this in V2.
>
>>
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
>>> ---
>>> .../bindings/iio/trigger/st,stm32-exti-trigger.txt | 17
>>> +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/iio/trigger/st,stm32-exti-trigger.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/iio/trigger/st,stm32-exti-trigger.txt
>>> b/Documentation/devicetree/bindings/iio/trigger/st,stm32-exti-trigger.txt
>>> new file mode 100644
>>> index 0000000..ebf2645
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/iio/trigger/st,stm32-exti-trigger.txt
>>> @@ -0,0 +1,17 @@
>>> +STMicroelectronics STM32 EXTI trigger bindings
>>> +
>>> +EXTi gpio signal can be routed internally as trigger source for various
>>
>>
>> s/gpio/GPIO/
>>
>>> +IPs (e.g. for ADC or DAC conversions).
>>> +
>>> +Contents of a stm32 exti trigger root node:
>>
>>
>> Drop "root"
>
> I'll fix these in V2.
>
>>
>>> +-------------------------------------------
>>> +Required properties:
>>> +- compatible: Should be "st,stm32-exti-trigger"
>>
>>
>> This whole binding looks a bit suspicious. Is this actually a h/w block?
>> What makes it stm32 specific? Seems like the gpio properties should just
>> be part of the ADC or DAC that they trigger.
>
>
> Please let me explain in more details.
> I think best is I add following documentation in binding. Something like:
>
> GPIO + EXTI controller side | ADC side (input trigger MUX, same for DAC)
> |
> IIO trigger ---> IIO device
> | __________
> | inX --| \
> | ... --| SAR ADC |-->
> __ | __ |__________/
> PA11 --| \ | TIMx --| \ trigger ^
> PB11 --| | ... --| }----------->'
> ... --| }---- EXTI11 ---------->--|__/
> --| | |
> PJ11 --|__/ |
>
> In fact, this driver configures GPIO and EXTI controllers (left side of
> above scheme), and it registers corresponding trigger in IIO. Then it can be
> used by other IPs registered as IIO devices.
>
> I think this should be outside of ADC or DAC IIO device drivers, to live in
> separate IIO trigger driver. It will avoid duplicating code (e.g. PATCH 4)
> into several IIO device drivers.
>
> Is it ok to declare this as separate IIO trigger driver?

Drivers and DT nodes are not necessarily 1 to 1.

What controls the GPIO line that is used for the trigger?

> Maybe Jonathan could also advise on this ?
>
> As I see it, this is stm32 specific, as any GPIO bank, (e.g. PA11,
> PB11...) can be selected to generate (EXTI11) hardware trigger signal.

This seems more like a pinmux'ing issue than a GPIO. This isn't really
a GPIO if it is routed to a h/w control.

A -gpios property for a trigger would make sense if you had a driver
handling GPIO interrupts to generate IIO triggers. This seems to be
just mux control.

>>> +- extiN-gpio: optional gpio line that may be used as external trigger
>>> source
>>
>>
>> -gpios is the preferred form.
>
>
> I apologize, I didn't make it clear in the first place. Please let me
> rephrase. Is bellow description more suitable ?

What I mean is the property name is wrong. It should be "extiN-gpios".

>
> - extiN-gpio: One or several named GPIO lines that may be used as
> external trigger source by STM32 ADC, DAC. N may be 0..15.
> For example, on stm32f4, exti11-gpio can trig ADC, exti9-gpio can
> trig DAC.
>
>>
>>> + (e.g. N may be 0..15. For example, exti11-gpio can trig ADC on
>>> stm32f4).
>>> +
>>> +Example:
>>> + triggers {
>>> + compatible = "st,stm32-exti-trigger";
>>> + exti11-gpio=<&gpioa 11 0>;
>>
>>
>> spaces around the "=".
>
> I'll fix it in V2, and also add example for more that one EXTI trigger:
>
> triggers {
> compatible = "st,stm32-exti-trigger";
> exti9-gpio = <&gpioa 9 0>;
> exti11-gpio = <&gpioa 11 0>;
> ...
> };
>
> Above binding can typically be used in board dt, in case on-board gpio is
> connected/used as trigger source for IIO device (STM32 ADC/DAC).
>
> Please let me know if this clarifies, so I'll do necessary changes in V2.
>
> Many thanks for your review.
> Best Regards,
> Fabrice
>
>>
>>> + };
>>> --
>>> 1.9.1
>>>
>