| Message ID | 20250404014500.2789830-2-sbellary@baylibre.com (mailing list archive) |
|---|---|
| State | Under Review |
| Headers | show |
| Series | dt-bindings: clock: ti: convert to yaml | expand |
On Thu, Apr 03, 2025 at 06:44:57PM GMT, Sukrut Bellary wrote: > +properties: > + reg: > + maxItems: 1 How reg is part of this? Every clock has reg, doesn't it? Otherwise how do you control it? Drop. > + > + ti,autoidle-shift: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + bit shift of the autoidle enable bit for the clock > + maximum: 31 > + default: 0 > + > + ti,invert-autoidle-bit: > + type: boolean > + description: > + autoidle is enabled by setting the bit to 0 required: - ti,autoidle-shift - ti,invert-autoidle-bit - although this makes no sense, so probably old binding was not correct here > + > +additionalProperties: true > + > +examples: > + - | > + bus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + clock@1b4 { > + reg = <0x01b4>; > + ti,autoidle-shift = <8>; > + ti,invert-autoidle-bit; > + }; Drop example, pointless here - noop due to lack of compatible. Instead provide complete examples in schemas referencing this. > + }; > -- > 2.34.1 >
On Thu, Apr 03, 2025 at 06:44:57PM -0700, Sukrut Bellary wrote: > Covert TI autoidle clock txt binding to yaml. Convert 2 patches in the series have the exact same subject. Really, nothing in all of the git history should ever repeat a subject. After all, you can't make the same change twice. > > AutoIdle clock is not an individual clock; it is always a > derivate of some basic clock like a gate, divider, or fixed-factor. > This binding will be referred in ti,divider-clock.yaml, and > ti,fixed-factor-clock.yaml. > > As all clocks don't support the autoidle feature e.g., > in DRA77xx/AM57xx[1], dpll_abe_x2* and dpll_per_x2 don't have > autoidle, remove required properties from the binding. > Clean up the example to meet the current standards. > > Add the creator of the original binding as a maintainer. > > [1] https://www.ti.com/lit/ug/spruhz6l/spruhz6l.pdf > > Signed-off-by: Sukrut Bellary <sbellary@baylibre.com> > --- > .../devicetree/bindings/clock/ti/autoidle.txt | 37 -------------- > .../bindings/clock/ti/ti,autoidle.yaml | 49 +++++++++++++++++++ > 2 files changed, 49 insertions(+), 37 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/clock/ti/autoidle.txt > create mode 100644 Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml > > diff --git a/Documentation/devicetree/bindings/clock/ti/autoidle.txt b/Documentation/devicetree/bindings/clock/ti/autoidle.txt > deleted file mode 100644 > index 05645a10a9e3..000000000000 > --- a/Documentation/devicetree/bindings/clock/ti/autoidle.txt > +++ /dev/null > @@ -1,37 +0,0 @@ > -Binding for Texas Instruments autoidle clock. > - > -This binding uses the common clock binding[1]. It assumes a register mapped > -clock which can be put to idle automatically by hardware based on the usage > -and a configuration bit setting. Autoidle clock is never an individual > -clock, it is always a derivative of some basic clock like a gate, divider, > -or fixed-factor. > - > -[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > - > -Required properties: > -- reg : offset for the register controlling the autoidle > -- ti,autoidle-shift : bit shift of the autoidle enable bit > -- ti,invert-autoidle-bit : autoidle is enabled by setting the bit to 0 > - > -Examples: > - dpll_core_m4_ck: dpll_core_m4_ck { > - #clock-cells = <0>; > - compatible = "ti,divider-clock"; > - clocks = <&dpll_core_x2_ck>; > - ti,max-div = <31>; > - ti,autoidle-shift = <8>; > - reg = <0x2d38>; > - ti,index-starts-at-one; > - ti,invert-autoidle-bit; > - }; > - > - dpll_usb_clkdcoldo_ck: dpll_usb_clkdcoldo_ck { > - #clock-cells = <0>; > - compatible = "ti,fixed-factor-clock"; > - clocks = <&dpll_usb_ck>; > - ti,clock-div = <1>; > - ti,autoidle-shift = <8>; > - reg = <0x01b4>; > - ti,clock-mult = <1>; > - ti,invert-autoidle-bit; > - }; > diff --git a/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml b/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml > new file mode 100644 > index 000000000000..c995dae65cd6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml > @@ -0,0 +1,49 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/ti/ti,autoidle.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TI autoidle clock > + > +maintainers: > + - Tero Kristo <kristo@kernel.org> > + - Sukrut Bellary <sbellary@baylibre.com> > + > +description: | Don't need '|' if no formatting to preserve. > + In TI SoC, some of the clocks support autoidle feature. > + It assumes a register mapped clock which can be put to idle automatically > + by hardware based on the usage and a configuration bit setting. > + Autoidle clock is never an individual clock, it is always a derivative > + of some basic clock like a gate, divider or fixed-factor. Is this 3 1 sentence paragraphs or 1 paragraph with odd line wrapping? Blank line between paragraphs or re-wrap at 80 char. I prefer the latter as 1 sentence paragraphs doesn't make much sense. > + > +properties: > + reg: > + maxItems: 1 > + > + ti,autoidle-shift: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + bit shift of the autoidle enable bit for the clock > + maximum: 31 > + default: 0 > + > + ti,invert-autoidle-bit: > + type: boolean > + description: > + autoidle is enabled by setting the bit to 0 > + > +additionalProperties: true > + > +examples: > + - | > + bus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + clock@1b4 { > + reg = <0x01b4>; > + ti,autoidle-shift = <8>; > + ti,invert-autoidle-bit; > + }; > + }; > -- > 2.34.1 >
Am Fri, 4 Apr 2025 12:44:39 +0200 schrieb Krzysztof Kozlowski <krzk@kernel.org>: > On Thu, Apr 03, 2025 at 06:44:57PM GMT, Sukrut Bellary wrote: > > +properties: > > + reg: > > + maxItems: 1 > > How reg is part of this? Every clock has reg, doesn't it? Otherwise how > do you control it? Drop. > > > + > > + ti,autoidle-shift: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + bit shift of the autoidle enable bit for the clock > > + maximum: 31 > > + default: 0 > > + > > + ti,invert-autoidle-bit: > > + type: boolean > > + description: > > + autoidle is enabled by setting the bit to 0 > > required: > - ti,autoidle-shift > - ti,invert-autoidle-bit - although this makes no sense, so probably > old binding was not correct here > well, the more informal definition in the txt file can be read as: if the clock supports autoidle, then ti,autoidle-shift is required. But that does not translate to the formal definition in the yaml file. So we have nothing required here. I am a bit wondering whether we should just drop the autoidle.txt. The only thing worth there is the description. Regards, Andreas
On Fri, Apr 04, 2025 at 12:44:39PM +0200, Krzysztof Kozlowski wrote: > On Thu, Apr 03, 2025 at 06:44:57PM GMT, Sukrut Bellary wrote: > > +properties: > > + reg: > > + maxItems: 1 > > How reg is part of this? Every clock has reg, doesn't it? Otherwise how > do you control it? Drop. Thanks for the review. I will drop. > > + > > + ti,autoidle-shift: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + bit shift of the autoidle enable bit for the clock > > + maximum: 31 > > + default: 0 > > + > > + ti,invert-autoidle-bit: > > + type: boolean > > + description: > > + autoidle is enabled by setting the bit to 0 > > required: > - ti,autoidle-shift > - ti,invert-autoidle-bit - although this makes no sense, so probably > old binding was not correct here Yes, all clocks don't support autoidle. So, required is not applicable here. > > + > > +additionalProperties: true > > + > > +examples: > > + - | > > + bus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + clock@1b4 { > > + reg = <0x01b4>; > > + ti,autoidle-shift = <8>; > > + ti,invert-autoidle-bit; > > + }; > > Drop example, pointless here - noop due to lack of compatible. Instead > provide complete examples in schemas referencing this. Ok, will remove from here. > > + }; > > -- > > 2.34.1 > >
On Sat, Apr 05, 2025 at 09:55:29PM +0200, Andreas Kemnade wrote: > Am Fri, 4 Apr 2025 12:44:39 +0200 > schrieb Krzysztof Kozlowski <krzk@kernel.org>: > > > On Thu, Apr 03, 2025 at 06:44:57PM GMT, Sukrut Bellary wrote: > > > +properties: > > > + reg: > > > + maxItems: 1 > > > > How reg is part of this? Every clock has reg, doesn't it? Otherwise how > > do you control it? Drop. > > > > > + > > > + ti,autoidle-shift: > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + description: > > > + bit shift of the autoidle enable bit for the clock > > > + maximum: 31 > > > + default: 0 > > > + > > > + ti,invert-autoidle-bit: > > > + type: boolean > > > + description: > > > + autoidle is enabled by setting the bit to 0 > > > > required: > > - ti,autoidle-shift > > - ti,invert-autoidle-bit - although this makes no sense, so probably > > old binding was not correct here > > > > well, the more informal definition in the txt file can be read as: if > the clock supports autoidle, then ti,autoidle-shift is required. But > that does not > translate to the formal definition in the yaml file. > So we have nothing required here. > > I am a bit wondering whether we should just drop the autoidle.txt. The > only thing worth there is the description. Thanks for the review. IMO, it would be good to keep this as other clocks refer to these properties. But yes, I will add details in the description about if the clock supports autoidle, then these properties are applicable. > Regards, > Andreas
On Fri, Apr 04, 2025 at 10:19:02AM -0500, Rob Herring wrote: > On Thu, Apr 03, 2025 at 06:44:57PM -0700, Sukrut Bellary wrote: > > Covert TI autoidle clock txt binding to yaml. > > Convert > > 2 patches in the series have the exact same subject. Really, nothing in > all of the git history should ever repeat a subject. After all, you > can't make the same change twice. Thanks for the review. I will fix this. > > > > AutoIdle clock is not an individual clock; it is always a > > derivate of some basic clock like a gate, divider, or fixed-factor. > > This binding will be referred in ti,divider-clock.yaml, and > > ti,fixed-factor-clock.yaml. > > > > As all clocks don't support the autoidle feature e.g., > > in DRA77xx/AM57xx[1], dpll_abe_x2* and dpll_per_x2 don't have > > autoidle, remove required properties from the binding. > > Clean up the example to meet the current standards. > > > > Add the creator of the original binding as a maintainer. > > > > [1] https://www.ti.com/lit/ug/spruhz6l/spruhz6l.pdf > > > > Signed-off-by: Sukrut Bellary <sbellary@baylibre.com> > > --- > > .../devicetree/bindings/clock/ti/autoidle.txt | 37 -------------- > > .../bindings/clock/ti/ti,autoidle.yaml | 49 +++++++++++++++++++ > > 2 files changed, 49 insertions(+), 37 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/clock/ti/autoidle.txt > > create mode 100644 Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml > > > > diff --git a/Documentation/devicetree/bindings/clock/ti/autoidle.txt b/Documentation/devicetree/bindings/clock/ti/autoidle.txt > > deleted file mode 100644 > > index 05645a10a9e3..000000000000 > > --- a/Documentation/devicetree/bindings/clock/ti/autoidle.txt > > +++ /dev/null > > @@ -1,37 +0,0 @@ > > -Binding for Texas Instruments autoidle clock. > > - > > -This binding uses the common clock binding[1]. It assumes a register mapped > > -clock which can be put to idle automatically by hardware based on the usage > > -and a configuration bit setting. Autoidle clock is never an individual > > -clock, it is always a derivative of some basic clock like a gate, divider, > > -or fixed-factor. > > - > > -[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > - > > -Required properties: > > -- reg : offset for the register controlling the autoidle > > -- ti,autoidle-shift : bit shift of the autoidle enable bit > > -- ti,invert-autoidle-bit : autoidle is enabled by setting the bit to 0 > > - > > -Examples: > > - dpll_core_m4_ck: dpll_core_m4_ck { > > - #clock-cells = <0>; > > - compatible = "ti,divider-clock"; > > - clocks = <&dpll_core_x2_ck>; > > - ti,max-div = <31>; > > - ti,autoidle-shift = <8>; > > - reg = <0x2d38>; > > - ti,index-starts-at-one; > > - ti,invert-autoidle-bit; > > - }; > > - > > - dpll_usb_clkdcoldo_ck: dpll_usb_clkdcoldo_ck { > > - #clock-cells = <0>; > > - compatible = "ti,fixed-factor-clock"; > > - clocks = <&dpll_usb_ck>; > > - ti,clock-div = <1>; > > - ti,autoidle-shift = <8>; > > - reg = <0x01b4>; > > - ti,clock-mult = <1>; > > - ti,invert-autoidle-bit; > > - }; > > diff --git a/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml b/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml > > new file mode 100644 > > index 000000000000..c995dae65cd6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml > > @@ -0,0 +1,49 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/clock/ti/ti,autoidle.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: TI autoidle clock > > + > > +maintainers: > > + - Tero Kristo <kristo@kernel.org> > > + - Sukrut Bellary <sbellary@baylibre.com> > > + > > +description: | > > Don't need '|' if no formatting to preserve. I will fix this. > > + In TI SoC, some of the clocks support autoidle feature. > > + It assumes a register mapped clock which can be put to idle automatically > > + by hardware based on the usage and a configuration bit setting. > > + Autoidle clock is never an individual clock, it is always a derivative > > + of some basic clock like a gate, divider or fixed-factor. > > Is this 3 1 sentence paragraphs or 1 paragraph with odd line wrapping? > Blank line between paragraphs or re-wrap at 80 char. I prefer the latter > as 1 sentence paragraphs doesn't make much sense. It's odd line wrapping. I will fix this. > > + > > +properties: > > + reg: > > + maxItems: 1 > > + > > + ti,autoidle-shift: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + bit shift of the autoidle enable bit for the clock > > + maximum: 31 > > + default: 0 > > + > > + ti,invert-autoidle-bit: > > + type: boolean > > + description: > > + autoidle is enabled by setting the bit to 0 > > + > > +additionalProperties: true > > + > > +examples: > > + - | > > + bus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + clock@1b4 { > > + reg = <0x01b4>; > > + ti,autoidle-shift = <8>; > > + ti,invert-autoidle-bit; > > + }; > > + }; > > -- > > 2.34.1 > >
diff --git a/Documentation/devicetree/bindings/clock/ti/autoidle.txt b/Documentation/devicetree/bindings/clock/ti/autoidle.txt deleted file mode 100644 index 05645a10a9e3..000000000000 --- a/Documentation/devicetree/bindings/clock/ti/autoidle.txt +++ /dev/null @@ -1,37 +0,0 @@ -Binding for Texas Instruments autoidle clock. - -This binding uses the common clock binding[1]. It assumes a register mapped -clock which can be put to idle automatically by hardware based on the usage -and a configuration bit setting. Autoidle clock is never an individual -clock, it is always a derivative of some basic clock like a gate, divider, -or fixed-factor. - -[1] Documentation/devicetree/bindings/clock/clock-bindings.txt - -Required properties: -- reg : offset for the register controlling the autoidle -- ti,autoidle-shift : bit shift of the autoidle enable bit -- ti,invert-autoidle-bit : autoidle is enabled by setting the bit to 0 - -Examples: - dpll_core_m4_ck: dpll_core_m4_ck { - #clock-cells = <0>; - compatible = "ti,divider-clock"; - clocks = <&dpll_core_x2_ck>; - ti,max-div = <31>; - ti,autoidle-shift = <8>; - reg = <0x2d38>; - ti,index-starts-at-one; - ti,invert-autoidle-bit; - }; - - dpll_usb_clkdcoldo_ck: dpll_usb_clkdcoldo_ck { - #clock-cells = <0>; - compatible = "ti,fixed-factor-clock"; - clocks = <&dpll_usb_ck>; - ti,clock-div = <1>; - ti,autoidle-shift = <8>; - reg = <0x01b4>; - ti,clock-mult = <1>; - ti,invert-autoidle-bit; - }; diff --git a/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml b/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml new file mode 100644 index 000000000000..c995dae65cd6 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/ti/ti,autoidle.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI autoidle clock + +maintainers: + - Tero Kristo <kristo@kernel.org> + - Sukrut Bellary <sbellary@baylibre.com> + +description: | + In TI SoC, some of the clocks support autoidle feature. + It assumes a register mapped clock which can be put to idle automatically + by hardware based on the usage and a configuration bit setting. + Autoidle clock is never an individual clock, it is always a derivative + of some basic clock like a gate, divider or fixed-factor. + +properties: + reg: + maxItems: 1 + + ti,autoidle-shift: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + bit shift of the autoidle enable bit for the clock + maximum: 31 + default: 0 + + ti,invert-autoidle-bit: + type: boolean + description: + autoidle is enabled by setting the bit to 0 + +additionalProperties: true + +examples: + - | + bus { + #address-cells = <1>; + #size-cells = <0>; + + clock@1b4 { + reg = <0x01b4>; + ti,autoidle-shift = <8>; + ti,invert-autoidle-bit; + }; + };
Covert TI autoidle clock txt binding to yaml. AutoIdle clock is not an individual clock; it is always a derivate of some basic clock like a gate, divider, or fixed-factor. This binding will be referred in ti,divider-clock.yaml, and ti,fixed-factor-clock.yaml. As all clocks don't support the autoidle feature e.g., in DRA77xx/AM57xx[1], dpll_abe_x2* and dpll_per_x2 don't have autoidle, remove required properties from the binding. Clean up the example to meet the current standards. Add the creator of the original binding as a maintainer. [1] https://www.ti.com/lit/ug/spruhz6l/spruhz6l.pdf Signed-off-by: Sukrut Bellary <sbellary@baylibre.com> --- .../devicetree/bindings/clock/ti/autoidle.txt | 37 -------------- .../bindings/clock/ti/ti,autoidle.yaml | 49 +++++++++++++++++++ 2 files changed, 49 insertions(+), 37 deletions(-) delete mode 100644 Documentation/devicetree/bindings/clock/ti/autoidle.txt create mode 100644 Documentation/devicetree/bindings/clock/ti/ti,autoidle.yaml