| Message ID | 20210701182012.3421679-1-sean.anderson@seco.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/3] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin | expand |
Hi Sean, On Thu, Jul 1, 2021 at 8:20 PM Sean Anderson <sean.anderson@seco.com> wrote: > These properties allow configuring the SD/OE pin as described in the > datasheet. > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > --- > > Changes in v4: > - Specify that bindings should specify these properties, but don't make > any guarantees about the driver's behavior when they are not present. > - Clarify description of idt,(en|dis)able-shutdown properties. > - Make opposing properties mutually exclusive. > - Add these properties to the example. Thanks for the update! > --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml > +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml > @@ -109,6 +152,22 @@ allOf: > required: > - clock-names > - clocks > + - if: > + true > + then: > + oneOf: > + - required: > + - idt,enable-shutdown > + - required: > + - idt,disable-shutdown > + - if: > + true > + then: > + oneOf: > + - required: > + - idt,output-enable-active-high > + - required: > + - idt,output-enable-active-low Do you really need the "if: true then:"? Just the "oneOf: ..." worked fine for me in another binding, but then I didn't have a surrounding "allOf" to interfere... Gr{oetje,eeting}s, Geert
On Fri, Jul 02, 2021 at 11:07:57AM -0400, Sean Anderson wrote: > > > On 7/2/21 3:14 AM, Geert Uytterhoeven wrote: > > Hi Sean, > > > > On Thu, Jul 1, 2021 at 8:20 PM Sean Anderson <sean.anderson@seco.com> wrote: > >> These properties allow configuring the SD/OE pin as described in the > >> datasheet. > >> > >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> > >> --- > >> > >> Changes in v4: > >> - Specify that bindings should specify these properties, but don't make > >> any guarantees about the driver's behavior when they are not present. > >> - Clarify description of idt,(en|dis)able-shutdown properties. > >> - Make opposing properties mutually exclusive. > >> - Add these properties to the example. > > > > Thanks for the update! > > > >> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml > >> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml > > > >> @@ -109,6 +152,22 @@ allOf: > >> required: > >> - clock-names > >> - clocks > >> + - if: > >> + true > >> + then: > >> + oneOf: > >> + - required: > >> + - idt,enable-shutdown > >> + - required: > >> + - idt,disable-shutdown > >> + - if: > >> + true > >> + then: > >> + oneOf: > >> + - required: > >> + - idt,output-enable-active-high > >> + - required: > >> + - idt,output-enable-active-low > > > > Do you really need the "if: true then:"? > > Just the "oneOf: ..." worked fine for me in another binding, but then I > > didn't have a surrounding "allOf" to interfere... > > Yes. If you want to have multiple oneOfs, they have to go under an > allOf. And allOf *only* allows if statements. This is a pretty big > deficiency, IMO, but not something I can address here. Humm, we should relax that, not work around it. Rob
On 7/2/21 5:15 PM, Rob Herring wrote: > On Fri, Jul 2, 2021 at 9:18 AM Rob Herring <robh@kernel.org> wrote: >> >> On Fri, Jul 02, 2021 at 11:07:57AM -0400, Sean Anderson wrote: >> > >> > >> > On 7/2/21 3:14 AM, Geert Uytterhoeven wrote: >> > > Hi Sean, >> > > >> > > On Thu, Jul 1, 2021 at 8:20 PM Sean Anderson <sean.anderson@seco.com> wrote: >> > >> These properties allow configuring the SD/OE pin as described in the >> > >> datasheet. >> > >> >> > >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> > >> --- >> > >> >> > >> Changes in v4: >> > >> - Specify that bindings should specify these properties, but don't make >> > >> any guarantees about the driver's behavior when they are not present. >> > >> - Clarify description of idt,(en|dis)able-shutdown properties. >> > >> - Make opposing properties mutually exclusive. >> > >> - Add these properties to the example. >> > > >> > > Thanks for the update! >> > > >> > >> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml >> > >> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml >> > > >> > >> @@ -109,6 +152,22 @@ allOf: >> > >> required: >> > >> - clock-names >> > >> - clocks >> > >> + - if: >> > >> + true >> > >> + then: >> > >> + oneOf: >> > >> + - required: >> > >> + - idt,enable-shutdown >> > >> + - required: >> > >> + - idt,disable-shutdown >> > >> + - if: >> > >> + true >> > >> + then: >> > >> + oneOf: >> > >> + - required: >> > >> + - idt,output-enable-active-high >> > >> + - required: >> > >> + - idt,output-enable-active-low >> > > >> > > Do you really need the "if: true then:"? >> > > Just the "oneOf: ..." worked fine for me in another binding, but then I >> > > didn't have a surrounding "allOf" to interfere... >> > >> > Yes. If you want to have multiple oneOfs, they have to go under an >> > allOf. And allOf *only* allows if statements. This is a pretty big >> > deficiency, IMO, but not something I can address here. >> >> Humm, we should relax that, not work around it. > > I've now relaxed this restriction in dt-schema master. > > Rob > > P.S. I probably broke something because it's Friday afternoon before > going on holiday for a week (so I'll do a tagged release when back). Will you be doing a release? I'm waiting on that before I send v5. --Sean
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml index 28675b0b80f1..2631a175612b 100644 --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml @@ -30,6 +30,21 @@ description: | 3 -- OUT3 4 -- OUT4 + The idt,(en|dis)able-shutdown and idt,output-enable-active-(high|low) + properties control the SH (en_global_shutdown) and SP bits of the + Primary Source and Shutdown Register, respectively. Their behavior is + summarized by the following table: + + SH SP Output when the SD/OE pin is Low/High + == == ===================================== + 0 0 Active/Inactive + 0 1 Inactive/Active + 1 0 Active/Shutdown + 1 1 Inactive/Shutdown + + One of idt,(en|dis)able-shutdown and one of + idt,output-enable-active-(high|low) should be specified. + maintainers: - Luca Ceresoli <luca@lucaceresoli.net> @@ -64,6 +79,34 @@ properties: maximum: 22760 description: Optional load capacitor for XTAL1 and XTAL2 + idt,enable-shutdown: + $ref: /schemas/types.yaml#/definitions/flag + description: | + Enable the shutdown functionality. The chip will be shut down if + the SD/OE pin is driven high. This corresponds to setting the SH + bit of the Primary Source and Shutdown Register. + + idt,disable-shutdown: + $ref: /schemas/types.yaml#/definitions/flag + description: | + Disable the shutdown functionality. The chip will never be shut + down based on the value of the SD/OE pin. This corresponds to + clearing the SH bit of the Primary Source and Shutdown Register. + + idt,output-enable-active-high: + $ref: /schemas/types.yaml#/definitions/flag + description: | + This enables output when the SD/OE pin is high, and disables + output when the SD/OE pin is low. This corresponds to setting the + SP bit of the Primary Source and Shutdown Register. + + idt,output-enable-active-low: + $ref: /schemas/types.yaml#/definitions/flag + description: | + This disables output when the SD/OE pin is high, and enables + output when the SD/OE pin is low. This corresponds to clearing the + SP bit of the Primary Source and Shutdown Register. + patternProperties: "^OUT[1-4]$": type: object @@ -109,6 +152,22 @@ allOf: required: - clock-names - clocks + - if: + true + then: + oneOf: + - required: + - idt,enable-shutdown + - required: + - idt,disable-shutdown + - if: + true + then: + oneOf: + - required: + - idt,output-enable-active-high + - required: + - idt,output-enable-active-low additionalProperties: false @@ -138,6 +197,10 @@ examples: clocks = <&ref25m>; clock-names = "xin"; + /* Set the SD/OE pin's settings */ + idt,disable-shutdown; + idt,output-enable-active-low; + OUT1 { idt,drive-mode = <VC5_CMOSD>; idt,voltage-microvolts = <1800000>;
These properties allow configuring the SD/OE pin as described in the datasheet. Signed-off-by: Sean Anderson <sean.anderson@seco.com> --- Changes in v4: - Specify that bindings should specify these properties, but don't make any guarantees about the driver's behavior when they are not present. - Clarify description of idt,(en|dis)able-shutdown properties. - Make opposing properties mutually exclusive. - Add these properties to the example. Changes in v3: - Add idt,disable-shutdown and idt,output-enable-active-low to allow for a default of not changing the SP/SH bits at all. Changes in v2: - Rename idt,sd-active-high to idt,output-enable-active-high - Add idt,enable-shutdown .../bindings/clock/idt,versaclock5.yaml | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+)