[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Stephen Warren]

On 04/02/2016 09:13 AM, Eric Nelson wrote:
> Hi Peng,
>
> On 04/01/2016 10:46 PM, Peng Fan wrote:
>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>
>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>> of the flag and setting the desc->flags field to the driver
>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>
>>>> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
>>>>
>>> Thanks for pointing this out.
>>>
>>> This patch also works, but it has me confused.
>>>
>>> How/why is parsing the ACTIVE_LOW flag specific to MXC?
>>>
>>> This is a general-purpose flag in the kernel, not something machine-
>>> specific.
>>>
>>> It also appears that there are a bunch of other copies
>>> of this same bit of code in the various mach_xlate() routines:
>>>
>>> desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
>>>
>>> If it's done in gpio-uclass, this isn't needed and xlate can
>>> be removed from mxc-gpio and quite a few other architectures.
>>>
>>> Please advise,
>>
>> I saw you have posted a patch set to convert other gpio drivers.
>> But actually the translation of gpio property should be done by
>> each gpio driver. Alought we have gpio-cells=<2> for most gpio
>> drivers, but if there is one case that gpio-cells=<3>, and it have
>> different meaning for each cell from other most drivers?
>
> Which case has gpio-cells=<3>?
>
> As far as I can tell, only tegra and sandbox have something other
> than parsing of offset and the GPIO_ACTIVE_LOW flag.
>
> Tegra seems to have gpio-cells=<2> and sandbox has either 0 or 1.
>
>> So I suggest we follow the linux way,
>>
>> 434         if (!chip->of_xlate) {
>> 435                 chip->of_gpio_n_cells = 2;
>> 436                 chip->of_xlate = of_gpio_simple_xlate;
>> 437         }
>>
>> If gpio drivers does not provide xlate function, then use a simple xlate
>> function. If gpio drivers have their own xlate function, then use their
>> own way.
>
> The recommendation in device-tree-bindings/gpio/gpio.txt is to have
> the GPIO_ACTIVE_LOW/HIGH flag in the second cell, so parsing that
> directly in gpio_find_and_xlate() seems right.

That's a recommendation when designing GPIO controller bindings, not a 
definition of something that is categorically true for all bindings. 
Each binding is free to represent its flags (if any) in whatever way it 
wants, and so as Peng says, each driver has be in full control over its 
own of_xlate functionality. Now, for drivers that happen to use a common 
flag representation, we can plug in a common implementation of the xlate 
function.