Re: [PATCH 24/34] arm64: dts: qcom: sc8180x: 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 both `phys` and `phy-names` to the root port nodes, but `phy-names` is deleted entirely instead of being moved.
--

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

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

This commit moves PCIe phy references and GPIO properties from the RC
controller node to the root port node. It drops phys and phy-names
from the controller node and adds phys to the root port node.

> arm64: dts: qcom: sc8180x: Move PCIe phy and GPIOs to root port node
>
> 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 and phy-names from the controller to
> pcie0_port0, pcie1_port0, pcie2_port0, and pcie3_port0, adding

[Severity: Medium]
The commit message claims to move both phys and phy-names to the root port
nodes, but phy-names is deleted entirely instead of being moved.

While dropping phy-names is technically correct and safe (the
qcom,pcie-common.yaml DT binding marks phy-names as deprecated and
explicitly states it is not required in the root port node), the commit message
is factually inaccurate.

Could the commit message be updated to document the intentional removal of the
deprecated property rather than claiming it was moved?

> diff --git a/arch/arm64/boot/dts/qcom/sc8180x.dtsi b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
> index de1b72b64d960..a85e3211958d1 100644
> --- a/arch/arm64/boot/dts/qcom/sc8180x.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
> @@ -1779,17 +1779,17 @@ pcie0: pcie@1c00000 {
>  					<&gem_noc MASTER_AMPSS_M0 0 &config_noc SLAVE_PCIE_0 0>;
>  			interconnect-names = "pcie-mem", "cpu-pcie";
>  
> -			phys = <&pcie0_phy>;
> -			phy-names = "pciephy";
>  			dma-coherent;
>  
>  			status = "disabled";
>  
> -			pcie@0 {
> +			pcie0_port0: pcie@0 {
>  				device_type = "pci";
>  				reg = <0x0 0x0 0x0 0x0 0x0>;
>  				bus-range = <0x01 0xff>;
>  
> +				phys = <&pcie0_phy>;
> +
>  				#address-cells = <3>;
>  				#size-cells = <2>;
>  				ranges;

[Severity: Medium]
As noted above, phys is moved to the pcie0_port0 node in
arch/arm64/boot/dts/qcom/sc8180x.dtsi, but phy-names is conspicuously missing,
contradicting the commit message.

[ ... ]

> @@ -1898,17 +1898,17 @@ pcie3: pcie@1c08000 {
>  					<&gem_noc MASTER_AMPSS_M0 0 &config_noc SLAVE_PCIE_3 0>;
>  			interconnect-names = "pcie-mem", "cpu-pcie";
>  
> -			phys = <&pcie3_phy>;
> -			phy-names = "pciephy";
>  			dma-coherent;
>  
>  			status = "disabled";
>  
> -			pcie@0 {
> +			pcie3_port0: pcie@0 {
>  				device_type = "pci";
>  				reg = <0x0 0x0 0x0 0x0 0x0>;
>  				bus-range = <0x01 0xff>;
>  
> +				phys = <&pcie3_phy>;
> +
>  				#address-cells = <3>;
>  				#size-cells = <2>;
>  				ranges;

[Severity: Medium]
The same discrepancy applies to pcie3_port0, where phy-names is also omitted.

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