Re: [PATCH v3 1/2] dt-bindings: GPIO: Add gpio-initval

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Tue, Nov 17, 2015 at 9:35 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
> On Tue, Nov 17, 2015 at 11:07:57AM +0100, Markus Pargmann wrote:
>> Add a binding for GPIO initialization.
>
> Some comments, but they are really more on the gpio hog. I must have
> missed them.

They are already merged, and in use :(

>> +Optional properties:
>> +- line-name:  The GPIO label name. If not present the node name is used.
>
> We already have "label" for this purpose.

I would rather say we already have another instance of "line-name"
for this purpose. See
Documentation/devicetree/bindings/gpio/gpio.txt

"label" was not chosen because it's a Linuxism (that is what the internal
API is using) and not to the point.

I like the clear "line" specifier over "pin" etc since GPIOs can be
chip-internal and what not.

>> + The following two options are mutually exclusive. One of them should be
>> + specified, but not both:
>> +- gpio-hog:   A property specifying that this child node represent a GPIO hog.
>> +- gpio-initval: This GPIO should be initialized to the specified configuration.
>> +
>> + The following three options are mutually exclusive. They change the setting of
>> + the GPIO pin. One of them should be specified:
>>  - input:      A property specifying to set the GPIO direction as input.
>>  - output-low  A property specifying to set the GPIO direction as output with
>>             the value low.
>>  - output-high A property specifying to set the GPIO direction as output with
>>             the value high.
>
> If they are mutually exclusive, then it should just be one property with
> values. That is much more simple to validate.
>
> But none of this is really even needed. We can do all this with existing
> bindings. Most gpio controllers use 2 cells and the 2nd cell was
> reserved for something like this. Simply adding a -gpios property in the
> gpio controller node would do the job:
>
> init-gpios = <&self n (GPIO_ACTIVE_HIGH | GPIO_OUTPUT)>;
> hog-gpios = <&self m GPIO_INPUT>;
>
> Perhaps we don't want to use GPIO_ACTIVE_x here, but we have 31 more
> free bits to use.

I prefer if the flags to be used for characteristics of the line rather
than driving it in any way. Anyway the input/output-low/output-high is
already merged, and in use for hogs.

We looked at using -gpios in a self-referential manner, but that
made it impossible to name the lines, and that is a desired
property. Hogged lines need names so we can figure out who is
using them during debug etc. It also makes the DT more
human-readable.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux