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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <eb06c8e0-ebda-47b9-94f6-6e6eca21097d@freeshell.de>
Date: Wed, 27 Nov 2024 03:52:10 -0800
From: E Shattow <e@...eshell.de>
To: Emil Renner Berthing <emil.renner.berthing@...onical.com>,
 Jisheng Zhang <jszhang@...nel.org>
Cc: Emil Renner Berthing <kernel@...il.dk>, Conor Dooley <conor@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, minda.chen@...rfivetech.com,
 hal.feng@...rfivetech.com
Subject: Re: [PATCH v5 0/1] riscv: dts: starfive: jh7110-milkv-mars: enable
 usb0 host function

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