Re: [PATCH v2 28/37] arm64: dts: qcom: sdm845: 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 to 'Move phys and phy-names' to the port node, but the `phy-names` property is entirely dropped from the device tree.
--

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

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

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

> 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 and pcie1_port0, adding labels to these nodes to allow

[Severity: Medium]
The commit message explicitly mentions moving phy-names to the port nodes,
but looking at the device tree changes below, this property is completely
dropped. Was the intention to keep phy-names in the port nodes as described
here?

> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 4ae8627d6dbc3..efaa71a1d13ca 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -2394,12 +2394,9 @@ pcie0: pcie@1c00000 {

[ ... ]

> -			phys = <&pcie0_phy>;
> -			phy-names = "pciephy";
> -
>  			status = "disabled";
>  
> -			pcie@0 {
> +			pcie0_port0: pcie@0 {

[ ... ]

>  				#address-cells = <3>;
>  				#size-cells = <2>;
>  				ranges;
> +
> +				phys = <&pcie0_phy>;
>  			};

[Severity: Medium]
Is this missing the phy-names = "pciephy"; property that was removed from
the controller node above?

[ ... ]

> @@ -2524,12 +2523,9 @@ pcie1: pcie@1c08000 {

[ ... ]

> -			phys = <&pcie1_phy>;
> -			phy-names = "pciephy";
> -
>  			status = "disabled";
>  
> -			pcie@0 {
> +			pcie1_port0: pcie@0 {

[ ... ]

>  				#address-cells = <3>;
>  				#size-cells = <2>;
>  				ranges;
> +
> +				phys = <&pcie1_phy>;
>  			};

[Severity: Medium]
Similarly here, the phy-names property is removed from the controller node
but not added to the pcie1_port0 node as the commit message describes. Does
this node also need the phy-names property?

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