Re: [PATCH v2 36/37] arm64: dts: qcom: kodiak: Move PCIe phy and GPIOs to root port node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Accidental reversion of PCIe wake GPIO polarity and undocumented property addition during relocation.
--

commit cfdfc7bf6f1000a6c06f30a3d29926ca3f427856
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

arm64: dts: qcom: kodiak: Move PCIe phy and GPIOs to root port node

This commit moves PCIe phy references and the perst/wake GPIO properties
from the PCIe RC controller nodes to the root port nodes across several
Qualcomm devicetrees, renaming perst-gpios to reset-gpios.

[ ... ]

> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts b/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts
> index b1ad1d7c346a2..92023aac967d7 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-particle-tachyon.dts

[ ... ]

> @@ -562,8 +564,6 @@ &pcie0_phy {
>  };
>  
>  &pcie1 {
> -	perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> -
>  	pinctrl-0 = <&pcie1_reset_n>, <&pcie1_wake_n>, <&pcie1_clkreq_n>;
>  	pinctrl-names = "default";
>  
> @@ -572,6 +572,11 @@ &pcie1 {
>  	status = "okay";
>  };
>  
> +&pcie1_port0 {
> +	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_LOW>;

[Severity: Medium]
Was this new wake-gpios property intended here? The original pcie1 node did
not define wake-gpios before the relocation.

> +};
> +
>  &pcie1_phy {

[ ... ]

> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts b/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
> index 3a9fbef89aff4..d0639eea398ee 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
> @@ -522,15 +522,17 @@ &lpass_va_macro {
>  };
>  
>  &pcie0 {
> -	perst-gpios = <&tlmm 87 GPIO_ACTIVE_LOW>;
> -	wake-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
> -
>  	pinctrl-0 = <&pcie0_clkreq_n>, <&pcie0_reset_n>, <&pcie0_wake_n>;
>  	pinctrl-names = "default";
>  
>  	status = "okay";
>  };
>  
> +&pcie0_port {
> +	reset-gpios = <&tlmm 87 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;

[Severity: Medium]
Does this accidentally revert the wake-gpios polarity? The removed property
specified GPIO_ACTIVE_LOW, but this new node specifies GPIO_ACTIVE_HIGH.

> +};
> +
>  &pcie0_phy {

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-wake-v2-0-2744251b1181@oss.qualcomm.com?part=36