Re: [PATCH v5 0/1] riscv: dts: starfive: jh7110-milkv-mars: enable usb0 host function

[E Shattow <e@freeshell.de>]

Hi Emil, thanks for taking time to review!

Added CC: Minda Chen, Hal Feng

Please Minda and Hal can you follow-up on Emil's comments as well?

On 11/27/24 03:00, Emil Renner Berthing wrote:
> E Shattow wrote:
>> Enable host mode JH7110 on-chip USB for Milk-V Mars by setting host mode
>> and connect vbus pinctrl.
>>
>> This functionality depends on setting the USB over-current register to
>> disable at bootloader phase, for example U-Boot:
>> https://patchwork.ozlabs.org/project/uboot/patch/20241012031328.4268-6-minda.chen@starfivetech.com/
> Hi E,
>
> Ideally the JH7110 pinctrl driver would be updated, so Linux can do this itself
> and doesn't need to rely on u-boot doing it. I already asked for this here:
>
> https://lore.kernel.org/all/CAJM55Z-+Cxdebcn4MLXfQdOVhx4c2SQ+zMH8cjn-Yq35xO8g0A@mail.gmail.com/

Yes, I agree, and Linux is not the only consumer of devicetree. I would 
like USB function to work for users of Linux and U-Boot on these boards 
by copying what the vendor Board Support Package does what is shipped 
with the products. If it is more in-depth than this I will defer to Hal 
or Minda.


For some wider context, upstream U-Boot is about to adopt the 
dt-rebasing via Hal's OF_UPSTREAM for JH7110 boards series and then also 
there is a patch set from Minda Chen to add the on-chip JH7110 USB 
support to U-Boot, and so then and there it will depend on these dts 
changes. If you have Milk-V Mars then already there are three of four 
USB-A receptacle ports which are functional on PCIe-connected VL805 USB 
chipset.

>
>> If the over-current register is not prepared for us then the result is no
>> change in functional outcome with this patch applied; there is an error
>> visible to the user and this additional usb configuration fails (same as
>> it is now). On Milk-V Mars with four USB-A ports this applies to one of the
>> ports and the remaining three VL805-connected ports via PCIe are not changed.
> Thanks for the patches. I don't quite understand when you write "no change in
> functional outcome with this patch applied". The USB-C port is already
> configured as a peripheral, and I just tried setting up an ethernet gadget on
> my VF2 running 6.12 and that works quite well. Does it not work on the Milk-V
> Mars board? If it does then these patches would break that functionality.
>
> Here is the script I used for that:
> https://paste.c-net.org/BravoLonely
>
> At the very least you'll need to explain in the commit message itself why
> changing the USB-C port from peripheral mode to host mode is OK. But ideally
> maybe you could make it work in OTG mode, so userspace can choose how they want
> to use the port. The same goes for the PINE64 board too.
>
> /Emil

USB-C port on Mars is not wired for data here, that is only true for 
VisionFive2. If the user wants to use their USB-A receptacle as OTG port 
I will not object to a future improvement, but here we want the basic 
expectations of users covered that they should have four working USB-A 
receptacle ports in U-Boot (and possibly in Linux, depending on the 
overcurrent register wherever it is set). This is what I am meaning, 
there may be somebody using a male-male USB-A USB-A cable for OTG but 
more likely is that people just want to plug in USB peripherals to host 
ports and use their mouse / keyboard / flash memory, I think.


There is no USB-C port on Star64.

>
>> Changes since v4:
>>   - Rebase on latest master
>>
>> Changes since v3:
>>   - Rebase on linux-next/master
>>   - use tabs for code indent
>>
>> Changes since v2:
>>   - Rebase on 6.12
>>
>> E Shattow (1):
>>    riscv: dts: starfive: jh7110-milkv-mars: enable usb0 host function
>>
>>   .../boot/dts/starfive/jh7110-milkv-mars.dts    | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> --
>> 2.45.2
>>
Thanks again Emil.  -E