Re: [PATCH v6 2/4] arm64: dts: intel: agilex5: add USB3.1 controller node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `usb31_bus` node defines a 1 TiB (40-bit) `dma-ranges` window that structurally overflows its parent bus (`soc`) which only has a 32-bit address space.
- [Medium] The `phys` property points to a `usb-nop-xceiv` node, but this is rejected by the generic PHY framework and ignored by the DWC3 legacy PHY fallback, making the property dead code.
--

commit ea5388a5a6d3ab6b2ff68927da6ac8bb6e1fe941
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

arm64: dts: intel: agilex5: add USB3.1 controller node

This commit adds a DTS node for the Synopsys DWC3 USB3.1 controller on the
Agilex5 SoC. It places it under a simple-bus with a 40-bit dma-ranges window
to match the interconnect's USB DMA address width. The controller is also
enabled in host mode on the SoCDK board.

> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> index f54767d1526e1..523f722a2a642 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi

[ ... ]

> @@ -493,6 +493,41 @@ usb0: usb@10b00000 {
>  			status = "disabled";
>  		};
>  
> +		usb31_bus: usb-bus@11000000 {
> +			compatible = "simple-bus";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges = <0x0 0x0 0x11000000 0x0 0x100000>;
> +			/*
> +			 * USB interconnect DMA is limited to 40-bit addresses
> +			 * (1 TiB window from 0).
> +			 */
> +			dma-ranges = <0x0 0x0 0x0 0x100 0x0>;

[Severity: High]
Does this 40-bit dma-ranges window structurally overflow its parent bus?

The usb31_bus node is defined as a child of the soc node, which only has
a 32-bit address space (#address-cells = <1>). Translating a 1 TiB DMA window
into a 32-bit parent bus mathematically overflows the parent's addressability.

The kernel's OF DMA parser evaluates this window against the parent's 32-bit
addressability and truncates DMA addresses to the parent bus size. Any DMA
address beyond 4 GB cannot be represented or routed correctly.

This breaks 64-bit DMA for the USB controller, which can cause memory
corruption or IOMMU faults if system memory > 4GB is used for DMA buffers.

> +
> +			usb31: usb@0 {
> +				compatible = "altr,agilex5-dwc3";
> +				reg = <0x0 0x0 0x0 0x100000>;
> +				interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> +				/*
> +				 * ref and suspend share the same hardware
> +				 * source; reuse the published suspend clock ID.
> +				 */
> +				clocks = <&clkmgr AGILEX5_USB31_BUS_CLK_EARLY>,
> +					 <&clkmgr AGILEX5_USB31_SUSPEND_CLK>,
> +					 <&clkmgr AGILEX5_USB31_SUSPEND_CLK>;
> +				clock-names = "bus_early", "ref", "suspend";
> +				resets = <&rst USB1_RESET>, <&rst USB1_OCP_RESET>;
> +				reset-names = "core", "ecc";
> +				iommus = <&smmu 7>;
> +				phys = <&usbphy0>, <&usbphy0>;
> +				phy-names = "usb2-phy", "usb3-phy";

[Severity: Medium]
Will this phys property actually be parsed, or is it treated as dead code?

The usbphy0 node has compatible = "usb-nop-xceiv". The generic PHY subsystem
in drivers/phy/phy-core.c explicitly rejects usb-nop-xceiv and returns -ENODEV.
The DWC3 core driver dwc3_core_get_phy() handles this error by falling back to
the legacy USB PHY subsystem.

However, the legacy subsystem explicitly expects the legacy property name
usb-phy. Since this node uses the generic phys property name, the legacy
fallback also fails.

The DWC3 driver ignores the resulting -ENODEV error and silently proceeds
without a PHY.

> +				maximum-speed = "super-speed";
> +				snps,dis_u2_susphy_quirk;
> +				snps,dis_u3_susphy_quirk;
> +				status = "disabled";
> +			};
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788886615.git.adrian.ho.yin.ng@altera.com?part=2