This adds the documentation to the compatible regulator-fixed-clock
Signed-off-by: Philippe Schenker <[email protected]>
---
.../bindings/regulator/fixed-regulator.yaml | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
index a650b457085d..5fd081e80b43 100644
--- a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
@@ -19,9 +19,19 @@ description:
allOf:
- $ref: "regulator.yaml#"
+select:
+ properties:
+ compatible:
+ contains:
+ const: regulator-fixed-clock
+ required:
+ - clocks
+
properties:
compatible:
- const: regulator-fixed
+ items:
+ - const: regulator-fixed
+ - const: regulator-fixed-clock
regulator-name: true
@@ -29,6 +39,12 @@ properties:
description: gpio to use for enable control
maxItems: 1
+ clocks:
+ description:
+ clock to use for enable control. This binding is only available if
+ the compatible is chosen to regulator-fixed-clock. The clock binding
+ is mandatory if compatible is chosen to regulator-fixed-clock.
+
startup-delay-us:
description: startup time in microseconds
$ref: /schemas/types.yaml#/definitions/uint32
--
2.23.0
On Tue, Sep 3, 2019 at 9:03 AM Philippe Schenker
<[email protected]> wrote:
>
> This adds the documentation to the compatible regulator-fixed-clock
Please explain what that is in this patch.
>
> Signed-off-by: Philippe Schenker <[email protected]>
>
> ---
>
> .../bindings/regulator/fixed-regulator.yaml | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
> index a650b457085d..5fd081e80b43 100644
> --- a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
> @@ -19,9 +19,19 @@ description:
> allOf:
> - $ref: "regulator.yaml#"
>
> +select:
> + properties:
> + compatible:
> + contains:
> + const: regulator-fixed-clock
> + required:
> + - clocks
You don't need this.
If you add a new compatible, then this should probably be a new schema
doc. Is the 'gpio' property valid in this case as if a clock is the
enable, can you also have a gpio enable? That said, it seems like the
new compatible is only for validating the DT in the driver. You could
just use a clock if present and default to current behavior if not.
It's not the kernel's job to validate DTs.
>
> properties:
> compatible:
> - const: regulator-fixed
> + items:
> + - const: regulator-fixed
> + - const: regulator-fixed-clock
This says you must have 'compatible = "regulator-fixed",
"regulator-fixed-clock";'.
What you want is:
enum:
- regulator-fixed
- regulator-fixed-clock
> regulator-name: true
>
> @@ -29,6 +39,12 @@ properties:
> description: gpio to use for enable control
> maxItems: 1
>
> + clocks:
> + description:
> + clock to use for enable control. This binding is only available if
> + the compatible is chosen to regulator-fixed-clock. The clock binding
> + is mandatory if compatible is chosen to regulator-fixed-clock.
Need to define how many clocks (maxItems: 1).
> +
> startup-delay-us:
> description: startup time in microseconds
> $ref: /schemas/types.yaml#/definitions/uint32
> --
> 2.23.0
>
On Tue, 2019-09-03 at 09:45 +0100, Rob Herring wrote:
> On Tue, Sep 3, 2019 at 9:03 AM Philippe Schenker
> <[email protected]> wrote:
> > This adds the documentation to the compatible regulator-fixed-clock
>
> Please explain what that is in this patch.
Hi Rob and thanks for your comments. I will change this commit message
for a possible v2 into this:
This adds the documentation to the compatible regulator-fixed-clock.
This binding is a special binding of regulator-fixed and adds the
ability to add a clock to regulator-fixed, so the regulator can be
enabled and disabled with that clock.
If the special compatible regulator-fixed-clock is used it is mandatory
to supply a clock.
> >
>
> > Signed-off-by: Philippe Schenker <[email protected]>
> >
> > ---
> >
> > .../bindings/regulator/fixed-regulator.yaml | 18
> > +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml
> > index a650b457085d..5fd081e80b43 100644
> > --- a/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml
> > +++ b/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml
> > @@ -19,9 +19,19 @@ description:
> > allOf:
> > - $ref: "regulator.yaml#"
> >
> > +select:
> > + properties:
> > + compatible:
> > + contains:
> > + const: regulator-fixed-clock
> > + required:
> > + - clocks
>
> You don't need this.
>
> If you add a new compatible, then this should probably be a new schema
> doc. Is the 'gpio' property valid in this case as if a clock is the
> enable, can you also have a gpio enable? That said, it seems like the
> new compatible is only for validating the DT in the driver. You could
> just use a clock if present and default to current behavior if not.
> It's not the kernel's job to validate DTs.
The gpio property is valid with this compatible. I added regulator-
fixed-clock to regulator-fixed hence I also don't want to create a new
schema file.
With the above select statement I wanted to state clocks as required
when the compatible regulator-fixed-clock is given.
>
> > properties:
> > compatible:
> > - const: regulator-fixed
> > + items:
> > + - const: regulator-fixed
> > + - const: regulator-fixed-clock
>
> This says you must have 'compatible = "regulator-fixed",
> "regulator-fixed-clock";'.
>
> What you want is:
>
> enum:
> - regulator-fixed
> - regulator-fixed-clock
Thanks, this was exactly what I wanted to do.
>
> > regulator-name: true
> >
> > @@ -29,6 +39,12 @@ properties:
> > description: gpio to use for enable control
> > maxItems: 1
> >
> > + clocks:
> > + description:
> > + clock to use for enable control. This binding is only
> > available if
> > + the compatible is chosen to regulator-fixed-clock. The clock
> > binding
> > + is mandatory if compatible is chosen to regulator-fixed-
> > clock.
>
> Need to define how many clocks (maxItems: 1).
Will do for a possible v2. I want to wait what Mark Brown says about the
addition in general, maybe I have to change all over how this speciality
is added into regulator subsystem and therefore also the binding will
change.
Philippe
>
> > +
> > startup-delay-us:
> > description: startup time in microseconds
> > $ref: /schemas/types.yaml#/definitions/uint32
> > --
> > 2.23.0
> >
On Wed, Sep 4, 2019 at 9:07 AM Philippe Schenker
<[email protected]> wrote:
>
> On Tue, 2019-09-03 at 09:45 +0100, Rob Herring wrote:
> > On Tue, Sep 3, 2019 at 9:03 AM Philippe Schenker
> > <[email protected]> wrote:
> > > This adds the documentation to the compatible regulator-fixed-clock
> >
> > Please explain what that is in this patch.
>
> Hi Rob and thanks for your comments. I will change this commit message
> for a possible v2 into this:
>
> This adds the documentation to the compatible regulator-fixed-clock.
> This binding is a special binding of regulator-fixed and adds the
> ability to add a clock to regulator-fixed, so the regulator can be
> enabled and disabled with that clock.
> If the special compatible regulator-fixed-clock is used it is mandatory
> to supply a clock.
>
> > >
> >
> > > Signed-off-by: Philippe Schenker <[email protected]>
> > >
> > > ---
> > >
> > > .../bindings/regulator/fixed-regulator.yaml | 18
> > > +++++++++++++++++-
> > > 1 file changed, 17 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml
> > > index a650b457085d..5fd081e80b43 100644
> > > --- a/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml
> > > +++ b/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml
> > > @@ -19,9 +19,19 @@ description:
> > > allOf:
> > > - $ref: "regulator.yaml#"
> > >
> > > +select:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: regulator-fixed-clock
> > > + required:
> > > + - clocks
> >
> > You don't need this.
> >
> > If you add a new compatible, then this should probably be a new schema
> > doc. Is the 'gpio' property valid in this case as if a clock is the
> > enable, can you also have a gpio enable? That said, it seems like the
> > new compatible is only for validating the DT in the driver. You could
> > just use a clock if present and default to current behavior if not.
> > It's not the kernel's job to validate DTs.
>
> The gpio property is valid with this compatible. I added regulator-
> fixed-clock to regulator-fixed hence I also don't want to create a new
> schema file.
Okay, if all the other properties are valid then adding to this file is fine.
> With the above select statement I wanted to state clocks as required
> when the compatible regulator-fixed-clock is given.
select is not the right way to do that. select is for whether to apply
the schema to a node or not. What you have will silently not apply the
schema if 'clocks' is missing or compatible is 'regulator-fixed'.
Essentially what you need to do here is:
if:
properties:
compatible:
contains:
const: regulator-fixed-clock
then:
required:
- clocks
Rob