Re: [PATCH] cmd: ums: Set Device Descriptor iSerial based on serial# env variable

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Marek,

On 6/13/26 4:54 AM, Marek Vasut wrote:
> The DFU code currently sets the content of "serial#" environment
> variable into Device Descriptor iSerial field. This is useful when
> there are multiple devices connected to the same host PC and they
> have to be told apart. Replicate the same behavior in the UMS code.
> 
> Example without serial# set, which is the original behavior:
> "
> u-boot=> ums 0 mmc 1
> UMS: LUN 0, dev mmc 1, hwpart 0, sector 0x0, count 0x1d5a000
> 
> $ dmesg -w
> ...
> usb 5-2.3.7: New USB device found, idVendor=0525, idProduct=a4a5, bcdDevice=7e.a7
> usb 5-2.3.7: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 5-2.3.7: Product: USB download gadget
> usb 5-2.3.7: Manufacturer: Vendor
>                              <------------------------------- NOT HERE
> usb-storage 5-2.3.7:1.0: USB Mass Storage device detected
> ...
> 
> $ lsusb -vd 0525:a4a5
> 
> Bus 005 Device 051: ID 0525:a4a5 Netchip Technology, Inc. Linux-USB File-backed Storage Gadget
> Negotiated speed: High Speed (480Mbps)
> Device Descriptor:
>    bLength                18
>    bDescriptorType         1
>    bcdUSB               2.00
>    bDeviceClass            0 [unknown]
>    bDeviceSubClass         0 [unknown]
>    bDeviceProtocol         0
>    bMaxPacketSize0        64
>    idVendor           0x0525 Netchip Technology, Inc.
>    idProduct          0xa4a5 Linux-USB File-backed Storage Gadget
>    bcdDevice           7e.a7
>    iManufacturer           1 Vendor
>    iProduct                2 USB download gadget
>    iSerial                 0 <------------------------------- NOT HERE
> ...
> "
> 
> Example with serial# set to 1234abcd, which is the optional new behavior:
> "
> u-boot=> setenv serial# 1234abcd <------------------------------ HERE
> u-boot=> ums 0 mmc 1
> UMS: LUN 0, dev mmc 1, hwpart 0, sector 0x0, count 0x1d5a000
> 
> $ dmesg -w
> ...
> usb 5-2.3.7: New USB device found, idVendor=0525, idProduct=a4a5, bcdDevice=7e.a7
> usb 5-2.3.7: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 5-2.3.7: Product: USB download gadget
> usb 5-2.3.7: Manufacturer: Vendor
> usb 5-2.3.7: SerialNumber: 1234abcd <--------------------------- HERE
> usb-storage 5-2.3.7:1.0: USB Mass Storage device detected
> ...
> 
> $ lsusb -vd 0525:a4a5
> 
> Bus 005 Device 052: ID 0525:a4a5 Netchip Technology, Inc. Linux-USB File-backed Storage Gadget
> Negotiated speed: High Speed (480Mbps)
> Device Descriptor:
>    bLength                18
>    bDescriptorType         1
>    bcdUSB               2.00
>    bDeviceClass            0 [unknown]
>    bDeviceSubClass         0 [unknown]
>    bDeviceProtocol         0
>    bMaxPacketSize0        64
>    idVendor           0x0525 Netchip Technology, Inc.
>    idProduct          0xa4a5 Linux-USB File-backed Storage Gadget
>    bcdDevice           7e.a7
>    iManufacturer           1 Vendor
>    iProduct                2 USB download gadget
>    iSerial                 3 1234abcd <-------------------------- HERE
> ...
> "
> 
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: Lukasz Majewski <lukma@nabladev.com>
> Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
>   cmd/usb_mass_storage.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/cmd/usb_mass_storage.c b/cmd/usb_mass_storage.c
> index e8b87045bdc..335258d7a56 100644
> --- a/cmd/usb_mass_storage.c
> +++ b/cmd/usb_mass_storage.c
> @@ -9,6 +9,7 @@
>   #include <blk.h>
>   #include <command.h>
>   #include <console.h>
> +#include <env.h>
>   #include <errno.h>
>   #include <g_dnl.h>
>   #include <malloc.h>
> @@ -65,7 +66,7 @@ static void ums_fini(void)
>   
>   static int ums_init(const char *devtype, const char *devnums_part_str)
>   {
> -	char *s, *t, *devnum_part_str, *name;
> +	char *s, *t, *devnum_part_str, *name, *sn;

Reduce the scope of the variable by declaring this variable in the 
if-block? I find it easier follow code this way. Additionally, I'm 
assuming we could get a "variable defined but never used" kinda warning 
if ENV_SUPPORT is not selected?

I don't know anything about USB, but I'm a bit puzzled by the choice of 
location, as the DFU, fastboot and rockusb USB gadget drivers are 
setting this in their bind function... shouldn't this be added in 
drivers/usb/gadget/f_mass_storage.c:fsg_bind() instead to match? I'm 
thinking, if someone programmatically configures the UMS gadget driver 
without going through the CLI, they probably would be missing this as well?

Cheers,
Quentin