[fixes] dt-bindings: PCI: fsl,imx6q: fix assigned-clocks warning

Message ID 20230508071837.68552-1-krzysztof.kozlowski@linaro.org
State New
Headers
Series [fixes] dt-bindings: PCI: fsl,imx6q: fix assigned-clocks warning |

Commit Message

Krzysztof Kozlowski May 8, 2023, 7:18 a.m. UTC
  assigned-clocks are a dependency of clocks, however the dtschema has
limitation and expects clocks to be present in the binding using
assigned-clocks, not in other referenced bindings.  The clocks were
defined in common fsl,imx6q-pcie-common.yaml, which is referenced by fsl,imx6q-pcie-ep.yaml.  The fsl,imx6q-pcie-ep.yaml used assigned-clocks thus leading to warnings:

  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.example.dtb: pcie-ep@33800000:
    Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected)
  From schema: Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml

Fix this by moving clocks to each specific schema from the common one
and narrowing them to strictly match what is expected for given device.

Fixes: b10f82380eeb ("dt-bindings: imx6q-pcie: Restruct i.MX PCIe schema")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Patch for current cycle (v6.4-rc).   Please take directly as fixes or
let me know, so I will send it to Linus.
---
 .../bindings/pci/fsl,imx6q-pcie-common.yaml   | 13 +---
 .../bindings/pci/fsl,imx6q-pcie-ep.yaml       | 38 +++++++++
 .../bindings/pci/fsl,imx6q-pcie.yaml          | 77 +++++++++++++++++++
 3 files changed, 117 insertions(+), 11 deletions(-)
  

Comments

Conor Dooley May 8, 2023, 8:23 a.m. UTC | #1
On Mon, May 08, 2023 at 09:18:37AM +0200, Krzysztof Kozlowski wrote:
> @@ -49,6 +62,31 @@ required:
>  allOf:
>    - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
>    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx8mq-pcie-ep

How come this is enum rather than const (and same for the other
single-compatible ones below)?

Cheers,
Conor.

> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +        clock-names:
> +          items:
> +            - const: pcie
> +            - const: pcie_bus
> +            - const: pcie_phy
> +            - const: pcie_aux
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pcie
> +            - const: pcie_bus
> +            - const: pcie_aux
> +
>  
>  unevaluatedProperties: false
>  
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index 2443641754d3..81bbb8728f0f 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> @@ -40,6 +40,19 @@ properties:
>        - const: dbi
>        - const: config
>  
> +  clocks:
> +    minItems: 3
> +    items:
> +      - description: PCIe bridge clock.
> +      - description: PCIe bus clock.
> +      - description: PCIe PHY clock.
> +      - description: Additional required clock entry for imx6sx-pcie,
> +           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
> +
> +  clock-names:
> +    minItems: 3
> +    maxItems: 4
> +
>    interrupts:
>      items:
>        - description: builtin MSI controller.
> @@ -77,6 +90,70 @@ required:
>  allOf:
>    - $ref: /schemas/pci/snps,dw-pcie.yaml#
>    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx6sx-pcie
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +        clock-names:
> +          items:
> +            - const: pcie
> +            - const: pcie_bus
> +            - const: pcie_phy
> +            - const: pcie_inbound_axi
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx8mq-pcie
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +        clock-names:
> +          items:
> +            - const: pcie
> +            - const: pcie_bus
> +            - const: pcie_phy
> +            - const: pcie_aux
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx6q-pcie
> +            - fsl,imx6qp-pcie
> +            - fsl,imx7d-pcie
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pcie
> +            - const: pcie_bus
> +            - const: pcie_phy
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx8mm-pcie
> +            - fsl,imx8mp-pcie
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pcie
> +            - const: pcie_bus
> +            - const: pcie_aux
>  
>  unevaluatedProperties: false
>  
> -- 
> 2.34.1
>
  
Rob Herring May 8, 2023, 8:27 a.m. UTC | #2
On Mon, 08 May 2023 09:18:37 +0200, Krzysztof Kozlowski wrote:
> assigned-clocks are a dependency of clocks, however the dtschema has
> limitation and expects clocks to be present in the binding using
> assigned-clocks, not in other referenced bindings.  The clocks were
> defined in common fsl,imx6q-pcie-common.yaml, which is referenced by fsl,imx6q-pcie-ep.yaml.  The fsl,imx6q-pcie-ep.yaml used assigned-clocks thus leading to warnings:
> 
>   Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.example.dtb: pcie-ep@33800000:
>     Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected)
>   From schema: Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> 
> Fix this by moving clocks to each specific schema from the common one
> and narrowing them to strictly match what is expected for given device.
> 
> Fixes: b10f82380eeb ("dt-bindings: imx6q-pcie: Restruct i.MX PCIe schema")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Patch for current cycle (v6.4-rc).   Please take directly as fixes or
> let me know, so I will send it to Linus.
> ---
>  .../bindings/pci/fsl,imx6q-pcie-common.yaml   | 13 +---
>  .../bindings/pci/fsl,imx6q-pcie-ep.yaml       | 38 +++++++++
>  .../bindings/pci/fsl,imx6q-pcie.yaml          | 77 +++++++++++++++++++
>  3 files changed, 117 insertions(+), 11 deletions(-)
> 

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/media/i2c/ovti,ov2685.example.dtb: camera-sensor@3c: port:endpoint:data-lanes: [[1]] is too short
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb: camera@3c: port:endpoint:data-lanes: [[1]] is too short
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml

doc reference errors (make refcheckdocs):
Documentation/usb/gadget_uvc.rst: Documentation/userspace-api/media/v4l/pixfmt-packed.yuv.rst
MAINTAINERS: Documentation/devicetree/bindings/pwm/pwm-apple.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230508071837.68552-1-krzysztof.kozlowski@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
  
Krzysztof Kozlowski May 8, 2023, 9:22 a.m. UTC | #3
On 08/05/2023 10:23, Conor Dooley wrote:
> On Mon, May 08, 2023 at 09:18:37AM +0200, Krzysztof Kozlowski wrote:
>> @@ -49,6 +62,31 @@ required:
>>  allOf:
>>    - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
>>    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          enum:
>> +            - fsl,imx8mq-pcie-ep
> 
> How come this is enum rather than const (and same for the other
> single-compatible ones below)?

I assume the list might grow, so enum will spare one indentation change.

Best regards,
Krzysztof
  
Conor Dooley May 8, 2023, 4:25 p.m. UTC | #4
On Mon, May 08, 2023 at 11:22:36AM +0200, Krzysztof Kozlowski wrote:
> On 08/05/2023 10:23, Conor Dooley wrote:
> > On Mon, May 08, 2023 at 09:18:37AM +0200, Krzysztof Kozlowski wrote:
> >> @@ -49,6 +62,31 @@ required:
> >>  allOf:
> >>    - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
> >>    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          enum:
> >> +            - fsl,imx8mq-pcie-ep
> > 
> > How come this is enum rather than const (and same for the other
> > single-compatible ones below)?
> 
> I assume the list might grow, so enum will spare one indentation change.

Figured it'd be either that or consistency across the entries.

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
  
Bjorn Helgaas May 8, 2023, 9:35 p.m. UTC | #5
On Mon, May 08, 2023 at 09:18:37AM +0200, Krzysztof Kozlowski wrote:
> assigned-clocks are a dependency of clocks, however the dtschema has
> limitation and expects clocks to be present in the binding using
> assigned-clocks, not in other referenced bindings.  The clocks were
> defined in common fsl,imx6q-pcie-common.yaml, which is referenced by fsl,imx6q-pcie-ep.yaml.  The fsl,imx6q-pcie-ep.yaml used assigned-clocks thus leading to warnings:

Weirdly long line here.  I think it's good to preserve longs lines in
warnings and error messages themselves, but in the normal text it
looks like an oversight.

>   Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.example.dtb: pcie-ep@33800000:
>     Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected)
>   From schema: Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
  
Krzysztof Kozlowski May 9, 2023, 6:02 a.m. UTC | #6
On Mon, 08 May 2023 09:18:37 +0200, Krzysztof Kozlowski wrote:
> assigned-clocks are a dependency of clocks, however the dtschema has
> limitation and expects clocks to be present in the binding using
> assigned-clocks, not in other referenced bindings.  The clocks were
> defined in common fsl,imx6q-pcie-common.yaml, which is referenced by fsl,imx6q-pcie-ep.yaml.  The fsl,imx6q-pcie-ep.yaml used assigned-clocks thus leading to warnings:
> 
>   Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.example.dtb: pcie-ep@33800000:
>     Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected)
>   From schema: Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> 
> [...]

Applied, thanks!

[1/1] dt-bindings: PCI: fsl,imx6q: fix assigned-clocks warning
      https://git.kernel.org/krzk/linux-dt/c/8bbec86ce6d66fb33530c679f7bb3a123fc9e7da

Best regards,
  
Krzysztof Kozlowski May 9, 2023, 6:03 a.m. UTC | #7
On 08/05/2023 23:35, Bjorn Helgaas wrote:
> On Mon, May 08, 2023 at 09:18:37AM +0200, Krzysztof Kozlowski wrote:
>> assigned-clocks are a dependency of clocks, however the dtschema has
>> limitation and expects clocks to be present in the binding using
>> assigned-clocks, not in other referenced bindings.  The clocks were
>> defined in common fsl,imx6q-pcie-common.yaml, which is referenced by fsl,imx6q-pcie-ep.yaml.  The fsl,imx6q-pcie-ep.yaml used assigned-clocks thus leading to warnings:
> 
> Weirdly long line here.  I think it's good to preserve longs lines in
> warnings and error messages themselves, but in the normal text it
> looks like an oversight.

Thanks, somehow I missed it. I fixed it and applied. Let me know if you
would prefer to take it.

Best regards,
Krzysztof
  

Patch

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
index 9bff8ecb653c..d91b639ae7ae 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
@@ -17,20 +17,11 @@  description:
 properties:
   clocks:
     minItems: 3
-    items:
-      - description: PCIe bridge clock.
-      - description: PCIe bus clock.
-      - description: PCIe PHY clock.
-      - description: Additional required clock entry for imx6sx-pcie,
-           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
+    maxItems: 4
 
   clock-names:
     minItems: 3
-    items:
-      - const: pcie
-      - const: pcie_bus
-      - enum: [ pcie_phy, pcie_aux ]
-      - enum: [ pcie_inbound_axi, pcie_aux ]
+    maxItems: 4
 
   num-lanes:
     const: 1
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
index f4a328ec1daa..ee155ed5f181 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
@@ -31,6 +31,19 @@  properties:
       - const: dbi
       - const: addr_space
 
+  clocks:
+    minItems: 3
+    items:
+      - description: PCIe bridge clock.
+      - description: PCIe bus clock.
+      - description: PCIe PHY clock.
+      - description: Additional required clock entry for imx6sx-pcie,
+           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
+
+  clock-names:
+    minItems: 3
+    maxItems: 4
+
   interrupts:
     items:
       - description: builtin eDMA interrupter.
@@ -49,6 +62,31 @@  required:
 allOf:
   - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
   - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx8mq-pcie-ep
+    then:
+      properties:
+        clocks:
+          minItems: 4
+        clock-names:
+          items:
+            - const: pcie
+            - const: pcie_bus
+            - const: pcie_phy
+            - const: pcie_aux
+    else:
+      properties:
+        clocks:
+          maxItems: 3
+        clock-names:
+          items:
+            - const: pcie
+            - const: pcie_bus
+            - const: pcie_aux
+
 
 unevaluatedProperties: false
 
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 2443641754d3..81bbb8728f0f 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -40,6 +40,19 @@  properties:
       - const: dbi
       - const: config
 
+  clocks:
+    minItems: 3
+    items:
+      - description: PCIe bridge clock.
+      - description: PCIe bus clock.
+      - description: PCIe PHY clock.
+      - description: Additional required clock entry for imx6sx-pcie,
+           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
+
+  clock-names:
+    minItems: 3
+    maxItems: 4
+
   interrupts:
     items:
       - description: builtin MSI controller.
@@ -77,6 +90,70 @@  required:
 allOf:
   - $ref: /schemas/pci/snps,dw-pcie.yaml#
   - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx6sx-pcie
+    then:
+      properties:
+        clocks:
+          minItems: 4
+        clock-names:
+          items:
+            - const: pcie
+            - const: pcie_bus
+            - const: pcie_phy
+            - const: pcie_inbound_axi
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx8mq-pcie
+    then:
+      properties:
+        clocks:
+          minItems: 4
+        clock-names:
+          items:
+            - const: pcie
+            - const: pcie_bus
+            - const: pcie_phy
+            - const: pcie_aux
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx6q-pcie
+            - fsl,imx6qp-pcie
+            - fsl,imx7d-pcie
+    then:
+      properties:
+        clocks:
+          maxItems: 3
+        clock-names:
+          items:
+            - const: pcie
+            - const: pcie_bus
+            - const: pcie_phy
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx8mm-pcie
+            - fsl,imx8mp-pcie
+    then:
+      properties:
+        clocks:
+          maxItems: 3
+        clock-names:
+          items:
+            - const: pcie
+            - const: pcie_bus
+            - const: pcie_aux
 
 unevaluatedProperties: false