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

[Junzhong Pan <panjunzhong@linux.spacemit.com>]

On 9/27/2026 7:49 PM, sashiko-bot@kernel.org wrote:
> 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.
> 

I think it's fine the keep current order, same as dtsi.

> [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?
> 

Not now, the current standard role switch framework support in
chipidea driver is not completely implemented. An extra patch series is
needed to fix that, but that's another topic, after that, we then could
add the attributes. Currently the sysfs attribute works fine.

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