Re: [PATCH v3 6/8] riscv: dts: spacemit: enable USB2.0 controllers on BananaPi F3

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] usb: missing usb-role-switch property for standard sysfs ABI
- [Low] dts: board file nodes not sorted alphabetically
--

commit 44aeea968dbe19365572cdf8df737d1b44de83c9
Author: Junzhong Pan <panjunzhong@linux.spacemit.com>

riscv: dts: spacemit: enable USB2.0 controllers on BananaPi F3

This commit enables the usb2_0 and usb2_1 nodes on the BananaPi F3 board.
It intends to allow role switching via sysfs for the Type-C port and 
support cellular modules on the Mini PCIe slot.

> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> index 58035d886bf67..ce36250e2debd 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts

[ ... ]

> @@ -383,10 +383,26 @@ &uart0 {
>  	status = "okay";
>  };
>  
> +&usbphy0 {
> +	status = "okay";
> +};
> +
> +&usbphy1 {
> +	status = "okay";
> +};
> +
>  &usbphy2 {
>  	status = "okay";
>  };
>  
> +&usb2_0 {

[Severity: Low]
Are the device tree board file nodes sorted alphabetically here?

Device tree board files mandate that node references be sorted alphabetically
by label to prevent merge conflicts. Based on ASCII sorting, 2 (0x32) comes
before p (0x70), so &usb2_0 and &usb2_1 should precede the &usbphy* nodes.

[Severity: Low]
Does this node omit the usb-role-switch property required to expose the
standard sysfs ABI?

The commit message notes that the user can switch the role in sysfs.
However, by omitting the usb-role-switch property, the driver defaults
to exposing its legacy, non-standard driver-specific sysfs attribute
(/sys/devices/platform/.../role) rather than registering with the
standard USB role switch framework (/sys/class/usb_role/).

Would it be better to include the usb-role-switch property so that
modern userspace tools can use the expected standard UAPI?

> +	status = "okay";
> +};
> +
> +&usb2_1 {
> +	status = "okay";
> +};
> +
>  &usb_dwc3 {
>  	dr_mode = "host";
>  	#address-cells = <1>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927-b4-spacemit-k1-usb2-v3-0-ce7095896039@linux.spacemit.com?part=6