Re: [PATCH] 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
> ---
> 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.
> ---
>  .../bindings/regulator/gpio-regulator.txt     | 23 +++++++++++++------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/gpio-regulator.txt b/Documentation/devicetree/bindings/regulator/gpio-regulator.txt
> index 1f496159e2bb..acca13c1eaf3 100644
> --- 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.
> +			  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.
> +- 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

I think this must be a required property.

> +			  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.

This is not very clear. Maybe add an example below or explain it better.

I guess we can't change it now anymore anyway, but it's not clear to
me, why we have this in the first place: A regulator should only be
active when it has a consumer or is "always-on". IIRC the regulator
core automatically selects the lowest voltage/current compatible with
all consumers. It seems the only usecase is to specify an initial
state that is different from all states in the "states" property, before
the regulator is turned on for the first time. However it also is not
an off-state, because it won't be set again on turning the regulator off.

>  - startup-delay-us	: Startup time in microseconds.

Not relevant for your patch, but I wonder why this is not in the
generic regulator binding.

> -- 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.

HTH,
Harald




[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