[PATCH] arm64: dts: rockchip: fix PCIe regulators for Radxa ROCK 3A

[FUKAUMI Naoki]

hi,

On 9/13/24 12:30, Chukun Pan wrote:
>> @@ -119,14 +123,10 @@ vcc3v3_pi6c_03: vcc3v3-pi6c-03-regulator {
> 
> Please add /* actually fed by vcc5v0_sys */
> 
>>   	vcc3v3_pcie: vcc3v3-pcie-regulator {
>>   		compatible = "regulator-fixed";
>> -		enable-active-high;
>> -		gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
>> -		pinctrl-names = "default";
>> -		pinctrl-0 = <&pcie_enable_h>;
>>   		regulator-name = "vcc3v3_pcie";
>>   		regulator-min-microvolt = <3300000>;
>>   		regulator-max-microvolt = <3300000>;
>> -		vin-supply = <&vcc5v0_sys>;
>> +		vin-supply = <&vcc3v3_pi6c_03>;
>>   	};
> 
> I recommend renaming vcc3v3_pcie to vcc3v3_pcie30x1, which better
> matches the schematic.

ok

>>   &pcie2x1 {
>>   	pinctrl-names = "default";
>> -	pinctrl-0 = <&pcie_reset_h>;
>> +	pinctrl-0 = <&pcie2x1m1_pins>;
>>   	reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
>> -	vpcie3v3-supply = <&vcc3v3_pcie>;
>> +	vpcie3v3-supply = <&vcc3v3_wf>;
>>   	status = "okay";
>>   };
> 
> Please separate the changes for pcie2x1 and pcie3 into 2 patches.

ok

>> +		pcie2x1m1_pins: pcie2x1m1-pins {
>> +			rockchip,pins =
>> +				/* pcie20_clkreqnm1 */
>> +				<2 RK_PD0 4 &pcfg_pull_none>,
>> +				/* pcie20_perstnm1 */
>> +				<3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>,
>> +				/* pcie20_wakenm1 */
>> +				<2 RK_PD1 4 &pcfg_pull_none>;
> 
> Why not pcie20m1_pins?

$ make dtbs
   DTC     arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dtb
arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts:656.32-664.5: ERROR 
(duplicate_label): /pinctrl/pcie/pcie20m1-pins: Duplicate label 
'pcie20m1_pins' on /pinctrl/pcie/pcie20m1-pins and 
/pinctrl/pcie20/pcie20m1-pins

we need gpio pin for reset.

>> +		pcie3x2m1_pins: pcie3x2m1-pins {
>> +			rockchip,pins =
>> +				/* pcie30x2_clkreqnm1 */
>> +				<2 RK_PD4 4 &pcfg_pull_none>,
>> +				/* pcie30x2_perstnm1 */
>> +				<2 RK_PD6 RK_FUNC_GPIO &pcfg_pull_none>,
>> +				/* pcie30x2_wakenm1 */
>> +				<2 RK_PD5 4 &pcfg_pull_none>;
>> +		};
> 
> Why not pcie30x2m1_pins?

$ make dtbs
   DTC     arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dtb
arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts:665.36-673.5: ERROR 
(duplicate_label): /pinctrl/pcie/pcie30x2m1-pins: Duplicate label 
'pcie30x2m1_pins' on /pinctrl/pcie/pcie30x2m1-pins and 
/pinctrl/pcie30x2/pcie30x2m1-pins

> Also missing blank line.
> 
>> +		pcie_pwren_h: pcie-pwren-h {
>> +			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
>>   		};
> 
> Thanks,
> Chukun

thank you very much for your review.

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.