Merge branch '2023-03-17-improve-read-command-add-write-command' into next

To quote the author:
The first patch simplies do_read somewhat by making use of an existing
helper instead of parsing the dev_part string manually. As a bonus
(and my actual motivation), it now understands dev#partname syntax -
hard-coded partition numbers are so last decade.

I also need the symmetrical operation, being able to write to a named
raw partition, and fortunately it doesn't require that many lines of
code to implement that.

There's a very minor change in the error reporting due to using
cmdtp->name to generate the new messages, but I don't think "Error
reading blocks" offers much that "read error" doesn't.

New in v2: the last three patches add documentation, ensure CMD_WRITE
is set for sandbox and adds some basic test cases for the various ways
of accessing the partitions (by number, name, or as raw offset within
the whole disk).

v3: Add Simon's R-b to patches 2, 4, 5, fixup whitespace in patch 5.

I don't want to duplicate the documentation, but I can see the value
in 'write' having its own entry in the TOC, so I added a stub
write.rst that just refers to the read.rst, which then explicitly
documents both.

10 files changed, 195 insertions, 31 deletions

diff --git a/cmd/Kconfig b/cmd/Kconfig
index a3512836c1a..ba5ec69293f 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1562,6 +1562,11 @@ config CMD_WDT
 	help
 	  This provides commands to control the watchdog timer devices.
 
+config CMD_WRITE
+	bool "write - Write binary data to a partition"
+	help
+	  Provides low-level write access to a partition.
+
 config CMD_AXI
 	bool "axi"
 	depends on AXI
diff --git a/cmd/Makefile b/cmd/Makefile
index 2d8bb4fc052..7198029f11e 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_CMD_PXE) += pxe.o
 obj-$(CONFIG_CMD_WOL) += wol.o
 obj-$(CONFIG_CMD_QFW) += qfw.o
 obj-$(CONFIG_CMD_READ) += read.o
+obj-$(CONFIG_CMD_WRITE) += read.o
 obj-$(CONFIG_CMD_REGINFO) += reginfo.o
 obj-$(CONFIG_CMD_REISER) += reiser.o
 obj-$(CONFIG_CMD_REMOTEPROC) += remoteproc.o
diff --git a/cmd/read.c b/cmd/read.c
index fecfadaa1fa..1218e7acfd0 100644
--- a/cmd/read.c
+++ b/cmd/read.c
@@ -13,70 +13,69 @@
 #include <mapmem.h>
 #include <part.h>
 
-int do_read(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+static int
+do_rw(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
-	char *ep;
 	struct blk_desc *dev_desc = NULL;
-	int dev;
-	int part = 0;
 	struct disk_partition part_info;
-	ulong offset = 0u;
-	ulong limit = 0u;
+	ulong offset, limit;
+	uint blk, cnt, res;
 	void *addr;
-	uint blk;
-	uint cnt;
+	int part;
 
 	if (argc != 6) {
 		cmd_usage(cmdtp);
 		return 1;
 	}
 
-	dev = (int)hextoul(argv[2], &ep);
-	if (*ep) {
-		if (*ep != ':') {
-			printf("Invalid block device %s\n", argv[2]);
-			return 1;
-		}
-		part = (int)hextoul(++ep, NULL);
-	}
-
-	dev_desc = blk_get_dev(argv[1], dev);
-	if (dev_desc == NULL) {
-		printf("Block device %s %d not supported\n", argv[1], dev);
+	part = part_get_info_by_dev_and_name_or_num(argv[1], argv[2],
+						    &dev_desc, &part_info, 1);
+	if (part < 0)
 		return 1;
-	}
 
 	addr = map_sysmem(hextoul(argv[3], NULL), 0);
 	blk = hextoul(argv[4], NULL);
 	cnt = hextoul(argv[5], NULL);
 
-	if (part != 0) {
-		if (part_get_info(dev_desc, part, &part_info)) {
-			printf("Cannot find partition %d\n", part);
-			return 1;
-		}
+	if (part > 0) {
 		offset = part_info.start;
 		limit = part_info.size;
 	} else {
 		/* Largest address not available in struct blk_desc. */
+		offset = 0;
 		limit = ~0;
 	}
 
 	if (cnt + blk > limit) {
-		printf("Read out of range\n");
+		printf("%s out of range\n", cmdtp->name);
 		return 1;
 	}
 
-	if (blk_dread(dev_desc, offset + blk, cnt, addr) != cnt) {
-		printf("Error reading blocks\n");
+	if (IS_ENABLED(CONFIG_CMD_WRITE) && !strcmp(cmdtp->name, "write"))
+		res = blk_dwrite(dev_desc, offset + blk, cnt, addr);
+	else
+		res = blk_dread(dev_desc, offset + blk, cnt, addr);
+
+	if (res != cnt) {
+		printf("%s error\n", cmdtp->name);
 		return 1;
 	}
 
 	return 0;
 }
 
+#ifdef CONFIG_CMD_READ
 U_BOOT_CMD(
-	read,	6,	0,	do_read,
+	read,	6,	0,	do_rw,
 	"Load binary data from a partition",
-	"<interface> <dev[:part]> addr blk# cnt"
+	"<interface> <dev[:part|#partname]> addr blk# cnt"
+);
+#endif
+
+#ifdef CONFIG_CMD_WRITE
+U_BOOT_CMD(
+	write,	6,	0,	do_rw,
+	"Store binary data to a partition",
+	"<interface> <dev[:part|#partname]> addr blk# cnt"
 );
+#endif
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index ccbc18aad01..b7737814afe 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -58,6 +58,7 @@ CONFIG_CMD_SPI=y
 CONFIG_CMD_TEMPERATURE=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_WDT=y
+CONFIG_CMD_WRITE=y
 CONFIG_CMD_CAT=y
 CONFIG_BOOTP_DNS2=y
 CONFIG_CMD_TFTPPUT=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index a0fbdad20a8..ac1e8bbbef0 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -84,6 +84,7 @@ CONFIG_CMD_SPI=y
 CONFIG_CMD_TEMPERATURE=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_WDT=y
+CONFIG_CMD_WRITE=y
 CONFIG_CMD_AXI=y
 CONFIG_CMD_CAT=y
 CONFIG_CMD_SETEXPR_FMT=y
diff --git a/doc/usage/cmd/read.rst b/doc/usage/cmd/read.rst
new file mode 100644
index 00000000000..840846728fc
--- /dev/null
+++ b/doc/usage/cmd/read.rst
@@ -0,0 +1,44 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later:
+
+read and write commands
+=======================
+
+Synopsis
+--------
+
+::
+
+    read <interface> <dev[:part|#partname]> <addr> <blk#> <cnt>
+    write <interface> <dev[:part|#partname]> <addr> <blk#> <cnt>
+
+The read and write commands can be used for raw access to data in
+block devices (or partitions therein), i.e. without going through a
+file system.
+
+read
+----
+
+The block device is specified using the <interface> (e.g. "mmc") and
+<dev> parameters. If the block device has a partition table, one can
+optionally specify a partition number (using the :part syntax) or
+partition name (using the #partname syntax). The command then reads
+the <cnt> blocks of data starting at block number <blk#> of the given
+device/partition to the memory address <addr>.
+
+write
+-----
+
+The write command is completely equivalent to the read command, except
+of course that the transfer direction is reversed.
+
+Examples
+--------
+
+    # Read 2 MiB from partition 3 of mmc device 2 to $loadaddr
+    read mmc 2.3 $loadaddr 0 0x1000
+
+    # Read 16 MiB from the partition named 'kernel' of mmc device 1 to $loadaddr
+    read mmc 1#kernel $loadaddr 0 0x8000
+
+    # Write to the third sector of the partition named 'bootdata' of mmc device 0
+    write mmc 0#bootdata $loadaddr 2 1
diff --git a/doc/usage/cmd/write.rst b/doc/usage/cmd/write.rst
new file mode 100644
index 00000000000..c16870d6dc1
--- /dev/null
+++ b/doc/usage/cmd/write.rst
@@ -0,0 +1,6 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later:
+
+write command
+=============
+
+See :doc:`read`.
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index ebf5eea9f8a..d01d38cbf9d 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -72,6 +72,7 @@ Shell commands
    cmd/printenv
    cmd/pstore
    cmd/qfw
+   cmd/read
    cmd/reset
    cmd/rng
    cmd/sbi
@@ -92,6 +93,7 @@ Shell commands
    cmd/ut
    cmd/wdt
    cmd/wget
+   cmd/write
    cmd/xxd
 
 Booting OS
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index 2ffde8703ab..7848f348bc4 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_CMD_PINMUX) += pinmux.o
 obj-$(CONFIG_CMD_PWM) += pwm.o
 obj-$(CONFIG_CMD_SEAMA) += seama.o
 ifdef CONFIG_SANDBOX
+obj-$(CONFIG_CMD_READ) += rw.o
 obj-$(CONFIG_CMD_SETEXPR) += setexpr.o
 endif
 obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
diff --git a/test/cmd/rw.c b/test/cmd/rw.c
new file mode 100644
index 00000000000..98302bf047b
--- /dev/null
+++ b/test/cmd/rw.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Tests for read and write commands
+ */
+
+#include <common.h>
+#include <dm/test.h>
+#include <mapmem.h>
+#include <part.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+static int setup_partitions(struct unit_test_state *uts, struct blk_desc **mmc_dev_desc)
+{
+	char str_disk_guid[UUID_STR_LEN + 1];
+	struct disk_partition parts[2] = {
+		{
+			.start = 48, /* GPT data takes up the first 34 blocks or so */
+			.size = 4,
+			.name = "data",
+		},
+		{
+			.start = 52,
+			.size = 10,
+			.name = "log",
+		},
+	};
+
+	ut_asserteq(2, blk_get_device_by_str("mmc", "2", mmc_dev_desc));
+	if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
+		gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
+		gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
+		gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
+	}
+	ut_assertok(gpt_restore(*mmc_dev_desc, str_disk_guid, parts,
+				ARRAY_SIZE(parts)));
+	return 0;
+}
+
+/* Fill the write buffer with pseudo-random data, clear the read buffer. */
+static void init_buffers(char *rb, char *wb, size_t size, unsigned seed)
+{
+	memset(rb, 0, size);
+	while (size--) {
+		*wb++ = seed;
+		seed *= 43;
+		seed += 17 + size/4;
+	}
+}
+
+static int dm_test_read_write(struct unit_test_state *uts)
+{
+	struct blk_desc *dev_desc;
+	char wbuf[1024], rbuf[1024];
+	ulong wa, ra;
+
+#define INIT_BUFFERS() init_buffers(rbuf, wbuf, sizeof(rbuf), __LINE__)
+
+	ut_assertok(setup_partitions(uts, &dev_desc));
+
+	wa = map_to_sysmem(wbuf);
+	ra = map_to_sysmem(rbuf);
+
+	/* Simple test, write to/read from same partition. */
+	INIT_BUFFERS();
+	ut_assertok(run_commandf("write mmc 2:1 0x%lx 0 2", wa));
+	ut_assertok(run_commandf("read  mmc 2:1 0x%lx 0 2", ra));
+	ut_assertok(memcmp(wbuf, rbuf, sizeof(wbuf)));
+	ut_assertok(run_commandf("read  mmc 2:1 0x%lx 1 1", ra));
+	ut_assertok(memcmp(&wbuf[512], rbuf, 512));
+
+	/* Use name for write, number for read. */
+	INIT_BUFFERS();
+	ut_assertok(run_commandf("write mmc 2#log 0x%lx 0 2", wa));
+	ut_assertok(run_commandf("read  mmc 2:2   0x%lx 0 2", ra));
+	ut_assertok(memcmp(wbuf, rbuf, sizeof(wbuf)));
+
+	/* Use full device for write, name for read. */
+	INIT_BUFFERS();
+	ut_assertok(run_commandf("write mmc 2:0    0x%lx 0x30 2", wa));
+	ut_assertok(run_commandf("read  mmc 2#data 0x%lx    0 2", ra));
+	ut_assertok(memcmp(wbuf, rbuf, sizeof(wbuf)));
+
+	/* Use name for write, full device for read */
+	INIT_BUFFERS();
+	ut_assertok(run_commandf("write mmc 2#log 0x%lx    1 2", wa));
+	ut_assertok(run_commandf("read  mmc 2:0   0x%lx 0x35 2", ra));
+	ut_assertok(memcmp(wbuf, rbuf, sizeof(wbuf)));
+
+	/* Read/write outside partition bounds should be rejected upfront. */
+	console_record_reset_enable();
+	ut_asserteq(1, run_commandf("read mmc 2#data 0x%lx 3 2", ra));
+	ut_assert_nextlinen("read out of range");
+	ut_assert_console_end();
+
+	console_record_reset_enable();
+	ut_asserteq(1, run_commandf("write mmc 2#log 0x%lx 9 2", wa));
+	ut_assert_nextlinen("write out of range");
+	ut_assert_console_end();
+
+	return 0;
+}
+
+DM_TEST(dm_test_read_write, UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);