[PATCH v3] arm64: dts: rockchip: change pinctrl for pcie2x1l2 for Radxa ROCK 5A

[Jonas Karlman]

On 2024-10-02 13:21, Heiko Stübner wrote:
> Am Mittwoch, 2. Oktober 2024, 12:50:29 CEST schrieb Jonas Karlman:
>> Hi,
>>
>> On 2024-10-02 01:50, FUKAUMI Naoki wrote:
>>> for pcie2x1l2, only pcie20x1_2_perstn_m0 is required, and its function
>>> needs to be GPIO to avoid freeze at "pci enum" without PCIe device on
>>> u-boot.
>>>
>>> change pinctrl definitions for pcie2x1l2. no functional change is
>>> intended on Linux kernel.
>>
>> After the split and addition of pcie2_reset I think this patch is no
>> longer needed? The issue this patch tried to fix was already
>> fixed/changed in "arm64: dts: rockchip: Split up RK3588's PCIe pinctrls".
>>
>> Looks like this now just rename pcie2_reset to pcie20x1_2_perstn_m0?
> 
> and removes the other pinctrl states clkreqn and waken .

This is not something that I think should have been done in the first
place, the pins/signals exists in hw and schematics, however software
is not using these signals.

> 
> In a previous version they mentioned that this somehow affects u-boot.

The issue with U-Boot is that for pcie the pin used for reset-gpios must
use gpio func pinconf, or pci enumerating in U-Boot will freeze the
board.

"arm64: dts: rockchip: Split up RK3588's PCIe pinctrls" already changed
to use the gpio func for the perstn pin, so the U-Boot issue should
already have been resolved with that patch.

For Linux the pins gpio func is implicitly configured when the pcie
driver request the reset-gpios pin. In U-Boot there exists some pinctrl
overrides to solve freeze issues, those should also be sent upstream ;-)

Regards,
Jonas

> 
> But you're right, that renaming of the pinctrl reset entry should
> definitly not be in there.
> 
> 
>>> Signed-off-by: FUKAUMI Naoki <naoki at radxa.com>
>>> ---
>>> Changed in v3:
>>> - rebase on next/master
>>> Changed in v2:
>>> - reword commit message
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>>> index 87fce8d9a964..841ac9a30628 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
>>> @@ -310,7 +310,7 @@ rgmii_phy1: ethernet-phy at 1 {
>>>  };
>>>  
>>>  &pcie2x1l2 {
>>> -	pinctrl-0 = <&pcie2_reset>, <&pcie20x1m0_clkreqn>, <&pcie20x1m0_waken>;
>>> +	pinctrl-0 = <&pcie20x1_2_perstn_m0>;
>>>  	pinctrl-names = "default";
>>>  	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
>>>  	vpcie3v3-supply = <&vcc3v3_wf>;
>>> @@ -325,12 +325,12 @@ io_led: io-led {
>>>  	};
>>>  
>>>  	pcie {
>>> -		pow_en: pow-en {
>>> -			rockchip,pins = <0 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>;
>>> +		pcie20x1_2_perstn_m0: pcie20x1-2-perstn-m0 {
>>> +			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_none>;
>>>  		};
>>>  
>>> -		pcie2_reset: pcie2-reset {
>>> -			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_none>;
>>> +		pow_en: pow-en {
>>> +			rockchip,pins = <0 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>;
>>>  		};
>>>  	};
>>>  
>>
>>
> 
> 
> 
>