linux-kernel - Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aQsIXcQzeYop6a0B@geday>
Date: Wed, 5 Nov 2025 05:18:37 -0300
From: Geraldo Nascimento <geraldogabriel@...il.com>
To: Shawn Lin <shawn.lin@...k-chips.com>
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Manivannan Sadhasivam <mani@...nel.org>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Heiko Stuebner <heiko@...ech.de>, linux-pci@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Johan Jonker <jbx6244@...il.com>,
	linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec

On Wed, Nov 05, 2025 at 02:35:28PM +0800, Shawn Lin wrote:
> Hi Geraldo,
> 
> 在 2025/11/05 星期三 13:55, Geraldo Nascimento 写道:
> > The PERST# side-band signal is defined by PCIe spec as an open-drain
> 
> I couldn't find any clue that says PERST# is an open-drain signal. Could
> you quote it from PCI Express Card Electromechanical Specification?
> 
> > active-low signal that depends on a pull-up resistor to keep the
> > signal high when deasserted. Align bindings to the spec.
> 
> This is not true from my POV. An open-drain PCIe side-band  signal
> is used for both of EP and RC to achieve some special work-flow, like
> CLKREQ# for L1ss, etc. Since both ends could control it. But PERST# is a
> fundamental reset signal driven by RC which should be in sure state,
> high or low, has nothing to do with open-drain.
> 
> > 
> > Note that the relevant driver hacks the active-low signal as
> > active-high and switches the normal polarity of PERST#
> > assertion/deassertion, 1 and 0 in that order, and instead uses
> > 0 to signal low (assertion) and 1 to signal deassertion.
> > 
> > Incidentally, this change makes hardware that refused to work
> > with the Rockchip-IP PCIe core working for me, which was the
> > object of many fool's errands.
> > 
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@...il.com>
> > ---
> >   arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi | 8 ++++++--
> >   1 file changed, 6 insertions(+), 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..8dcb03708145 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399pro-vmarc-som.dtsi
> > @@ -383,9 +383,9 @@ &pcie_phy {
> >   };
> >   
> >   &pcie0 {
> > -	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
> > +	ep-gpios = <&gpio0 RK_PB4 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> 
> So my biggest guess is we don't need this change at all.
> gpio0b4 is used as gpio function, the problem you faced is that it
> didn't set gpio0b4 as pull-up, because the defaut state is pull-down.
> 
> Maybe the drive current of this IO is too weak, making it unable to 
> fully drive high in the pull-down state? If that's the case, can you see 
> a half-level signal on the oscilloscope?
> 
> >   	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 */
> > @@ -408,6 +408,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 {
>

Hi Shawn, glad to hear from you.

Perhaps the following change is better? It resolves the issue
without the added complication of open drain. After you questioned
if open drain is actually part of the spec, I remembered that
GPIO_OPEN_DRAIN is actually (GPIO_SINGLE_ENDED | GPIO_LINE_OPEN_DRAIN)
so I decided to test with just GPIO_SINGLE_ENDED and it works.

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