On 2/18/19 10:30 AM, Geert Uytterhoeven wrote:
> Hi Marek,
>
> On Sun, Feb 17, 2019 at 8:00 AM <marek.vasut@xxxxxxxxx> wrote:
>> 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
>> ---
>> 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, adding more worms to the can, below...
>
>> --- a/Documentation/devicetree/bindings/regulator/gpio-regulator.txt
>> +++ b/Documentation/devicetree/bindings/regulator/gpio-regulator.txt
>> @@ -4,16 +4,25 @@ Required properties:
>> - compatible : Must be "regulator-gpio".
>> - regulator-name : Defined in regulator.txt as optional, but required
>> here.
>> -- states : Selection of available voltages and GPIO configs.
>> - if there are no states, then use a fixed regulator
>> +- states : Selection of available voltages/currents provided by
>> + this regulator and matching GPIO configurations to
>> + achieve them. If there are no states in the "states"
>> + array, use a fixed regulator instead.
>>
>> Optional properties:
>> -- enable-gpio : GPIO to use to enable/disable the regulator.
>> -- gpios : GPIO group used to control voltage.
>> -- gpios-states : gpios pin's initial states array. 0: LOW, 1: HIGH.
>> - defualt is LOW if nothing is specified.
>> +- enable-gpio : GPIO used to enable/disable the regulator.
>
> According to modern GPIO rules, that should be "enable-gpios" (plural) ...
Isn't that changing the ABI spec then ?
>> + Warning, the GPIO phandle flags are ignored and the
>> + GPIO polarity is controlled solely by the presence
>> + of "enable-active-high" DT property. This is due to
>> + compatibility with old DTs.
>
> Wasn't the purpose of the various *active-* flags to accommodate GPIOs
> lacking a flags cell (i.e. typically #gpio-cells = <1>)?
Yes
> When the flags cell is present, there's indeed opportunity for confusion
> (and breakage), combined with the presence/lack of *active-* below...
The code is rather clear that the flags cell is ignored.
drivers/gpio/gpiolib-of.c:
91 /*
92 * The regulator GPIO handles are specified such that the
93 * presence or absence of "enable-active-high" solely controls
94 * the polarity of the GPIO line. Any phandle flags must
95 * be actively ignored.
96 */
>> +- enable-active-high : Polarity of "enable-gpio" GPIO is active HIGH.
>> + Default is active LOW.
>> +- gpios : Array of one or more GPIO pins used to select the
>> + regulator voltage/current listed in "states".
>> +- gpios-states : Initial state of GPIO pins in "gpios" array.
>> + 0: LOW, 1: HIGH.
>> + Default is LOW if nothing else is specified.
>> - startup-delay-us : Startup time in microseconds.
>> -- enable-active-high : Polarity of GPIO is active high (default is low).
>> - regulator-type : Specifies what is being regulated, must be either
>> "voltage" or "current", defaults to voltage.
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
Best regards,
Marek Vasut
