linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] PCI: rockchip-host: support quirky devices
@ 2025-11-03  6:26 Geraldo Nascimento
  2025-11-03  6:27 ` [RFC PATCH 1/2] arm64: dts: rockchip: drop PCIe 3v3 always-on/boot-on Geraldo Nascimento
  2025-11-03  6:27 ` [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle Geraldo Nascimento
  0 siblings, 2 replies; 11+ messages in thread
From: Geraldo Nascimento @ 2025-11-03  6:26 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	linux-pci, linux-arm-kernel, linux-kernel, devicetree,
	Krzysztof Kozlowski, Conor Dooley, Johan Jonker,
	Geraldo Nascimento

With these two changes I'm able to work with a Samsung PM981a OEM SSD
that is known to be not-working with Rockchip-IP PCIe.

Previously I attempted a contrived solution that mostly worked for my
simple purposes but was rather inelegant and impractical.

Now I have isolated the behavior to the three lines in the two commits.
Omit those three lines and you get a working set with the kernel.

I have no idea how to actually implement this in a way that makes sense
and doesn't break the PCIe spec but it is my sincere wish that
interested RK3399 parties test the change and report any regressions
with already-working devices and specifically, successes or failures of
initial link-training with these changes.

Geraldo Nascimento (2):
  arm64: dts: rockchip: drop PCIe 3v3 always-on/boot-on
  PCI: rockchip-host: drop wait on PERST# toggle

 arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi | 2 --
 drivers/pci/controller/pcie-rockchip-host.c           | 1 -
 2 files changed, 3 deletions(-)

-- 
2.49.0


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

* [RFC PATCH 1/2] arm64: dts: rockchip: drop PCIe 3v3 always-on/boot-on
  2025-11-03  6:26 [RFC PATCH 0/2] PCI: rockchip-host: support quirky devices Geraldo Nascimento
@ 2025-11-03  6:27 ` Geraldo Nascimento
  2025-11-03  6:27 ` [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle Geraldo Nascimento
  1 sibling, 0 replies; 11+ messages in thread
From: Geraldo Nascimento @ 2025-11-03  6:27 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	linux-pci, linux-arm-kernel, linux-kernel, devicetree,
	Krzysztof Kozlowski, Conor Dooley, Johan Jonker,
	Geraldo Nascimento

Example commit of needed dropping of regulator always-on/boot-on
declarations to make sure quirky devices known to not be working on
RK3399 are able to enumerate on the PCI bus.

One example only, tested on my ROCK PI N10 board, to avoid patch-bomb

Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
index aa70776e898a..ad99a8558bf0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
@@ -25,8 +25,6 @@ vcc3v3_pcie: regulator-vcc-pcie {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pcie_pwr>;
 		regulator-name = "vcc3v3_pcie";
-		regulator-always-on;
-		regulator-boot-on;
 		vin-supply = <&vcc5v0_sys>;
 	};
 
-- 
2.49.0


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

* [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-03  6:26 [RFC PATCH 0/2] PCI: rockchip-host: support quirky devices Geraldo Nascimento
  2025-11-03  6:27 ` [RFC PATCH 1/2] arm64: dts: rockchip: drop PCIe 3v3 always-on/boot-on Geraldo Nascimento
@ 2025-11-03  6:27 ` Geraldo Nascimento
  2025-11-03 18:10   ` Bjorn Helgaas
  1 sibling, 1 reply; 11+ messages in thread
From: Geraldo Nascimento @ 2025-11-03  6:27 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	linux-pci, linux-arm-kernel, linux-kernel, devicetree,
	Krzysztof Kozlowski, Conor Dooley, Johan Jonker,
	Geraldo Nascimento

With this change PCIe will complete link-training with a known quirky
device - Samsung OEM PM981a SSD. This is completely against the PCIe
spec and yet it works as long as the power regulator for 3v3 PCIe
power is not defined as always-on or boot-on.

Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ee1822ca01db..6add0faf6dc9 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -314,7 +314,6 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 	rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
 			    PCIE_CLIENT_CONFIG);
 
-	msleep(PCIE_T_PVPERL_MS);
 	gpiod_set_value_cansleep(rockchip->perst_gpio, 1);
 
 	msleep(PCIE_RESET_CONFIG_WAIT_MS);
-- 
2.49.0


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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-03  6:27 ` [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle Geraldo Nascimento
@ 2025-11-03 18:10   ` Bjorn Helgaas
  2025-11-03 20:55     ` Geraldo Nascimento
  2025-11-05  3:55     ` Geraldo Nascimento
  0 siblings, 2 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2025-11-03 18:10 UTC (permalink / raw)
  To: Geraldo Nascimento
  Cc: linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

On Mon, Nov 03, 2025 at 03:27:25AM -0300, Geraldo Nascimento wrote:
> With this change PCIe will complete link-training with a known quirky
> device - Samsung OEM PM981a SSD. This is completely against the PCIe
> spec and yet it works as long as the power regulator for 3v3 PCIe
> power is not defined as always-on or boot-on.

What is against the spec?  In what way is this SSD "known quirky"?  Is
there a published erratum for it?

Removing this delay might make this SSD work, but if this delay is
required per PCIe spec, how can we be confident that other devices
will still work?

Reports of devices that still work is not really enough to move this
from the "hack that makes one device work" column to the "safe and
effective for all devices" column.

It's easy to see how *lack* of a delay can break something, but much
harder to imagine how *removing* a delay can make something work.
Devices must be able to tolerate pretty much arbitrary delays.

> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip-host.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index ee1822ca01db..6add0faf6dc9 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -314,7 +314,6 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
>  	rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
>  			    PCIE_CLIENT_CONFIG);
>  
> -	msleep(PCIE_T_PVPERL_MS);
>  	gpiod_set_value_cansleep(rockchip->perst_gpio, 1);
>  
>  	msleep(PCIE_RESET_CONFIG_WAIT_MS);
> -- 
> 2.49.0
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-03 18:10   ` Bjorn Helgaas
@ 2025-11-03 20:55     ` Geraldo Nascimento
  2025-11-05  3:55     ` Geraldo Nascimento
  1 sibling, 0 replies; 11+ messages in thread
From: Geraldo Nascimento @ 2025-11-03 20:55 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

On Mon, Nov 03, 2025 at 12:10:38PM -0600, Bjorn Helgaas wrote:
> On Mon, Nov 03, 2025 at 03:27:25AM -0300, Geraldo Nascimento wrote:
> > With this change PCIe will complete link-training with a known quirky
> > device - Samsung OEM PM981a SSD. This is completely against the PCIe
> > spec and yet it works as long as the power regulator for 3v3 PCIe
> > power is not defined as always-on or boot-on.
> 
> What is against the spec?  In what way is this SSD "known quirky"?  Is
> there a published erratum for it?

Hi Bjorn!

My proposed change itself is against the spec.

This SSD is known to simply not complete initial link-training with
Rockchip-IP PCIe. This is not officialy documented but based on
reports across boards (like the Armbian one).

I think it's the other way around though, it's the Rockchip-IP PCIe
controller that is quirky somehow and doesn't play nice with
many, many devices.

> 
> Removing this delay might make this SSD work, but if this delay is
> required per PCIe spec, how can we be confident that other devices
> will still work?

We really can't be sure there will be no side-effects to other devices.

> 
> Reports of devices that still work is not really enough to move this
> from the "hack that makes one device work" column to the "safe and
> effective for all devices" column.

I agree, and I knew from the start I would not get encouragements from
the community relative to this change. Still, it seemed selfish not to
share the workaround.

> 
> It's easy to see how *lack* of a delay can break something, but much
> harder to imagine how *removing* a delay can make something work.
> Devices must be able to tolerate pretty much arbitrary delays.

That said, the problem of quirky Rockchip-IP PCIe remains. The fact
that removing the delay makes it work with devices that previously
did not complete initial link training must at least point to a
resolution somehow, even if the present change is unacceptable.

I'll continue to hack around this, see if I can pinpoint exactly why
that delay screws up my initial link training.

Thanks,
Geraldo Nascimento

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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-03 18:10   ` Bjorn Helgaas
  2025-11-03 20:55     ` Geraldo Nascimento
@ 2025-11-05  3:55     ` Geraldo Nascimento
  2025-11-05  9:06       ` Diederik de Haas
  2025-11-09 23:51       ` Dragan Simic
  1 sibling, 2 replies; 11+ messages in thread
From: Geraldo Nascimento @ 2025-11-05  3:55 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

On Mon, Nov 03, 2025 at 12:10:38PM -0600, Bjorn Helgaas wrote:
> On Mon, Nov 03, 2025 at 03:27:25AM -0300, Geraldo Nascimento wrote:
> > With this change PCIe will complete link-training with a known quirky
> > device - Samsung OEM PM981a SSD. This is completely against the PCIe
> > spec and yet it works as long as the power regulator for 3v3 PCIe
> > power is not defined as always-on or boot-on.
> 
> What is against the spec?  In what way is this SSD "known quirky"?  Is
> there a published erratum for it?
> 
> Removing this delay might make this SSD work, but if this delay is
> required per PCIe spec, how can we be confident that other devices
> will still work?
> 
> Reports of devices that still work is not really enough to move this
> from the "hack that makes one device work" column to the "safe and
> effective for all devices" column.
> 
> It's easy to see how *lack* of a delay can break something, but much
> harder to imagine how *removing* a delay can make something work.
> Devices must be able to tolerate pretty much arbitrary delays.

Hi Bjorn!

I did some more testing, intrigued by why would a delay of more than
5 ms after the enablement of the power rails trigger failure in
initial link-training.

Something in my intuition kept telling me this was PERST# related,
and so I followed that rabbit-hole.

It seems the following change will allow the SSD to work with the
Rockchip-IP PCIe core without any other changes. So it is purely
a DT change and we are able to keep the mandatory 100ms delay
after driving PERST# low, as well as the always-on/boot-on
properties of the 3v3 power regulator.

This time everything is within the PCIe spec AFAICT, PERST# indeed
is an Open Drain signal, and indeed it does requires pull-up resistor
to maintain the drive after driving it high.

I'm still testing the overall stability of this, let's hope for the
best!

Thanks,
Geraldo Nascimento

diff --git a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
index aa70776e898a..1c5afc0413bc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
@@ -383,13 +383,14 @@ &pcie_phy {
 };
 
 &pcie0 {
-	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
+	ep-gpios = <&gpio0 RK_PB4 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
 	num-lanes = <4>;
-	pinctrl-0 = <&pcie_clkreqnb_cpm>;
+	pinctrl-0 = <&pcie_clkreqnb_cpm>, <&pcie_perst>;
 	pinctrl-names = "default";
 	vpcie0v9-supply = <&vcca_0v9>;	/* VCC_0V9_S0 */
 	vpcie1v8-supply = <&vcca_1v8>;	/* VCC_1V8_S0 */
 	vpcie3v3-supply = <&vcc3v3_pcie>;
+	max-link-speed = <2>;
 	status = "okay";
 };
 
@@ -408,6 +409,10 @@ pcie {
 		pcie_pwr: pcie-pwr {
 			rockchip,pins = <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
 		};
+		pcie_perst: pcie-perst {
+			rockchip,pins = <0 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+
 	};
 
 	pmic {

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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-05  3:55     ` Geraldo Nascimento
@ 2025-11-05  9:06       ` Diederik de Haas
  2025-11-05 21:22         ` Geraldo Nascimento
  2025-11-09 23:51       ` Dragan Simic
  1 sibling, 1 reply; 11+ messages in thread
From: Diederik de Haas @ 2025-11-05  9:06 UTC (permalink / raw)
  To: Geraldo Nascimento, Bjorn Helgaas
  Cc: linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

On Wed Nov 5, 2025 at 4:55 AM CET, Geraldo Nascimento wrote:
> On Mon, Nov 03, 2025 at 12:10:38PM -0600, Bjorn Helgaas wrote:
>> On Mon, Nov 03, 2025 at 03:27:25AM -0300, Geraldo Nascimento wrote:
>> > With this change PCIe will complete link-training with a known quirky
>> > device - Samsung OEM PM981a SSD. This is completely against the PCIe
>
> Something in my intuition kept telling me this was PERST# related,
> and so I followed that rabbit-hole.
>
> It seems the following change will allow the SSD to work with the
> Rockchip-IP PCIe core without any other changes. So it is purely
> a DT change and we are able to keep the mandatory 100ms delay
> after driving PERST# low, as well as the always-on/boot-on
> properties of the 3v3 power regulator.
>
> This time everything is within the PCIe spec AFAICT, PERST# indeed
> is an Open Drain signal, and indeed it does requires pull-up resistor
> to maintain the drive after driving it high.
>
> I'm still testing the overall stability of this, let's hope for the
> best!

I have a Samsung PM981 (without the 'a') and AFAICT it works fine.
I had noticed that the PERST# (pinctrl) was missing, but 'ep-gpios'
was/is new to me and I hadn't had time to research that properly yet.
Good to see you already found it yourself :-)

Cheers,
  Diederik

> diff --git a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> index aa70776e898a..1c5afc0413bc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> @@ -383,13 +383,14 @@ &pcie_phy {
>  };
>  
>  &pcie0 {
> -	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> +	ep-gpios = <&gpio0 RK_PB4 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  	num-lanes = <4>;
> -	pinctrl-0 = <&pcie_clkreqnb_cpm>;
> +	pinctrl-0 = <&pcie_clkreqnb_cpm>, <&pcie_perst>;
>  	pinctrl-names = "default";
>  	vpcie0v9-supply = <&vcca_0v9>;	/* VCC_0V9_S0 */
>  	vpcie1v8-supply = <&vcca_1v8>;	/* VCC_1V8_S0 */
>  	vpcie3v3-supply = <&vcc3v3_pcie>;
> +	max-link-speed = <2>;
>  	status = "okay";
>  };
>  
> @@ -408,6 +409,10 @@ pcie {
>  		pcie_pwr: pcie-pwr {
>  			rockchip,pins = <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
>  		};
> +		pcie_perst: pcie-perst {
> +			rockchip,pins = <0 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>;
> +		};
> +
>  	};
>  
>  	pmic {
>

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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-05  9:06       ` Diederik de Haas
@ 2025-11-05 21:22         ` Geraldo Nascimento
  2025-11-07 10:27           ` Diederik de Haas
  0 siblings, 1 reply; 11+ messages in thread
From: Geraldo Nascimento @ 2025-11-05 21:22 UTC (permalink / raw)
  To: Diederik de Haas
  Cc: Bjorn Helgaas, linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

On Wed, Nov 05, 2025 at 10:06:53AM +0100, Diederik de Haas wrote:
> I have a Samsung PM981 (without the 'a') and AFAICT it works fine.
> I had noticed that the PERST# (pinctrl) was missing, but 'ep-gpios'
> was/is new to me and I hadn't had time to research that properly yet.
> Good to see you already found it yourself :-)
> 
> Cheers,
>   Diederik

Hello Diederik,

Thanks for the heads up!

Would you mind testing the following DT change to make sure your PM981
continues to work fine?

Thank you,
Geraldo Nascimento

---

diff --git a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
index aa70776e898a..b3d19dce539f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
@@ -383,7 +383,7 @@ &pcie_phy {
 };
 
 &pcie0 {
-	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
+	ep-gpios = <&gpio0 RK_PB4 (GPIO_ACTIVE_HIGH | GPIO_SINGLE_ENDED)>;
 	num-lanes = <4>;
 	pinctrl-0 = <&pcie_clkreqnb_cpm>;
 	pinctrl-names = "default";

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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-05 21:22         ` Geraldo Nascimento
@ 2025-11-07 10:27           ` Diederik de Haas
  0 siblings, 0 replies; 11+ messages in thread
From: Diederik de Haas @ 2025-11-07 10:27 UTC (permalink / raw)
  To: Geraldo Nascimento
  Cc: Bjorn Helgaas, linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

Hi Geraldo,

On Wed Nov 5, 2025 at 10:22 PM CET, Geraldo Nascimento wrote:
> On Wed, Nov 05, 2025 at 10:06:53AM +0100, Diederik de Haas wrote:
>> I have a Samsung PM981 (without the 'a') and AFAICT it works fine.
>> I had noticed that the PERST# (pinctrl) was missing, but 'ep-gpios'
>> was/is new to me and I hadn't had time to research that properly yet.
>> Good to see you already found it yourself :-)
>
> Would you mind testing the following DT change to make sure your PM981
> continues to work fine?

I just learned the the PCIe system on rk3399 is indeed different from
the systems where I use it with (rk3568 & rk3588). And 'ep-gpios' is
only used with rk3399 based devices (in the Rockchip dts tree), which
explains why I hadn't seen that before.
Which in turn means I can't test your proposed change.

I guess I was triggered by RFC patch 2 which said 'a known quirky
device' while my Samsung PM981's are working fine ... but with DW based
IP. You did specify in your cover letter the connection with Rockchip
PCI IP, which apparently can make a (lot of?) difference.
Me finding the PERST# pinctrl in a few minutes and we finding
improvements in RockPro64's PCI 'config' recently, indicated to me that
a new look into the dts definition may be warranted, before changing
``drivers/pci/controller/pcie-rockchip-host.c`` for everyone.

Cheers,
  Diederik

> Thank you,
> Geraldo Nascimento
>
> ---
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> index aa70776e898a..b3d19dce539f 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> @@ -383,7 +383,7 @@ &pcie_phy {
>  };
>  
>  &pcie0 {
> -	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> +	ep-gpios = <&gpio0 RK_PB4 (GPIO_ACTIVE_HIGH | GPIO_SINGLE_ENDED)>;
>  	num-lanes = <4>;
>  	pinctrl-0 = <&pcie_clkreqnb_cpm>;
>  	pinctrl-names = "default";


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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-05  3:55     ` Geraldo Nascimento
  2025-11-05  9:06       ` Diederik de Haas
@ 2025-11-09 23:51       ` Dragan Simic
  2025-11-09 23:57         ` Geraldo Nascimento
  1 sibling, 1 reply; 11+ messages in thread
From: Dragan Simic @ 2025-11-09 23:51 UTC (permalink / raw)
  To: Geraldo Nascimento
  Cc: Bjorn Helgaas, linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

Hello Geraldo,

On Wednesday, November 05, 2025 04:55 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> I did some more testing, intrigued by why would a delay of more than
> 5 ms after the enablement of the power rails trigger failure in
> initial link-training.
> 
> Something in my intuition kept telling me this was PERST# related,
> and so I followed that rabbit-hole.
> 
> It seems the following change will allow the SSD to work with the
> Rockchip-IP PCIe core without any other changes. So it is purely
> a DT change and we are able to keep the mandatory 100ms delay
> after driving PERST# low, as well as the always-on/boot-on
> properties of the 3v3 power regulator.
> 
> This time everything is within the PCIe spec AFAICT, PERST# indeed
> is an Open Drain signal, and indeed it does requires pull-up resistor
> to maintain the drive after driving it high.
> 
> I'm still testing the overall stability of this, let's hope for the
> best!
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> index aa70776e898a..1c5afc0413bc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> @@ -383,13 +383,14 @@ &pcie_phy {
>  };
>  
>  &pcie0 {
> -	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> +	ep-gpios = <&gpio0 RK_PB4 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  	num-lanes = <4>;
> -	pinctrl-0 = <&pcie_clkreqnb_cpm>;
> +	pinctrl-0 = <&pcie_clkreqnb_cpm>, <&pcie_perst>;
>  	pinctrl-names = "default";
>  	vpcie0v9-supply = <&vcca_0v9>;	/* VCC_0V9_S0 */
>  	vpcie1v8-supply = <&vcca_1v8>;	/* VCC_1V8_S0 */
>  	vpcie3v3-supply = <&vcc3v3_pcie>;
> +	max-link-speed = <2>;

FWIW, we shouldn't be enabling PCIe Gen2 here, because it's been
already disabled for the RK3399 due to unknown errata in the commit
712fa1777207 ("arm64: dts: rockchip: add max-link-speed for rk3399",
2016-12-16).  It's perfectly reasonable to assume the same for the
RK3399Pro, which is basically RK3399 packaged together with RK1808,
AFAIK with no on-package interconnects.

>  	status = "okay";
>  };
>  
> @@ -408,6 +409,10 @@ pcie {
>  		pcie_pwr: pcie-pwr {
>  			rockchip,pins = <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
>  		};
> +		pcie_perst: pcie-perst {
> +			rockchip,pins = <0 RK_PB4 RK_FUNC_GPIO &pcfg_pull_up>;
> +		};
> +
>  	};
>  
>  	pmic {


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

* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
  2025-11-09 23:51       ` Dragan Simic
@ 2025-11-09 23:57         ` Geraldo Nascimento
  0 siblings, 0 replies; 11+ messages in thread
From: Geraldo Nascimento @ 2025-11-09 23:57 UTC (permalink / raw)
  To: Dragan Simic
  Cc: Bjorn Helgaas, linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, linux-arm-kernel,
	linux-kernel, devicetree, Krzysztof Kozlowski, Conor Dooley,
	Johan Jonker

On Mon, Nov 10, 2025 at 12:51:49AM +0100, Dragan Simic wrote:
> Hello Geraldo,
> 
> On Wednesday, November 05, 2025 04:55 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > I did some more testing, intrigued by why would a delay of more than
> > 5 ms after the enablement of the power rails trigger failure in
> > initial link-training.
> > 
> > Something in my intuition kept telling me this was PERST# related,
> > and so I followed that rabbit-hole.
> > 
> > It seems the following change will allow the SSD to work with the
> > Rockchip-IP PCIe core without any other changes. So it is purely
> > a DT change and we are able to keep the mandatory 100ms delay
> > after driving PERST# low, as well as the always-on/boot-on
> > properties of the 3v3 power regulator.
> > 
> > This time everything is within the PCIe spec AFAICT, PERST# indeed
> > is an Open Drain signal, and indeed it does requires pull-up resistor
> > to maintain the drive after driving it high.
> > 
> > I'm still testing the overall stability of this, let's hope for the
> > best!
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> > index aa70776e898a..1c5afc0413bc 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> > @@ -383,13 +383,14 @@ &pcie_phy {
> >  };
> >  
> >  &pcie0 {
> > -	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> > +	ep-gpios = <&gpio0 RK_PB4 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >  	num-lanes = <4>;
> > -	pinctrl-0 = <&pcie_clkreqnb_cpm>;
> > +	pinctrl-0 = <&pcie_clkreqnb_cpm>, <&pcie_perst>;
> >  	pinctrl-names = "default";
> >  	vpcie0v9-supply = <&vcca_0v9>;	/* VCC_0V9_S0 */
> >  	vpcie1v8-supply = <&vcca_1v8>;	/* VCC_1V8_S0 */
> >  	vpcie3v3-supply = <&vcc3v3_pcie>;
> > +	max-link-speed = <2>;
> 
> FWIW, we shouldn't be enabling PCIe Gen2 here, because it's been
> already disabled for the RK3399 due to unknown errata in the commit
> 712fa1777207 ("arm64: dts: rockchip: add max-link-speed for rk3399",
> 2016-12-16).  It's perfectly reasonable to assume the same for the
> RK3399Pro, which is basically RK3399 packaged together with RK1808,
> AFAIK with no on-package interconnects.

Hi Dragan!

Thanks for the catch, you are correct. But in this case it was just
for my tests and it crept in in the git diff. I wasn't really proposing
to make that change.

Thanks,
Geraldo Nascimento


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

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

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-03  6:26 [RFC PATCH 0/2] PCI: rockchip-host: support quirky devices Geraldo Nascimento
2025-11-03  6:27 ` [RFC PATCH 1/2] arm64: dts: rockchip: drop PCIe 3v3 always-on/boot-on Geraldo Nascimento
2025-11-03  6:27 ` [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle Geraldo Nascimento
2025-11-03 18:10   ` Bjorn Helgaas
2025-11-03 20:55     ` Geraldo Nascimento
2025-11-05  3:55     ` Geraldo Nascimento
2025-11-05  9:06       ` Diederik de Haas
2025-11-05 21:22         ` Geraldo Nascimento
2025-11-07 10:27           ` Diederik de Haas
2025-11-09 23:51       ` Dragan Simic
2025-11-09 23:57         ` Geraldo Nascimento

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).