Re: [PATCH v3 1/3] arm64: dts: qcom: eliza: Add PCIe PHY and controller nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Gen3 operating points in PCIe OPP tables incorrectly require 'low_svs' instead of 'nom'.
- [High] The pcie0 and pcie1 nodes are missing the msi-map property.
--

--- Patch [1]: [PATCH v3 1/3] arm64: dts: qcom: eliza: Add PCIe PHY and controller nodes ---
commit be92db70e138e71a2f5b523e4b36113ca9171196
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Subject: arm64: dts: qcom: eliza: Add PCIe PHY and controller nodes

Eliza supports two PCIe instances: one 8GT/s x1 (PCIe0) and one 8GT/s x2
(PCIe1). This commit adds PCIe controller and PHY nodes for both instances,
and updates the GCC clock references to use the newly added PHY nodes.

> diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
> index 357e8066c1768..41a1203117681 100644
> --- a/arch/arm64/boot/dts/qcom/eliza.dtsi
> +++ b/arch/arm64/boot/dts/qcom/eliza.dtsi
[ ... ]
> +		pcie0: pcie@1c00000 {
[ ... ]
> +			iommu-map = <0 &apps_smmu 0x1480 0x1>,
> +				    <0x100 &apps_smmu 0x1481 0x1>;
> +
> +			interrupt-map = <0 0 0 1 &intc 0 0 0 564 IRQ_TYPE_LEVEL_HIGH>,

[Severity: High]
Is the msi-map property missing from the pcie0 node?

Without an msi-map property, the PCIe Requester ID cannot be translated to the
GIC ITS Device ID. Does this break MSI/MSI-X delivery for connected PCIe
endpoints?

> +					<0 0 0 2 &intc 0 0 0 565 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 3 &intc 0 0 0 566 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 4 &intc 0 0 0 567 IRQ_TYPE_LEVEL_HIGH>;
[ ... ]
> +			pcie0_opp_table: opp-table {
[ ... ]
> +				/* 8 GT/s x1 */
> +				opp-8000000-3 {
> +					opp-hz = /bits/ 64 <8000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;

[Severity: High]
Are the voltage requirements correct for the 8 GT/s operating point?

Voting for a lower voltage (low_svs) instead of the nominal voltage (nom) for
Gen3 speeds might result in PCIe link instability or negotiation failures due
to insufficient PHY power. Should this require rpmhpd_opp_nom instead?

> +					opp-peak-kBps = <984500 1>;
> +					opp-level = <3>;
> +				};
> +
> +			};
[ ... ]
> +		pcie1: pcie@1c08000 {
[ ... ]
> +			iommu-map = <0 &apps_smmu 0x1400 0x1>,
> +				    <0x100 &apps_smmu 0x1401 0x1>;
> +
> +			interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>,

[Severity: High]
Is the msi-map property missing here in the pcie1 node as well?

> +					<0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 3 &intc 0 0 0 151 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 4 &intc 0 0 0 152 IRQ_TYPE_LEVEL_HIGH>;
[ ... ]
> +			pcie1_opp_table: opp-table {
[ ... ]
> +				/* 8 GT/s x1 */
> +				opp-8000000-3 {
> +					opp-hz = /bits/ 64 <8000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;

[Severity: High]
Should this Gen3 8 GT/s OPP also vote for the nominal voltage (nom) instead of
low_svs to maintain signal integrity?

> +					opp-peak-kBps = <984500 1>;
> +					opp-level = <3>;
> +				};
> +
> +				/* 8 GT/s x2 */
> +				opp-16000000-3 {
> +					opp-hz = /bits/ 64 <16000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;

[Severity: High]
Does this 8 GT/s x2 OPP need to require rpmhpd_opp_nom similar to the other
Gen3 operating points?

> +					opp-peak-kBps = <1969000 1>;
> +					opp-level = <3>;
> +				};
> +
> +			};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-eliza_evk-v3-0-92b10038f13d@oss.qualcomm.com?part=1