Re: [PATCH v2 32/37] arm64: dts: qcom: sm8450: 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] The commit message claims to move `phy-names` to the root port node, but the property is completely removed instead.
--

commit 9d7dd05726a6daa00789e9da0c80d284e0e6db66
Author: Krishna Chaitanya Chundru <krishna.chaitanya.chundru@oss.qualcomm.com>

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

This commit moves the PCIe PHY reference and perst/wake GPIO properties
from the RC controller node to the root port nodes. It also renames
perst-gpios to reset-gpios to match the bindings.

> The PCIe phy reference and the perst/wake GPIO properties are
> per root port and belong in the root port node (pcie@0), not in the
> RC controller node. Move phys, phy-names, perst-gpios, and wake-gpios
> from the controller to the existing pcieport0 and newly labeled
> pcie1_port0, allowing board-level overrides. Rename perst-gpios to
> reset-gpios to match the binding used in the root port context.

[Severity: Medium]
The commit message states that phy-names is being moved from the controller
to the root port nodes. However, looking at the diff below, the property is
removed from the controller but does not seem to be added to the destination
nodes. Should the commit message be updated to reflect the removal, or was
the property supposed to be added to the root ports?

> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -2031,12 +2031,6 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>  
>  			power-domains = <&gcc PCIE_0_GDSC>;
>  
> -			phys = <&pcie0_phy>;
> -			phy-names = "pciephy";
> -
> -			perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>;
> -			wake-gpios = <&tlmm 96 GPIO_ACTIVE_LOW>;
> -
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&pcie0_default_state>;
>  
> @@ -2080,6 +2074,10 @@ pcieport0: pcie@0 {
>  				#address-cells = <3>;
>  				#size-cells = <2>;
>  				ranges;
> +
> +				phys = <&pcie0_phy>;
> +				reset-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>;
> +				wake-gpios = <&tlmm 96 GPIO_ACTIVE_LOW>;
>  			};
>  		};

[Severity: Medium]
Does pcieport0 need phy-names = "pciephy"; added here, as suggested by the
commit message?

[ ... ]

> @@ -2285,6 +2277,10 @@ pcie@0 {
>  				#address-cells = <3>;
>  				#size-cells = <2>;
>  				ranges;
> +
> +				phys = <&pcie1_phy>;
> +				reset-gpios = <&tlmm 97 GPIO_ACTIVE_LOW>;
> +				wake-gpios = <&tlmm 99 GPIO_ACTIVE_LOW>;
>  			};
>  		};

[Severity: Medium]
Similarly, should phy-names be added here to pcie1_port0?

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