Re: [PATCH RFC] dt-bindings: regulator: define a mux regulator

2019-08-20 Thread Uwe Kleine-König
Hello Rob,

On Tue, Aug 20, 2019 at 11:39:27AM -0500, Rob Herring wrote:
> On Tue, Aug 20, 2019 at 10:25 AM Uwe Kleine-König
>  wrote:
> >
> > A mux regulator is used to provide current on one of several outputs. It
> > might look as follows:
> >
> >   ,.
> > -- > -- > -- > -- > -- > -- > -- > -- >   `'
> >
> > Depending on which address is encoded on the three address inputs A0, A1
> > and A2 the current provided on IN is provided on one of the eight
> > outputs.
> >
> > What is new here is that the binding makes use of a #regulator-cells
> > property. This uses the approach known from other bindings (e.g. gpio)
> > to allow referencing all eight outputs with phandle arguments. This
> > requires an extention in of_get_regulator to use a new variant of
> > of_parse_phandle_with_args that has a cell_count_default parameter that
> > is used in absence of a $cell_name property. Even if we'd choose to
> > update all regulator-bindings to add #regulator-cells = <0>; we still
> > needed something to implement compatibility to the currently defined
> > bindings.
> >
> > Signed-off-by: Uwe Kleine-König 
> > ---
> > Hello,
> >
> > the obvious alternative is to add (here) eight subnodes to represent the
> > eight outputs. This is IMHO less pretty, but wouldn't need to introduce
> > #regulator-cells.
> 
> I'm okay with #regulator-cells approach.

OK, then I will look into that in more detail; unless the regulator guys
don't agree with this approach of course.

> > Apart from reg = <..> and a phandle there is (I think) nothing that
> > needs to be specified in the subnodes because all properties of an
> > output (apart from the address) apply to all outputs.
> >
> > What do you think?
> >
> > Best regards
> > Uwe
> >
> >  .../bindings/regulator/mux-regulator.yaml | 52 +++
> >  1 file changed, 52 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/regulator/mux-regulator.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/mux-regulator.yaml 
> > b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml
> > new file mode 100644
> > index ..f06dbb969090
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml
> > @@ -0,0 +1,52 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> (GPL-2.0-only OR BSD-2-Clause) is preferred.

OK.

> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/regulator/mux-regulator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MUX regulators
> > +
> > +properties:
> > +  compatible:
> > +const: XXX,adb708
> 
> ? I assume you will split this into a common and specific schemas. I
> suppose there could be differing ways to control the mux just like all
> other muxes.

Not sure if a specific schema is necessary. I wrote XXX because I was
offline while I authored the binding and so couldn't determine the right
vendor to use.

> > +  enable-gpios:
> > +maxItems: 1
> > +
> > +  address-gpios:
> > +description: Array of typically three GPIO pins used to select the
> > +  regulator's output. The least significant address GPIO must be listed
> > +  first. The others follow in order of significance.
> > +minItems: 1
> > +
> > +  "#regulator-cells":
> 
> How is this not required?

It should. For the RFC patch I didn't took the time to iron all the
details. My main concern was/is how the binding should look like and if
an #regulator-cells with a default would be acceptable.
 
Best regards and thanks for your feedback,
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |


Re: [PATCH RFC] dt-bindings: regulator: define a mux regulator

2019-08-20 Thread Rob Herring
On Tue, Aug 20, 2019 at 10:25 AM Uwe Kleine-König
 wrote:
>
> A mux regulator is used to provide current on one of several outputs. It
> might look as follows:
>
>   ,.
> -- -- -- -- -- -- -- --   `'
>
> Depending on which address is encoded on the three address inputs A0, A1
> and A2 the current provided on IN is provided on one of the eight
> outputs.
>
> What is new here is that the binding makes use of a #regulator-cells
> property. This uses the approach known from other bindings (e.g. gpio)
> to allow referencing all eight outputs with phandle arguments. This
> requires an extention in of_get_regulator to use a new variant of
> of_parse_phandle_with_args that has a cell_count_default parameter that
> is used in absence of a $cell_name property. Even if we'd choose to
> update all regulator-bindings to add #regulator-cells = <0>; we still
> needed something to implement compatibility to the currently defined
> bindings.
>
> Signed-off-by: Uwe Kleine-König 
> ---
> Hello,
>
> the obvious alternative is to add (here) eight subnodes to represent the
> eight outputs. This is IMHO less pretty, but wouldn't need to introduce
> #regulator-cells.

I'm okay with #regulator-cells approach.

>
> Apart from reg = <..> and a phandle there is (I think) nothing that
> needs to be specified in the subnodes because all properties of an
> output (apart from the address) apply to all outputs.
>
> What do you think?
>
> Best regards
> Uwe
>
>  .../bindings/regulator/mux-regulator.yaml | 52 +++
>  1 file changed, 52 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/regulator/mux-regulator.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/mux-regulator.yaml 
> b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml
> new file mode 100644
> index ..f06dbb969090
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: GPL-2.0

(GPL-2.0-only OR BSD-2-Clause) is preferred.


> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/mux-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MUX regulators
> +
> +properties:
> +  compatible:
> +const: XXX,adb708

? I assume you will split this into a common and specific schemas. I
suppose there could be differing ways to control the mux just like all
other muxes.

> +
> +  enable-gpios:
> +maxItems: 1
> +
> +  address-gpios:
> +description: Array of typically three GPIO pins used to select the
> +  regulator's output. The least significant address GPIO must be listed
> +  first. The others follow in order of significance.
> +minItems: 1
> +
> +  "#regulator-cells":

How is this not required?

> +const: 1
> +
> +  regulator-name:
> +description: A string used to construct the sub regulator's names
> +$ref: "/schemas/types.yaml#/definitions/string"
> +
> +  supply:
> +description: input supply
> +
> +required:
> +  - compatible
> +  - regulator-name
> +  - supply
> +
> +
> +examples:
> +  - |
> +mux-regulator {
> +  compatible = "regulator-mux";
> +
> +  regulator-name = "blafasel";
> +
> +  supply = <_regulator>;
> +
> +  enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
> +  address-gpios = < 2 GPIO_ACTIVE_HIGH>,
> +< 3 GPIO_ACTIVE_HIGH>,
> +< 4 GPIO_ACTIVE_HIGH>,
> +};
> +...
> --
> 2.20.1
>


[PATCH RFC] dt-bindings: regulator: define a mux regulator

2019-08-20 Thread Uwe Kleine-König
A mux regulator is used to provide current on one of several outputs. It
might look as follows:

  ,.
--; we still
needed something to implement compatibility to the currently defined
bindings.

Signed-off-by: Uwe Kleine-König 
---
Hello,

the obvious alternative is to add (here) eight subnodes to represent the
eight outputs. This is IMHO less pretty, but wouldn't need to introduce
#regulator-cells.

Apart from reg = <..> and a phandle there is (I think) nothing that
needs to be specified in the subnodes because all properties of an
output (apart from the address) apply to all outputs.

What do you think?

Best regards
Uwe

 .../bindings/regulator/mux-regulator.yaml | 52 +++
 1 file changed, 52 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/regulator/mux-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/mux-regulator.yaml 
b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml
new file mode 100644
index ..f06dbb969090
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/mux-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MUX regulators
+
+properties:
+  compatible:
+const: XXX,adb708
+
+  enable-gpios:
+maxItems: 1
+
+  address-gpios:
+description: Array of typically three GPIO pins used to select the
+  regulator's output. The least significant address GPIO must be listed
+  first. The others follow in order of significance.
+minItems: 1
+
+  "#regulator-cells":
+const: 1
+
+  regulator-name:
+description: A string used to construct the sub regulator's names
+$ref: "/schemas/types.yaml#/definitions/string"
+
+  supply:
+description: input supply
+
+required:
+  - compatible
+  - regulator-name
+  - supply
+  
+
+examples:
+  - |
+mux-regulator {
+  compatible = "regulator-mux";
+
+  regulator-name = "blafasel";
+
+  supply = <_regulator>;
+
+  enable-gpios = < 5 GPIO_ACTIVE_HIGH>;
+  address-gpios = < 2 GPIO_ACTIVE_HIGH>,
+< 3 GPIO_ACTIVE_HIGH>,
+< 4 GPIO_ACTIVE_HIGH>,
+};
+...
-- 
2.20.1