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

[Marek Vasut]

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);
-- 
2.53.0