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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to