Re: [PATCH v6 3/6] arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The pcie3a controller is incorrectly configured for an 8-lane PCIe link instead of a bifurcated 4-lane link.
- [Medium] The 32-bit PCI memory window in the `pcie3a` `ranges` property uses an incorrect child address, breaking identity mapping.
- [Medium] The 'pcie3a_port0' root port node is missing the 'compatible = "pciclass,0604";' property.
--

--- Patch [3]: [PATCH v6 3/6] arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY ---
commit 563e57122857b4066dc260031df9a220f41387c4
Author: Qiang Yu <qiang.yu@oss.qualcomm.com>

arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY

Glymur's PCIe3a and PCIe3b controllers share a single Gen5x8 QMP PHY block
that can be bifurcated into two independent x4 links, rather than each
controller owning its own dedicated PHY. This commit adds a pcie3_phy node
and wires up both controllers.

> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> index f32af66a3f1c8..6355499ee5e50 100644
> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi

[ ... ]

> +		pcie3a: pci@1c10000 {
> +			device_type = "pci";
> +			compatible = "qcom,glymur-pcie", "qcom,pcie-x1e80100";
> +			reg = <0x0 0x01c10000 0x0 0x3000>,
> +			      <0x0 0x70000000 0x0 0xf20>,
> +			      <0x0 0x70000f40 0x0 0xa8>,
> +			      <0x0 0x70001000 0x0 0x4000>,
> +			      <0x0 0x70100000 0x0 0x100000>,
> +			      <0x0 0x01c13000 0x0 0x1000>;
> +			reg-names = "parf",
> +				    "dbi",
> +				    "elbi",
> +				    "atu",
> +				    "config",
> +				    "mhi";
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges = <0x01000000 0x0 0x00000000 0x0 0x70200000 0x0 0x100000>,
> +				 <0x02000000 0x0 0x70000000 0x0 0x70300000 0x0 0x3d00000>,

[Severity: Medium]
Is this 32-bit PCI memory window child address correct? 

The standard for Qualcomm PCIe controllers in this device tree is to have
identical parent and child addresses for the 32-bit outbound memory window
(like pcie3b). Here it looks like cpu address 0x70300000 is mapped to pci
address 0x70000000, which appears to be copied from the dbi register base.
Could this break the identity mapping convention?

> +				 <0x03000000 0x7 0x00000000 0x7 0x00000000 0x0 0x40000000>,
> +				 <0x43000000 0x70 0x00000000 0x70 0x00000000 0x10 0x00000000>;
> +
> +			bus-range = <0 0xff>;
> +
> +			dma-coherent;
> +
> +			linux,pci-domain = <3>;
> +			num-lanes = <8>;

[Severity: High]
Should this controller be configured for 8 lanes instead of 4?

The commit message explicitly states that the single Gen5x8 PHY block
is "bifurcated into two independent x4 links", and pcie3b correctly requests
4 lanes. By requesting 8 lanes here, and further down setting eq-presets and
an OPP table for 8 lanes, won't this cause the PCIe controller to attempt an
unsupported 8-lane training on the bifurcated PHY?

> +
> +			operating-points-v2 = <&pcie3a_opp_table>;

[ ... ]

> +			pcie3a_port0: pcie@0 {
> +				device_type = "pci";
> +				reg = <0x0 0x0 0x0 0x0 0x0>;
> +				bus-range = <0x01 0xff>;
> +

[Severity: Medium]
Is this node missing the compatible = "pciclass,0604"; property?

All other PCIe root port nodes in the file (including pcie3b_port0) specify
this string. Omitting it here might break compatibility matching for generic
OF-based PCI bridge drivers.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-glymur_linkmode_0722-v6-0-bba8156b8cbb@oss.qualcomm.com?part=3

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy