| Message ID | 20221018072106.2391771-1-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/1] dt-bindings: clock: ti,cdce925: Convert to DT schema | expand |
On Tue, 18 Oct 2022 09:21:06 +0200, Alexander Stein wrote: > Convert the TI CDCE925 clock binding to DT schema format. > Including a small fix: Add the missing 'ti' prefix in the example > compatible. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > I have to admit I only have one specific addon platform for this > hardware, which is actually a CECD813 tbh. > > .../devicetree/bindings/clock/ti,cdce925.txt | 53 --------- > .../devicetree/bindings/clock/ti,cdce925.yaml | 104 ++++++++++++++++++ > 2 files changed, 104 insertions(+), 53 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.txt > create mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.example.dtb: clock-controller@64: $nodename:0: 'clock-controller@64' does not match '^clock-controller$' From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.example.dtb: clock-controller@64: 'ret' is a required property From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On 18/10/2022 03:21, Alexander Stein wrote: > Convert the TI CDCE925 clock binding to DT schema format. > Including a small fix: Add the missing 'ti' prefix in the example > compatible. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> Thank you for your patch. There is something to discuss/improve. > diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml > new file mode 100644 > index 000000000000..1e68ee68e458 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml > @@ -0,0 +1,104 @@ > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node bindings Drop "node bindings" > + > +maintainers: > + - Mike Looijmans <mike.looijmans@topic.nl> > + > +description: | > + Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI Reduction > + > + - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913 > + - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925 > + - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937 > + - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949 > + > +properties: > + $nodename: > + pattern: "^clock-controller$" Drop this requirement. It is in general expected, but there is no need for each binding to specify it. Other problem is that you did not actually test these bindings before sending... > + > + compatible: > + enum: > + - ti,cdce913 > + - ti,cdce925 > + - ti,cdce937 > + - ti,cdce949 > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: fixed parent clock > + > + "#clock-cells": > + const: 1 > + > + vdd-supply: > + description: Regulator that provides 1.8V Vdd power supply > + > + vddout-supply: > + description: | > + Regulator that provides Vddout power supply. > + non-L variant: 2.5V or 3.3V for > + L variant: 1.8V for > + > + xtal-load-pf: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Crystal load-capacitor value to fine-tune performance on a > + board, or to compensate for external influences. > + > +patternProperties: > + "^PLL[1-4]$": > + type: object > + description: | > + optional child node can be used to specify spread > + spectrum clocking parameters for a board > + additionalProperties: false > + properties: > + spread-spectrum: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: SSC mode as defined in the data sheet > + > + spread-spectrum-center: > + type: boolean > + description: | > + Use "centered" mode instead of "max" mode. When > + present, the clock runs at the requested frequency on average. > + Otherwise the requested frequency is the maximum value of the > + SCC range. > + Best regards, Krzysztof
Hi Krzysztof, thanks for your review. Am Dienstag, 18. Oktober 2022, 15:51:35 CEST schrieb Krzysztof Kozlowski: > On 18/10/2022 03:21, Alexander Stein wrote: > > Convert the TI CDCE925 clock binding to DT schema format. > > Including a small fix: Add the missing 'ti' prefix in the example > > compatible. > > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > Thank you for your patch. There is something to discuss/improve. > > > diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml > > b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml new file mode > > 100644 > > index 000000000000..1e68ee68e458 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml > > @@ -0,0 +1,104 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node > > bindings > Drop "node bindings" Thanks, will do so. > > + > > +maintainers: > > + - Mike Looijmans <mike.looijmans@topic.nl> > > + > > +description: | > > + Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI > > Reduction + > > + - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913 > > + - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925 > > + - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937 > > + - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949 > > + > > +properties: > > + $nodename: > > + pattern: "^clock-controller$" > > Drop this requirement. It is in general expected, but there is no need > for each binding to specify it. Should this be put in a common binding then? > Other problem is that you did not actually test these bindings before > sending... > > > + > > + compatible: > > + enum: > > + - ti,cdce913 > > + - ti,cdce925 > > + - ti,cdce937 > > + - ti,cdce949 > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: fixed parent clock > > + > > + "#clock-cells": > > + const: 1 > > + > > + vdd-supply: > > + description: Regulator that provides 1.8V Vdd power supply > > + > > + vddout-supply: > > + description: | > > + Regulator that provides Vddout power supply. > > + non-L variant: 2.5V or 3.3V for > > + L variant: 1.8V for > > + > > + xtal-load-pf: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + Crystal load-capacitor value to fine-tune performance on a > > + board, or to compensate for external influences. > > + > > +patternProperties: > > + "^PLL[1-4]$": > > + type: object > > + description: | > > + optional child node can be used to specify spread > > + spectrum clocking parameters for a board > > + > > additionalProperties: false Will do. Thanks and best regards, Alexander > > + properties: > > + spread-spectrum: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: SSC mode as defined in the data sheet > > + > > + spread-spectrum-center: > > + type: boolean > > + description: | > > + Use "centered" mode instead of "max" mode. When > > + present, the clock runs at the requested frequency on average. > > + Otherwise the requested frequency is the maximum value of the > > + SCC range. > > + > > Best regards, > Krzysztof
On 19/10/2022 01:49, Alexander Stein wrote: > Hi Krzysztof, > > thanks for your review. > > Am Dienstag, 18. Oktober 2022, 15:51:35 CEST schrieb Krzysztof Kozlowski: >> On 18/10/2022 03:21, Alexander Stein wrote: >>> Convert the TI CDCE925 clock binding to DT schema format. >>> Including a small fix: Add the missing 'ti' prefix in the example >>> compatible. >>> >>> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> >> >> Thank you for your patch. There is something to discuss/improve. >> >>> diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml >>> b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml new file mode >>> 100644 >>> index 000000000000..1e68ee68e458 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml >>> @@ -0,0 +1,104 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node >>> bindings >> Drop "node bindings" > > Thanks, will do so. > >>> + >>> +maintainers: >>> + - Mike Looijmans <mike.looijmans@topic.nl> >>> + >>> +description: | >>> + Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI >>> Reduction + >>> + - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913 >>> + - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925 >>> + - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937 >>> + - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949 >>> + >>> +properties: >>> + $nodename: >>> + pattern: "^clock-controller$" >> >> Drop this requirement. It is in general expected, but there is no need >> for each binding to specify it. > > Should this be put in a common binding then? Could be but we do not have dedicated binding for clock controllers, so this would be a common binding only for name. :) Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.txt b/Documentation/devicetree/bindings/clock/ti,cdce925.txt deleted file mode 100644 index df42ab72718f..000000000000 --- a/Documentation/devicetree/bindings/clock/ti,cdce925.txt +++ /dev/null @@ -1,53 +0,0 @@ -Binding for TI CDCE913/925/937/949 programmable I2C clock synthesizers. - -Reference -This binding uses the common clock binding[1]. - -[1] Documentation/devicetree/bindings/clock/clock-bindings.txt -[2] https://www.ti.com/product/cdce913 -[3] https://www.ti.com/product/cdce925 -[4] https://www.ti.com/product/cdce937 -[5] https://www.ti.com/product/cdce949 - -The driver provides clock sources for each output Y1 through Y5. - -Required properties: - - compatible: Shall be one of the following: - - "ti,cdce913": 1-PLL, 3 Outputs - - "ti,cdce925": 2-PLL, 5 Outputs - - "ti,cdce937": 3-PLL, 7 Outputs - - "ti,cdce949": 4-PLL, 9 Outputs - - reg: I2C device address. - - clocks: Points to a fixed parent clock that provides the input frequency. - - #clock-cells: From common clock bindings: Shall be 1. - -Optional properties: - - xtal-load-pf: Crystal load-capacitor value to fine-tune performance on a - board, or to compensate for external influences. -- vdd-supply: A regulator node for Vdd -- vddout-supply: A regulator node for Vddout - -For all PLL1, PLL2, ... an optional child node can be used to specify spread -spectrum clocking parameters for a board. - - spread-spectrum: SSC mode as defined in the data sheet. - - spread-spectrum-center: Use "centered" mode instead of "max" mode. When - present, the clock runs at the requested frequency on average. Otherwise - the requested frequency is the maximum value of the SCC range. - - -Example: - - clockgen: cdce925pw@64 { - compatible = "cdce925"; - reg = <0x64>; - clocks = <&xtal_27Mhz>; - #clock-cells = <1>; - xtal-load-pf = <5>; - vdd-supply = <&1v8-reg>; - vddout-supply = <&3v3-reg>; - /* PLL options to get SSC 1% centered */ - PLL2 { - spread-spectrum = <4>; - spread-spectrum-center; - }; - }; diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml new file mode 100644 index 000000000000..1e68ee68e458 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml @@ -0,0 +1,104 @@ +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node bindings + +maintainers: + - Mike Looijmans <mike.looijmans@topic.nl> + +description: | + Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI Reduction + + - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913 + - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925 + - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937 + - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949 + +properties: + $nodename: + pattern: "^clock-controller$" + + compatible: + enum: + - ti,cdce913 + - ti,cdce925 + - ti,cdce937 + - ti,cdce949 + + reg: + maxItems: 1 + + clocks: + items: + - description: fixed parent clock + + "#clock-cells": + const: 1 + + vdd-supply: + description: Regulator that provides 1.8V Vdd power supply + + vddout-supply: + description: | + Regulator that provides Vddout power supply. + non-L variant: 2.5V or 3.3V for + L variant: 1.8V for + + xtal-load-pf: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Crystal load-capacitor value to fine-tune performance on a + board, or to compensate for external influences. + +patternProperties: + "^PLL[1-4]$": + type: object + description: | + optional child node can be used to specify spread + spectrum clocking parameters for a board + + properties: + spread-spectrum: + $ref: /schemas/types.yaml#/definitions/uint32 + description: SSC mode as defined in the data sheet + + spread-spectrum-center: + type: boolean + description: | + Use "centered" mode instead of "max" mode. When + present, the clock runs at the requested frequency on average. + Otherwise the requested frequency is the maximum value of the + SCC range. + +required: + - compatible + - ret + - clocks + - "#clock-cells" + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + cdce925: clock-controller@64 { + compatible = "ti,cdce925"; + reg = <0x64>; + clocks = <&xtal_27Mhz>; + #clock-cells = <1>; + xtal-load-pf = <5>; + vdd-supply = <®_1v8>; + vddout-supply = <®_3v3>; + /* PLL options to get SSC 1% centered */ + PLL2 { + spread-spectrum = <4>; + spread-spectrum-center; + }; + }; + };
Convert the TI CDCE925 clock binding to DT schema format. Including a small fix: Add the missing 'ti' prefix in the example compatible. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- I have to admit I only have one specific addon platform for this hardware, which is actually a CECD813 tbh. .../devicetree/bindings/clock/ti,cdce925.txt | 53 --------- .../devicetree/bindings/clock/ti,cdce925.yaml | 104 ++++++++++++++++++ 2 files changed, 104 insertions(+), 53 deletions(-) delete mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.txt create mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.yaml