Re: [PATCH PRELIMINARY 2/5] arm64: dts: qcom: shikra: add ethernet nodes
From: sashiko-bot@kernel.org
Date: 2026-09-09 06:24:26
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] The 'qcom,shikra-ethqos' compatible string is undocumented in DT bindings, lacks a generic fallback, and has no driver support, rendering the nodes dead code. - [Medium] The ethernet nodes define 6 clocks including 'axi-noc' and 'pcie-tile-axi-noc', violating the 4-clock limit in bindings and introducing a copy-paste error. - [Low] Incorrect interconnect tag macro `QCOM_ICC_TAG_ALWAYS` used for RPM-based interconnects. -- commit ab9a1a40b65e83ff44f2ceb6b0d365afac0a9e34 Author: Mohd Ayaan Anwar [off-list ref] arm64: dts: qcom: shikra: add ethernet nodes Add the two Gigabit Ethernet controllers present on Shikra (ethernet0 at 0x5d00000, ethernet1 at 0x5d20000). Both nodes are left disabled; board files supply the PHY details.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi index c1f06f692f664..a55e20be83fe2 100644 --- a/arch/arm64/boot/dts/qcom/shikra.dtsi +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1977,6 +2027,222 @@ &clk_virt SLAVE_QUP_CORE_0 RPM_ALWAYS_TAG>, }; }; + ethernet0: ethernet@5d00000 { + compatible = "qcom,shikra-ethqos";
[Severity: High] Does this compatible string exist in the dt-bindings and does it have driver support? Without a standard fallback string like "qcom,sc8280xp-ethqos" and no driver support in dwmac-qcom-ethqos.c, won't these nodes fail to probe and remain completely uninitialized when board files try to enable them?
+ reg = <0x0 0x05d00000 0x0 0x10000>, + <0x0 0x05d16000 0x0 0x100>; + reg-names = "stmmaceth", "rgmii"; + + interrupts = <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH 0>; + interrupt-names = "macirq"; + + clocks = <&gcc GCC_EMAC0_AXI_CLK>, + <&gcc GCC_EMAC0_AHB_CLK>, + <&gcc GCC_EMAC0_PTP_CLK>, + <&gcc GCC_EMAC0_RGMII_CLK>, + <&gcc GCC_EMAC0_AXI_SYS_NOC_CLK>, + <&gcc GCC_PCIE_TILE_AXI_SYS_NOC_CLK>; + clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii", + "axi-noc", "pcie-tile-axi-noc";
[Severity: Medium] Does the qcom,ethqos.yaml binding allow 6 clocks? The binding strictly limits the clocks array to 4 items. Additionally, does it make sense to assign a PCIe clock to the Ethernet MAC? NOC clock scaling is usually handled by the interconnect framework via the interconnects property, so this might be a copy-paste error that breaks static validation.
+ + power-domains = <&gcc GCC_EMAC0_GDSC>; + resets = <&gcc GCC_EMAC0_BCR>; + iommus = <&apps_smmu 0x0380 0x0007>; + + interconnects = <&mem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ALWAYS + &config_noc SLAVE_EMAC0_CFG QCOM_ICC_TAG_ALWAYS>,
[Severity: Low] Is QCOM_ICC_TAG_ALWAYS the correct tag macro here? The Shikra SoC uses an RPM-based interconnect provider, which expects tags from <dt-bindings/interconnect/qcom,rpm-icc.h> such as RPM_ALWAYS_TAG. QCOM_ICC_TAG_ALWAYS is an RPMh macro defined in <dt-bindings/interconnect/qcom,icc.h>. [ ... ]
+ ethernet1: ethernet@5d20000 {
+ compatible = "qcom,shikra-ethqos";[Severity: High] Does this compatible string exist in the dt-bindings and does it have driver support? Without a standard fallback string like "qcom,sc8280xp-ethqos" and no driver support in dwmac-qcom-ethqos.c, won't these nodes fail to probe and remain completely uninitialized when board files try to enable them?
+ reg = <0x0 0x05d20000 0x0 0x10000>, + <0x0 0x05d36000 0x0 0x100>; + reg-names = "stmmaceth", "rgmii"; + + interrupts = <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH 0>; + interrupt-names = "macirq"; + + clocks = <&gcc GCC_EMAC1_AXI_CLK>, + <&gcc GCC_EMAC1_AHB_CLK>, + <&gcc GCC_EMAC1_PTP_CLK>, + <&gcc GCC_EMAC1_RGMII_CLK>, + <&gcc GCC_EMAC1_AXI_SYS_NOC_CLK>, + <&gcc GCC_PCIE_TILE_AXI_SYS_NOC_CLK>; + clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii", + "axi-noc", "pcie-tile-axi-noc";
[Severity: Medium] Does the qcom,ethqos.yaml binding allow 6 clocks? The binding strictly limits the clocks array to 4 items. Additionally, does it make sense to assign a PCIe clock to the Ethernet MAC? NOC clock scaling is usually handled by the interconnect framework via the interconnects property, so this might be a copy-paste error that breaks static validation.
+ + power-domains = <&gcc GCC_EMAC1_GDSC>; + resets = <&gcc GCC_EMAC1_BCR>; + iommus = <&apps_smmu 0x03a0 0x0007>; + + interconnects = <&mem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ALWAYS + &config_noc SLAVE_EMAC1_CFG QCOM_ICC_TAG_ALWAYS>,
[Severity: Low] Is QCOM_ICC_TAG_ALWAYS the correct tag macro here? The Shikra SoC uses an RPM-based interconnect provider, which expects tags from <dt-bindings/interconnect/qcom,rpm-icc.h> such as RPM_ALWAYS_TAG. QCOM_ICC_TAG_ALWAYS is an RPMh macro defined in <dt-bindings/interconnect/qcom,icc.h>. [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260908-shikra_ethernet_dts-v1-0-69c0c5c7c124@oss.qualcomm.com?part=2