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.0On 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
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 {
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 {
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
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 {
>
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";
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";
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
© 2016 - 2026 Red Hat, Inc.