Re: [PATCH V2] regulator: gpio: Reword the binding document

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

 



marek.vasut@xxxxxxxxx writes:
> From: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
> 
> Reword the binding document to make it clear how the propeties work
> and which properties affect which other properties.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
> Cc: Harald Geyer <harald@xxxxxxxxx>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
> To: devicetree@xxxxxxxxxxxxxxx
> ---
> V2: - Make "gpios" a mandatory property
>     - Reword "gpio-states" property description
>     - Change "enable-gpio" to "enable-gpios" to match modern DT rules
> Note: The recent gpio-regulator rework caused breakage. While the
>       changes in the gpio-regulator code were according to the DT
>       binding document, they stopped working with older DTs. Make
>       the binding document clearer to prevent such breakage in the
>       future.

Thanks for the update. I think it addresses all my concerns except for
one:

> +- gpios-states	: State of GPIO pins in "gpios" array that is set until
> +			  changed by the first consumer. 0: LOW, 1: HIGH.
> +			  Default is LOW if nothing else is specified.

I still believe this not true: There is no guarantee that the regulator
core won't change the state of GPIO pins before the first consumer comes
up.

I still think my proposal describes the property more acurately:
gpios-states : On operating systems, that don't support reading back gpio
               values in output mode (most notably linux), this array
               provides the state of GPIO pins set when requesting them
               from the gpio controller. Systems, that are capable of
               preserving state when requesting the lines, are free to
               ignore this property. 0: LOW, 1: HIGH. Default is LOW if
               nothing else is specified.

Since we had this discussion already in the V1 thread and you clearly don't
agree with me, the maintainers will need to decide. You can add 
Reviewed-by: Harald Geyer <harald@xxxxxxxxx>
once Rob and/or Mark have addressed this issue.

Thanks for your work!
Harald



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux