Re: [PATCH 2/4] arm64: dts: qcom: Introduce Shikra SoC base dtsi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Invalid PPI interrupt numbers (1, 2, 3, 0) are specified for the ARMv8 generic timer instead of the architecturally required numbers (13, 14, 11, 10).
- [Medium] The `sdhc_1` node incorrectly uses RPMh interconnect tags (`QCOM_ICC_TAG_ALWAYS`, `QCOM_ICC_TAG_ACTIVE_ONLY`) on an RPM-based interconnect topology.
- [Medium] The USB SuperSpeed wakeup interrupt (`ss_phy_irq`) directly references the GIC instead of routing through the MPM, breaking system wake-up from deep sleep.
- [Low] The `sdhc_1` node uses the undeclared macro `RPMHPD_CX` for its `power-domains` property, which will cause a Device Tree compiler syntax error.
--

commit e2ee94ef3a5741f2cb9aae14dbef52c5f4a760f1
Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
Subject: arm64: dts: qcom: Introduce Shikra SoC base dtsi

This commit adds initial device tree support for the Qualcomm Shikra SoC, an
IoT-focused platform featuring a Cortex-A55 and Cortex-A78C cluster. It
configures RPM-based power and clock management alongside various peripherals
like USB, eMMC, and the generic timer.

> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
> new file mode 100644
> index 0000000000000..262c488add1ec
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi

[ ... ]

> +		sdhc_1: mmc@4744000 {
> +			compatible = "qcom,shikra-sdhci", "qcom,sdhci-msm-v5";
> +
> +			reg = <0x0 0x04744000 0x0 0x1000>,
> +			      <0x0 0x04745000 0x0 0x1000>;

[ ... ]

> +			interconnects = <&system_noc MASTER_SDCC_1 QCOM_ICC_TAG_ALWAYS
> +					&mc_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>,
> +					<&mem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY
> +					&config_noc SLAVE_SDCC_1 QCOM_ICC_TAG_ACTIVE_ONLY>;

Since the Shikra SoC utilizes an RPM-based architecture, should this node use
interconnect tags from qcom,rpm-icc.h (such as RPM_ALWAYS_TAG) instead? 
Passing RPMh interconnect tags from qcom,icc.h to an RPM interconnect provider
might result in unhandled parameters or silent bandwidth request failures.

> +			interconnect-names = "sdhc-ddr",
> +					     "cpu-sdhc";
> +
> +			power-domains = <&rpmpd RPMHPD_CX>;

Does this power-domains assignment use an undeclared macro? RPMHPD_CX belongs
to RPMh-based power domains, while this platform relies on RPMPD. This looks
like it will cause a dtc syntax error during the build.

[ ... ]

> +		usb_1: usb@4e00000 {
> +			compatible = "qcom,shikra-dwc3", "qcom,snps-dwc3";
> +			reg = <0x0 0x04e00000 0x0 0xfc100>;

[ ... ]

> +			interrupts-extended = <&intc GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&intc GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&intc GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&intc GIC_SPI 254 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&intc GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "dwc_usb3",
> +					  "pwr_event",
> +					  "qusb2_phy",
> +					  "hs_phy_irq",
> +					  "ss_phy_irq";

The mpm node correctly maps pin 12 to GIC SPI 422 for the DWC3 ss_phy_irq.
Could routing this directly to the GIC here instead of hierarchically through
the mpm (e.g., <&mpm 12 IRQ_TYPE_LEVEL_HIGH>) prevent the hardware from
waking up from low-power states upon USB activity?

[ ... ]

> +	timer {
> +		compatible = "arm,armv8-timer";
> +
> +		interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 2 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 3 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
> +	};

Are these PPI interrupt numbers correct for the ARMv8 generic timer? The
architecturally defined numbers are typically 13 for Secure, 14 for Non-secure,
11 for Virtual, and 10 for Hypervisor. Using invalid hardware IRQ lines here
could prevent the kernel from receiving scheduler ticks and stall the boot.

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-shikra-dt-v1-0-716438330dd0@oss.qualcomm.com?part=2