[PATCH] commands: mmc_extcsd: split background operations

[PATCH] commands: mmc_extcsd: split background operations

[Stefan Kerkmann]

The current implementation of the  `mmc_extcsd -b` subcommand enables
manual background operations and automatic background operations if
available, which is unexpected and can be problematic:

With the eMMC JEDEC Standard 4.41 eMMCs gained support for decoupling
internal maintenance operations from foreground operations like read and
write commands. This is feature is called background operations. This
allows the host to manually trigger these maintenance operations e.g.
during idle times.

The advantages are reduced latencies during regular reads and writes. If
the host doesn't schedule these maintenance operations the standard
suggest[1] that they will be executed during foreground operations once
the maintenance level `critical` is reached. Thus enabling manual
background operations is a safe opt-in which resorts to maintenance
during foreground operations, even if the host isn't aware of the
enablement and doesn't start them at all.

With JEDEC standard 5.1 eMMCs gained support for schedueling these
background operations autonomously at their own will. This *not* a safe
opt-in as it needs support from the host:

> When AUTO_EN bit is set, the host should keep the device power active.

As there is no register to query the current status of a background
operation, the safe way seems to issue a power off notification and wait
for completion. This might not be implemented by the host.

This commit refactors the `mmc_extcsd -b` subcommand to only set the
`MANUAL_EN` bit, which is a safe default.

To still allow the enablement of  automatic background operations the
new `mmc_extcsd -B` subcommand is introduced which sets `MANUAL_EN` and
`AUTO_EN` bits.

[1]: See JEDEC Standard No. 84-B51, Page 93, 6.6.25 Background
Operations

Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>
---
 commands/Kconfig      |  9 ++++++---
 commands/mmc_extcsd.c | 20 +++++++++++++++-----
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/commands/Kconfig b/commands/Kconfig
index 4a04868611..c186568fae 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -325,14 +325,17 @@ config CMD_MMC_EXTCSD
 	help
 	  Read or write the extended CSD register of a MMC device.
 
-	  Usage: mmc_extcsd dev [-r | -i index [-r | -v value -y]]
-
+	  Usage: mmc_extcsd dev [-r | -b | -B | -i index [-r | -v value [-y]]]
+	  
 	  Options:
 		  -i      field index of the register
 		  -r      print the register as raw data
 		  -v      value which will be written
 		  -y      don't request when writing to one time programmable fields
-		  __CAUTION__: this could damage the device!
+			  __CAUTION__: this could damage the device!
+		  -b      set bkops-enable (EXT_CSD_BKOPS_EN[163])
+		  -B      set bkops-enable and auto-enable (EXT_CSD_BKOPS_EN[163])
+			  __WARNING__: this is a write-once setting!
 
 config CMD_POLLER
 	tristate
diff --git a/commands/mmc_extcsd.c b/commands/mmc_extcsd.c
index 993a6dd12b..7fa6235a4d 100644
--- a/commands/mmc_extcsd.c
+++ b/commands/mmc_extcsd.c
@@ -2379,7 +2379,7 @@ static int do_mmc_extcsd(int argc, char *argv[])
 	if (argc < 2)
 		return COMMAND_ERROR_USAGE;
 
-	while ((opt = getopt(argc, argv, "i:v:yrb")) > 0)
+	while ((opt = getopt(argc, argv, "i:v:yrbB")) > 0)
 		switch (opt) {
 		case 'i':
 			index = simple_strtoul(optarg, NULL, 0);
@@ -2401,6 +2401,13 @@ static int do_mmc_extcsd(int argc, char *argv[])
 			write_operation = 1;
 			always_write = 1;
 			break;
+		case 'B':
+			set_bkops_en = 1;
+			index = EXT_CSD_BKOPS_EN;
+			value = BIT(0) | BIT(1);
+			write_operation = 1;
+			always_write = 1;
+			break;
 		}
 
 	if (optind == argc)
@@ -2431,13 +2438,15 @@ static int do_mmc_extcsd(int argc, char *argv[])
 	}
 
 	if (set_bkops_en) {
-		if (dst[index]) {
+		if (dst[index] == value) {
 			printf("Abort: EXT_CSD [%u] already set to %#02x!\n",
 			       index, dst[index]);
 			goto error_with_mem;
 		}
-		if (dst[EXT_CSD_REV] >= 7)
-			value |= BIT(1);	/* set AUTO_EN bit too */
+
+		if (value & BIT(1)) {
+			pr_notice("Enabling automatic background operations, this needs support from the OS.\n");
+		}
 	}
 
 	if (write_operation)
@@ -2486,13 +2495,14 @@ BAREBOX_CMD_HELP_OPT("-v", "value which will be written")
 BAREBOX_CMD_HELP_OPT("-y", "don't request when writing to one time programmable fields")
 BAREBOX_CMD_HELP_OPT("",   "__CAUTION__: this could damage the device!")
 BAREBOX_CMD_HELP_OPT("-b", "set bkops-enable (EXT_CSD_BKOPS_EN[163])")
+BAREBOX_CMD_HELP_OPT("-B", "set bkops-enable and auto-enable (EXT_CSD_BKOPS_EN[163])")
 BAREBOX_CMD_HELP_OPT("",   "__WARNING__: this is a write-once setting!")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(mmc_extcsd)
 	.cmd		= do_mmc_extcsd,
 	BAREBOX_CMD_DESC("Read/write the extended CSD register.")
-	BAREBOX_CMD_OPTS("dev [-r | -b | -i index [-r | -v value [-y]]]")
+	BAREBOX_CMD_OPTS("dev [-r | -b | -B | -i index [-r | -v value [-y]]]")
 	BAREBOX_CMD_GROUP(CMD_GRP_CONSOLE)
 	BAREBOX_CMD_HELP(cmd_mmc_extcsd_help)
 BAREBOX_CMD_END

---
base-commit: a7e9f7f096b030962513eeea0e91e96f6e13ea40
change-id: 20241023-feature-emmc-background-ops-7451a7ec69c7

Best regards,
-- 
Stefan Kerkmann <s.kerkmann@pengutronix.de>

Re: [PATCH] commands: mmc_extcsd: split background operations

[Sascha Hauer]

On Wed, 23 Oct 2024 10:28:03 +0200, Stefan Kerkmann wrote:
> The current implementation of the  `mmc_extcsd -b` subcommand enables
> manual background operations and automatic background operations if
> available, which is unexpected and can be problematic:
> 
> With the eMMC JEDEC Standard 4.41 eMMCs gained support for decoupling
> internal maintenance operations from foreground operations like read and
> write commands. This is feature is called background operations. This
> allows the host to manually trigger these maintenance operations e.g.
> during idle times.
> 
> [...]

Applied, thanks!

[1/1] commands: mmc_extcsd: split background operations
      https://git.pengutronix.de/cgit/barebox/commit/?id=2eaaaf5da732 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>