Re: [PATCH 2/3] riscv: dts: spacemit: k3-deepcomputing-fml13v05: 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] dt: incorrect clock assignment for USB3 port B and C companion PHYs
- [High] dt: incorrect hardware interrupt number assigned to usb3b controller
- [Low] dt: missing Type-C and PD bindings for claimed port features
- [Low] dt: space indentation used instead of tabs for usb3c node
--

commit 01e320732a69960438751a5d51ee72cfcf2179bc
Author: Sandie Cao <sandie.cao@deepcomputing.io>

riscv: dts: spacemit: k3-deepcomputing-fml13v05: add USB controller and USB phy support

This commit adds USB device nodes to the DeepComputing FML13V05 board,
configuring four USB-C ports with various features like PD 3.0 and DP 1.4.

> diff --git a/arch/riscv/boot/dts/spacemit/k3-deepcomputing-fml13v05.dts b/arch/riscv/boot/dts/spacemit/k3-deepcomputing-fml13v05.dts
> index 804abc85602ab..55b0f63dcb2e3 100644
> --- a/arch/riscv/boot/dts/spacemit/k3-deepcomputing-fml13v05.dts
> +++ b/arch/riscv/boot/dts/spacemit/k3-deepcomputing-fml13v05.dts

[ ... ]

> +&usb3b {
> +	status = "okay";
> +};

[Severity: Low]
Are the 'usb-c-connector' nodes and OF graph linkages missing here? The
commit message claims Power Delivery and DisplayPort Alt-Mode capabilities,
but without these bindings, any kernel-managed role switching, PD negotiation,
or DisplayPort muxing might not function when a Type-C device is connected.

[ ... ]

> diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> index b9977e78e096a..ae3f8bb1e5ed1 100644
> --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi

[ ... ]

> +		usb3b: usb@81400000 {
> +			compatible = "spacemit,k3-dwc3";
> +			reg = <0x0 0x81400000 0x0 0x10000>;
> +			interrupts = <125 IRQ_TYPE_LEVEL_HIGH>;
                                      ^^^

[Severity: High]
Is this the correct hardware interrupt number for usb3b? Since the adjacent
controllers usb3c and usb3d use 148 and 149, 125 appears to be a copy-paste
error (perhaps it should be 147?). Using the wrong interrupt vector could
prevent the kernel from processing USB events on Port B, causing it to fail on
boot.

> +			interrupt-parent = <&saplic>;
> +			clocks = <&syscon_apmu CLK_APMU_USB3_PORTB_BUS>;

[ ... ]

> +		usb3b_u2phy: phy@81500000 {
> +			compatible = "spacemit,k3-usb2-phy";
> +			reg = <0x0 0x81500000 0x0 0x200>;
> +			clocks = <&syscon_apmu CLK_APMU_USB2_BUS>;
                                                ^^^^^^^^^^^^^^^^^

[Severity: High]
Should this use the port-specific clock (CLK_APMU_USB3_PORTB_BUS) instead of
the shared USB2 bus clock? If the independent USB2 host controller is disabled
or runtime gated, the USB3 ports could lose their companion PHY clocks,
leading to initialization failures or USB disconnects during power management.

> +			#phy-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +               usb3c: usb@81700000 {
  ^^^^^^^^^^^^^^^

[Severity: Low]
Does this node use space indentation? It looks like 15 spaces were used
instead of tabs.

> +			compatible = "spacemit,k3-dwc3";
> +			reg = <0x0 0x81700000 0x0 0x10000>;

[ ... ]

> +		usb3c_u2phy: phy@81800000 {
> +			compatible = "spacemit,k3-usb2-phy";
> +			reg = <0x0 0x81800000 0x0 0x200>;
> +			clocks = <&syscon_apmu CLK_APMU_USB2_BUS>;
                                                ^^^^^^^^^^^^^^^^^

[Severity: High]
Should this also use its port-specific clock (CLK_APMU_USB3_PORTC_BUS) instead
of the shared USB2 bus clock? Similar to usb3b_u2phy above, relying on the
standalone USB2 bus clock could cause power management issues for this PHY.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922110231.3426011-1-sandie.cao@deepcomputing.io?part=2