* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
@ 2025-11-05 3:55 ` Geraldo Nascimento
0 siblings, 0 replies; 22+ 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 {
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 22+ 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
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
@ 2025-11-05 9:06 ` Diederik de Haas
0 siblings, 0 replies; 22+ 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 {
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 22+ 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
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
@ 2025-11-05 21:22 ` Geraldo Nascimento
0 siblings, 0 replies; 22+ 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";
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 22+ 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
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
@ 2025-11-07 10:27 ` Diederik de Haas
0 siblings, 0 replies; 22+ 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";
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 22+ 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-09 23:51 ` Dragan Simic
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
@ 2025-11-09 23:51 ` Dragan Simic
0 siblings, 0 replies; 22+ 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 {
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 22+ 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
-1 siblings, 0 replies; 22+ 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] 22+ messages in thread* Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle
@ 2025-11-09 23:57 ` Geraldo Nascimento
0 siblings, 0 replies; 22+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 22+ messages in thread