diff mbox series

[v4,2/3] dt-bindings: arm64: dts: add dt-bindings for Marvell COM Express boards

Message ID 20231029174814.559583-3-enachman@marvell.com
State Changes Requested
Headers show
Series arm64: dts: cn913x: add COM Express boards | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dt-meta-schema fail build log

Commit Message

Elad Nachman Oct. 29, 2023, 5:48 p.m. UTC
From: Elad Nachman <enachman@marvell.com>

Add dt bindings for:
CN9130 COM Express CPU module
CN9131 COM Express CPU module
AC5X RD COM Express Type 7 carrier board.
AC5X RD COM Express board with a CN9131 COM Express Type 7 CPU module.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
 .../bindings/arm/marvell/armada-7k-8k.yaml        | 15 +++++++++++++++
 .../bindings/arm/marvell/marvell,ac5.yaml         | 14 ++++++++++++++
 2 files changed, 29 insertions(+)

Comments

Rob Herring (Arm) Oct. 30, 2023, 12:48 p.m. UTC | #1
On Sun, 29 Oct 2023 19:48:13 +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add dt bindings for:
> CN9130 COM Express CPU module
> CN9131 COM Express CPU module
> AC5X RD COM Express Type 7 carrier board.
> AC5X RD COM Express board with a CN9131 COM Express Type 7 CPU module.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../bindings/arm/marvell/armada-7k-8k.yaml        | 15 +++++++++++++++
>  .../bindings/arm/marvell/marvell,ac5.yaml         | 14 ++++++++++++++
>  2 files changed, 29 insertions(+)
> 

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:
./Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml:37:12: [warning] wrong indentation: expected 10 but found 11 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231029174814.559583-3-enachman@marvell.com

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.
Rob Herring (Arm) Oct. 30, 2023, 1:22 p.m. UTC | #2
On Sun, Oct 29, 2023 at 07:48:13PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>

Drop 'dts:' from the subject. This is not a .dts file.

Also drop the 2nd 'dt-bindings'. Subject line space is precious.

> 
> Add dt bindings for:
> CN9130 COM Express CPU module
> CN9131 COM Express CPU module
> AC5X RD COM Express Type 7 carrier board.
> AC5X RD COM Express board with a CN9131 COM Express Type 7 CPU module.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../bindings/arm/marvell/armada-7k-8k.yaml        | 15 +++++++++++++++
>  .../bindings/arm/marvell/marvell,ac5.yaml         | 14 ++++++++++++++
>  2 files changed, 29 insertions(+)
Krzysztof Kozlowski Oct. 30, 2023, 1:31 p.m. UTC | #3
On 29/10/2023 18:48, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 

A nit, subject: drop second/last, redundant "dt-bindings for". The
"dt-bindings" prefix is already stating that these are bindings.

> Add dt bindings for:
> CN9130 COM Express CPU module
> CN9131 COM Express CPU module
> AC5X RD COM Express Type 7 carrier board.
> AC5X RD COM Express board with a CN9131 COM Express Type 7 CPU module.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../bindings/arm/marvell/armada-7k-8k.yaml        | 15 +++++++++++++++
>  .../bindings/arm/marvell/marvell,ac5.yaml         | 14 ++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> index 52d78521e412..7e0ac5110eef 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> @@ -60,4 +60,19 @@ properties:
>            - const: marvell,armada-ap807-quad
>            - const: marvell,armada-ap807
>  
> +      - description: Armada CN9130 SoC without external CP as COM Express CPU module
> +        items:
> +          - const: marvell,cn9130-cpu-module

How CPU module can be used alone?

> +          - const: marvell,cn9130
> +          - const: marvell,armada-ap807-quad
> +          - const: marvell,armada-ap807
> +
> +      - description: Armada CN9131 SoC with one external CP as COM Express CPU module
> +        items:
> +          - const: marvell,cn9131-cpu-module
> +          - const: marvell,cn9131
> +          - const: marvell,cn9130
> +          - const: marvell,armada-ap807-quad
> +          - const: marvell,armada-ap807
> +
>  additionalProperties: true
> diff --git a/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml b/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml
> index 8960fb8b2b2f..734e1716a3e9 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml
> @@ -27,6 +27,20 @@ properties:
>            - const: marvell,ac5x
>            - const: marvell,ac5
>  
> +      - description: Alleycat5X (98DX35xx) Reference Design as COM Express Carrier
> +        items:
> +          - enum:
> +              - marvell,rd-ac5x-carrier
> +          - const: marvell,ac5x
> +
> +      - description:
> +           Alleycat5X (98DX35xx) Reference Design as COM Express Carrier plus
> +           Armada CN9131 COM Express CPU module
> +        items:
> +          - enum:
> +              - marvell,rd-ac5x-carrier-with-cn9131
> +          - const: marvell,ac5x

You never responded to me concerns, so I still do not know what's this.
This claims it has CN9131, but compatibles are missing.

Can anyone help us understand why AC5 has CN9130 but these are in
different files?

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
index 52d78521e412..7e0ac5110eef 100644
--- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
+++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
@@ -60,4 +60,19 @@  properties:
           - const: marvell,armada-ap807-quad
           - const: marvell,armada-ap807
 
+      - description: Armada CN9130 SoC without external CP as COM Express CPU module
+        items:
+          - const: marvell,cn9130-cpu-module
+          - const: marvell,cn9130
+          - const: marvell,armada-ap807-quad
+          - const: marvell,armada-ap807
+
+      - description: Armada CN9131 SoC with one external CP as COM Express CPU module
+        items:
+          - const: marvell,cn9131-cpu-module
+          - const: marvell,cn9131
+          - const: marvell,cn9130
+          - const: marvell,armada-ap807-quad
+          - const: marvell,armada-ap807
+
 additionalProperties: true
diff --git a/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml b/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml
index 8960fb8b2b2f..734e1716a3e9 100644
--- a/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml
+++ b/Documentation/devicetree/bindings/arm/marvell/marvell,ac5.yaml
@@ -27,6 +27,20 @@  properties:
           - const: marvell,ac5x
           - const: marvell,ac5
 
+      - description: Alleycat5X (98DX35xx) Reference Design as COM Express Carrier
+        items:
+          - enum:
+              - marvell,rd-ac5x-carrier
+          - const: marvell,ac5x
+
+      - description:
+           Alleycat5X (98DX35xx) Reference Design as COM Express Carrier plus
+           Armada CN9131 COM Express CPU module
+        items:
+          - enum:
+              - marvell,rd-ac5x-carrier-with-cn9131
+          - const: marvell,ac5x
+
 additionalProperties: true
 
 ...