Re: [PATCH 1/2] riscv: dts: spacemit: k3: add USB controller and USB phy support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The `spacemit,apmu` property for `combophy` uses an unaligned register offset (`0x11`) and is misplaced in the board file.
- [High] Missing driver for `spacemit,k3-combo-phy` causes the USB3 controller probe to defer indefinitely.
- [Medium] The `combophy` node specifies an abnormally large 6MB register space (`0x600000`), which is highly likely a typo.
- [Medium] The `combophy` node introduces undocumented custom properties and an invalid syscon reference, while lacking standard required properties.
--

--- Patch [1]: [PATCH 1/2] riscv: dts: spacemit: k3: add USB controller and USB phy support ---
commit d2902167c4edb4e87ae123e61df4d877f88a5943
Author: Inochi Amaoto <inochiama@gmail.com>

riscv: dts: spacemit: k3: add USB controller and USB phy support

This commit adds USB device nodes and combo PHY support to the Spacemit K3
device tree and the K3 Pico-ITX board.

> diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> index b89c1521e6649..2a6d35a64d5ce 100644
> --- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> +++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts

[ ... ]

> @@ -182,6 +182,11 @@ dldo7: dldo7 {
>  	};
>  };
>  
> +&combophy {
> +	spacemit,apmu = <&syscon_apmu 0x11>;

[Severity: High]
Does this offset cause unaligned access faults? The offset 0x11 (17 decimal)
is not 4-byte aligned, which can result in -EINVAL stride violations when the
underlying driver attempts regmap_read or regmap_update_bits.

Additionally, should SoC-level APMU offsets be placed in the main k3.dtsi
instead of the board-specific k3-pico-itx.dts?

> +	status = "okay";
> +};
> +

[ ... ]

> diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> index 19fc9b49668ed..82c9e2da82e9f 100644
> --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi

[ ... ]

> @@ -438,6 +439,47 @@ soc: soc {
>  		dma-noncoherent;
>  		ranges;
>  
> +		usb3d: usb@81a00000 {
> +			compatible = "spacemit,k3-dwc3";

[ ... ]

> +			reset-names = "ahb", "vcc", "phy";
> +			phys = <&usb3d_u2phy>,
> +			       <&combophy 4 PHY_TYPE_USB3>;

[Severity: High]
Is there a corresponding driver for the spacemit,k3-combo-phy compatible
string currently available in the tree?

Because the usb3d node depends on this PHY, the DWC3 core may continuously
fail to acquire the usb3-phy dependency and return -EPROBE_DEFER, leaving
the USB3 port uninitialized.

> +			phy-names = "usb2-phy", "usb3-phy";

[ ... ]

> +		combophy: phy@81d00000 {
> +			compatible = "spacemit,k3-combo-phy";
> +			reg = <0x0 0x81d00000 0x0 0x600000>;

[Severity: Medium]
Is this register map size intentional? Mapping 6MB of physical address space
for a PHY controller seems abnormally large and might be a typo.

> +			#phy-cells = <2>;
> +			spacemit,apb-spare = <&pll>;

[Severity: Medium]
Will this syscon reference work properly? The pll node being referenced is a
clock controller that lacks the syscon compatible string, meaning standard
syscon_regmap_lookup_by_phandle operations could fail.

Are the standard clocks and resets properties missing from this node as well?

> +			status = "disabled";
> +		};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709040415.977784-1-inochiama@gmail.com?part=1