linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4
@ 2025-10-30 15:16 Josua Mayer
  2025-10-30 15:16 ` [PATCH 1/2] Revert "arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports" Josua Mayer
  2025-10-30 15:16 ` [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Josua Mayer
  0 siblings, 2 replies; 7+ messages in thread
From: Josua Mayer @ 2025-10-30 15:16 UTC (permalink / raw)
  To: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd
  Cc: Rabeeh Khoury, Yazan Shhady, Mikhail Anikin, Jon Nettleton,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Josua Mayer

This patch-set implements a fix for pci errors on armada cp11x based
platforms due to timing of clock driver, clock coreand pci controller
driver.

It further reverts an ineffective previous fix attempt in device-tree.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
Josua Mayer (2):
      Revert "arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports"
      clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4

 arch/arm64/boot/dts/marvell/cn9132-clearfog.dts | 16 ++--------------
 drivers/clk/mvebu/cp110-system-controller.c     | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 14 deletions(-)
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251030-cn913x-pci-clk-1bb9a6419ff0

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] Revert "arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports"
  2025-10-30 15:16 [PATCH 0/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Josua Mayer
@ 2025-10-30 15:16 ` Josua Mayer
  2025-11-10 14:19   ` Gregory CLEMENT
  2025-10-30 15:16 ` [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Josua Mayer
  1 sibling, 1 reply; 7+ messages in thread
From: Josua Mayer @ 2025-10-30 15:16 UTC (permalink / raw)
  To: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd
  Cc: Rabeeh Khoury, Yazan Shhady, Mikhail Anikin, Jon Nettleton,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Josua Mayer

This reverts commit 794a066688038df46c01e177cc6faebded0acba4 because it
misunderstood interworking between arm trusted firmware and the common
phy driver, and does not consistently resolve the issue it was intended
to address.

Further diagnostics have revealed the root cause for the reported system
lock-up in a race condition between pci driver probe and clock core
disabling unused clocks.

Revert the wrong change restoring driver control over all pci lanes.
As a temporary workaround for the original issue, users can boot with
"clk_ignore_unused".

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm64/boot/dts/marvell/cn9132-clearfog.dts | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
index 5cf83d8ca1f59..2507896d58f9b 100644
--- a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
+++ b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
@@ -413,13 +413,7 @@ fixed-link {
 /* SRDS #0,#1,#2,#3 - PCIe */
 &cp0_pcie0 {
 	num-lanes = <4>;
-	/*
-	 * 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>;
-	 */
+	phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;
 	status = "okay";
 };
 
@@ -481,13 +475,7 @@ &cp1_eth0 {
 /* SRDS #0,#1 - PCIe */
 &cp1_pcie0 {
 	num-lanes = <2>;
-	/*
-	 * 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>;
-	 */
+	phys = <&cp1_comphy0 0>, <&cp1_comphy1 0>;
 	status = "okay";
 };
 

-- 
2.51.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4
  2025-10-30 15:16 [PATCH 0/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Josua Mayer
  2025-10-30 15:16 ` [PATCH 1/2] Revert "arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports" Josua Mayer
@ 2025-10-30 15:16 ` Josua Mayer
  2025-10-30 15:33   ` Andrew Lunn
  1 sibling, 1 reply; 7+ messages in thread
From: Josua Mayer @ 2025-10-30 15:16 UTC (permalink / raw)
  To: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd
  Cc: Rabeeh Khoury, Yazan Shhady, Mikhail Anikin, Jon Nettleton,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Josua Mayer

CP110 based platforms rely on the bootloader for pci port
initialization.
TF-A actively prevents non-uboot re-configuration of pci lanes, and many
boards do not have software control over the pci card reset.

If a pci port had link at boot-time and the clock is stopped at a later
point, the link fails and can not be recovered.

PCI controller driver probe - and by extension ownership of a driver for
the pci clocks - may be delayed especially on large modular kernels,
causing the clock core to start disabling unused clocks.

Add the CLK_IGNORE_UNUSED flag to the three pci port's clocks to ensure
they are not stopped before the pci controller driver has taken
ownership and tested for an existing link.

This fixes failed pci link detection when controller driver probes late,
e.g. with arm64 defconfig and CONFIG_PHY_MVEBU_CP110_COMPHY=m.

Closes: https://lore.kernel.org/r/b71596c7-461b-44b6-89ab-3cfbd492639f@solid-run.com
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/clk/mvebu/cp110-system-controller.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/clk/mvebu/cp110-system-controller.c b/drivers/clk/mvebu/cp110-system-controller.c
index 03c59bf221060..b47c869060466 100644
--- a/drivers/clk/mvebu/cp110-system-controller.c
+++ b/drivers/clk/mvebu/cp110-system-controller.c
@@ -110,6 +110,25 @@ static const char * const gate_base_names[] = {
 	[CP110_GATE_EIP197]	= "eip197"
 };
 
+static unsigned long gate_flags(const u8 bit_idx)
+{
+	switch (bit_idx) {
+	case CP110_GATE_PCIE_X1_0:
+	case CP110_GATE_PCIE_X1_1:
+	case CP110_GATE_PCIE_X4:
+		/*
+		 * If a port had an active link at boot time, stopping
+		 * the clock creates a failed state from which controller
+		 * driver can not recover.
+		 * Prevent stopping this clock till after a driver has taken
+		 * ownership.
+		 */
+		return CLK_IGNORE_UNUSED;
+	default:
+		return 0;
+	}
+};
+
 struct cp110_gate_clk {
 	struct clk_hw hw;
 	struct regmap *regmap;
@@ -171,6 +190,7 @@ static struct clk_hw *cp110_register_gate(const char *name,
 	init.ops = &cp110_gate_ops;
 	init.parent_names = &parent_name;
 	init.num_parents = 1;
+	init.flags = gate_flags(bit_idx);
 
 	gate->regmap = regmap;
 	gate->bit_idx = bit_idx;

-- 
2.51.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4
  2025-10-30 15:16 ` [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Josua Mayer
@ 2025-10-30 15:33   ` Andrew Lunn
  2025-10-30 15:51     ` Josua Mayer
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Lunn @ 2025-10-30 15:33 UTC (permalink / raw)
  To: Josua Mayer
  Cc: Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Rabeeh Khoury, Yazan Shhady, Mikhail Anikin,
	Jon Nettleton, linux-arm-kernel, devicetree, linux-kernel,
	linux-clk

On Thu, Oct 30, 2025 at 04:16:26PM +0100, Josua Mayer wrote:
> CP110 based platforms rely on the bootloader for pci port
> initialization.
> TF-A actively prevents non-uboot re-configuration of pci lanes, and many
> boards do not have software control over the pci card reset.
> 
> If a pci port had link at boot-time and the clock is stopped at a later
> point, the link fails and can not be recovered.
> 
> PCI controller driver probe - and by extension ownership of a driver for
> the pci clocks - may be delayed especially on large modular kernels,
> causing the clock core to start disabling unused clocks.
> 
> Add the CLK_IGNORE_UNUSED flag to the three pci port's clocks to ensure
> they are not stopped before the pci controller driver has taken
> ownership and tested for an existing link.
> 
> This fixes failed pci link detection when controller driver probes late,
> e.g. with arm64 defconfig and CONFIG_PHY_MVEBU_CP110_COMPHY=m.

Seems like a reasonable compromise, given that TF-A could be classed
as broken. This must also prevent suspend/resume powering off PCI
devices, and then reconnecting them on resume.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4
  2025-10-30 15:33   ` Andrew Lunn
@ 2025-10-30 15:51     ` Josua Mayer
  2025-11-10 14:20       ` Gregory CLEMENT
  0 siblings, 1 reply; 7+ messages in thread
From: Josua Mayer @ 2025-10-30 15:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Gregory Clement, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Rabeeh Khoury, Yazan Shhady, Mikhail Anikin,
	Jon Nettleton, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-clk@vger.kernel.org

I missed a colon in the subject line "cp110:",
should I roll v2 for this?

Am 30.10.25 um 16:33 schrieb Andrew Lunn:
> On Thu, Oct 30, 2025 at 04:16:26PM +0100, Josua Mayer wrote:
>> CP110 based platforms rely on the bootloader for pci port
>> initialization.
>> TF-A actively prevents non-uboot re-configuration of pci lanes, and many
>> boards do not have software control over the pci card reset.
>>
>> If a pci port had link at boot-time and the clock is stopped at a later
>> point, the link fails and can not be recovered.
>>
>> PCI controller driver probe - and by extension ownership of a driver for
>> the pci clocks - may be delayed especially on large modular kernels,
>> causing the clock core to start disabling unused clocks.
>>
>> Add the CLK_IGNORE_UNUSED flag to the three pci port's clocks to ensure
>> they are not stopped before the pci controller driver has taken
>> ownership and tested for an existing link.
>>
>> This fixes failed pci link detection when controller driver probes late,
>> e.g. with arm64 defconfig and CONFIG_PHY_MVEBU_CP110_COMPHY=m.
> Seems like a reasonable compromise, given that TF-A could be classed
> as broken. This must also prevent suspend/resume powering off PCI
> devices, and then reconnecting them on resume.
Currently pcie-armada8k (unlike e.g. pci-imx6) does not currently define
any dev_pm_ops - so we should be safe from any power-management.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
>     Andrew

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] Revert "arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports"
  2025-10-30 15:16 ` [PATCH 1/2] Revert "arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports" Josua Mayer
@ 2025-11-10 14:19   ` Gregory CLEMENT
  0 siblings, 0 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2025-11-10 14:19 UTC (permalink / raw)
  To: Josua Mayer, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd
  Cc: Rabeeh Khoury, Yazan Shhady, Mikhail Anikin, Jon Nettleton,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk,
	Josua Mayer

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

> This reverts commit 794a066688038df46c01e177cc6faebded0acba4 because it
> misunderstood interworking between arm trusted firmware and the common
> phy driver, and does not consistently resolve the issue it was intended
> to address.
>
> Further diagnostics have revealed the root cause for the reported system
> lock-up in a race condition between pci driver probe and clock core
> disabling unused clocks.
>
> Revert the wrong change restoring driver control over all pci lanes.
> As a temporary workaround for the original issue, users can boot with
> "clk_ignore_unused".
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>


Applied on mvebu/dt64

Thanks,

Gregory


> ---
>  arch/arm64/boot/dts/marvell/cn9132-clearfog.dts | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> index 5cf83d8ca1f59..2507896d58f9b 100644
> --- a/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> +++ b/arch/arm64/boot/dts/marvell/cn9132-clearfog.dts
> @@ -413,13 +413,7 @@ fixed-link {
>  /* SRDS #0,#1,#2,#3 - PCIe */
>  &cp0_pcie0 {
>  	num-lanes = <4>;
> -	/*
> -	 * 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>;
> -	 */
> +	phys = <&cp0_comphy0 0>, <&cp0_comphy1 0>, <&cp0_comphy2 0>, <&cp0_comphy3 0>;
>  	status = "okay";
>  };
>  
> @@ -481,13 +475,7 @@ &cp1_eth0 {
>  /* SRDS #0,#1 - PCIe */
>  &cp1_pcie0 {
>  	num-lanes = <2>;
> -	/*
> -	 * 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>;
> -	 */
> +	phys = <&cp1_comphy0 0>, <&cp1_comphy1 0>;
>  	status = "okay";
>  };
>  
>
> -- 
> 2.51.0
>

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4
  2025-10-30 15:51     ` Josua Mayer
@ 2025-11-10 14:20       ` Gregory CLEMENT
  0 siblings, 0 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2025-11-10 14:20 UTC (permalink / raw)
  To: Josua Mayer, Andrew Lunn
  Cc: Sebastian Hesselbarth, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Michael Turquette, Stephen Boyd, Rabeeh Khoury,
	Yazan Shhady, Mikhail Anikin, Jon Nettleton,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org

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

> I missed a colon in the subject line "cp110:",
> should I roll v2 for this?

it is up to the clock maintainer.

>
> Am 30.10.25 um 16:33 schrieb Andrew Lunn:
>> On Thu, Oct 30, 2025 at 04:16:26PM +0100, Josua Mayer wrote:
>>> CP110 based platforms rely on the bootloader for pci port
>>> initialization.
>>> TF-A actively prevents non-uboot re-configuration of pci lanes, and many
>>> boards do not have software control over the pci card reset.
>>>
>>> If a pci port had link at boot-time and the clock is stopped at a later
>>> point, the link fails and can not be recovered.
>>>
>>> PCI controller driver probe - and by extension ownership of a driver for
>>> the pci clocks - may be delayed especially on large modular kernels,
>>> causing the clock core to start disabling unused clocks.
>>>
>>> Add the CLK_IGNORE_UNUSED flag to the three pci port's clocks to ensure
>>> they are not stopped before the pci controller driver has taken
>>> ownership and tested for an existing link.
>>>
>>> This fixes failed pci link detection when controller driver probes late,
>>> e.g. with arm64 defconfig and CONFIG_PHY_MVEBU_CP110_COMPHY=m.
>> Seems like a reasonable compromise, given that TF-A could be classed
>> as broken. This must also prevent suspend/resume powering off PCI
>> devices, and then reconnecting them on resume.
> Currently pcie-armada8k (unlike e.g. pci-imx6) does not currently define
> any dev_pm_ops - so we should be safe from any power-management.
>>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>


Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Thanks,

Gregory


>>
>>     Andrew

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-10 14:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-30 15:16 [PATCH 0/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Josua Mayer
2025-10-30 15:16 ` [PATCH 1/2] Revert "arm64: dts: marvell: cn9132-clearfog: fix multi-lane pci x2 and x4 ports" Josua Mayer
2025-11-10 14:19   ` Gregory CLEMENT
2025-10-30 15:16 ` [PATCH 2/2] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Josua Mayer
2025-10-30 15:33   ` Andrew Lunn
2025-10-30 15:51     ` Josua Mayer
2025-11-10 14:20       ` Gregory CLEMENT

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).