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

Peng Fan
2016-04-02T05:46:14Z

Hi Eric,

On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>Hi Peng,
>
>On 03/28/2016 09:57 PM, Peng Fan wrote:
>> Hi Eric,
>>
>> 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?

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. Also I do no think delete the xlate implementation is not a good idea.

Simon may give more comments on this.

Regards,
Peng.
>
>
>Eric