Message ID | 20231229085156.1490233-1-chunyan.zhang@unisoc.com |
---|---|
Headers | show |
Series | Add UMS9620 clocks driver and bindings | expand |
On 29/12/2023 09:51, Chunyan Zhang wrote: > Add UMS9620's clock compatible strings and descriptions for clock > and clock-names for UMS9620. > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com> > --- > .../bindings/clock/sprd,ums512-clk.yaml | 79 ++++++++++++++++--- > 1 file changed, 68 insertions(+), 11 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > index 43d2b6c31357..dcad41a1fea7 100644 > --- a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > +++ b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > @@ -30,25 +30,38 @@ properties: > - sprd,ums512-mm-clk > - sprd,ums512-mm-gate-clk > - sprd,ums512-apapb-gate > + - sprd,ums9620-pmu-gate > + - sprd,ums9620-g1-pll > + - sprd,ums9620-g5l-pll > + - sprd,ums9620-g5r-pll > + - sprd,ums9620-g8-pll > + - sprd,ums9620-g9-pll > + - sprd,ums9620-g10-pll > + - sprd,ums9620-apapb-gate > + - sprd,ums9620-ap-clk > + - sprd,ums9620-apahb-gate > + - sprd,ums9620-aon-gate > + - sprd,ums9620-aonapb-clk > + - sprd,ums9620-topdvfs-clk > + - sprd,ums9620-ipaapb-gate > + - sprd,ums9620-ipa-clk > + - sprd,ums9620-ipaglb-gate > + - sprd,ums9620-ipadispc-gate > + - sprd,ums9620-pcieapb-gate > + - sprd,ums9620-pcie-clk > + - sprd,ums9620-mm-gate > + - sprd,ums9620-mm-clk > + - sprd,ums9620-dpu-vsp-gate > + - sprd,ums9620-dpu-vsp-clk > > "#clock-cells": > const: 1 > > clocks: > - minItems: 1 > - maxItems: 4 No, constraints stay here. > description: | > The input parent clock(s) phandle for the clock, only list > fixed clocks which are declared in devicetree. > > - clock-names: No, you cannot just drop properties. Widest constraints stay here. > - minItems: 1 > - items: > - - const: ext-26m > - - const: ext-32k > - - const: ext-4m > - - const: rco-100m > - > reg: > maxItems: 1 > > @@ -57,7 +70,43 @@ required: > - '#clock-cells' > - reg > > -additionalProperties: false > +allOf: > + - if: > + properties: > + compatible: > + contains: > + pattern: "^sprd,ums512-.*" > + > + then: > + properties: > + clocks: > + minItems: 1 > + maxItems: 4 > + > + clock-names: > + minItems: 1 > + items: > + - const: ext-26m > + - const: ext-32k > + - const: ext-4m > + - const: rco-100m > + > + - if: > + properties: > + compatible: > + contains: > + pattern: "^sprd,ums9620-.*" > + then: > + properties: > + clocks: > + minItems: 1 > + maxItems: 6 > + > + clock-names: > + minItems: 1 > + maxItems: 6 Missing names. Most likely you are trying to stuff too many devices into one binding. I suggest split, could be with some shared/common schema (see Qualcomm GCC bindings). > + > +unevaluatedProperties: false > > examples: > - | > @@ -68,4 +117,12 @@ examples: > clock-names = "ext-26m"; > #clock-cells = <1>; > }; > + > + aonapb_gate: clock-controller@0 { > + compatible = "sprd,ums9620-aon-gate"; Drop entire example, no need for new example for every new compatible. Best regards, Krzysztof
Hi Krzysztof, On Thu, 4 Jan 2024 at 16:46, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 29/12/2023 09:51, Chunyan Zhang wrote: > > Add UMS9620's clock compatible strings and descriptions for clock > > and clock-names for UMS9620. > > > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com> > > --- > > .../bindings/clock/sprd,ums512-clk.yaml | 79 ++++++++++++++++--- > > 1 file changed, 68 insertions(+), 11 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > > index 43d2b6c31357..dcad41a1fea7 100644 > > --- a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > > +++ b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > > @@ -30,25 +30,38 @@ properties: > > - sprd,ums512-mm-clk > > - sprd,ums512-mm-gate-clk > > - sprd,ums512-apapb-gate > > + - sprd,ums9620-pmu-gate > > + - sprd,ums9620-g1-pll > > + - sprd,ums9620-g5l-pll > > + - sprd,ums9620-g5r-pll > > + - sprd,ums9620-g8-pll > > + - sprd,ums9620-g9-pll > > + - sprd,ums9620-g10-pll > > + - sprd,ums9620-apapb-gate > > + - sprd,ums9620-ap-clk > > + - sprd,ums9620-apahb-gate > > + - sprd,ums9620-aon-gate > > + - sprd,ums9620-aonapb-clk > > + - sprd,ums9620-topdvfs-clk > > + - sprd,ums9620-ipaapb-gate > > + - sprd,ums9620-ipa-clk > > + - sprd,ums9620-ipaglb-gate > > + - sprd,ums9620-ipadispc-gate > > + - sprd,ums9620-pcieapb-gate > > + - sprd,ums9620-pcie-clk > > + - sprd,ums9620-mm-gate > > + - sprd,ums9620-mm-clk > > + - sprd,ums9620-dpu-vsp-gate > > + - sprd,ums9620-dpu-vsp-clk > > > > "#clock-cells": > > const: 1 > > > > clocks: > > - minItems: 1 > > - maxItems: 4 > > No, constraints stay here. > > > description: | > > The input parent clock(s) phandle for the clock, only list > > fixed clocks which are declared in devicetree. > > > > - clock-names: > > No, you cannot just drop properties. Widest constraints stay here. > > > - minItems: 1 > > - items: > > - - const: ext-26m > > - - const: ext-32k > > - - const: ext-4m > > - - const: rco-100m > > - > > reg: > > maxItems: 1 > > > > @@ -57,7 +70,43 @@ required: > > - '#clock-cells' > > - reg > > > > -additionalProperties: false > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + pattern: "^sprd,ums512-.*" > > + > > + then: > > + properties: > > + clocks: > > + minItems: 1 > > + maxItems: 4 > > + > > + clock-names: > > + minItems: 1 > > + items: > > + - const: ext-26m > > + - const: ext-32k > > + - const: ext-4m > > + - const: rco-100m > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + pattern: "^sprd,ums9620-.*" > > + then: > > + properties: > > + clocks: > > + minItems: 1 > > + maxItems: 6 > > + > > + clock-names: > > + minItems: 1 > > + maxItems: 6 > > Missing names. The names are fixed, but the order are not. For example: clk_a { clocks = <&ext_26m>, <&ext_32k>; clock-names = "ext-26m", "ext-32k"; }; clk_b { clocks = <&ext_26m>, <&ext_4m>; clock-names = "ext-26m", "ext-4m"; }; How to list the names for this kind of case? (Please forgive me for being lazy, didn't find a similar case after a fast search) Thanks, Chunyan
On 04/01/2024 10:09, Chunyan Zhang wrote: > Hi Krzysztof, > > On Thu, 4 Jan 2024 at 16:46, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 29/12/2023 09:51, Chunyan Zhang wrote: >>> Add UMS9620's clock compatible strings and descriptions for clock >>> and clock-names for UMS9620. >>> >>> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com> >>> --- >>> .../bindings/clock/sprd,ums512-clk.yaml | 79 ++++++++++++++++--- >>> 1 file changed, 68 insertions(+), 11 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml >>> index 43d2b6c31357..dcad41a1fea7 100644 >>> --- a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml >>> +++ b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml >>> @@ -30,25 +30,38 @@ properties: >>> - sprd,ums512-mm-clk >>> - sprd,ums512-mm-gate-clk >>> - sprd,ums512-apapb-gate >>> + - sprd,ums9620-pmu-gate >>> + - sprd,ums9620-g1-pll >>> + - sprd,ums9620-g5l-pll >>> + - sprd,ums9620-g5r-pll >>> + - sprd,ums9620-g8-pll >>> + - sprd,ums9620-g9-pll >>> + - sprd,ums9620-g10-pll >>> + - sprd,ums9620-apapb-gate >>> + - sprd,ums9620-ap-clk >>> + - sprd,ums9620-apahb-gate >>> + - sprd,ums9620-aon-gate >>> + - sprd,ums9620-aonapb-clk >>> + - sprd,ums9620-topdvfs-clk >>> + - sprd,ums9620-ipaapb-gate >>> + - sprd,ums9620-ipa-clk >>> + - sprd,ums9620-ipaglb-gate >>> + - sprd,ums9620-ipadispc-gate >>> + - sprd,ums9620-pcieapb-gate >>> + - sprd,ums9620-pcie-clk >>> + - sprd,ums9620-mm-gate >>> + - sprd,ums9620-mm-clk >>> + - sprd,ums9620-dpu-vsp-gate >>> + - sprd,ums9620-dpu-vsp-clk >>> >>> "#clock-cells": >>> const: 1 >>> >>> clocks: >>> - minItems: 1 >>> - maxItems: 4 >> >> No, constraints stay here. >> >>> description: | >>> The input parent clock(s) phandle for the clock, only list >>> fixed clocks which are declared in devicetree. >>> >>> - clock-names: >> >> No, you cannot just drop properties. Widest constraints stay here. >> >>> - minItems: 1 >>> - items: >>> - - const: ext-26m >>> - - const: ext-32k >>> - - const: ext-4m >>> - - const: rco-100m >>> - >>> reg: >>> maxItems: 1 >>> >>> @@ -57,7 +70,43 @@ required: >>> - '#clock-cells' >>> - reg >>> >>> -additionalProperties: false >>> +allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + pattern: "^sprd,ums512-.*" >>> + >>> + then: >>> + properties: >>> + clocks: >>> + minItems: 1 >>> + maxItems: 4 >>> + >>> + clock-names: >>> + minItems: 1 >>> + items: >>> + - const: ext-26m >>> + - const: ext-32k >>> + - const: ext-4m >>> + - const: rco-100m >>> + >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + pattern: "^sprd,ums9620-.*" >>> + then: >>> + properties: >>> + clocks: >>> + minItems: 1 >>> + maxItems: 6 >>> + >>> + clock-names: >>> + minItems: 1 >>> + maxItems: 6 >> >> Missing names. > > The names are fixed, but the order are not. Order must be fixed. > > For example: > clk_a { > clocks = <&ext_26m>, <&ext_32k>; > clock-names = "ext-26m", "ext-32k"; > }; > > clk_b { > clocks = <&ext_26m>, <&ext_4m>; > clock-names = "ext-26m", "ext-4m"; And here the order is fixed... > }; > > How to list the names for this kind of case? (Please forgive me for > being lazy, didn't find a similar case after a fast search) The same ums512 lists them or mentioned other bindings. You somehow cut the quote... Best regards, Krzysztof
On Thu, 4 Jan 2024 at 17:21, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 04/01/2024 10:09, Chunyan Zhang wrote: > > Hi Krzysztof, > > > > On Thu, 4 Jan 2024 at 16:46, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 29/12/2023 09:51, Chunyan Zhang wrote: > >>> Add UMS9620's clock compatible strings and descriptions for clock > >>> and clock-names for UMS9620. > >>> > >>> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com> > >>> --- > >>> .../bindings/clock/sprd,ums512-clk.yaml | 79 ++++++++++++++++--- > >>> 1 file changed, 68 insertions(+), 11 deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > >>> index 43d2b6c31357..dcad41a1fea7 100644 > >>> --- a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > >>> +++ b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml > >>> @@ -30,25 +30,38 @@ properties: > >>> - sprd,ums512-mm-clk > >>> - sprd,ums512-mm-gate-clk > >>> - sprd,ums512-apapb-gate > >>> + - sprd,ums9620-pmu-gate > >>> + - sprd,ums9620-g1-pll > >>> + - sprd,ums9620-g5l-pll > >>> + - sprd,ums9620-g5r-pll > >>> + - sprd,ums9620-g8-pll > >>> + - sprd,ums9620-g9-pll > >>> + - sprd,ums9620-g10-pll > >>> + - sprd,ums9620-apapb-gate > >>> + - sprd,ums9620-ap-clk > >>> + - sprd,ums9620-apahb-gate > >>> + - sprd,ums9620-aon-gate > >>> + - sprd,ums9620-aonapb-clk > >>> + - sprd,ums9620-topdvfs-clk > >>> + - sprd,ums9620-ipaapb-gate > >>> + - sprd,ums9620-ipa-clk > >>> + - sprd,ums9620-ipaglb-gate > >>> + - sprd,ums9620-ipadispc-gate > >>> + - sprd,ums9620-pcieapb-gate > >>> + - sprd,ums9620-pcie-clk > >>> + - sprd,ums9620-mm-gate > >>> + - sprd,ums9620-mm-clk > >>> + - sprd,ums9620-dpu-vsp-gate > >>> + - sprd,ums9620-dpu-vsp-clk > >>> > >>> "#clock-cells": > >>> const: 1 > >>> > >>> clocks: > >>> - minItems: 1 > >>> - maxItems: 4 > >> > >> No, constraints stay here. > >> > >>> description: | > >>> The input parent clock(s) phandle for the clock, only list > >>> fixed clocks which are declared in devicetree. > >>> > >>> - clock-names: > >> > >> No, you cannot just drop properties. Widest constraints stay here. > >> > >>> - minItems: 1 > >>> - items: > >>> - - const: ext-26m > >>> - - const: ext-32k > >>> - - const: ext-4m > >>> - - const: rco-100m > >>> - > >>> reg: > >>> maxItems: 1 > >>> > >>> @@ -57,7 +70,43 @@ required: > >>> - '#clock-cells' > >>> - reg > >>> > >>> -additionalProperties: false > >>> +allOf: > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + pattern: "^sprd,ums512-.*" > >>> + > >>> + then: > >>> + properties: > >>> + clocks: > >>> + minItems: 1 > >>> + maxItems: 4 > >>> + > >>> + clock-names: > >>> + minItems: 1 > >>> + items: > >>> + - const: ext-26m > >>> + - const: ext-32k > >>> + - const: ext-4m > >>> + - const: rco-100m > >>> + > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + pattern: "^sprd,ums9620-.*" > >>> + then: > >>> + properties: > >>> + clocks: > >>> + minItems: 1 > >>> + maxItems: 6 > >>> + > >>> + clock-names: > >>> + minItems: 1 > >>> + maxItems: 6 > >> > >> Missing names. > > > > The names are fixed, but the order are not. > > Order must be fixed. > > > > > For example: > > clk_a { > > clocks = <&ext_26m>, <&ext_32k>; > > clock-names = "ext-26m", "ext-32k"; > > }; > > > > clk_b { > > clocks = <&ext_26m>, <&ext_4m>; > > clock-names = "ext-26m", "ext-4m"; > > And here the order is fixed... The order is not fixed, 'clk_b' will cause dtb_check error, since it skips the second one i.e. ext-32k in the clock-names list. > > > }; > > > > How to list the names for this kind of case? (Please forgive me for > > being lazy, didn't find a similar case after a fast search) > > The same ums512 lists them or mentioned other bindings. You somehow cut > the quote... > > Best regards, > Krzysztof >
On 04/01/2024 10:26, Chunyan Zhang wrote: >>>> Missing names. >>> >>> The names are fixed, but the order are not. >> >> Order must be fixed. Did you read this? It must be. If it is not, your patchset has issues you must fix. >> >>> >>> For example: >>> clk_a { >>> clocks = <&ext_26m>, <&ext_32k>; >>> clock-names = "ext-26m", "ext-32k"; >>> }; >>> >>> clk_b { >>> clocks = <&ext_26m>, <&ext_4m>; >>> clock-names = "ext-26m", "ext-4m"; >> >> And here the order is fixed... > > The order is not fixed, 'clk_b' will cause dtb_check error, since it > skips the second one i.e. ext-32k in the clock-names list. Then why do you use the same compatible for two different devices? Best regards, Krzysztof