devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
@ 2025-11-05  5:55 Geraldo Nascimento
  2025-11-05  6:35 ` Shawn Lin
  0 siblings, 1 reply; 9+ messages in thread
From: Geraldo Nascimento @ 2025-11-05  5:55 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

The PERST# side-band signal is defined by PCIe spec as an open-drain
active-low signal that depends on a pull-up resistor to keep the
signal high when deasserted. Align bindings to the spec.

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@gmail.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)>;
 	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 {
-- 
2.49.0


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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-05  5:55 [PATCH] arm64: dts: rockchip: align bindings to PCIe spec Geraldo Nascimento
@ 2025-11-05  6:35 ` Shawn Lin
  2025-11-05  8:18   ` Geraldo Nascimento
  0 siblings, 1 reply; 9+ messages in thread
From: Shawn Lin @ 2025-11-05  6:35 UTC (permalink / raw)
  To: Geraldo Nascimento
  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, linux-rockchip

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@gmail.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 {


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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-05  6:35 ` Shawn Lin
@ 2025-11-05  8:18   ` Geraldo Nascimento
  2025-11-05  8:56     ` Shawn Lin
  0 siblings, 1 reply; 9+ messages in thread
From: Geraldo Nascimento @ 2025-11-05  8:18 UTC (permalink / raw)
  To: Shawn Lin
  Cc: 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, linux-rockchip

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

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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-05  8:18   ` Geraldo Nascimento
@ 2025-11-05  8:56     ` Shawn Lin
  2025-11-05 20:02       ` Geraldo Nascimento
  2025-11-07  2:43       ` Geraldo Nascimento
  0 siblings, 2 replies; 9+ messages in thread
From: Shawn Lin @ 2025-11-05  8:56 UTC (permalink / raw)
  To: Geraldo Nascimento
  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, linux-rockchip

在 2025/11/05 星期三 16:18, Geraldo Nascimento 写道:
> 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@gmail.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.


Does that work for you too?

&pcie0 {
	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
  	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>;
+		};
+
  	};

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


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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-05  8:56     ` Shawn Lin
@ 2025-11-05 20:02       ` Geraldo Nascimento
  2025-11-07  2:43       ` Geraldo Nascimento
  1 sibling, 0 replies; 9+ messages in thread
From: Geraldo Nascimento @ 2025-11-05 20:02 UTC (permalink / raw)
  To: Shawn Lin
  Cc: 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, linux-rockchip

On Wed, Nov 05, 2025 at 04:56:36PM +0800, Shawn Lin wrote:
> 在 2025/11/05 星期三 16:18, Geraldo Nascimento 写道:
> > 
> > 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.
> 
> 
> Does that work for you too?
> 
> &pcie0 {
> 	ep-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
>   	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>;
> +		};
> +
>   	};

Hi Shawn,

No, that does not work.

I believe the pull-up mux became needed because I was forcing open drain
on PERST#.

Thanks,
Geraldo Nascimento

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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-05  8:56     ` Shawn Lin
  2025-11-05 20:02       ` Geraldo Nascimento
@ 2025-11-07  2:43       ` Geraldo Nascimento
  2025-11-07  3:01         ` Shawn Lin
  1 sibling, 1 reply; 9+ messages in thread
From: Geraldo Nascimento @ 2025-11-07  2:43 UTC (permalink / raw)
  To: Shawn Lin
  Cc: 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, linux-rockchip

On Wed, Nov 05, 2025 at 04:56:36PM +0800, Shawn Lin wrote:
> 在 2025/11/05 星期三 16:18, Geraldo Nascimento 写道:
> > 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.

Shawn,

I quote from the PCIe Mini Card Electromechanical Specification Rev 1.2

"3.4.1. Logic Signal Requirements

The 3.3V card logic levels for single-ended digital signals (WAKE#,
CLKREQ#, PERST#, and W_DISABLE#) are given in Table 3-7. [...]"

So while you are correct that PERST# is most definitely not Open Drain,
there's evidence on the spec that defines this signal as Single-Ended.

Thanks,
Geraldo Nascimento

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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-07  2:43       ` Geraldo Nascimento
@ 2025-11-07  3:01         ` Shawn Lin
  2025-11-08 22:12           ` Sebastian Reichel
  0 siblings, 1 reply; 9+ messages in thread
From: Shawn Lin @ 2025-11-07  3:01 UTC (permalink / raw)
  To: Geraldo Nascimento, Ye Zhang
  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, linux-rockchip

+ Ye Zhang

在 2025/11/07 星期五 10:43, Geraldo Nascimento 写道:
> On Wed, Nov 05, 2025 at 04:56:36PM +0800, Shawn Lin wrote:
>> 在 2025/11/05 星期三 16:18, Geraldo Nascimento 写道:
>>> 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.
> 
> Shawn,
> 
> I quote from the PCIe Mini Card Electromechanical Specification Rev 1.2
> 
> "3.4.1. Logic Signal Requirements
> 
> The 3.3V card logic levels for single-ended digital signals (WAKE#,
> CLKREQ#, PERST#, and W_DISABLE#) are given in Table 3-7. [...]"
> 
> So while you are correct that PERST# is most definitely not Open Drain,
> there's evidence on the spec that defines this signal as Single-Ended.
> 

This's true. But I couldn't find any user in dts using either
GPIO_SINGLE_ENDED or GPIO_OPEN_DRAIN for PCIe PERST#. I'm curious
how these two flags affect actual behavior of chips. Ye, could you
please help check it?

> Thanks,
> Geraldo Nascimento
> 


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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-07  3:01         ` Shawn Lin
@ 2025-11-08 22:12           ` Sebastian Reichel
  2025-11-08 22:43             ` Geraldo Nascimento
  0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Reichel @ 2025-11-08 22:12 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Geraldo Nascimento, Ye Zhang, 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, linux-rockchip

[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]

Hi,

On Fri, Nov 07, 2025 at 11:01:04AM +0800, Shawn Lin wrote:
> + Ye Zhang
> 
> 在 2025/11/07 星期五 10:43, Geraldo Nascimento 写道:
> > On Wed, Nov 05, 2025 at 04:56:36PM +0800, Shawn Lin wrote:
> > > 在 2025/11/05 星期三 16:18, Geraldo Nascimento 写道:
> > > > 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.
> > 
> > Shawn,
> > 
> > I quote from the PCIe Mini Card Electromechanical Specification Rev 1.2
> > 
> > "3.4.1. Logic Signal Requirements
> > 
> > The 3.3V card logic levels for single-ended digital signals (WAKE#,
> > CLKREQ#, PERST#, and W_DISABLE#) are given in Table 3-7. [...]"
> > 
> > So while you are correct that PERST# is most definitely not Open Drain,
> > there's evidence on the spec that defines this signal as Single-Ended.
> > 
> 
> This's true. But I couldn't find any user in dts using either
> GPIO_SINGLE_ENDED or GPIO_OPEN_DRAIN for PCIe PERST#. I'm curious
> how these two flags affect actual behavior of chips. Ye, could you
> please help check it?

FWIW I assume single-ended in the spec means it's not differential
like all the highspeed signals on the PCIe connection. This says
nothing about open-drain, open-source or push-pull being used. The
kernel on the other hand has a very specific understanding of
GPIO_SINGLE_ENDED:

	if (flags & OF_GPIO_SINGLE_ENDED) {
		if (flags & OF_GPIO_OPEN_DRAIN)
			lflags |= GPIO_OPEN_DRAIN;
		else
			lflags |= GPIO_OPEN_SOURCE;
	}

I.e. it is the same as configuring open-source ;)

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
  2025-11-08 22:12           ` Sebastian Reichel
@ 2025-11-08 22:43             ` Geraldo Nascimento
  0 siblings, 0 replies; 9+ messages in thread
From: Geraldo Nascimento @ 2025-11-08 22:43 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Shawn Lin, Ye Zhang, 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, linux-rockchip

On Sat, Nov 08, 2025 at 11:12:54PM +0100, Sebastian Reichel wrote:
> Hi,

Hi Sebastian,

> 
> On Fri, Nov 07, 2025 at 11:01:04AM +0800, Shawn Lin wrote:
> > + Ye Zhang
> > 
> > 在 2025/11/07 星期五 10:43, Geraldo Nascimento 写道:
> > > On Wed, Nov 05, 2025 at 04:56:36PM +0800, Shawn Lin wrote:
> > > > 在 2025/11/05 星期三 16:18, Geraldo Nascimento 写道:
> > > > > 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.
> > > 
> > > Shawn,
> > > 
> > > I quote from the PCIe Mini Card Electromechanical Specification Rev 1.2
> > > 
> > > "3.4.1. Logic Signal Requirements
> > > 
> > > The 3.3V card logic levels for single-ended digital signals (WAKE#,
> > > CLKREQ#, PERST#, and W_DISABLE#) are given in Table 3-7. [...]"
> > > 
> > > So while you are correct that PERST# is most definitely not Open Drain,
> > > there's evidence on the spec that defines this signal as Single-Ended.
> > > 
> > 
> > This's true. But I couldn't find any user in dts using either
> > GPIO_SINGLE_ENDED or GPIO_OPEN_DRAIN for PCIe PERST#. I'm curious
> > how these two flags affect actual behavior of chips. Ye, could you
> > please help check it?
> 
> FWIW I assume single-ended in the spec means it's not differential
> like all the highspeed signals on the PCIe connection. This says
> nothing about open-drain, open-source or push-pull being used. The

yes, I agree. It was an oversight on my part to assume open-drain on
PERST# was part of the spec just because many cores implement it that
way. Kudos to Shawn for correcting me.

> kernel on the other hand has a very specific understanding of
> GPIO_SINGLE_ENDED:
> 
> 	if (flags & OF_GPIO_SINGLE_ENDED) {
> 		if (flags & OF_GPIO_OPEN_DRAIN)
> 			lflags |= GPIO_OPEN_DRAIN;
> 		else
> 			lflags |= GPIO_OPEN_SOURCE;
> 	}
> 
> I.e. it is the same as configuring open-source ;)

Yup, I had noticed that. This works because the reset value of PMU GRF
register PMUGRF_GPIO0B_P sets the relevant PERST# GPIO (GPIO0-12) on my
board to pull-down, which can work with Open Source/Emitter. If we set
the GPIO to Open Drain/Collector we must on the other hand set that pin
to pull-up. Either way it works.

I've been investigating why that GPIO isn't properly working as
Push-Pull for my board (Rock PI N10) but so far I'm clueless.

Thank you,
Geraldo Nascimento

> 
> Greetings,
> 
> -- Sebastian



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

end of thread, other threads:[~2025-11-08 22:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-05  5:55 [PATCH] arm64: dts: rockchip: align bindings to PCIe spec Geraldo Nascimento
2025-11-05  6:35 ` Shawn Lin
2025-11-05  8:18   ` Geraldo Nascimento
2025-11-05  8:56     ` Shawn Lin
2025-11-05 20:02       ` Geraldo Nascimento
2025-11-07  2:43       ` Geraldo Nascimento
2025-11-07  3:01         ` Shawn Lin
2025-11-08 22:12           ` Sebastian Reichel
2025-11-08 22:43             ` 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).