[PATCH] cmd: ums: Switch HW partition before block access

[Christoph Niedermaier]

From: Marek Vasut <marex at nabladev.com>
Sent: Friday, April 24, 2026 5:34 AM
> An UMS session with eMMC device specifier "ums C mmc dev.part1,dev.part2"
> exposes the same eMMC HW partition 'part2' twice instead of exposing both
> HW partitions 'part1' and 'part2'. Fix this by switching the eMMC HW
> partition before block device read/write access.
> 
> An eMMC is represented by a single struct blk_desc, with the currently
> selected HW partition being stored in this struct blk_desc. Each call to
> part_get_info_by_dev_and_name_or_num() with partition string dev[.partN]
> does trigger HW partition switch by calling blk_get_device_part_str() ->
> blk_get_device_part_str() -> get_dev_hwpart() -> get_dev_hwpart() ->
> blk_dselect_hwpart(). The ums_init() iterates over the device specifier
> string and calls part_get_info_by_dev_and_name_or_num() in a loop for
> each dev[.partN] entry used as the partition string. If the device
> specifier string contains more than one dev[.partN] partition strings
> for the same dev device, then it is the HW partition described in the
> last dev[.partN] partition string entry that is accessed for all dev
> device partition strings in the device specifier string, because that
> last dev[.partN] partition string entry was the last one that triggered
> blk_dselect_hwpart() call for that device.
> 
> To access the expected HW partition for every dev[.partN] partition string
> entry, it is necessary to call blk_dselect_hwpart() before each block read
> or write. Store HW partition described for each dev[.partN] partition string
> in struct ums and use the stored value to make it so.
> 
> The blk_dselect_hwpart() does test whether the currently selected HW
> partition is already configured in hardware and does not reconfigure
> the hardware if that is the case, therefore for the majority of block
> reads and writes, blk_dselect_hwpart() is a no-op with negligible
> performance impact.
> 
> Example reproducer is listed below. The last sector of both eMMC HW BOOT
> partitions is populated with distinct test pattern and UMS is launched:
> 
> "
> => mmc dev 1 1 ; mmc read $loadaddr 0x1fff 1 ; md $loadaddr 4
> switch to partitions #1, OK
> mmc1(part 1) is current device
> MMC read: dev # 1, block # 8191, count 1 ... 1 blocks read: OK
> 84000000: 1234abcd 1234abcd 1234abcd 1234abcd  ..4...4...4...4.
> 
> => mmc dev 1 2 ; mmc read $loadaddr 0x1fff 1 ; md $loadaddr 4
> switch to partitions #2, OK
> mmc1(part 2) is current device
> MMC read: dev # 1, block # 8191, count 1 ... 1 blocks read: OK
> 84000000: 567890ef 567890ef 567890ef 567890ef  ..xV..xV..xV..xV
> 
> => ums 0 mmc 1.1,1.2
> UMS: LUN 0, dev mmc 1, hwpart 1, sector 0x0, count 0x2000
> UMS: LUN 1, dev mmc 1, hwpart 2, sector 0x0, count 0x2000
> "
> 
> Read of the two block devices on host without this fix produces
> identical data present in HW BOOT partition 2:
> 
> "
> $ dd if=/dev/sdX of=mmc-a.bin ; dd if=/dev/sdY of=mmc-b.bin
> $ hexdump -C mmc-a.bin | tail -n 3 | head -n 1 ; \
>   hexdump -C mmc-b.bin | tail -n 3 | head -n 1
> 003ffe00  ef 90 78 56 ef 90 78 56  ef 90 78 56 ef 90 78 56  |..xV..xV..xV..xV|
> 003ffe00  ef 90 78 56 ef 90 78 56  ef 90 78 56 ef 90 78 56  |..xV..xV..xV..xV|
> "
> 
> Read of the two block devices on host with this fix produces the
> expected distinct data from either HW BOOT partition 1 or 2:
> 
> "
> $ dd if=/dev/sdX of=mmc-a.bin ; dd if=/dev/sdY of=mmc-b.bin
> $ hexdump -C mmc-a.bin | tail -n 3 | head -n 1 ; \
>   hexdump -C mmc-b.bin | tail -n 3 | head -n 1
> 003ffe00  cd ab 34 12 cd ab 34 12  cd ab 34 12 cd ab 34 12  |..4...4...4...4.|
> 003ffe00  ef 90 78 56 ef 90 78 56  ef 90 78 56 ef 90 78 56  |..xV..xV..xV..xV|
> "
> 
> Reported-by: Christoph Niedermaier <cniedermaier at dh-electronics.com>
> Signed-off-by: Marek Vasut <marex at nabladev.com>
> ---
> Cc: Lukasz Majewski <lukma at denx.de>
> Cc: Mattijs Korpershoek <mkorpershoek at kernel.org>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: u-boot at lists.denx.de
> ---
>  cmd/usb_mass_storage.c     | 11 +++++++++++
>  include/usb_mass_storage.h |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/cmd/usb_mass_storage.c b/cmd/usb_mass_storage.c
> index 47e8b70cd10..e8b87045bdc 100644
> --- a/cmd/usb_mass_storage.c
> +++ b/cmd/usb_mass_storage.c
> @@ -24,6 +24,11 @@ static int ums_read_sector(struct ums *ums_dev,
>  {
>         struct blk_desc *block_dev = &ums_dev->block_dev;
>         lbaint_t blkstart = start + ums_dev->start_sector;
> +       int ret;
> +
> +       ret = blk_dselect_hwpart(block_dev, ums_dev->hwpart);
> +       if (ret && ret != -ENOSYS)
> +               return ret;
> 
>         return blk_dread(block_dev, blkstart, blkcnt, buf);
>  }
> @@ -33,6 +38,11 @@ static int ums_write_sector(struct ums *ums_dev,
>  {
>         struct blk_desc *block_dev = &ums_dev->block_dev;
>         lbaint_t blkstart = start + ums_dev->start_sector;
> +       int ret;
> +
> +       ret = blk_dselect_hwpart(block_dev, ums_dev->hwpart);
> +       if (ret && ret != -ENOSYS)
> +               return ret;
> 
>         return blk_dwrite(block_dev, blkstart, blkcnt, buf);
>  }
> @@ -110,6 +120,7 @@ static int ums_init(const char *devtype, const char *devnums_part_str)
>                 snprintf(name, UMS_NAME_LEN, "UMS disk %d", ums_count);
>                 ums[ums_count].name = name;
>                 ums[ums_count].block_dev = *block_dev;
> +               ums[ums_count].hwpart = block_dev->hwpart;
> 
>                 printf("UMS: LUN %d, dev %s %d, hwpart %d, sector %#x, count %#x\n",
>                        ums_count, devtype, ums[ums_count].block_dev.devnum,
> diff --git a/include/usb_mass_storage.h b/include/usb_mass_storage.h
> index 6d83d93cad7..9be704ea8b7 100644
> --- a/include/usb_mass_storage.h
> +++ b/include/usb_mass_storage.h
> @@ -22,6 +22,7 @@ struct ums {
>         unsigned int num_sectors;
>         const char *name;
>         struct blk_desc block_dev;
> +       int hwpart;
>  };
> 
>  int fsg_init(struct ums *ums_devs, int count, struct udevice *udc);

Tested-by: Christoph Niedermaier <cniedermaier at dh-electronics.com>

Thanks and regards
Christoph