Re: [PATCH 26/34] arm64: dts: qcom: sm8150: 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 explicitly claims that `phy-names` is moved to the root port nodes, but it is only removed from the controller nodes and never added to the new locations.
--

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

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

This commit moves the PCIe PHY and perst/wake GPIO properties from the root
complex node to the root port node for sm8150. It also renames perst-gpios
to reset-gpios to match root port bindings.

> Move phys, phy-names, perst-gpios, and wake-gpios from the controller to
> pcie0_port0 and pcie1_port0, adding labels to these nodes to allow board-level
> overrides, and renaming 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 moved to the pcie0_port0 and
pcie1_port0 nodes, but the property appears to only be removed from the
parent nodes and never added back. Could this omission cause a regression
during initialization if the driver or PHY framework strictly expects this
property to be present?

> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index 74df5cd86f04f..d75f714632872 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi

[ ... ]

> @@ -1901,22 +1901,20 @@ pcie0: pcie@1c00000 {
>  
>  			power-domains = <&gcc PCIE_0_GDSC>;
>  
> -			phys = <&pcie0_phy>;
> -			phy-names = "pciephy";
> -
> -			perst-gpios = <&tlmm 35 GPIO_ACTIVE_HIGH>;
> -			wake-gpios = <&tlmm 37 GPIO_ACTIVE_LOW>;
> -

[ ... ]

> -			pcie@0 {
> +			pcie0_port0: pcie@0 {
>  				device_type = "pci";
>  				reg = <0x0 0x0 0x0 0x0 0x0>;
>  				bus-range = <0x01 0xff>;
>  
> +				phys = <&pcie0_phy>;
> +				reset-gpios = <&tlmm 35 GPIO_ACTIVE_HIGH>;
> +				wake-gpios = <&tlmm 37 GPIO_ACTIVE_LOW>;
> +

[Severity: Medium]
Should phy-names = "pciephy" be added here for pcie0_port0, as indicated by
the commit message?

>  				#address-cells = <3>;
>  				#size-cells = <2>;
>  				ranges;

[ ... ]

> @@ -2019,10 +2017,6 @@ pcie1: pcie@1c08000 {
>  
>  			power-domains = <&gcc PCIE_1_GDSC>;
>  
> -			phys = <&pcie1_phy>;
> -			phy-names = "pciephy";
> -
> -			perst-gpios = <&tlmm 102 GPIO_ACTIVE_HIGH>;
>  			enable-gpio = <&tlmm 104 GPIO_ACTIVE_HIGH>;

[ ... ]

> -			pcie@0 {
> +			pcie1_port0: pcie@0 {
>  				device_type = "pci";
>  				reg = <0x0 0x0 0x0 0x0 0x0>;
>  				bus-range = <0x01 0xff>;
>  
> +				phys = <&pcie1_phy>;
> +				reset-gpios = <&tlmm 102 GPIO_ACTIVE_HIGH>;
> +

[Severity: Medium]
Similarly for pcie1_port0, was the phy-names property accidentally dropped
during the move?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-root_port-v1-0-90a252ddbbfa@oss.qualcomm.com?part=26