On Tue, 2019-09-03 at 09:45 +0100, Rob Herring wrote: > On Tue, Sep 3, 2019 at 9:03 AM Philippe Schenker > <philippe.schenker@xxxxxxxxxxx> 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 <philippe.schenker@xxxxxxxxxxx> > > > > --- > > > > .../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 > >