All of lore.kernel.org
 help / color / mirror / Atom feed
* Correct meaning of the GPIO active low flag
@ 2014-02-10 14:33 Laurent Pinchart
  2014-02-10 14:50 ` Alexandre Courbot
  0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-10 14:33 UTC (permalink / raw)
  To: linux-gpio; +Cc: Linus Walleij, Alexandre Courbot, Stephen Warren

Hello everybody,

While working on Dt support for a driver that uses GPIO I came to ponder about 
the correct meaning of the GPIO active low flag. I'm bringing the question to 
the mailing list to get feedback.

When a device has an active low input, the fact that the input is active low 
is a property of the device, and thus known to the driver. On the other hand, 
if an inverter is present on the board, that information isn't known to device 
drivers and need to be expressed in DT.

Does the active low flag express the latter only, or both of them ? To ask the 
question differently, should the low flag model the inverter inside the 
device, known to the device driver, effectively moving handling of that 
inverter out of the device driver to the core code, or should it stop at the 
device boundary and only model the board ?

As an example, if my device datasheet states that the reset input is active 
low, an no inverter is present on the GPIO line, should I set the GPIO active 
low flag in DT and set the GPIO value to 1 in software (assuming I use the 
gpiod_* API) to make the reset signal active, or should I set the GPIO active 
high flag in DT and the the GPIO value to 0 in software ?

Let's also keep in mind that devices can have programmable polarities for 
inputs connected to GPIO-driven signals, in which case the internal polarity 
inversion is dynamic and can't be expressed in the GPIO propery in DT.

-- 
Regards,

Laurent Pinchart


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 14:33 Correct meaning of the GPIO active low flag Laurent Pinchart
@ 2014-02-10 14:50 ` Alexandre Courbot
  2014-02-10 15:13   ` Laurent Pinchart
  0 siblings, 1 reply; 16+ messages in thread
From: Alexandre Courbot @ 2014-02-10 14:50 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-gpio, Linus Walleij, Stephen Warren

Hi Laurent,

On Mon, Feb 10, 2014 at 11:33 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hello everybody,
>
> While working on Dt support for a driver that uses GPIO I came to ponder about
> the correct meaning of the GPIO active low flag. I'm bringing the question to
> the mailing list to get feedback.
>
> When a device has an active low input, the fact that the input is active low
> is a property of the device, and thus known to the driver. On the other hand,
> if an inverter is present on the board, that information isn't known to device
> drivers and need to be expressed in DT.
>
> Does the active low flag express the latter only, or both of them ? To ask the
> question differently, should the low flag model the inverter inside the
> device, known to the device driver, effectively moving handling of that
> inverter out of the device driver to the core code, or should it stop at the
> device boundary and only model the board ?

The intent behind the current behavior of the gpiod interface is to
avoid the need for code like this:

        if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
            gpio_set_value(pb->enable_gpio, 0);
        else
            gpio_set_value(pb->enable_gpio, 1);

which could simply be replaced by:

        gpiod_set_value(pb->enable_gpio, 1);

and the GPIO framework will invert the signal if needed according to
the active low property of the GPIO.

This behavior is based on the assumption that the active low flag
represents an inverter present on the board, and thus unknown to the
device driver. Now I just hope this assumption is correct. On the
other hand I'm not sure I would understand the need for it if it were
to model a property of the device, as the driver would be aware of it
anyway, and thus should not need to be told about it explicitly.

> As an example, if my device datasheet states that the reset input is active
> low, an no inverter is present on the GPIO line, should I set the GPIO active
> low flag in DT and set the GPIO value to 1 in software (assuming I use the
> gpiod_* API) to make the reset signal active, or should I set the GPIO active
> high flag in DT and the the GPIO value to 0 in software ?

If your datasheet explicitly states that the reset input is active
low, then this fact should be captured by the compatible property
already (since the active low status is true for each and every
compatible device) and I don't think your node would need to state it
in addition. Unless, of course, there is an inverter on the way.

This is my understanding but let's see what others have to say and
let's also make sure to update the documentation to lift the
ambiguity. :)

Alex.

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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 14:50 ` Alexandre Courbot
@ 2014-02-10 15:13   ` Laurent Pinchart
  2014-02-10 16:56     ` Stephen Warren
  0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-10 15:13 UTC (permalink / raw)
  To: Alexandre Courbot; +Cc: linux-gpio, Linus Walleij, Stephen Warren

Hi Alexander,

Thank you for your quick answer.

On Monday 10 February 2014 23:50:40 Alexandre Courbot wrote:
> On Mon, Feb 10, 2014 at 11:33 PM, Laurent Pinchart wrote:
> > Hello everybody,
> > 
> > While working on Dt support for a driver that uses GPIO I came to ponder
> > about the correct meaning of the GPIO active low flag. I'm bringing the
> > question to the mailing list to get feedback.
> > 
> > When a device has an active low input, the fact that the input is active
> > low is a property of the device, and thus known to the driver. On the
> > other hand, if an inverter is present on the board, that information
> > isn't known to device drivers and need to be expressed in DT.
> > 
> > Does the active low flag express the latter only, or both of them ? To ask
> > the question differently, should the low flag model the inverter inside
> > the device, known to the device driver, effectively moving handling of
> > that inverter out of the device driver to the core code, or should it
> > stop at the device boundary and only model the board ?
> 
> The intent behind the current behavior of the gpiod interface is to
> avoid the need for code like this:
> 
>         if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
>             gpio_set_value(pb->enable_gpio, 0);
>         else
>             gpio_set_value(pb->enable_gpio, 1);
> 
> which could simply be replaced by:
> 
>         gpiod_set_value(pb->enable_gpio, 1);
> 
> and the GPIO framework will invert the signal if needed according to
> the active low property of the GPIO.

Yes, I had got that so far.

> This behavior is based on the assumption that the active low flag
> represents an inverter present on the board, and thus unknown to the
> device driver. Now I just hope this assumption is correct. On the
> other hand I'm not sure I would understand the need for it if it were
> to model a property of the device, as the driver would be aware of it
> anyway, and thus should not need to be told about it explicitly.
> 
> > As an example, if my device datasheet states that the reset input is
> > active low, an no inverter is present on the GPIO line, should I set the
> > GPIO active low flag in DT and set the GPIO value to 1 in software
> > (assuming I use the gpiod_* API) to make the reset signal active, or
> > should I set the GPIO active high flag in DT and the the GPIO value to 0
> > in software ?
> 
> If your datasheet explicitly states that the reset input is active low, then
> this fact should be captured by the compatible property already (since the
> active low status is true for each and every compatible device) and I don't
> think your node would need to state it in addition. Unless, of course, there
> is an inverter on the way.

Just to clarify.

Any inverter on the board would of course need to be modeled in DT, using the 
active low/high flag. The inverter inside the device is indeed captured by the 
compatible property, so I would expect my driver to assert reset with

	gpiod_set_value(dev->reset, 0);

and use the active high flag in DT. This will result in a low level on the 
reset pin, asserting the reset. If an inverter is then present on the board, 
the active low flag would be specified in DT, resulting in a high level on the 
input of the inverter and a low level on the reset pin, without any change to 
the driver.

I think this is my preferred way of operation, as opposed to calling

	gpiod_set_value(dev->reset, 1);

in the driver and setting the active low flag in DT when no inverter is 
present on the board and the active high flag when an inverter is present.

Now, this should be documented, and device drivers will likely need to be 
reviewed. I'm pretty sure most devices with active low inputs currently use 
the active low flag in DT, in contradiction with the above.

> This is my understanding but let's see what others have to say and let's
> also make sure to update the documentation to lift the ambiguity. :)

-- 
Regards,

Laurent Pinchart


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 15:13   ` Laurent Pinchart
@ 2014-02-10 16:56     ` Stephen Warren
  2014-02-10 16:57       ` Stephen Warren
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2014-02-10 16:56 UTC (permalink / raw)
  To: Laurent Pinchart, Alexandre Courbot; +Cc: linux-gpio, Linus Walleij

On 02/10/2014 08:13 AM, Laurent Pinchart wrote:
> Hi Alexander,
> 
> Thank you for your quick answer.
> 
> On Monday 10 February 2014 23:50:40 Alexandre Courbot wrote:
>> On Mon, Feb 10, 2014 at 11:33 PM, Laurent Pinchart wrote:
>>> Hello everybody,
>>>
>>> While working on Dt support for a driver that uses GPIO I came to ponder
>>> about the correct meaning of the GPIO active low flag. I'm bringing the
>>> question to the mailing list to get feedback.
>>>
>>> When a device has an active low input, the fact that the input is active
>>> low is a property of the device, and thus known to the driver. On the
>>> other hand, if an inverter is present on the board, that information
>>> isn't known to device drivers and need to be expressed in DT.
>>>
>>> Does the active low flag express the latter only, or both of them ? To ask
>>> the question differently, should the low flag model the inverter inside
>>> the device, known to the device driver, effectively moving handling of
>>> that inverter out of the device driver to the core code, or should it
>>> stop at the device boundary and only model the board ?
>>
>> The intent behind the current behavior of the gpiod interface is to
>> avoid the need for code like this:
>>
>>         if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
>>             gpio_set_value(pb->enable_gpio, 0);
>>         else
>>             gpio_set_value(pb->enable_gpio, 1);
>>
>> which could simply be replaced by:
>>
>>         gpiod_set_value(pb->enable_gpio, 1);
>>
>> and the GPIO framework will invert the signal if needed according to
>> the active low property of the GPIO.
> 
> Yes, I had got that so far.
> 
>> This behavior is based on the assumption that the active low flag
>> represents an inverter present on the board, and thus unknown to the
>> device driver. Now I just hope this assumption is correct. On the
>> other hand I'm not sure I would understand the need for it if it were
>> to model a property of the device, as the driver would be aware of it
>> anyway, and thus should not need to be told about it explicitly.
>>
>>> As an example, if my device datasheet states that the reset input is
>>> active low, an no inverter is present on the GPIO line, should I set the
>>> GPIO active low flag in DT and set the GPIO value to 1 in software
>>> (assuming I use the gpiod_* API) to make the reset signal active, or
>>> should I set the GPIO active high flag in DT and the the GPIO value to 0
>>> in software ?
>>
>> If your datasheet explicitly states that the reset input is active low, then
>> this fact should be captured by the compatible property already (since the
>> active low status is true for each and every compatible device) and I don't
>> think your node would need to state it in addition. Unless, of course, there
>> is an inverter on the way.
> 
> Just to clarify.
> 
> Any inverter on the board would of course need to be modeled in DT, using the 
> active low/high flag. The inverter inside the device is indeed captured by the 
> compatible property, so I would expect my driver to assert reset with
> 
> 	gpiod_set_value(dev->reset, 0);
> 
> and use the active high flag in DT.

I think the flag should represent the physical level of the signal on
the board at the device pin. I'm pretty sure that's what's most
consistent with existing DT properties.

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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 16:56     ` Stephen Warren
@ 2014-02-10 16:57       ` Stephen Warren
  2014-02-10 17:52         ` Laurent Pinchart
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2014-02-10 16:57 UTC (permalink / raw)
  To: Laurent Pinchart, Alexandre Courbot; +Cc: linux-gpio, Linus Walleij

On 02/10/2014 09:56 AM, Stephen Warren wrote:
> On 02/10/2014 08:13 AM, Laurent Pinchart wrote:
>> Hi Alexander,
>>
>> Thank you for your quick answer.
>>
>> On Monday 10 February 2014 23:50:40 Alexandre Courbot wrote:
>>> On Mon, Feb 10, 2014 at 11:33 PM, Laurent Pinchart wrote:
>>>> Hello everybody,
>>>>
>>>> While working on Dt support for a driver that uses GPIO I came to ponder
>>>> about the correct meaning of the GPIO active low flag. I'm bringing the
>>>> question to the mailing list to get feedback.
>>>>
>>>> When a device has an active low input, the fact that the input is active
>>>> low is a property of the device, and thus known to the driver. On the
>>>> other hand, if an inverter is present on the board, that information
>>>> isn't known to device drivers and need to be expressed in DT.
>>>>
>>>> Does the active low flag express the latter only, or both of them ? To ask
>>>> the question differently, should the low flag model the inverter inside
>>>> the device, known to the device driver, effectively moving handling of
>>>> that inverter out of the device driver to the core code, or should it
>>>> stop at the device boundary and only model the board ?
>>>
>>> The intent behind the current behavior of the gpiod interface is to
>>> avoid the need for code like this:
>>>
>>>         if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
>>>             gpio_set_value(pb->enable_gpio, 0);
>>>         else
>>>             gpio_set_value(pb->enable_gpio, 1);
>>>
>>> which could simply be replaced by:
>>>
>>>         gpiod_set_value(pb->enable_gpio, 1);
>>>
>>> and the GPIO framework will invert the signal if needed according to
>>> the active low property of the GPIO.
>>
>> Yes, I had got that so far.
>>
>>> This behavior is based on the assumption that the active low flag
>>> represents an inverter present on the board, and thus unknown to the
>>> device driver. Now I just hope this assumption is correct. On the
>>> other hand I'm not sure I would understand the need for it if it were
>>> to model a property of the device, as the driver would be aware of it
>>> anyway, and thus should not need to be told about it explicitly.
>>>
>>>> As an example, if my device datasheet states that the reset input is
>>>> active low, an no inverter is present on the GPIO line, should I set the
>>>> GPIO active low flag in DT and set the GPIO value to 1 in software
>>>> (assuming I use the gpiod_* API) to make the reset signal active, or
>>>> should I set the GPIO active high flag in DT and the the GPIO value to 0
>>>> in software ?
>>>
>>> If your datasheet explicitly states that the reset input is active low, then
>>> this fact should be captured by the compatible property already (since the
>>> active low status is true for each and every compatible device) and I don't
>>> think your node would need to state it in addition. Unless, of course, there
>>> is an inverter on the way.
>>
>> Just to clarify.
>>
>> Any inverter on the board would of course need to be modeled in DT, using the 
>> active low/high flag. The inverter inside the device is indeed captured by the 
>> compatible property, so I would expect my driver to assert reset with
>>
>> 	gpiod_set_value(dev->reset, 0);
>>
>> and use the active high flag in DT.
> 
> I think the flag should represent the physical level of the signal on
> the board at the device pin. I'm pretty sure that's what's most
> consistent with existing DT properties.

(That would have to be the GPIO source device, in order to account for
any board-induced inversion)


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 16:57       ` Stephen Warren
@ 2014-02-10 17:52         ` Laurent Pinchart
  2014-02-10 23:04           ` Stephen Warren
  0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-10 17:52 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

Hi Stephen,

On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
> On 02/10/2014 09:56 AM, Stephen Warren wrote:
> > On 02/10/2014 08:13 AM, Laurent Pinchart wrote:
> >> On Monday 10 February 2014 23:50:40 Alexandre Courbot wrote:
> >>> On Mon, Feb 10, 2014 at 11:33 PM, Laurent Pinchart wrote:
> >>>> Hello everybody,
> >>>> 
> >>>> While working on Dt support for a driver that uses GPIO I came to
> >>>> ponder about the correct meaning of the GPIO active low flag. I'm
> >>>> bringing the question to the mailing list to get feedback.
> >>>> 
> >>>> When a device has an active low input, the fact that the input is
> >>>> active low is a property of the device, and thus known to the driver.
> >>>> On the other hand, if an inverter is present on the board, that
> >>>> information isn't known to device drivers and need to be expressed in
> >>>> DT.
> >>>> 
> >>>> Does the active low flag express the latter only, or both of them ? To
> >>>> ask the question differently, should the low flag model the inverter
> >>>> inside the device, known to the device driver, effectively moving
> >>>> handling of that inverter out of the device driver to the core code, or
> >>>> should it stop at the device boundary and only model the board ?
> >>> 
> >>> The intent behind the current behavior of the gpiod interface is to
> >>> 
> >>> avoid the need for code like this:
> >>>         if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
> >>>             gpio_set_value(pb->enable_gpio, 0);
> >>>         else
> >>>             gpio_set_value(pb->enable_gpio, 1);
> >>> 
> >>> which could simply be replaced by:
> >>>         gpiod_set_value(pb->enable_gpio, 1);
> >>> 
> >>> and the GPIO framework will invert the signal if needed according to
> >>> the active low property of the GPIO.
> >> 
> >> Yes, I had got that so far.
> >> 
> >>> This behavior is based on the assumption that the active low flag
> >>> represents an inverter present on the board, and thus unknown to the
> >>> device driver. Now I just hope this assumption is correct. On the
> >>> other hand I'm not sure I would understand the need for it if it were
> >>> to model a property of the device, as the driver would be aware of it
> >>> anyway, and thus should not need to be told about it explicitly.
> >>> 
> >>>> As an example, if my device datasheet states that the reset input is
> >>>> active low, an no inverter is present on the GPIO line, should I set
> >>>> the GPIO active low flag in DT and set the GPIO value to 1 in software
> >>>> (assuming I use the gpiod_* API) to make the reset signal active, or
> >>>> should I set the GPIO active high flag in DT and the the GPIO value to
> >>>> 0 in software ?
> >>> 
> >>> If your datasheet explicitly states that the reset input is active low,
> >>> then this fact should be captured by the compatible property already
> >>> (since the active low status is true for each and every compatible
> >>> device) and I don't think your node would need to state it in addition.
> >>> Unless, of course, there is an inverter on the way.
> >> 
> >> Just to clarify.
> >> 
> >> Any inverter on the board would of course need to be modeled in DT, using
> >> the active low/high flag. The inverter inside the device is indeed
> >> captured by the compatible property, so I would expect my driver to
> >> assert reset with
> >>
> >> 	gpiod_set_value(dev->reset, 0);
> >> 
> >> and use the active high flag in DT.
> > 
> > I think the flag should represent the physical level of the signal on
> > the board at the device pin. I'm pretty sure that's what's most
> > consistent with existing DT properties.
> 
> (That would have to be the GPIO source device, in order to account for
> any board-induced inversion)

Would that be the physical level at the GPIO source device output to achieve a 
high level at the target device input pin, or the physical level at the GPIO 
source device output to assert the signal at the target device input pin ? The 
first case wouldn't take the receiver device internal inverter into account 
while the second case would. In the second case, how should we handle receiver 
devices that have configurable signal polarities (essentially 
enabling/disabling the internal inverter from a software-controller 
configuration) ?

-- 
Regards,

Laurent Pinchart


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 17:52         ` Laurent Pinchart
@ 2014-02-10 23:04           ` Stephen Warren
  2014-02-10 23:21             ` Laurent Pinchart
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2014-02-10 23:04 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
...
>>> I think the flag should represent the physical level of the signal on
>>> the board at the device pin. I'm pretty sure that's what's most
>>> consistent with existing DT properties.
>>
>> (That would have to be the GPIO source device, in order to account for
>> any board-induced inversion)
> 
> Would that be the physical level at the GPIO source device output to achieve a 
> high level at the target device input pin, or the physical level at the GPIO 
> source device output to assert the signal at the target device input pin ? The 
> first case wouldn't take the receiver device internal inverter into account 
> while the second case would. In the second case, how should we handle receiver 
> devices that have configurable signal polarities (essentially 
> enabling/disabling the internal inverter from a software-controller 
> configuration) ?

I would expect the flag to represent the physical level that achieves
(or represents, for inputs) a logically asserted value at the device.

I don't think we should make the level flag influence any kind of
configurable level within the device; that's a separate orthogonal, but
related, concept. It'd be best if the DT binding for the device either
(a) provided a separate property to configure that, or (b) picked a
single one of the configurable values, and documented that all DTs
should assume that value.

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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 23:04           ` Stephen Warren
@ 2014-02-10 23:21             ` Laurent Pinchart
  2014-02-12 16:50               ` Stephen Warren
  0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-10 23:21 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

Hi Stephen,

On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
> > On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
> >> On 02/10/2014 09:56 AM, Stephen Warren wrote:
> ...
> 
> >>> I think the flag should represent the physical level of the signal on
> >>> the board at the device pin. I'm pretty sure that's what's most
> >>> consistent with existing DT properties.
> >> 
> >> (That would have to be the GPIO source device, in order to account for
> >> any board-induced inversion)
> > 
> > Would that be the physical level at the GPIO source device output to
> > achieve a high level at the target device input pin, or the physical
> > level at the GPIO source device output to assert the signal at the target
> > device input pin ? The first case wouldn't take the receiver device
> > internal inverter into account while the second case would. In the second
> > case, how should we handle receiver devices that have configurable signal
> > polarities (essentially enabling/disabling the internal inverter from a
> > software-controller configuration) ?
> 
> I would expect the flag to represent the physical level that achieves (or
> represents, for inputs) a logically asserted value at the device.

I assume you mean "the physical level at the GPIO controller output".

> I don't think we should make the level flag influence any kind of
> configurable level within the device; that's a separate orthogonal, but
> related, concept. It'd be best if the DT binding for the device either
> (a) provided a separate property to configure that, or (b) picked a
> single one of the configurable values, and documented that all DTs
> should assume that value.

Agreed. I've phrased my question incorrectly though.

My concern with devices that have configurable input polarities is that the 
"physical level [at the GPIO controller output] that achieves (or represents, 
for inputs) a logically asserted value at the device" depends on runtime 
configuration of the device, and is thus ill-defined.

We could consider that the flag represents the physical level at the GPIO 
controller output that achieves (or represents, for inputs) a logically 
asserted value at the device, for the default configuration of the device. The 
default configuration of the device would then need to be defined. I'm unsure 
whether the default configuration should be constant, or could depend on other 
DT properties.

-- 
Regards,

Laurent Pinchart


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-10 23:21             ` Laurent Pinchart
@ 2014-02-12 16:50               ` Stephen Warren
  2014-02-13 14:43                 ` Laurent Pinchart
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2014-02-12 16:50 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

On 02/10/2014 04:21 PM, Laurent Pinchart wrote:
> Hi Stephen,
> 
> On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
>> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
>>> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
>>>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
>> ...
>>
>>>>> I think the flag should represent the physical level of the signal on
>>>>> the board at the device pin. I'm pretty sure that's what's most
>>>>> consistent with existing DT properties.
>>>>
>>>> (That would have to be the GPIO source device, in order to account for
>>>> any board-induced inversion)
>>>
>>> Would that be the physical level at the GPIO source device output to
>>> achieve a high level at the target device input pin, or the physical
>>> level at the GPIO source device output to assert the signal at the target
>>> device input pin ? The first case wouldn't take the receiver device
>>> internal inverter into account while the second case would. In the second
>>> case, how should we handle receiver devices that have configurable signal
>>> polarities (essentially enabling/disabling the internal inverter from a
>>> software-controller configuration) ?
>>
>> I would expect the flag to represent the physical level that achieves (or
>> represents, for inputs) a logically asserted value at the device.
> 
> I assume you mean "the physical level at the GPIO controller output".

Yes.

>> I don't think we should make the level flag influence any kind of
>> configurable level within the device; that's a separate orthogonal, but
>> related, concept. It'd be best if the DT binding for the device either
>> (a) provided a separate property to configure that, or (b) picked a
>> single one of the configurable values, and documented that all DTs
>> should assume that value.
> 
> Agreed. I've phrased my question incorrectly though.
> 
> My concern with devices that have configurable input polarities is that the 

s/input/output/ I assume?

> "physical level [at the GPIO controller output] that achieves (or represents, 
> for inputs) a logically asserted value at the device" depends on runtime 
> configuration of the device, and is thus ill-defined.

I think for DT, we can define what the runtime state must be, as I
mentioned above.

> We could consider that the flag represents the physical level at the GPIO 
> controller output that achieves (or represents, for inputs) a logically 
> asserted value at the device, for the default configuration of the device. The 
> default configuration of the device would then need to be defined. I'm unsure 
> whether the default configuration should be constant, or could depend on other 
> DT properties.

Either would work, so long as the exact meaning of the DT content was
well-defined, and statically defined.

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

* Re: Correct meaning of the GPIO active low flag
  2014-02-12 16:50               ` Stephen Warren
@ 2014-02-13 14:43                 ` Laurent Pinchart
  2014-02-13 16:49                   ` Stephen Warren
  0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-13 14:43 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

Hi Stephen,

On Wednesday 12 February 2014 09:50:37 Stephen Warren wrote:
> On 02/10/2014 04:21 PM, Laurent Pinchart wrote:
> > On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
> >> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
> >>> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
> >>>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
> >> ...
> >> 
> >>>>> I think the flag should represent the physical level of the signal on
> >>>>> the board at the device pin. I'm pretty sure that's what's most
> >>>>> consistent with existing DT properties.
> >>>> 
> >>>> (That would have to be the GPIO source device, in order to account for
> >>>> any board-induced inversion)
> >>> 
> >>> Would that be the physical level at the GPIO source device output to
> >>> achieve a high level at the target device input pin, or the physical
> >>> level at the GPIO source device output to assert the signal at the
> >>> target device input pin ? The first case wouldn't take the receiver
> >>> device internal inverter into account while the second case would. In
> >>> the second case, how should we handle receiver devices that have
> >>> configurable signal polarities (essentially enabling/disabling the
> >>> internal inverter from a software-controller configuration) ?
> >> 
> >> I would expect the flag to represent the physical level that achieves (or
> >> represents, for inputs) a logically asserted value at the device.
> > 
> > I assume you mean "the physical level at the GPIO controller output".
> 
> Yes.
> 
> >> I don't think we should make the level flag influence any kind of
> >> configurable level within the device; that's a separate orthogonal, but
> >> related, concept. It'd be best if the DT binding for the device either
> >> (a) provided a separate property to configure that, or (b) picked a
> >> single one of the configurable values, and documented that all DTs
> >> should assume that value.
> > 
> > Agreed. I've phrased my question incorrectly though.
> > 
> > My concern with devices that have configurable input polarities is that
> > the
> 
> s/input/output/ I assume?

No, I mean input. Think about video vertical/horizontal sync inputs, they 
usually have configurable polarities on the receiver side. In that case the 
physical level at the GPIO controller output that achieves a logically 
asserted value at the device depends on how the device is configured at 
runtime.

Moving the input's internal inverter out of the GPIO flag would solve this 
problem in that the flag would instead represent the physical level at the 
GPIO controller output that achieves a high level at the device (option 'a').

That might be considered counterintuitive though. The other option ('b') I can 
think of is the one I mentioned, representing "the physical level at the GPIO 
controller output that achieves (or represents, for inputs) a logically 
asserted value at the device, for the default configuration of the device" 
with the default configuration of the device defined in the DT bindings.

Option 'b' is more complex in the sense that it requires expressing more 
information in the DT bindings.

> > "physical level [at the GPIO controller output] that achieves (or
> > represents, for inputs) a logically asserted value at the device" depends
> > on runtime configuration of the device, and is thus ill-defined.
> 
> I think for DT, we can define what the runtime state must be, as I mentioned
> above.
> 
> > We could consider that the flag represents the physical level at the GPIO
> > controller output that achieves (or represents, for inputs) a logically
> > asserted value at the device, for the default configuration of the device.
> > The default configuration of the device would then need to be defined.
> > I'm unsure whether the default configuration should be constant, or could
> > depend on other DT properties.
> 
> Either would work, so long as the exact meaning of the DT content was well-
> defined, and statically defined.

-- 
Regards,

Laurent Pinchart


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-13 14:43                 ` Laurent Pinchart
@ 2014-02-13 16:49                   ` Stephen Warren
  2014-02-14 23:48                     ` Laurent Pinchart
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2014-02-13 16:49 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

On 02/13/2014 07:43 AM, Laurent Pinchart wrote:
> Hi Stephen,
> 
> On Wednesday 12 February 2014 09:50:37 Stephen Warren wrote:
>> On 02/10/2014 04:21 PM, Laurent Pinchart wrote:
>>> On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
>>>> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
>>>>> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
>>>>>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
>>>> ...
>>>>
>>>>>>> I think the flag should represent the physical level of the signal on
>>>>>>> the board at the device pin. I'm pretty sure that's what's most
>>>>>>> consistent with existing DT properties.
>>>>>>
>>>>>> (That would have to be the GPIO source device, in order to account for
>>>>>> any board-induced inversion)
>>>>>
>>>>> Would that be the physical level at the GPIO source device output to
>>>>> achieve a high level at the target device input pin, or the physical
>>>>> level at the GPIO source device output to assert the signal at the
>>>>> target device input pin ? The first case wouldn't take the receiver
>>>>> device internal inverter into account while the second case would. In
>>>>> the second case, how should we handle receiver devices that have
>>>>> configurable signal polarities (essentially enabling/disabling the
>>>>> internal inverter from a software-controller configuration) ?
>>>>
>>>> I would expect the flag to represent the physical level that achieves (or
>>>> represents, for inputs) a logically asserted value at the device.
>>>
>>> I assume you mean "the physical level at the GPIO controller output".
>>
>> Yes.
>>
>>>> I don't think we should make the level flag influence any kind of
>>>> configurable level within the device; that's a separate orthogonal, but
>>>> related, concept. It'd be best if the DT binding for the device either
>>>> (a) provided a separate property to configure that, or (b) picked a
>>>> single one of the configurable values, and documented that all DTs
>>>> should assume that value.
>>>
>>> Agreed. I've phrased my question incorrectly though.
>>>
>>> My concern with devices that have configurable input polarities is that
>>> the
>>
>> s/input/output/ I assume?
> 
> No, I mean input.

OK, I guess I was thinking about GPIO inputs then; the same discussion
applies in reverse.

> Think about video vertical/horizontal sync inputs, they 
> usually have configurable polarities on the receiver side. In that case the 
> physical level at the GPIO controller output that achieves a logically 
> asserted value at the device depends on how the device is configured at 
> runtime.

Sure.

I think the GPIO specifier should specify the signal polarity required
to get a logically asserted signal to the device. If the device can be
configured to accept different signal polarities as logically asserted,
then that must indeed be a separate DT property to the GPIO specifier,
since the GPIO specifier's format and semantics are only meaningful (and
parsable/interpretable) to the GPIO controller, and not the GPIO consumer.

Similarly, for inputs to the GPIO controller, the GPIO specifier should
specify the value at the GPIO controller too, and any configuration of
the output polarity of the device should be a separate property of that
device.

The same argument applies to IRQs.

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

* Re: Correct meaning of the GPIO active low flag
  2014-02-13 16:49                   ` Stephen Warren
@ 2014-02-14 23:48                     ` Laurent Pinchart
  2014-02-15  0:07                       ` Stephen Warren
  0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-14 23:48 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

Hi Stephen,

On Thursday 13 February 2014 09:49:57 Stephen Warren wrote:
> On 02/13/2014 07:43 AM, Laurent Pinchart wrote:
> > On Wednesday 12 February 2014 09:50:37 Stephen Warren wrote:
> >> On 02/10/2014 04:21 PM, Laurent Pinchart wrote:
> >>> On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
> >>>> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
> >>>>> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
> >>>>>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
> >>>> ...
> >>>> 
> >>>>>>> I think the flag should represent the physical level of the signal
> >>>>>>> on the board at the device pin. I'm pretty sure that's what's most
> >>>>>>> consistent with existing DT properties.
> >>>>>> 
> >>>>>> (That would have to be the GPIO source device, in order to account
> >>>>>> for any board-induced inversion)
> >>>>> 
> >>>>> Would that be the physical level at the GPIO source device output to
> >>>>> achieve a high level at the target device input pin, or the physical
> >>>>> level at the GPIO source device output to assert the signal at the
> >>>>> target device input pin ? The first case wouldn't take the receiver
> >>>>> device internal inverter into account while the second case would. In
> >>>>> the second case, how should we handle receiver devices that have
> >>>>> configurable signal polarities (essentially enabling/disabling the
> >>>>> internal inverter from a software-controller configuration) ?
> >>>> 
> >>>> I would expect the flag to represent the physical level that achieves
> >>>> (or represents, for inputs) a logically asserted value at the device.
> >>> 
> >>> I assume you mean "the physical level at the GPIO controller output".
> >> 
> >> Yes.
> >> 
> >>>> I don't think we should make the level flag influence any kind of
> >>>> configurable level within the device; that's a separate orthogonal, but
> >>>> related, concept. It'd be best if the DT binding for the device either
> >>>> (a) provided a separate property to configure that, or (b) picked a
> >>>> single one of the configurable values, and documented that all DTs
> >>>> should assume that value.
> >>> 
> >>> Agreed. I've phrased my question incorrectly though.
> >>> 
> >>> My concern with devices that have configurable input polarities is that
> >>> the
> >> 
> >> s/input/output/ I assume?
> > 
> > No, I mean input.
> 
> OK, I guess I was thinking about GPIO inputs then; the same discussion
> applies in reverse.
> 
> > Think about video vertical/horizontal sync inputs, they usually have
> > configurable polarities on the receiver side. In that case the physical
> > level at the GPIO controller output that achieves a logically asserted
> > value at the device depends on how the device is configured at runtime.
> 
> Sure.
> 
> I think the GPIO specifier should specify the signal polarity required
> to get a logically asserted signal to the device. If the device can be
> configured to accept different signal polarities as logically asserted,
> then that must indeed be a separate DT property to the GPIO specifier,
> since the GPIO specifier's format and semantics are only meaningful (and
> parsable/interpretable) to the GPIO controller, and not the GPIO consumer.

Agreed, but that's not my point (or maybe I've just not understood that you 
got my point). The small detail that made me concerned in the first place is 
that, when the device input polarity is configurable, the "logically asserted 
signal" state becomes dynamic. How do we define the DT GPIO polarity in that 
case ?

Let me take a (slightly made up) example. Let's assume a chip with a control 
input signal connected to a GPIO output of an SoC without any inverter on the 
board. The input polarity is runtime configurable (through I2C for instance). 
The chip has two modes of operation (USB host or USB function for instance, or 
ethernet link speed, ...), and for some reason, we need the input to be active 
high in mode A and active low in mode B. This is handled in the chip device 
driver that configures the input polarity based on the mode.

The chip DT bindings will have a property that contains a reference to the 
GPIO connected to the control input, and a flag to set the GPIO polarity. We 
want that flag to express the physical level at the GPIO source to achieve an 
asserted level at the chip input. That's active high in mode A and active low 
in mode B (and would be the opposite if we had an inverter on the board). Now, 
as the mode of operation is fully dynamic, what value should the DT flag have 
(assuming there's no default mode from a chip point of view) ?

> Similarly, for inputs to the GPIO controller, the GPIO specifier should
> specify the value at the GPIO controller too, and any configuration of
> the output polarity of the device should be a separate property of that
> device.
> 
> The same argument applies to IRQs.

-- 
Regards,

Laurent Pinchart


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-14 23:48                     ` Laurent Pinchart
@ 2014-02-15  0:07                       ` Stephen Warren
  2014-02-15  0:20                         ` Laurent Pinchart
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2014-02-15  0:07 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

On 02/14/2014 04:48 PM, Laurent Pinchart wrote:
> Hi Stephen,
> 
> On Thursday 13 February 2014 09:49:57 Stephen Warren wrote:
>> On 02/13/2014 07:43 AM, Laurent Pinchart wrote:
>>> On Wednesday 12 February 2014 09:50:37 Stephen Warren wrote:
>>>> On 02/10/2014 04:21 PM, Laurent Pinchart wrote:
>>>>> On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
>>>>>> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
>>>>>>> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
>>>>>>>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
>>>>>> ...
>>>>>>
>>>>>>>>> I think the flag should represent the physical level of the signal
>>>>>>>>> on the board at the device pin. I'm pretty sure that's what's most
>>>>>>>>> consistent with existing DT properties.
>>>>>>>>
>>>>>>>> (That would have to be the GPIO source device, in order to account
>>>>>>>> for any board-induced inversion)
>>>>>>>
>>>>>>> Would that be the physical level at the GPIO source device output to
>>>>>>> achieve a high level at the target device input pin, or the physical
>>>>>>> level at the GPIO source device output to assert the signal at the
>>>>>>> target device input pin ? The first case wouldn't take the receiver
>>>>>>> device internal inverter into account while the second case would. In
>>>>>>> the second case, how should we handle receiver devices that have
>>>>>>> configurable signal polarities (essentially enabling/disabling the
>>>>>>> internal inverter from a software-controller configuration) ?
>>>>>>
>>>>>> I would expect the flag to represent the physical level that achieves
>>>>>> (or represents, for inputs) a logically asserted value at the device.
>>>>>
>>>>> I assume you mean "the physical level at the GPIO controller output".
>>>>
>>>> Yes.
>>>>
>>>>>> I don't think we should make the level flag influence any kind of
>>>>>> configurable level within the device; that's a separate orthogonal, but
>>>>>> related, concept. It'd be best if the DT binding for the device either
>>>>>> (a) provided a separate property to configure that, or (b) picked a
>>>>>> single one of the configurable values, and documented that all DTs
>>>>>> should assume that value.
>>>>>
>>>>> Agreed. I've phrased my question incorrectly though.
>>>>>
>>>>> My concern with devices that have configurable input polarities is that
>>>>> the
>>>>
>>>> s/input/output/ I assume?
>>>
>>> No, I mean input.
>>
>> OK, I guess I was thinking about GPIO inputs then; the same discussion
>> applies in reverse.
>>
>>> Think about video vertical/horizontal sync inputs, they usually have
>>> configurable polarities on the receiver side. In that case the physical
>>> level at the GPIO controller output that achieves a logically asserted
>>> value at the device depends on how the device is configured at runtime.
>>
>> Sure.
>>
>> I think the GPIO specifier should specify the signal polarity required
>> to get a logically asserted signal to the device. If the device can be
>> configured to accept different signal polarities as logically asserted,
>> then that must indeed be a separate DT property to the GPIO specifier,
>> since the GPIO specifier's format and semantics are only meaningful (and
>> parsable/interpretable) to the GPIO controller, and not the GPIO consumer.
> 
> Agreed, but that's not my point (or maybe I've just not understood that you 
> got my point). The small detail that made me concerned in the first place is 
> that, when the device input polarity is configurable, the "logically asserted 
> signal" state becomes dynamic. How do we define the DT GPIO polarity in that 
> case ?

Pick whichever polarity you want. Write that polarity into the GPIO
specifier's flags. If there's an inverter on the board between the GPIO
controller pin and the remote device pin, you need an extra property in
the device to specify how to program the polarity of the signal at the
device end too.

Perhaps the following will make my thoughts clearer:

http://www.spinics.net/lists/arm-kernel/msg307927.html
[PATCH 1/2] mfd: palmas: support IRQ inversion at the board level

http://www.spinics.net/lists/arm-kernel/msg307926.html
[PATCH 2/2] ARM: tegra: fix Dalmore PMIC IRQ polarity

> Let me take a (slightly made up) example. Let's assume a chip with a control 
> input signal connected to a GPIO output of an SoC without any inverter on the 
> board. The input polarity is runtime configurable (through I2C for instance). 
> The chip has two modes of operation (USB host or USB function for instance, or 
> ethernet link speed, ...), and for some reason, we need the input to be active 
> high in mode A and active low in mode B. This is handled in the chip device 
> driver that configures the input polarity based on the mode.

Oh, if you're talking about fiddling around at run-time, then that's
just something the driver has to deal with internally. In that case,
let's just make the GPIO active-high in DT. When the driver programs the
device into whatever mode requires it to be active-low, the driver needs
to be written to set that GPIO value correctly. I don't think DT has any
influence on this at all, since DT is about a static setup, whereas your
use-case is dynamic.

> The chip DT bindings will have a property that contains a reference to the 
> GPIO connected to the control input, and a flag to set the GPIO polarity. We 
> want that flag to express the physical level at the GPIO source to achieve an 
> asserted level at the chip input. That's active high in mode A and active low 
> in mode B (and would be the opposite if we had an inverter on the board). Now, 
> as the mode of operation is fully dynamic, what value should the DT flag have 
> (assuming there's no default mode from a chip point of view) ?
> 
>> Similarly, for inputs to the GPIO controller, the GPIO specifier should
>> specify the value at the GPIO controller too, and any configuration of
>> the output polarity of the device should be a separate property of that
>> device.
>>
>> The same argument applies to IRQs.


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-15  0:07                       ` Stephen Warren
@ 2014-02-15  0:20                         ` Laurent Pinchart
  2014-02-18 17:58                           ` Stephen Warren
  0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-15  0:20 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

Hi Stephen,

On Friday 14 February 2014 17:07:32 Stephen Warren wrote:
> On 02/14/2014 04:48 PM, Laurent Pinchart wrote:
> > On Thursday 13 February 2014 09:49:57 Stephen Warren wrote:
> >> On 02/13/2014 07:43 AM, Laurent Pinchart wrote:
> >>> On Wednesday 12 February 2014 09:50:37 Stephen Warren wrote:
> >>>> On 02/10/2014 04:21 PM, Laurent Pinchart wrote:
> >>>>> On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
> >>>>>> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
> >>>>>>> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
> >>>>>>>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
> >>>>>> ...
> >>>>>> 
> >>>>>>>>> I think the flag should represent the physical level of the signal
> >>>>>>>>> on the board at the device pin. I'm pretty sure that's what's most
> >>>>>>>>> consistent with existing DT properties.
> >>>>>>>> 
> >>>>>>>> (That would have to be the GPIO source device, in order to account
> >>>>>>>> for any board-induced inversion)
> >>>>>>> 
> >>>>>>> Would that be the physical level at the GPIO source device output to
> >>>>>>> achieve a high level at the target device input pin, or the physical
> >>>>>>> level at the GPIO source device output to assert the signal at the
> >>>>>>> target device input pin ? The first case wouldn't take the receiver
> >>>>>>> device internal inverter into account while the second case would.
> >>>>>>> In the second case, how should we handle receiver devices that have
> >>>>>>> configurable signal polarities (essentially enabling/disabling the
> >>>>>>> internal inverter from a software-controller configuration) ?
> >>>>>> 
> >>>>>> I would expect the flag to represent the physical level that achieves
> >>>>>> (or represents, for inputs) a logically asserted value at the device.
> >>>>> 
> >>>>> I assume you mean "the physical level at the GPIO controller output".
> >>>> 
> >>>> Yes.
> >>>> 
> >>>>>> I don't think we should make the level flag influence any kind of
> >>>>>> configurable level within the device; that's a separate orthogonal,
> >>>>>> but related, concept. It'd be best if the DT binding for the device
> >>>>>> either (a) provided a separate property to configure that, or (b)
> >>>>>> picked a single one of the configurable values, and documented that
> >>>>>> all DTs should assume that value.
> >>>>> 
> >>>>> Agreed. I've phrased my question incorrectly though.
> >>>>> 
> >>>>> My concern with devices that have configurable input polarities is
> >>>>> that the
> >>>> 
> >>>> s/input/output/ I assume?
> >>> 
> >>> No, I mean input.
> >> 
> >> OK, I guess I was thinking about GPIO inputs then; the same discussion
> >> applies in reverse.
> >> 
> >>> Think about video vertical/horizontal sync inputs, they usually have
> >>> configurable polarities on the receiver side. In that case the physical
> >>> level at the GPIO controller output that achieves a logically asserted
> >>> value at the device depends on how the device is configured at runtime.
> >> 
> >> Sure.
> >> 
> >> I think the GPIO specifier should specify the signal polarity required
> >> to get a logically asserted signal to the device. If the device can be
> >> configured to accept different signal polarities as logically asserted,
> >> then that must indeed be a separate DT property to the GPIO specifier,
> >> since the GPIO specifier's format and semantics are only meaningful (and
> >> parsable/interpretable) to the GPIO controller, and not the GPIO
> >> consumer.
> > 
> > Agreed, but that's not my point (or maybe I've just not understood that
> > you
> > got my point). The small detail that made me concerned in the first place
> > is that, when the device input polarity is configurable, the "logically
> > asserted signal" state becomes dynamic. How do we define the DT GPIO
> > polarity in that case ?
> 
> Pick whichever polarity you want. Write that polarity into the GPIO
> specifier's flags. If there's an inverter on the board between the GPIO
> controller pin and the remote device pin, you need an extra property in
> the device to specify how to program the polarity of the signal at the
> device end too.
> 
> Perhaps the following will make my thoughts clearer:
> 
> http://www.spinics.net/lists/arm-kernel/msg307927.html
> [PATCH 1/2] mfd: palmas: support IRQ inversion at the board level
> 
> http://www.spinics.net/lists/arm-kernel/msg307926.html
> [PATCH 2/2] ARM: tegra: fix Dalmore PMIC IRQ polarity
> 
> > Let me take a (slightly made up) example. Let's assume a chip with a
> > control input signal connected to a GPIO output of an SoC without any
> > inverter on the board. The input polarity is runtime configurable
> > (through I2C for instance). The chip has two modes of operation (USB host
> > or USB function for instance, or ethernet link speed, ...), and for some
> > reason, we need the input to be active high in mode A and active low in
> > mode B. This is handled in the chip device driver that configures the
> > input polarity based on the mode.
> 
> Oh, if you're talking about fiddling around at run-time, then that's
> just something the driver has to deal with internally. In that case,
> let's just make the GPIO active-high in DT. When the driver programs the
> device into whatever mode requires it to be active-low, the driver needs
> to be written to set that GPIO value correctly. I don't think DT has any
> influence on this at all, since DT is about a static setup, whereas your
> use-case is dynamic.

Except that there could be an inverter, which would need to be expressed in 
DT.

I propose wording the documentation as follows.

The GPIO polarity flag should represent the physical level that achieves (or 
represents, for inputs) a logically asserted value at the device. When the 
device signal polarity is dynamically configurable (as opposed to the 
statically configured case where the polarity is set based on DT properties 
only) the flag should bet set to the polarity required by the default 
logically asserted value, and that default logically asserted value should be 
documented in the device DT bindings.

> > The chip DT bindings will have a property that contains a reference to the
> > GPIO connected to the control input, and a flag to set the GPIO polarity.
> > We want that flag to express the physical level at the GPIO source to
> > achieve an asserted level at the chip input. That's active high in mode A
> > and active low in mode B (and would be the opposite if we had an inverter
> > on the board). Now, as the mode of operation is fully dynamic, what value
> > should the DT flag have (assuming there's no default mode from a chip
> > point of view) ?
> > 
> >> Similarly, for inputs to the GPIO controller, the GPIO specifier should
> >> specify the value at the GPIO controller too, and any configuration of
> >> the output polarity of the device should be a separate property of that
> >> device.
> >> 
> >> The same argument applies to IRQs.

-- 
Regards,

Laurent Pinchart


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

* Re: Correct meaning of the GPIO active low flag
  2014-02-15  0:20                         ` Laurent Pinchart
@ 2014-02-18 17:58                           ` Stephen Warren
  2014-02-19  0:19                             ` Laurent Pinchart
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2014-02-18 17:58 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

On 02/14/2014 05:20 PM, Laurent Pinchart wrote:
> On Friday 14 February 2014 17:07:32 Stephen Warren wrote:
...
>> Oh, if you're talking about fiddling around at run-time, then that's
>> just something the driver has to deal with internally. In that case,
>> let's just make the GPIO active-high in DT. When the driver programs the
>> device into whatever mode requires it to be active-low, the driver needs
>> to be written to set that GPIO value correctly. I don't think DT has any
>> influence on this at all, since DT is about a static setup, whereas your
>> use-case is dynamic.
> 
> Except that there could be an inverter, which would need to be expressed in 
> DT.

Of course, but that statement doesn't invalidate anything I said.

> I propose wording the documentation as follows.
> 
> The GPIO polarity flag should represent the physical level that achieves (or 
> represents, for inputs) a logically asserted value at the device. When the 
> device signal polarity is dynamically configurable (as opposed to the 
> statically configured case where the polarity is set based on DT properties 
> only) the flag should bet set to the polarity required by the default 
> logically asserted value, and that default logically asserted value should be 
> documented in the device DT bindings.

To me, that wording:

a) Doesn't explicitly state that the GPIO specifier should contain the
level at the GPIO controller rather than at the device.

b) Implies that devices with configurable polarity should derive the
polarity from the GPIO specifier, whereas in fact this is exactly what
we don't want.

How about:

The GPIO specifier's polarity flag should represent the physical level
at the GPIO controller that achieves (or represents, for inputs) a
logically asserted value at the device. Note that if the board inverts
the signal between the GPIO controller and device, then the GPIO
specifier will represent the opposite physical level than signal at the
device's.

When the device's signal polarity is configurable, the binding for the
device must either:

a) Define a single static polarity for the signal, with the expectation
that any software using that binding would statically program the device
to use that signal polarity.

The static choice of polarity may be either:

a1) Defined statically by the DT binding itself.

or:

a2) Dictated by a binding-specific DT property.

In particular, the polarity cannot be derived from the GPIO specifier,
since that would prevent the DT from separately representing the two
orthogonal concepts of: configurable signal polarity in the device, and
possible board-level signal inversion.

or:

b) Pick a single option for device signal polarity, and document this
choice in the binding. The GPIO specifier should represent the polarity
of the signal (at the GPIO controller) assuming that the device is
configured for this particular signal polarity choice. If the software
chooses to program the device to generate or receive a signal of the
opposite polarity, software will be responsible for correctly
interpreting (inverting) the GPIO signal at the GPIO controller.

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

* Re: Correct meaning of the GPIO active low flag
  2014-02-18 17:58                           ` Stephen Warren
@ 2014-02-19  0:19                             ` Laurent Pinchart
  0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2014-02-19  0:19 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Alexandre Courbot, linux-gpio, Linus Walleij

Hi Stephen,

On Tuesday 18 February 2014 10:58:30 Stephen Warren wrote:
> On 02/14/2014 05:20 PM, Laurent Pinchart wrote:
> > On Friday 14 February 2014 17:07:32 Stephen Warren wrote:
> ...
> 
> >> Oh, if you're talking about fiddling around at run-time, then that's
> >> just something the driver has to deal with internally. In that case,
> >> let's just make the GPIO active-high in DT. When the driver programs the
> >> device into whatever mode requires it to be active-low, the driver needs
> >> to be written to set that GPIO value correctly. I don't think DT has any
> >> influence on this at all, since DT is about a static setup, whereas your
> >> use-case is dynamic.
> > 
> > Except that there could be an inverter, which would need to be expressed
> > in DT.
> 
> Of course, but that statement doesn't invalidate anything I said.
> 
> > I propose wording the documentation as follows.
> > 
> > The GPIO polarity flag should represent the physical level that achieves
> > (or represents, for inputs) a logically asserted value at the device.
> > When the device signal polarity is dynamically configurable (as opposed
> > to the statically configured case where the polarity is set based on DT
> > properties only) the flag should bet set to the polarity required by the
> > default logically asserted value, and that default logically asserted
> > value should be documented in the device DT bindings.
> 
> To me, that wording:
> 
> a) Doesn't explicitly state that the GPIO specifier should contain the
> level at the GPIO controller rather than at the device.
> 
> b) Implies that devices with configurable polarity should derive the
> polarity from the GPIO specifier, whereas in fact this is exactly what
> we don't want.
> 
> How about:
> 
> The GPIO specifier's polarity flag should represent the physical level
> at the GPIO controller that achieves (or represents, for inputs) a
> logically asserted value at the device. Note that if the board inverts
> the signal between the GPIO controller and device, then the GPIO
> specifier will represent the opposite physical level than signal at the
> device's.

"Note that" sounds to me like we're introducing an exception. I would replace 
it with "In particular,". Apart from that your proposal looks perfect to me. 
Thanks a lot for coping with my constant nitpicking on this issue :-) Would 
you like to submit a documentation patch ?

> When the device's signal polarity is configurable, the binding for the
> device must either:
> 
> a) Define a single static polarity for the signal, with the expectation
> that any software using that binding would statically program the device
> to use that signal polarity.
> 
> The static choice of polarity may be either:
> 
> a1) Defined statically by the DT binding itself.
> 
> or:
> 
> a2) Dictated by a binding-specific DT property.
> 
> In particular, the polarity cannot be derived from the GPIO specifier,
> since that would prevent the DT from separately representing the two
> orthogonal concepts of: configurable signal polarity in the device, and
> possible board-level signal inversion.
> 
> or:
> 
> b) Pick a single option for device signal polarity, and document this
> choice in the binding. The GPIO specifier should represent the polarity
> of the signal (at the GPIO controller) assuming that the device is
> configured for this particular signal polarity choice. If the software
> chooses to program the device to generate or receive a signal of the
> opposite polarity, software will be responsible for correctly
> interpreting (inverting) the GPIO signal at the GPIO controller.

-- 
Regards,

Laurent Pinchart


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

end of thread, other threads:[~2014-02-19  0:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-10 14:33 Correct meaning of the GPIO active low flag Laurent Pinchart
2014-02-10 14:50 ` Alexandre Courbot
2014-02-10 15:13   ` Laurent Pinchart
2014-02-10 16:56     ` Stephen Warren
2014-02-10 16:57       ` Stephen Warren
2014-02-10 17:52         ` Laurent Pinchart
2014-02-10 23:04           ` Stephen Warren
2014-02-10 23:21             ` Laurent Pinchart
2014-02-12 16:50               ` Stephen Warren
2014-02-13 14:43                 ` Laurent Pinchart
2014-02-13 16:49                   ` Stephen Warren
2014-02-14 23:48                     ` Laurent Pinchart
2014-02-15  0:07                       ` Stephen Warren
2014-02-15  0:20                         ` Laurent Pinchart
2014-02-18 17:58                           ` Stephen Warren
2014-02-19  0:19                             ` Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.