[PATCH v2 0/4] arm64: dts: marvell: cn913x-solidrun: fix sata ports status

[PATCH v2 0/4] arm64: dts: marvell: cn913x-solidrun: fix sata ports status

[Josua Mayer]

The SolidRun CN9130 SoC based boards have a variety of functional
problems, in particular

- SATA ports
- CN9132 CEX-7 eMMC
- CN9132 Clearfog PCI-E x2 / x4 ports

are not functional.

The SATA issue was recently introduced via changes to the
armada-cp11x.dtsi, wheras the eMMC and SPI problems were present in the
board dts from the very beginning.

This patch-set aims to resolve the problems after testing on Debian 13
release (Linux v6.12).

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
Changes in v2:
- fixed mistakes in the original board device-trees that caused
  functional issues with eMMC and pci.
- Link to v1: https://lore.kernel.org/r/20250911-cn913x-sr-fix-sata-v1-1-9e72238d0988@solid-run.com

---
Josua Mayer (4):
      arm64: dts: marvell: cn913x-solidrun: fix sata ports status
      arm64: dts: marvell: cn9132-clearfog: disable eMMC high-speed modes
      arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports
      arm64: dts: marvell: cn9130-sr-som: add missing properties to emmc

 arch/arm64/boot/dts/marvell/cn9130-cf.dtsi         |  7 ++++---
 arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi     |  2 ++
 arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts |  6 ++++--
 arch/arm64/boot/dts/marvell/cn9132-clearfog.dts    | 22 ++++++++++++++++------
 arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi    |  8 ++++++++
 5 files changed, 34 insertions(+), 11 deletions(-)
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250911-cn913x-sr-fix-sata-5c737ebdb97f

Best regards,
-- 
Josua Mayer <josua@solid-run.com>

[PATCH v2 1/4] arm64: dts: marvell: cn913x-solidrun: fix sata ports status

[Josua Mayer]

Commit "arm64: dts: marvell: only enable complete sata nodes" changed
armada-cp11x.dtsi disabling all sata ports status by default.

The author missed some dts which relied on the dtsi enabling all ports,
and just disabled unused ones instead.

Update dts for SolidRun cn913x based boards to enable the available
ports, rather than disabling the unvavailable one.

Further according to dt bindings the serdes phys are to be specified in
the port node, not the controller node.
Move those phys properties accordingly in clearfog base/pro/solidwan.

Fixes: 30023876aef4 ("arm64: dts: marvell: only enable complete sata nodes")
Cc: <stable@vger.kernel.org>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm64/boot/dts/marvell/cn9130-cf.dtsi         | 7 ++++---
 arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts | 6 ++++--
 arch/arm64/boot/dts/marvell/cn9132-clearfog.dts    | 6 ++----
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi b/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi
index ad0ab34b66028c53b8a18b3e8ee0c0aec869759f..bd42bfbe408bbe2a4d58dbd40204bcfb3c126312 100644
--- a/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi
+++ b/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi
@@ -152,11 +152,12 @@ expander0_pins: cp0-expander0-pins {
 
 /* SRDS #0 - SATA on M.2 connector */
 &cp0_sata0 {
-	phys = <&cp0_comphy0 1>;
 	status = "okay";
 
-	/* only port 1 is available */
-	/delete-node/ sata-port@0;
+	sata-port@1 {
+		phys = <&cp0_comphy0 1>;
+		status = "okay";
+	};
 };
 
 /* microSD */
diff --git a/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts b/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts
index 47234d0858dd2195bb1485f25768ad3c757b7ac2..338853d3b179bb5cb742e975bb830fdb9d62d4cc 100644
--- a/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts
+++ b/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts
@@ -563,11 +563,13 @@ &cp1_rtc {
 
 /* SRDS #1 - SATA on M.2 (J44) */
 &cp1_sata0 {
-	phys = <&cp1_comphy1 0>;
 	status = "okay";
 
 	/* only port 0 is available */
-	/delete-node/ sata-port@1;
+	sata-port@0 {
+		phys = <&cp1_comphy1 0>;
+		status = "okay";
+	};
 };
 
 &cp1_syscon0 {
diff --git a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
index 0f53745a6fa0d8cbd3ab9cdc28a972ed748c275f..115c55d73786e2b9265e1caa4c62ee26f498fb41 100644
--- a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
+++ b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
@@ -512,10 +512,9 @@ &cp1_sata0 {
 	status = "okay";
 
 	/* only port 1 is available */
-	/delete-node/ sata-port@0;
-
 	sata-port@1 {
 		phys = <&cp1_comphy3 1>;
+		status = "okay";
 	};
 };
 
@@ -631,9 +630,8 @@ &cp2_sata0 {
 	status = "okay";
 
 	/* only port 1 is available */
-	/delete-node/ sata-port@0;
-
 	sata-port@1 {
+		status = "okay";
 		phys = <&cp2_comphy3 1>;
 	};
 };

-- 
2.51.0

[PATCH v2 2/4] arm64: dts: marvell: cn9132-clearfog: disable eMMC high-speed modes

[Josua Mayer]

Similar to MacchiatoBIN the high-speed modes are unstable on the CN9132
CEX-7 module, leading to failed transactions under normal use.

Disable all high-speed modes including UHS.

Additionally add no-sdio and non-removable properties as appropriate for
eMMC.

Fixes: e9ff907f4076 ("arm64: dts: add description for solidrun cn9132 cex7 module and clearfog board")
Cc: <stable@vger.kernel.org>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi b/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi
index afc041c1c448c3e49e1c35d817e91e75db6cfad6..bb2bb47fd77c12f1461b5b9f6ef5567a32cc0153 100644
--- a/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi
+++ b/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi
@@ -137,6 +137,14 @@ &ap_sdhci0 {
 	pinctrl-0 = <&ap_mmc0_pins>;
 	pinctrl-names = "default";
 	vqmmc-supply = <&v_1_8>;
+	/*
+	 * Not stable in HS modes - phy needs "more calibration", so disable
+	 * UHS (by preventing voltage switch), SDR104, SDR50 and DDR50 modes.
+	 */
+	no-1-8-v;
+	no-sd;
+	no-sdio;
+	non-removable;
 	status = "okay";
 };
 

-- 
2.51.0

[PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Josua Mayer]

The mvebu-comphy driver does not currently know how to pass correct
lane-count to ATF while configuring the serdes lanes.

This causes the system to hard reset during reconfiguration, if a pci
card is present and has established a link during bootloader.

Remove the comphy handles from the respective pci nodes to avoid runtime
reconfiguration, relying solely on bootloader configuration - while
avoiding the hard reset.

When bootloader has configured the lanes correctly, the pci ports are
functional under Linux.

This issue may be addressed in the comphy driver at a future point.

Fixes: e9ff907f4076 ("arm64: dts: add description for solidrun cn9132 cex7 module and clearfog board")
Cc: <stable@vger.kernel.org>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm64/boot/dts/marvell/cn9132-clearfog.dts | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
index 115c55d73786e2b9265e1caa4c62ee26f498fb41..6f237d3542b9102695f8a48457f43340da994a2c 100644
--- a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
+++ b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
@@ -413,7 +413,13 @@ fixed-link {
 /* SRDS #0,#1,#2,#3 - PCIe */
 &cp0_pcie0 {
 	num-lanes = <4>;
-	phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;
+	/*
+	 * The mvebu-comphy driver does not currently know how to pass correct
+	 * lane-count to ATF while configuring the serdes lanes.
+	 * Rely on bootloader configuration only.
+	 *
+	 * phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;
+	 */
 	status = "okay";
 };
 
@@ -475,7 +481,13 @@ &cp1_eth0 {
 /* SRDS #0,#1 - PCIe */
 &cp1_pcie0 {
 	num-lanes = <2>;
-	phys = <&cp1_comphy0 0>, <&cp1_comphy1 0>;
+	/*
+	 * The mvebu-comphy driver does not currently know how to pass correct
+	 * lane-count to ATF while configuring the serdes lanes.
+	 * Rely on bootloader configuration only.
+	 *
+	 * phys = <&cp1_comphy0 0>, <&cp1_comphy1 0>;
+	 */
 	status = "okay";
 };
 

-- 
2.51.0

[PATCH v2 4/4] arm64: dts: marvell: cn9130-sr-som: add missing properties to emmc

[Josua Mayer]

CN9130 System on Module connects an eMMC to ap_sdhci0, but the common
properties indicating eMMC were not added to device-tree.

Add no-sdio and non-removable as applicable to eMMC.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi b/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi
index a997bbabedd8a9679e9d209225666d7696dd7da2..f95202decfceb5cc9dc777ddd1870d5972a1bc54 100644
--- a/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi
+++ b/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi
@@ -61,6 +61,8 @@ &ap_sdhci0 {
 	pinctrl-0 = <&ap_mmc0_pins>;
 	pinctrl-names = "default";
 	vqmmc-supply = <&v_1_8>;
+	no-sdio;
+	non-removable;
 	status = "okay";
 };
 

-- 
2.51.0

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Andrew Lunn]

On Thu, Sep 11, 2025 at 08:28:06PM +0200, Josua Mayer wrote:
> The mvebu-comphy driver does not currently know how to pass correct
> lane-count to ATF while configuring the serdes lanes.
> 
> This causes the system to hard reset during reconfiguration, if a pci
> card is present and has established a link during bootloader.
> 
> Remove the comphy handles from the respective pci nodes to avoid runtime
> reconfiguration, relying solely on bootloader configuration - while
> avoiding the hard reset.
> 
> When bootloader has configured the lanes correctly, the pci ports are
> functional under Linux.

Does this require a specific bootloader? Can i use mainline grub or
bareboot?

	Andrew

Re: [PATCH v2 1/4] arm64: dts: marvell: cn913x-solidrun: fix sata ports status

[Gregory CLEMENT]

Josua Mayer <josua@solid-run.com> writes:

> Commit "arm64: dts: marvell: only enable complete sata nodes" changed
> armada-cp11x.dtsi disabling all sata ports status by default.
>
> The author missed some dts which relied on the dtsi enabling all ports,
> and just disabled unused ones instead.
>
> Update dts for SolidRun cn913x based boards to enable the available
> ports, rather than disabling the unvavailable one.
>
> Further according to dt bindings the serdes phys are to be specified in
> the port node, not the controller node.
> Move those phys properties accordingly in clearfog base/pro/solidwan.
>
> Fixes: 30023876aef4 ("arm64: dts: marvell: only enable complete sata nodes")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Applied on mvebu/fixes

Thanks,

Gregory
> ---
>  arch/arm64/boot/dts/marvell/cn9130-cf.dtsi         | 7 ++++---
>  arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts | 6 ++++--
>  arch/arm64/boot/dts/marvell/cn9132-clearfog.dts    | 6 ++----
>  3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi b/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi
> index ad0ab34b66028c53b8a18b3e8ee0c0aec869759f..bd42bfbe408bbe2a4d58dbd40204bcfb3c126312 100644
> --- a/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi
> +++ b/arch/arm64/boot/dts/marvell/cn9130-cf.dtsi
> @@ -152,11 +152,12 @@ expander0_pins: cp0-expander0-pins {
>  
>  /* SRDS #0 - SATA on M.2 connector */
>  &cp0_sata0 {
> -	phys = <&cp0_comphy0 1>;
>  	status = "okay";
>  
> -	/* only port 1 is available */
> -	/delete-node/ sata-port@0;
> +	sata-port@1 {
> +		phys = <&cp0_comphy0 1>;
> +		status = "okay";
> +	};
>  };
>  
>  /* microSD */
> diff --git a/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts b/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts
> index 47234d0858dd2195bb1485f25768ad3c757b7ac2..338853d3b179bb5cb742e975bb830fdb9d62d4cc 100644
> --- a/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts
> +++ b/arch/arm64/boot/dts/marvell/cn9131-cf-solidwan.dts
> @@ -563,11 +563,13 @@ &cp1_rtc {
>  
>  /* SRDS #1 - SATA on M.2 (J44) */
>  &cp1_sata0 {
> -	phys = <&cp1_comphy1 0>;
>  	status = "okay";
>  
>  	/* only port 0 is available */
> -	/delete-node/ sata-port@1;
> +	sata-port@0 {
> +		phys = <&cp1_comphy1 0>;
> +		status = "okay";
> +	};
>  };
>  
>  &cp1_syscon0 {
> diff --git a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> index 0f53745a6fa0d8cbd3ab9cdc28a972ed748c275f..115c55d73786e2b9265e1caa4c62ee26f498fb41 100644
> --- a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> +++ b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> @@ -512,10 +512,9 @@ &cp1_sata0 {
>  	status = "okay";
>  
>  	/* only port 1 is available */
> -	/delete-node/ sata-port@0;
> -
>  	sata-port@1 {
>  		phys = <&cp1_comphy3 1>;
> +		status = "okay";
>  	};
>  };
>  
> @@ -631,9 +630,8 @@ &cp2_sata0 {
>  	status = "okay";
>  
>  	/* only port 1 is available */
> -	/delete-node/ sata-port@0;
> -
>  	sata-port@1 {
> +		status = "okay";
>  		phys = <&cp2_comphy3 1>;
>  	};
>  };
>
> -- 
> 2.51.0
>
>

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 2/4] arm64: dts: marvell: cn9132-clearfog: disable eMMC high-speed modes

[Gregory CLEMENT]

Josua Mayer <josua@solid-run.com> writes:

> Similar to MacchiatoBIN the high-speed modes are unstable on the CN9132
> CEX-7 module, leading to failed transactions under normal use.
>
> Disable all high-speed modes including UHS.
>
> Additionally add no-sdio and non-removable properties as appropriate for
> eMMC.
>
> Fixes: e9ff907f4076 ("arm64: dts: add description for solidrun cn9132 cex7 module and clearfog board")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Applied on mvebu/fixes

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi b/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi
> index afc041c1c448c3e49e1c35d817e91e75db6cfad6..bb2bb47fd77c12f1461b5b9f6ef5567a32cc0153 100644
> --- a/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi
> +++ b/arch/arm64/boot/dts/marvell/cn9132-sr-cex7.dtsi
> @@ -137,6 +137,14 @@ &ap_sdhci0 {
>  	pinctrl-0 = <&ap_mmc0_pins>;
>  	pinctrl-names = "default";
>  	vqmmc-supply = <&v_1_8>;
> +	/*
> +	 * Not stable in HS modes - phy needs "more calibration", so disable
> +	 * UHS (by preventing voltage switch), SDR104, SDR50 and DDR50 modes.
> +	 */
> +	no-1-8-v;
> +	no-sd;
> +	no-sdio;
> +	non-removable;
>  	status = "okay";
>  };
>  
>
> -- 
> 2.51.0
>
>

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Gregory CLEMENT]

Josua Mayer <josua@solid-run.com> writes:

> The mvebu-comphy driver does not currently know how to pass correct
> lane-count to ATF while configuring the serdes lanes.
>
> This causes the system to hard reset during reconfiguration, if a pci
> card is present and has established a link during bootloader.
>
> Remove the comphy handles from the respective pci nodes to avoid runtime
> reconfiguration, relying solely on bootloader configuration - while
> avoiding the hard reset.
>
> When bootloader has configured the lanes correctly, the pci ports are
> functional under Linux.
>
> This issue may be addressed in the comphy driver at a future point.
>
> Fixes: e9ff907f4076 ("arm64: dts: add description for solidrun cn9132 cex7 module and clearfog board")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Applied on mvebu/fixes

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/cn9132-clearfog.dts | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> index 115c55d73786e2b9265e1caa4c62ee26f498fb41..6f237d3542b9102695f8a48457f43340da994a2c 100644
> --- a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> +++ b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> @@ -413,7 +413,13 @@ fixed-link {
>  /* SRDS #0,#1,#2,#3 - PCIe */
>  &cp0_pcie0 {
>  	num-lanes = <4>;
> -	phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;
> +	/*
> +	 * The mvebu-comphy driver does not currently know how to pass correct
> +	 * lane-count to ATF while configuring the serdes lanes.
> +	 * Rely on bootloader configuration only.
> +	 *
> +	 * phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;
> +	 */
>  	status = "okay";
>  };
>  
> @@ -475,7 +481,13 @@ &cp1_eth0 {
>  /* SRDS #0,#1 - PCIe */
>  &cp1_pcie0 {
>  	num-lanes = <2>;
> -	phys = <&cp1_comphy0 0>, <&cp1_comphy1 0>;
> +	/*
> +	 * The mvebu-comphy driver does not currently know how to pass correct
> +	 * lane-count to ATF while configuring the serdes lanes.
> +	 * Rely on bootloader configuration only.
> +	 *
> +	 * phys = <&cp1_comphy0 0>, <&cp1_comphy1 0>;
> +	 */
>  	status = "okay";
>  };
>  
>
> -- 
> 2.51.0
>
>

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 4/4] arm64: dts: marvell: cn9130-sr-som: add missing properties to emmc

[Gregory CLEMENT]

Josua Mayer <josua@solid-run.com> writes:

> CN9130 System on Module connects an eMMC to ap_sdhci0, but the common
> properties indicating eMMC were not added to device-tree.
>
> Add no-sdio and non-removable as applicable to eMMC.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Applied on mvebu/dt64

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi b/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi
> index a997bbabedd8a9679e9d209225666d7696dd7da2..f95202decfceb5cc9dc777ddd1870d5972a1bc54 100644
> --- a/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi
> +++ b/arch/arm64/boot/dts/marvell/cn9130-sr-som.dtsi
> @@ -61,6 +61,8 @@ &ap_sdhci0 {
>  	pinctrl-0 = <&ap_mmc0_pins>;
>  	pinctrl-names = "default";
>  	vqmmc-supply = <&v_1_8>;
> +	no-sdio;
> +	non-removable;
>  	status = "okay";
>  };
>  
>
> -- 
> 2.51.0
>
>

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Josua Mayer]

Am 12.09.25 um 00:12 schrieb Andrew Lunn:
> On Thu, Sep 11, 2025 at 08:28:06PM +0200, Josua Mayer wrote:
>> The mvebu-comphy driver does not currently know how to pass correct
>> lane-count to ATF while configuring the serdes lanes.
>>
>> This causes the system to hard reset during reconfiguration, if a pci
>> card is present and has established a link during bootloader.
>>
>> Remove the comphy handles from the respective pci nodes to avoid runtime
>> reconfiguration, relying solely on bootloader configuration - while
>> avoiding the hard reset.
>>
>> When bootloader has configured the lanes correctly, the pci ports are
>> functional under Linux.
> Does this require a specific bootloader? Can i use mainline grub or
> bareboot?

In this case it means U-Boot, i.e. before one would start grub.

I am never quite sure if in this situation I should say "firmware" instead ...

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Andrew Lunn]

On Thu, Sep 18, 2025 at 10:46:03AM +0000, Josua Mayer wrote:
> Am 12.09.25 um 00:12 schrieb Andrew Lunn:
> > On Thu, Sep 11, 2025 at 08:28:06PM +0200, Josua Mayer wrote:
> >> The mvebu-comphy driver does not currently know how to pass correct
> >> lane-count to ATF while configuring the serdes lanes.
> >>
> >> This causes the system to hard reset during reconfiguration, if a pci
> >> card is present and has established a link during bootloader.
> >>
> >> Remove the comphy handles from the respective pci nodes to avoid runtime
> >> reconfiguration, relying solely on bootloader configuration - while
> >> avoiding the hard reset.
> >>
> >> When bootloader has configured the lanes correctly, the pci ports are
> >> functional under Linux.
> > Does this require a specific bootloader? Can i use mainline grub or
> > bareboot?
> 
> In this case it means U-Boot, i.e. before one would start grub.
> 
> I am never quite sure if in this situation I should say "firmware" instead ...

What you failed to answer is my question about 'mainline'? Do i need a
specific vendor u-boot, or can i just use mainline u-boot, or mainline
bareboot.

I personally like to replace the bootloader, because the one shipped
with the board often has useful features disabled, or is old. If i do
that, will the board work? I would much prefer the kernel makes no
assumptions about the bootloader. You said:

> The mvebu-comphy driver does not currently know how to pass correct
> lane-count to ATF while configuring the serdes lanes.

Why not just teach mvebu-comphy to pass the correct line-count? That
sounds like the proper fix, and that makes the kernel independent of
the bootloader.

	Andrew

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Josua Mayer]

Am 18.09.25 um 16:09 schrieb Andrew Lunn:
> On Thu, Sep 18, 2025 at 10:46:03AM +0000, Josua Mayer wrote:
>> Am 12.09.25 um 00:12 schrieb Andrew Lunn:
>>> On Thu, Sep 11, 2025 at 08:28:06PM +0200, Josua Mayer wrote:
>>>> The mvebu-comphy driver does not currently know how to pass correct
>>>> lane-count to ATF while configuring the serdes lanes.
>>>>
>>>> This causes the system to hard reset during reconfiguration, if a pci
>>>> card is present and has established a link during bootloader.
>>>>
>>>> Remove the comphy handles from the respective pci nodes to avoid runtime
>>>> reconfiguration, relying solely on bootloader configuration - while
>>>> avoiding the hard reset.
>>>>
>>>> When bootloader has configured the lanes correctly, the pci ports are
>>>> functional under Linux.
>>> Does this require a specific bootloader? Can i use mainline grub or
>>> bareboot?
>> In this case it means U-Boot, i.e. before one would start grub.
>>
>> I am never quite sure if in this situation I should say "firmware" instead ...
> What you failed to answer is my question about 'mainline'? Do i need a
> specific vendor u-boot, or can i just use mainline u-boot, or mainline
> bareboot.

Ah.

There is no mainline u-boot for these boards (yet).
I submitted v1 on u-boot ml a while back but didn't have time to rework it.

U-Boot has a different comphy driver that appears to configure the lanes
correctly.

>
> I personally like to replace the bootloader, because the one shipped
> with the board often has useful features disabled, or is old. If i do
> that, will the board work?
Conversationally if the bootloader configured the board correctly,
then it also correctly configured all pci lanes considering u-boot
handles link-up itself on mvebu.
> I would much prefer the kernel makes no
> assumptions about the bootloader.
Same.
> You said:
>
>> The mvebu-comphy driver does not currently know how to pass correct
>> lane-count to ATF while configuring the serdes lanes.
> Why not just teach mvebu-comphy to pass the correct line-count? That
> sounds like the proper fix, and that makes the kernel independent of
> the bootloader.
That would be a feature on the comphy driver, not a bug-fix backported
to stable. The core goal was to fix bugs found in Debian 13.

The Linux comphy driver should be updated going forward.
I did not due it (yet) because tracing the bitmask of atf smc call is
very tedious due to layers of #define referencing each other.

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Andrew Lunn]

> >> The mvebu-comphy driver does not currently know how to pass correct
> >> lane-count to ATF while configuring the serdes lanes.
> > Why not just teach mvebu-comphy to pass the correct line-count? That
> > sounds like the proper fix, and that makes the kernel independent of
> > the bootloader.

> That would be a feature on the comphy driver, not a bug-fix backported
> to stable. The core goal was to fix bugs found in Debian 13.

It is not so simple.

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

  It must either fix a real bug that bothers people or just add a device ID

Crashing at boot would be a real bug that bothers people, not just a
new feature.

Lets see how big the patch is. If its 1000 lines of hard to understand
code, it will probably be rejected for stable. If its 100 lines or
less, it will likely be accepted.

It is also hard to argue the DT is wrong. It just describes the
hardware. I assume the description is actually correct? The issue is
the driver, not the description. Also, i assume this affects all
boards using this SoC? Removing the nodes in one board 'fixes' one
board. Fixing the driver fixes all boards...

    Andrew

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Josua Mayer]

Am 18.09.25 um 17:41 schrieb Andrew Lunn:

>>>> The mvebu-comphy driver does not currently know how to pass correct
>>>> lane-count to ATF while configuring the serdes lanes.
>>> Why not just teach mvebu-comphy to pass the correct line-count? That
>>> sounds like the proper fix, and that makes the kernel independent of
>>> the bootloader.
>> That would be a feature on the comphy driver, not a bug-fix backported
>> to stable. The core goal was to fix bugs found in Debian 13.
> It is not so simple.
>
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>
>   It must either fix a real bug that bothers people or just add a device ID
>
> Crashing at boot would be a real bug that bothers people, not just a
> new feature.
>
> Lets see how big the patch is. If its 1000 lines of hard to understand
> code, it will probably be rejected for stable. If its 100 lines or
> less, it will likely be accepted.
I see.
> It is also hard to argue the DT is wrong. It just describes the
> hardware. I assume the description is actually correct?
The x4 port linked  comphy as below:

phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;

At the time of submitting my patch I was  not convinced the above was right, or wrong.
I labeled it wrong for causing a fault which I should have noticed much earlier.

The  numeric argument after the comphy-lane handle is the port number,
for those functions that can have multiple ports (e.g. ethernet #2).

This means above dts linked pci port 0 on lanes 0-4, which appears correct.
Further lanes 1-3 have no other pci ports, there is no other configuration to confuse it with.

> The issue is
> the driver, not the description. Also, i assume this affects all
> boards using this SoC? Removing the nodes in one board 'fixes' one
> board. Fixing the driver fixes all boards...

I missed to check whether other boards share similar description.

Today I found two other dts that reference multiple lanes:

arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi

Both cases the function is PCI - first one x2, secondx4.

I will try to look into a more correct solution soon.

Re: [PATCH v2 3/4] arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports

[Josua Mayer]

Hi Andrew,

Am 18.09.25 um 19:40 schrieb Josua Mayer:
> Am 18.09.25 um 17:41 schrieb Andrew Lunn:
>
>>>>> The mvebu-comphy driver does not currently know how to pass correct
>>>>> lane-count to ATF while configuring the serdes lanes.
>>>> Why not just teach mvebu-comphy to pass the correct line-count? That
>>>> sounds like the proper fix, and that makes the kernel independent of
>>>> the bootloader.
>>> That would be a feature on the comphy driver, not a bug-fix backported
>>> to stable. The core goal was to fix bugs found in Debian 13.
>> It is not so simple.
>>
>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>
>>   It must either fix a real bug that bothers people or just add a device ID
>>
>> Crashing at boot would be a real bug that bothers people, not just a
>> new feature.
>>
>> Lets see how big the patch is. If its 1000 lines of hard to understand
>> code, it will probably be rejected for stable. If its 100 lines or
>> less, it will likely be accepted.
> I see.
>> It is also hard to argue the DT is wrong. It just describes the
>> hardware. I assume the description is actually correct?
> The x4 port linked  comphy as below:
>
> phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;
>
> At the time of submitting my patch I was  not convinced the above was right, or wrong.
> I labeled it wrong for causing a fault which I should have noticed much earlier.
>
> The  numeric argument after the comphy-lane handle is the port number,
> for those functions that can have multiple ports (e.g. ethernet #2).
>
> This means above dts linked pci port 0 on lanes 0-4, which appears correct.
> Further lanes 1-3 have no other pci ports, there is no other configuration to confuse it with.
>
>> The issue is
>> the driver, not the description. Also, i assume this affects all
>> boards using this SoC? Removing the nodes in one board 'fixes' one
>> board. Fixing the driver fixes all boards...
> I missed to check whether other boards share similar description.
>
> Today I found two other dts that reference multiple lanes:
>
> arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
> arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
>
> Both cases the function is PCI - first one x2, secondx4.
>
> I will try to look into a more correct solution soon.

I did some extensive testing outside of Debian world, with v6.12.0, v6.12.48, Marvell v6.1 ...
and made a range of interesting / confusing observations:

1. I was able to produce the problem in a self-compiled kernel outside Debian.
This is with arm64 defconfig and some small adaptations.

The system only got stuck during boot when the comphy driver was a module.

In this case there are two suspicious messages in boot log:

[    2.742966] armada8k-pcie f4600000.pcie: No available PHY
[    3.732084] armada8k-pcie f4600000.pcie: Phy link never came up

The link timeout comes to mind first, which is unexpected as in my testing
there was always a card connected.
This card was detected fine with comphy driver builtin.

The "No available PHY" likely leads to some bad error handling in pci probe,
and should be investigated further.

2. When the system got stuck during boot, it was never in the middle of an smc to atf.
I confirmed this by adding locking to the smc function handler in atf, and logging
activity to serial console.

3. I was wrong in that the linux driver does not know how to configure the lane count,
the comphy driver does indeed pass the port width (as indicated by num-lanes dt prop)
in the format that ATF expects.

However ATF does nothing with kernel driver pci lane configuration.
Instead any power-on or power-off call from kernel driver via smcc
is tested for originating within linux vs. uboot. Only when source is uboot,
it performs any configuration ... :

https://github.com/ARM-software/arm-trusted-firmware/blob/master/drivers/marvell/comphy/phy-comphy-cp110.c#L1257

4. The "mode" argument (x2) to the smc function for comphy lane power-on/setup
differs between Marvell U-Boot and Linux. I found this by dumping them from ATF itself.

In particular the bits indicating the port number were invalid due to an overflow error
in solidrun u-boot (based on Marvell u-boot).
The mode specified by kernel driver however seemed correct in that regard.

Further the bits indicating the serdes lane speed are 0 in linux driver, and all one in vendor u-boot.

As atf ignores pci lane configuration when originating from kernel, this had no impact so far.

5. The port number passed from u-boot to atf appears to have no effect.
Fixing it in vendor u-boot had so far no apparent impact.


To conclude, I think my device-tree patch is not correct and should be replaced
once a better workaround or solution is discovered.