diff mbox series

[1/5] dt-bindings: clock: fix wrong clock documentation for qcom,rpmcc

Message ID 20220705202837.667-2-ansuelsmth@gmail.com (mailing list archive)
State Superseded, archived
Headers show
Series clk: qcom: Drop use of pxo/cxo_board for rpm devices | expand

Commit Message

Christian Marangi July 5, 2022, 8:28 p.m. UTC
qcom,rpmcc describe 2 different kind of device.
Currently we have definition for rpm-smd based device but we lack
Documentation for simple rpm based device.

Add the missing clk for ipq806x, apq8060, msm8660 and apq8064 and
provide and additional example.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/clock/qcom,rpmcc.yaml | 77 ++++++++++++++++++-
 1 file changed, 73 insertions(+), 4 deletions(-)

Comments

Krzysztof Kozlowski July 6, 2022, 7:43 a.m. UTC | #1
On 05/07/2022 22:28, Christian Marangi wrote:
> qcom,rpmcc describe 2 different kind of device.
> Currently we have definition for rpm-smd based device but we lack
> Documentation for simple rpm based device.
> 
> Add the missing clk for ipq806x, apq8060, msm8660 and apq8064 and
> provide and additional example.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/clock/qcom,rpmcc.yaml | 77 ++++++++++++++++++-
>  1 file changed, 73 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> index 9d296b89a8d0..028eb0277495 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> @@ -48,16 +48,65 @@ properties:
>    '#clock-cells':
>      const: 1
>  
> -  clocks:
> -    maxItems: 1
> +  clocks: true
>  
> -  clock-names:
> -    const: xo
> +  clock-names: true

That's not correct way to describe these. Check other files, we've
already talked about this. You need min and maxItems here.

>  
>  required:
>    - compatible
>    - '#clock-cells'
>  
> +if:

allOf

> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - qcom,rpmcc-apq8060
> +          - qcom,rpmcc-ipq806x
> +          - qcom,rpmcc-msm8660
> +
> +then:
> +  properties:
> +    clocks:
> +      description: pxo clock
> +
> +    clock-names:
> +      const: pxo
> +
> +  required:
> +    - clocks
> +    - clock-names
> +
> +else:
> +  if:

No nested ifs.

> +    properties:
> +      compatible:
> +        contains:
> +          const: qcom,rpmcc-apq8064
> +  then:
> +    properties:
> +      clocks:
> +        items:
> +          - description: pxo clock
> +          - description: cxo clock
> +
> +      clock-names:
> +        items:
> +          - const: pxo
> +          - const: cxo
> +
> +    required:
> +      - clocks
> +      - clock-names
> +
> +  else:
> +    properties:
> +      clocks:
> +        description: xo clock
> +
> +      clock-names:
> +        const: xo
> +
>  additionalProperties: false
>  
>  examples:
> @@ -73,3 +122,23 @@ examples:
>              };
>          };
>      };
> +
> +  - |
> +    rpm {
> +        clock-controller {
> +            compatible = "qcom,rpmcc-ipq806x", "qcom,rpmcc";
> +            #clock-cells = <1>;
> +            clocks = <&pxo_board>;
> +            clock-names = "pxo";
> +        };
> +    };
> +
> +  - |
> +    rpm {
> +        clock-controller {
> +            compatible = "qcom,rpmcc-apq8064", "qcom,rpmcc";
> +            #clock-cells = <1>;
> +            clocks = <&pxo_board>, <&cxo_board>;
> +            clock-names = "pxo", "cxo";
> +        };

These are the same. Provide examples only for blocks which are
significantly different.


Best regards,
Krzysztof
Dmitry Baryshkov July 6, 2022, 8:23 a.m. UTC | #2
On Tue, 5 Jul 2022 at 23:56, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> qcom,rpmcc describe 2 different kind of device.
> Currently we have definition for rpm-smd based device but we lack
> Documentation for simple rpm based device.
>
> Add the missing clk for ipq806x, apq8060, msm8660 and apq8064 and
> provide and additional example.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/clock/qcom,rpmcc.yaml | 77 ++++++++++++++++++-
>  1 file changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> index 9d296b89a8d0..028eb0277495 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
[,,,,]

> +
> +then:
> +  properties:
> +    clocks:
> +      description: pxo clock
> +
> +    clock-names:
> +      const: pxo
> +
> +  required:
> +    - clocks
> +    - clock-names

I don't think you can not mark these properties as required, older
schemas do not have them.

> +
> +else:
> +  if:
> +    properties:
> +      compatible:
> +        contains:
> +          const: qcom,rpmcc-apq8064
> +  then:
> +    properties:
> +      clocks:
> +        items:
> +          - description: pxo clock
> +          - description: cxo clock
[...]
Christian Marangi July 6, 2022, 10:19 a.m. UTC | #3
On Wed, Jul 06, 2022 at 11:23:46AM +0300, Dmitry Baryshkov wrote:
> On Tue, 5 Jul 2022 at 23:56, Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > qcom,rpmcc describe 2 different kind of device.
> > Currently we have definition for rpm-smd based device but we lack
> > Documentation for simple rpm based device.
> >
> > Add the missing clk for ipq806x, apq8060, msm8660 and apq8064 and
> > provide and additional example.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../devicetree/bindings/clock/qcom,rpmcc.yaml | 77 ++++++++++++++++++-
> >  1 file changed, 73 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> > index 9d296b89a8d0..028eb0277495 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> [,,,,]
> 
> > +
> > +then:
> > +  properties:
> > +    clocks:
> > +      description: pxo clock
> > +
> > +    clock-names:
> > +      const: pxo
> > +
> > +  required:
> > +    - clocks
> > +    - clock-names
> 
> I don't think you can not mark these properties as required, older
> schemas do not have them.
>

Well considering we changed rpmcc to parent_data and rpm clock require
pxo clock as parents it seems to be they should be required.

Actually no idea why this wasn't required before. Probably because this
schema described only rpm-smd and not old rpm?

> > +
> > +else:
> > +  if:
> > +    properties:
> > +      compatible:
> > +        contains:
> > +          const: qcom,rpmcc-apq8064
> > +  then:
> > +    properties:
> > +      clocks:
> > +        items:
> > +          - description: pxo clock
> > +          - description: cxo clock
> [...]
> 
> -- 
> With best wishes
> Dmitry
Dmitry Baryshkov July 6, 2022, 11:58 a.m. UTC | #4
On Wed, 6 Jul 2022 at 13:35, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Wed, Jul 06, 2022 at 11:23:46AM +0300, Dmitry Baryshkov wrote:
> > On Tue, 5 Jul 2022 at 23:56, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > >
> > > qcom,rpmcc describe 2 different kind of device.
> > > Currently we have definition for rpm-smd based device but we lack
> > > Documentation for simple rpm based device.
> > >
> > > Add the missing clk for ipq806x, apq8060, msm8660 and apq8064 and
> > > provide and additional example.
> > >
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > >  .../devicetree/bindings/clock/qcom,rpmcc.yaml | 77 ++++++++++++++++++-
> > >  1 file changed, 73 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> > > index 9d296b89a8d0..028eb0277495 100644
> > > --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
> > [,,,,]
> >
> > > +
> > > +then:
> > > +  properties:
> > > +    clocks:
> > > +      description: pxo clock
> > > +
> > > +    clock-names:
> > > +      const: pxo
> > > +
> > > +  required:
> > > +    - clocks
> > > +    - clock-names
> >
> > I don't think you can not mark these properties as required, older
> > schemas do not have them.
> >
>
> Well considering we changed rpmcc to parent_data and rpm clock require
> pxo clock as parents it seems to be they should be required.

parent_data specifies both the normal flow (.fw_name) and the fallback
(.name), so they are not required.
I actually miss 'recommended: true' in YAML for such cases.

> Actually no idea why this wasn't required before. Probably because this
> schema described only rpm-smd and not old rpm?
>
> > > +
> > > +else:
> > > +  if:
> > > +    properties:
> > > +      compatible:
> > > +        contains:
> > > +          const: qcom,rpmcc-apq8064
> > > +  then:
> > > +    properties:
> > > +      clocks:
> > > +        items:
> > > +          - description: pxo clock
> > > +          - description: cxo clock
> > [...]
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
index 9d296b89a8d0..028eb0277495 100644
--- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.yaml
@@ -48,16 +48,65 @@  properties:
   '#clock-cells':
     const: 1
 
-  clocks:
-    maxItems: 1
+  clocks: true
 
-  clock-names:
-    const: xo
+  clock-names: true
 
 required:
   - compatible
   - '#clock-cells'
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - qcom,rpmcc-apq8060
+          - qcom,rpmcc-ipq806x
+          - qcom,rpmcc-msm8660
+
+then:
+  properties:
+    clocks:
+      description: pxo clock
+
+    clock-names:
+      const: pxo
+
+  required:
+    - clocks
+    - clock-names
+
+else:
+  if:
+    properties:
+      compatible:
+        contains:
+          const: qcom,rpmcc-apq8064
+  then:
+    properties:
+      clocks:
+        items:
+          - description: pxo clock
+          - description: cxo clock
+
+      clock-names:
+        items:
+          - const: pxo
+          - const: cxo
+
+    required:
+      - clocks
+      - clock-names
+
+  else:
+    properties:
+      clocks:
+        description: xo clock
+
+      clock-names:
+        const: xo
+
 additionalProperties: false
 
 examples:
@@ -73,3 +122,23 @@  examples:
             };
         };
     };
+
+  - |
+    rpm {
+        clock-controller {
+            compatible = "qcom,rpmcc-ipq806x", "qcom,rpmcc";
+            #clock-cells = <1>;
+            clocks = <&pxo_board>;
+            clock-names = "pxo";
+        };
+    };
+
+  - |
+    rpm {
+        clock-controller {
+            compatible = "qcom,rpmcc-apq8064", "qcom,rpmcc";
+            #clock-cells = <1>;
+            clocks = <&pxo_board>, <&cxo_board>;
+            clock-names = "pxo", "cxo";
+        };
+    };