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)

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to