[PATCH v2 04/17] fdt: Add basic support for decoding GPIO definitions

[Simon Glass]

Hi Stephen,

On Mon, Dec 5, 2011 at 1:46 PM, Stephen Warren <swarren at nvidia.com> wrote:
> On 12/02/2011 07:11 PM, Simon Glass wrote:
>> This adds some support into fdtdec for reading GPIO definitions from
>> the fdt. We permit up to FDT_GPIO_MAX GPIOs in the system. Each GPIO
>> is of the form:
>>
>> gpio-function-name = <phandle gpio_num flags>;
>>
>> where:
>>
>> phandle is a pointer to the GPIO node
>> gpio_num is the number of the GPIO (0 to 223)
>> flags is some flags, proposed as follows:
>>
>>    bit    meaning
>>    0      0=input, 1=output
>>    1      for output only: inital value of output
>>    2      0=polarity normal, 1=active low (inverted)
>
> The meaning of the flags (and even whether there are any flags any if so
> how many cells there are to contain them) is defined by the GPIO
> controller's binding. It's not something that can be interpreted in
> isolation by a generic DT parsing function. See for example #gpio-cells
> in tegra20.dtsi's gpio node and kernel file
> Documentation/devicetree/bindings/gpio/gpio_nvidia.txt.

I see this in my version:

Required properties:
- compatible : "nvidia,tegra20-gpio"
- #gpio-cells : Should be two. The first cell is the pin number and the
  second cell is used to specify optional parameters:
  - bit 0 specifies polarity (0 for normal, 1 for inverted)
- gpio-controller : Marks the device node as a GPIO controller.

so how do I go about adding the other two bits?

>
> This implies that a lot of the code isn't correct, but I haven't
> explicitly mentioned this everywhere for brevity.

Well it's ok - these flags are only used in one place, and only the
input/output flag in fact.

It would be nice to use constants for these in the fdt instead of
numbers. Is that in progress?

enable-propounder = <&gpio 43 OUTPUT>;

>
>> An example is:
>>
>> enable-propounder = <&gpio 43 1>;
>
> I /think/ convention is to name such properties foo-gpios rather than
> just foo. Still, this is only a comment on the example and not the code,
> since the complete property name is passed into the new functions by the
> caller.

ok, will update.

>
>> +/* For now we allow 224 GPIOs. We can extend this later if required */
>> +enum {
>> +     FDT_GPIO_NONE = 255,    /* an invalid GPIO used to end our list */
>
> Can't you use 0 for that? (the kernel currently uses -1, but it seems
> there's agreement that was a mistake). If you use 255, the number will
> have to keep getting bumped as more complex systems become supported. If
> not 0, perhaps U32_MAX or whatever the relevant ${type}_MAX is?

But 0 is a valid GPIO isn't it?

I currently use the max value available to the u8. We can change it at
will when we update the u8 type to u16 which is why I made it a
constant.

Thanks for all your help with this.

Regards,
Simon

>
> --
> nvpublic