Re: [RFC PATCH 2/2] PCI: rockchip-host: drop wait on PERST# toggle

Diederik de Haas
2025-11-07T10:28:03Z

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";