[PATCH v6 0/3] arm64: dts: cn913x: add COM Express boards

[PATCH v6 0/3] arm64: dts: cn913x: add COM Express boards

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Add support for CN9130 and CN9131 COM Express Type 7 CPU
module boards by Marvell.
Add device tree bindings for this board.
Define these COM Express CPU modules as dtsi, and
provide a dtsi file for a carrier board (Marvell AC5X RD
COM Express type 7 carrier board).
This Carrier board only utilizes the PCIe link, hence no
special device / driver support is provided by this dtsi file.
Finally, add dts file for the combined carrier and CPU module.

v6:
   1) Add cn9130 COM Express system

   2) Drop with from compatibility name of COM Express system

   3) Fix identation issues of dt bindings

v5:

   1) List only carrier compatibility on carrier dtsi

   2) Fix dt_bindings_check warnings using latest yamllint/dtschema

   3) Fix subject lines to remove unnecessary wordings.

   4) Remove dt bindings for standalone CPU modules

   5) Move CN913x dt bindings to A7K dt bindings file

   6) Fix dtbs_check warnings for dtb and bindings,
      using latest yamllint/dtschema.

   7) Move memory definition to main dts file, as memory
      is socket based.

v4:
   1) reorder patches - dt bindings before dts/dtsi files

   2) correct description in dt bindings

   3) separate dt bindings for CPU module, carrier and combination

   4) make carrier board dts into dtsi, make dts for combination of
      carrier and CPU module

   5) correct compatibility strings and file names to use dashes
      instead of underscores

v3:
   1) Remove acronym which creates warnings for checkpatch.pl

   2) Correct compatibility string for ac5x rd board

   3) Add above compatibility string to dt bindings

   4) update MAINTAINERS file with ac5 series dts files

   5) remove memory property from carrier dts

   6) add comment explaining that OOB RGMII ethernet port
      connector and PHY are both on CPU module

v2:
   1) add compatibility string for the board

   2) remove unneeded hard-coded PHY LED blinking mode initialization

   3) Split the CPU portion of the carrier board to 
      dtsi files, and define a dts file for the AC5X RD
      carrier board.

Elad Nachman (3):
  MAINTAINERS: add ac5 to list of maintained Marvell dts files
  dt-bindings: arm64: add Marvell COM Express boards
  arm64: dts: cn913x: add device trees for COM Express boards

 .../bindings/arm/marvell/armada-7k-8k.yaml    |  24 ++++
 MAINTAINERS                                   |   1 +
 arch/arm64/boot/dts/marvell/Makefile          |   1 +
 .../dts/marvell/ac5x-rd-carrier-cn9131.dts    |  25 ++++
 .../boot/dts/marvell/ac5x-rd-carrier.dtsi     |  14 +++
 .../dts/marvell/cn9130-db-comexpress.dtsi     |  96 ++++++++++++++++
 .../dts/marvell/cn9131-db-comexpress.dtsi     | 108 ++++++++++++++++++
 7 files changed, 269 insertions(+)
 create mode 100644 arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dts
 create mode 100644 arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
 create mode 100644 arch/arm64/boot/dts/marvell/cn9130-db-comexpress.dtsi
 create mode 100644 arch/arm64/boot/dts/marvell/cn9131-db-comexpress.dtsi

-- 
2.25.1

[PATCH v6 1/3] MAINTAINERS: add ac5 to list of maintained Marvell dts files

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Add ac5 dts files to the list of maintained Marvell Armada dts files

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b81da7a36a36..6f863a0c3248 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2339,6 +2339,7 @@ F:	arch/arm/boot/dts/marvell/armada*
 F:	arch/arm/boot/dts/marvell/kirkwood*
 F:	arch/arm/configs/mvebu_*_defconfig
 F:	arch/arm/mach-mvebu/
+F:	arch/arm64/boot/dts/marvell/ac5*
 F:	arch/arm64/boot/dts/marvell/armada*
 F:	arch/arm64/boot/dts/marvell/cn913*
 F:	drivers/clk/mvebu/
-- 
2.25.1

[PATCH v6 2/3] dt-bindings: arm64: add Marvell COM Express boards

[Elad Nachman]

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    | 24 +++++++++++++++++++
 1 file changed, 24 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..d00866aeaa8d 100644
--- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
+++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
@@ -60,4 +60,28 @@ properties:
           - const: marvell,armada-ap807-quad
           - const: marvell,armada-ap807
 
+      - description:
+          Alleycat5X (98DX35xx) Reference Design as COM Express Carrier plus
+          Armada CN9130 COM Express CPU module
+        items:
+          - enum:
+              - marvell,cn9130-ac5x-carrier
+          - const: marvell,rd-ac5x-carrier
+          - const: marvell,cn9130-cpu-module
+          - const: marvell,cn9130
+          - const: marvell,armada-ap807-quad
+          - const: marvell,armada-ap807
+
+      - description:
+          Alleycat5X (98DX35xx) Reference Design as COM Express Carrier plus
+          Armada CN9131 COM Express CPU module
+        items:
+          - enum:
+              - marvell,cn9131-ac5x-carrier
+          - const: marvell,rd-ac5x-carrier
+          - const: marvell,cn9131-cpu-module
+          - const: marvell,cn9131
+          - const: marvell,armada-ap807-quad
+          - const: marvell,armada-ap807
+
 additionalProperties: true
-- 
2.25.1

[PATCH v6 3/3] arm64: dts: cn913x: add device trees for COM Express boards

[Elad Nachman]

From: Elad Nachman <enachman@marvell.com>

Add support for CN9130 and CN9131 COM Express Type 7 CPU
module boards by Marvell.
Define these COM Express CPU modules as dtsi and
provide a dtsi file for a carrier board (Marvell AC5X RD
COM Express type 7 carrier board).
This Carrier board only utilizes the PCIe link, hence no
special device / driver support is provided by this dtsi file.
Finally, provide a dts file for the com express carrier and
CPU module combination.

These COM Express boards differ from the existing CN913x DB
boards by the type of ethernet connection (RGMII),
the type of voltage regulators (not i2c expander based)
and the USB phy (not UTMI based).
Note - PHY + RGMII connector is OOB on CPU module.
CN9131 COM Express board is basically CN9130 COM Express board
with an additional CP115 I/O co-processor, which in this case
provides an additional USB host controller on the board.

Signed-off-by: Elad Nachman <enachman@marvell.com>
---
 arch/arm64/boot/dts/marvell/Makefile          |   1 +
 .../dts/marvell/ac5x-rd-carrier-cn9131.dts    |  25 ++++
 .../boot/dts/marvell/ac5x-rd-carrier.dtsi     |  14 +++
 .../dts/marvell/cn9130-db-comexpress.dtsi     |  96 ++++++++++++++++
 .../dts/marvell/cn9131-db-comexpress.dtsi     | 108 ++++++++++++++++++
 5 files changed, 244 insertions(+)
 create mode 100644 arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dts
 create mode 100644 arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
 create mode 100644 arch/arm64/boot/dts/marvell/cn9130-db-comexpress.dtsi
 create mode 100644 arch/arm64/boot/dts/marvell/cn9131-db-comexpress.dtsi

diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
index 79ac09b58a89..99b8cb3c49e1 100644
--- a/arch/arm64/boot/dts/marvell/Makefile
+++ b/arch/arm64/boot/dts/marvell/Makefile
@@ -26,4 +26,5 @@ dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db-B.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-A.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-B.dtb
+dtb-$(CONFIG_ARCH_MVEBU) += ac5x-rd-carrier-cn9131.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += ac5-98dx35xx-rd.dtb
diff --git a/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dts b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dts
new file mode 100644
index 000000000000..e83b9ae379ec
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dts
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 Marvell International Ltd.
+ *
+ * Device tree for the AC5X RD Type 7 Com Express carrier board,
+ * Utilizing the CN913x COM Express CPU module board.
+ * This specific board only maintains a PCIe link with the CPU CPU module
+ * module, which does not require any special DTS definitions.
+ */
+
+#include "cn9131-db-comexpress.dtsi"
+#include "ac5x-rd-carrier.dtsi"
+
+/ {
+	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board with CN9131 CPU module";
+	compatible = "marvell,cn9131-ac5x-carrier", "marvell,rd-ac5x-carrier",
+			"marvell,cn9131-cpu-module", "marvell,cn9131",
+			"marvell,armada-ap807-quad", "marvell,armada-ap807";
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x2 0x00000000>;
+	};
+
+};
diff --git a/arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
new file mode 100644
index 000000000000..fd45d5582233
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 Marvell International Ltd.
+ *
+ * Device tree for the AC5X RD Type 7 Com Express carrier board,
+ * This specific board only maintains a PCIe link with the CPU CPU module
+ * module, which does not require any special DTS definitions.
+ */
+
+/ {
+	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board";
+	compatible = "marvell,rd-ac5x-carrier";
+
+};
diff --git a/arch/arm64/boot/dts/marvell/cn9130-db-comexpress.dtsi b/arch/arm64/boot/dts/marvell/cn9130-db-comexpress.dtsi
new file mode 100644
index 000000000000..028496ebc473
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/cn9130-db-comexpress.dtsi
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 Marvell International Ltd.
+ *
+ * Device tree for the CN9130-DB Com Express CPU module board.
+ */
+
+#include "cn9130-db.dtsi"
+
+/ {
+	model = "Marvell Armada CN9130-DB COM EXPRESS type 7 CPU module board";
+	compatible = "marvell,cn9130-cpu-module", "marvell,cn9130",
+		     "marvell,armada-ap807-quad", "marvell,armada-ap807";
+
+};
+
+&ap0_reg_sd_vccq {
+	regulator-max-microvolt = <1800000>;
+	states = <1800000 0x1 1800000 0x0>;
+	/delete-property/ gpios;
+};
+
+&cp0_reg_usb3_vbus0 {
+	/delete-property/ gpio;
+};
+
+&cp0_reg_usb3_vbus1 {
+	/delete-property/ gpio;
+};
+
+&cp0_reg_sd_vcc {
+	status = "disabled";
+};
+
+&cp0_reg_sd_vccq {
+	status = "disabled";
+};
+
+&cp0_sdhci0 {
+	status = "disabled";
+};
+
+&cp0_eth0 {
+	status = "disabled";
+};
+
+&cp0_eth1 {
+	status = "okay";
+	phy = <&phy0>;
+	phy-mode = "rgmii-id";
+};
+
+&cp0_eth2 {
+	status = "disabled";
+};
+
+&cp0_mdio {
+	status = "okay";
+	pinctrl-0 = <&cp0_ge_mdio_pins>;
+	phy0: ethernet-phy@0 {
+		status = "okay";
+	};
+};
+
+&cp0_syscon0 {
+	cp0_pinctrl: pinctrl {
+		compatible = "marvell,cp115-standalone-pinctrl";
+
+		cp0_ge_mdio_pins: ge-mdio-pins {
+			marvell,pins = "mpp40", "mpp41";
+			marvell,function = "ge";
+		};
+	};
+};
+
+&cp0_sdhci0 {
+	status = "disabled";
+};
+
+&cp0_spi1 {
+	status = "okay";
+};
+
+&cp0_usb3_0 {
+	status = "okay";
+	usb-phy = <&cp0_usb3_0_phy0>;
+	phy-names = "usb";
+	/delete-property/ phys;
+};
+
+&cp0_usb3_1 {
+	status = "okay";
+	usb-phy = <&cp0_usb3_0_phy1>;
+	phy-names = "usb";
+	/delete-property/ phys;
+};
diff --git a/arch/arm64/boot/dts/marvell/cn9131-db-comexpress.dtsi b/arch/arm64/boot/dts/marvell/cn9131-db-comexpress.dtsi
new file mode 100644
index 000000000000..6f3914bcfd01
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/cn9131-db-comexpress.dtsi
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2023 Marvell International Ltd.
+ *
+ * Device tree for the CN9131-DB Com Express CPU module board.
+ */
+
+#include "cn9131-db.dtsi"
+
+/ {
+	model = "Marvell Armada CN9131-DB COM EXPRESS type 7 CPU module board";
+	compatible = "marvell,cn9131-cpu-module", "marvell,cn9131", "marvell,cn9130",
+		     "marvell,armada-ap807-quad", "marvell,armada-ap807";
+
+};
+
+&ap0_reg_sd_vccq {
+	regulator-max-microvolt = <1800000>;
+	states = <1800000 0x1 1800000 0x0>;
+	/delete-property/ gpios;
+};
+
+&cp0_reg_usb3_vbus0 {
+	/delete-property/ gpio;
+};
+
+&cp0_reg_usb3_vbus1 {
+	/delete-property/ gpio;
+};
+
+&cp1_reg_usb3_vbus0 {
+	/delete-property/ gpio;
+};
+
+&cp0_reg_sd_vcc {
+	status = "disabled";
+};
+
+&cp0_reg_sd_vccq {
+	status = "disabled";
+};
+
+&cp0_sdhci0 {
+	status = "disabled";
+};
+
+&cp0_eth0 {
+	status = "disabled";
+};
+
+&cp0_eth1 {
+	status = "okay";
+	phy = <&phy0>;
+	phy-mode = "rgmii-id";
+};
+
+&cp0_eth2 {
+	status = "disabled";
+};
+
+&cp0_mdio {
+	status = "okay";
+	pinctrl-0 = <&cp0_ge_mdio_pins>;
+	phy0: ethernet-phy@0 {
+		status = "okay";
+	};
+};
+
+&cp0_syscon0 {
+	cp0_pinctrl: pinctrl {
+		compatible = "marvell,cp115-standalone-pinctrl";
+
+		cp0_ge_mdio_pins: ge-mdio-pins {
+			marvell,pins = "mpp40", "mpp41";
+			marvell,function = "ge";
+		};
+	};
+};
+
+&cp0_sdhci0 {
+	status = "disabled";
+};
+
+&cp0_spi1 {
+	status = "okay";
+};
+
+&cp0_usb3_0 {
+	status = "okay";
+	usb-phy = <&cp0_usb3_0_phy0>;
+	phy-names = "usb";
+	/delete-property/ phys;
+};
+
+&cp0_usb3_1 {
+	status = "okay";
+	usb-phy = <&cp0_usb3_0_phy1>;
+	phy-names = "usb";
+	/delete-property/ phys;
+};
+
+&cp1_usb3_1 {
+	status = "okay";
+	usb-phy = <&cp1_usb3_0_phy0>;
+	/* Generic PHY, providing serdes lanes */
+	phys = <&cp1_comphy3 1>;
+	phy-names = "usb";
+};
-- 
2.25.1

Re: [PATCH v6 1/3] MAINTAINERS: add ac5 to list of maintained Marvell dts files

[Rob Herring]

On Mon, Nov 27, 2023 at 1:09 PM Elad Nachman <enachman@marvell.com> wrote:
>
> From: Elad Nachman <enachman@marvell.com>
>
> Add ac5 dts files to the list of maintained Marvell Armada dts files
>
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b81da7a36a36..6f863a0c3248 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2339,6 +2339,7 @@ F:        arch/arm/boot/dts/marvell/armada*
>  F:     arch/arm/boot/dts/marvell/kirkwood*
>  F:     arch/arm/configs/mvebu_*_defconfig
>  F:     arch/arm/mach-mvebu/
> +F:     arch/arm64/boot/dts/marvell/ac5*
>  F:     arch/arm64/boot/dts/marvell/armada*
>  F:     arch/arm64/boot/dts/marvell/cn913*

$ ls arch/arm64/boot/dts/marvell/
ac5-98dx25xx.dtsi                    armada-7040.dtsi
armada-ap810-ap0.dtsi
ac5-98dx35xx.dtsi                    armada-7040-mochabin.dts
armada-ap810-ap0-octa-core.dtsi
ac5-98dx35xx-rd.dts                  armada-70x0.dtsi
armada-common.dtsi
armada-371x.dtsi                     armada-8020.dtsi
armada-cp110.dtsi
armada-3720-db.dts                   armada-8040-clearfog-gt-8k.dts
armada-cp115.dtsi
armada-3720-eDPU.dts                 armada-8040-db.dts
armada-cp11x.dtsi
armada-3720-espressobin.dts          armada-8040.dtsi
cn9130-crb-A.dts
armada-3720-espressobin.dtsi         armada-8040-mcbin.dts
cn9130-crb-B.dts
armada-3720-espressobin-emmc.dts     armada-8040-mcbin.dtsi
cn9130-crb.dtsi
armada-3720-espressobin-ultra.dts    armada-8040-mcbin-singleshot.dts
cn9130-db-B.dts
armada-3720-espressobin-v7.dts       armada-8040-puzzle-m801.dts
cn9130-db.dts
armada-3720-espressobin-v7-emmc.dts  armada-8080-db.dts
cn9130-db.dtsi
armada-3720-gl-mv1000.dts            armada-8080.dtsi
cn9130.dtsi
armada-3720-turris-mox.dts           armada-80x0.dtsi
cn9131-db-B.dts
armada-3720-uDPU.dts                 armada-ap806.dtsi
cn9131-db.dts
armada-3720-uDPU.dtsi                armada-ap806-dual.dtsi
cn9131-db.dtsi
armada-372x.dtsi                     armada-ap806-quad.dtsi
cn9132-db-B.dts
armada-37xx.dtsi                     armada-ap807.dtsi
cn9132-db.dts
armada-7020.dtsi                     armada-ap807-quad.dtsi
cn9132-db.dtsi
armada-7040-db.dts                   armada-ap80x.dtsi                 Makefile

Looks to me like a single entry will do:

F: arch/arm64/boot/dts/marvell/

Rob

Re: [PATCH v6 1/3] MAINTAINERS: add ac5 to list of maintained Marvell dts files

[Andrew Lunn]

On Mon, Nov 27, 2023 at 01:49:31PM -0600, Rob Herring wrote:
> On Mon, Nov 27, 2023 at 1:09 PM Elad Nachman <enachman@marvell.com> wrote:
> >
> > From: Elad Nachman <enachman@marvell.com>
> >
> > Add ac5 dts files to the list of maintained Marvell Armada dts files
> >
> > Signed-off-by: Elad Nachman <enachman@marvell.com>
> > ---
> >  MAINTAINERS | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b81da7a36a36..6f863a0c3248 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2339,6 +2339,7 @@ F:        arch/arm/boot/dts/marvell/armada*
> >  F:     arch/arm/boot/dts/marvell/kirkwood*
> >  F:     arch/arm/configs/mvebu_*_defconfig
> >  F:     arch/arm/mach-mvebu/
> > +F:     arch/arm64/boot/dts/marvell/ac5*
> >  F:     arch/arm64/boot/dts/marvell/armada*

> >  F:     arch/arm64/boot/dts/marvell/cn913*
> Looks to me like a single entry will do:
> 
> F: arch/arm64/boot/dts/marvell/

Agreed. I guess this is a left over from moving all the files into
vendor sub directories. Probably all the MAINTAINER entries need
updating.

Elad, please update the Marvell entry as suggested.

      Andrew

Re: [PATCH v6 3/3] arm64: dts: cn913x: add device trees for COM Express boards

[Andrew Lunn]

> +++ b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dts
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2023 Marvell International Ltd.
> + *
> + * Device tree for the AC5X RD Type 7 Com Express carrier board,
> + * Utilizing the CN913x COM Express CPU module board.
> + * This specific board only maintains a PCIe link with the CPU CPU module
> + * module, which does not require any special DTS definitions.
> + */
> +
> +#include "cn9131-db-comexpress.dtsi"
> +#include "ac5x-rd-carrier.dtsi"
> +
> +/ {
> +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board with CN9131 CPU module";
> +	compatible = "marvell,cn9131-ac5x-carrier", "marvell,rd-ac5x-carrier",
> +			"marvell,cn9131-cpu-module", "marvell,cn9131",
> +			"marvell,armada-ap807-quad", "marvell,armada-ap807";

> diff --git a/arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
> new file mode 100644
> index 000000000000..fd45d5582233

> +/ {
> +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board";
> +	compatible = "marvell,rd-ac5x-carrier";

Now i'm confused. What does rd mean?

I would expect RD mean Reference Design, and that is the complete
device in its box.

Yet, here you have RD for the carrier?

The box itself is called cn9131-ac5x-carrier?

This makes no sense to me.

Maybe i'm understanding this all wrong, and its the carrier which you
are producing a reference design for? The CPU module does not really
matter? I could use any off the shelf ComExpress 7 SOM. The bits you
are trying to sell are on the carrier? But since you are Marvell, you
don't want to recommend using an AMD ComExpress board when you happen
to also have CPU module which would work? But the CPU is not really
the point of this, its the carrier?

	Andrew

Re: [PATCH v6 2/3] dt-bindings: arm64: add Marvell COM Express boards

[Krzysztof Kozlowski]

On 27/11/2023 20:08, 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.
> 

I hope this time this was tested before sending.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

RE: [EXT] Re: [PATCH v6 3/3] arm64: dts: cn913x: add device trees for COM Express boards

[Elad Nachman]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, November 28, 2023 12:34 AM
> To: Elad Nachman <enachman@marvell.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; gregory.clement@bootlin.com;
> sebastian.hesselbarth@gmail.com; pali@kernel.org; mrkiko.rs@gmail.com;
> chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Yuval Caduri
> <cyuval@marvell.com>
> Subject: [EXT] Re: [PATCH v6 3/3] arm64: dts: cn913x: add device trees for
> COM Express boards
> 
> External Email
> 
> ----------------------------------------------------------------------
> > +++ b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-cn9131.dts
> > @@ -0,0 +1,25 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (C) 2023 Marvell International Ltd.
> > + *
> > + * Device tree for the AC5X RD Type 7 Com Express carrier board,
> > + * Utilizing the CN913x COM Express CPU module board.
> > + * This specific board only maintains a PCIe link with the CPU CPU
> > +module
> > + * module, which does not require any special DTS definitions.
> > + */
> > +
> > +#include "cn9131-db-comexpress.dtsi"
> > +#include "ac5x-rd-carrier.dtsi"
> > +
> > +/ {
> > +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier
> board with CN9131 CPU module";
> > +	compatible = "marvell,cn9131-ac5x-carrier", "marvell,rd-ac5x-
> carrier",
> > +			"marvell,cn9131-cpu-module", "marvell,cn9131",
> > +			"marvell,armada-ap807-quad", "marvell,armada-
> ap807";
> 
> > diff --git a/arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
> > b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
> > new file mode 100644
> > index 000000000000..fd45d5582233
> 
> > +/ {
> > +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier
> board";
> > +	compatible = "marvell,rd-ac5x-carrier";
> 
> Now i'm confused. What does rd mean?
> 
> I would expect RD mean Reference Design, and that is the complete device in
> its box.

AC5X RD can either work as you would expect, as a complete standalone box using the internal CPU, or you can move the switch on the back of the box to "external" mode, and connect via an external cable a kit which would allow it to use an external CPU COM Express module, mounted on top of an interposer kit.

> 
> Yet, here you have RD for the carrier?
> 
> The box itself is called cn9131-ac5x-carrier?
> 
> This makes no sense to me.
> 
> Maybe i'm understanding this all wrong, and its the carrier which you are
> producing a reference design for? The CPU module does not really matter? I

So in this case, once the switch is set to external as explained above, the AC5X RD becomes part of the carrier solution.
This is a development/reference solution, not a full commercial solution, hence it has the flexibility to be configured in different modes of operation.

> could use any off the shelf ComExpress 7 SOM. The bits you are trying to sell

Basically, yes. We have it validated versus few x86_64 system in our labs.

> are on the carrier? But since you are Marvell, you don't want to recommend
> using an AMD ComExpress board when you happen to also have CPU

To the best of my knowledge, we did not validate specifically against AMD COM Express solutions.
Since some of these modules utilize non-standard implementation of the COM Express standard (for example, few AMD CPUs do not have 10G signals, hence few AMD COM Express designs drive PCIe signals via the 10G-KR Ethernet pins of the COM Express standard), it is up to the customer, if he chooses to use such module(s), to validate them against the Marvell AC5X RD, acting as carrier via the interposer kit.

> module which would work? But the CPU is not really the point of this, its the
> carrier?

We have tested and validated a complete reference/development solution combining CN9131 Com Express CPU module, interposer kit and AC5X RD as carrier.
We only push to upstream solutions which we have validated in the lab, hence we push device tree files for the combination tested - specific CPU and specific carrier.
Customers are free to use other COM Express CPU modules, but they will have to validate them by themselves, to account for any deviation from the COM Express standard.
After that, if they wish, they can choose to go for the process of upstreaming their device tree files by their own, like we chose.

> 
> 	Andrew

FYI,

Elad.

Re: [EXT] Re: [PATCH v6 3/3] arm64: dts: cn913x: add device trees for COM Express boards

[Andrew Lunn]

> > Now i'm confused. What does rd mean?
> > 
> > I would expect RD mean Reference Design, and that is the complete device in
> > its box.
> 
> AC5X RD can either work as you would expect, as a complete standalone box using the internal CPU, or you can move the switch on the back of the box to "external" mode, and connect via an external cable a kit which would allow it to use an external CPU COM Express module, mounted on top of an interposer kit.
> 
> > 
> > Yet, here you have RD for the carrier?
> > 
> > The box itself is called cn9131-ac5x-carrier?
> > 
> > This makes no sense to me.
> > 
> > Maybe i'm understanding this all wrong, and its the carrier which you are
> > producing a reference design for? The CPU module does not really matter? I
> 
> So in this case, once the switch is set to external as explained above, the AC5X RD becomes part of the carrier solution.
> This is a development/reference solution, not a full commercial solution, hence it has the flexibility to be configured in different modes of operation.

O.K, now this make more sense. Please expand the documentation, in
particularly the carrier, explaining how it can be used, and the .dts
file about it giving a complete system, but again the carrier is the
focus.

Is the internal CPU open? Or is it a black box which only Marvell
Firmware can use? I'm just wondering if we will need another .dtsi
file describing the internal CPU, and a .dts file which includes both
the carrier and the internal CPU .dtsi to give an image you can boot
on the carrier?

	Andrew

RE: [EXT] Re: [PATCH v6 3/3] arm64: dts: cn913x: add device trees for COM Express boards

[Elad Nachman]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Wednesday, November 29, 2023 3:12 PM
> To: Elad Nachman <enachman@marvell.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; gregory.clement@bootlin.com;
> sebastian.hesselbarth@gmail.com; pali@kernel.org; mrkiko.rs@gmail.com;
> chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Yuval Caduri
> <cyuval@marvell.com>
> Subject: Re: [EXT] Re: [PATCH v6 3/3] arm64: dts: cn913x: add device trees for
> COM Express boards
> 
> > > Now i'm confused. What does rd mean?
> > >
> > > I would expect RD mean Reference Design, and that is the complete
> > > device in its box.
> >
> > AC5X RD can either work as you would expect, as a complete standalone
> > box using the internal CPU, or you can move the switch on the back of the
> > box to "external" mode, and connect via an external cable a kit which would
> > allow it to use an external CPU COM Express module, mounted on top of an
> > interposer kit.
> >
> > >
> > > Yet, here you have RD for the carrier?
> > >
> > > The box itself is called cn9131-ac5x-carrier?
> > >
> > > This makes no sense to me.
> > >
> > > Maybe i'm understanding this all wrong, and its the carrier which
> > > you are producing a reference design for? The CPU module does not
> > > really matter? I
> >
> > So in this case, once the switch is set to external as explained above, the
> AC5X RD becomes part of the carrier solution.
> > This is a development/reference solution, not a full commercial solution,
>> hence it has the flexibility to be configured in different modes of operation.
> 
> O.K, now this make more sense. Please expand the documentation, in
> particularly the carrier, explaining how it can be used, and the .dts file about
> it giving a complete system, but again the carrier is the focus.

Will add comments on the dts and dtsi files on the next patch version.

> 
> Is the internal CPU open? Or is it a black box which only Marvell Firmware
> can use? I'm just wondering if we will need another .dtsi file describing the
> internal CPU, and a .dts file which includes both the carrier and the internal
> CPU .dtsi to give an image you can boot on the carrier?

When the board boots in the internal (standalone) CPU mode, the following dts, which is
Already upstreamed, is to be used to boot Linux on the internal, standalone CPU:

arch/arm64/boot/dts/marvell/ac5-98dx35xx-rd.dts

When the board boots in the external CPU mode, the internal CPU
is disabled, and only the switch portion of the SOC acts as a PCIe end-point,
Hence there is little use to describe a CPU which is disabled.

In this mode, the AC5X RD carrier portion only provides a non-CPU PCIe end-point to the
COM Express CPU module (in this case, containing the CN9131 CPU).
There is no CPU booting in this mode on the carrier, only on the COM Express
CPU module.
What runs the Linux is the CN9131 on the COM Express CPU module,
And it accesses the switch end-point on the AC5X RD portion of the carrier via PCIe.

This is briefly documented (will elaborate further in next patch version) in the following file covered in the patch:
arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi

> 
> 	Andrew

FYI,

Elad.