diff mbox series

[1/2] mmc-utils: Support MANUAL_EN and AUTO_EN in BKOPS_EN register

Message ID 20191016111542.31358-2-laa@kamstrup.com (mailing list archive)
State New, archived
Headers show
Series mmc-utils: BKOPS_EN auto, manual and start | expand

Commit Message

Lars Pedersen Oct. 16, 2019, 11:15 a.m. UTC 
From: Lars Pedersen <lapeddk@gmail.com>

The BKOPS_EN register in eMMC 5.1 now has two fields AUTO_EN and MANUAL_EN.
---
 mmc.c      | 14 ++++++++----
 mmc.h      |  7 ++++--
 mmc_cmds.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 mmc_cmds.h |  3 ++-
 4 files changed, 80 insertions(+), 10 deletions(-)

Comments

Avri Altman Oct. 17, 2019, 1:13 p.m. UTC | #1 
Hi,

> 
> From: Lars Pedersen <lapeddk@gmail.com>
Please sign your patch (use -s when running git commit)

> 
> The BKOPS_EN register in eMMC 5.1 now has two fields AUTO_EN and
> MANUAL_EN.
It was already in 5.0


> +       { do_write_bkops_auto_en, -2,
> +         "bkops auto", "<mode> <device>\n"
> +               "Enable/disable eMMC BKOPS AUTO_EN feature on <device>.\nThe
> device may perform background operations while not servicing the host.\n"
> +               "NOTE!  Only supported in eMMC-5.1 and
> + MANUAL_EN(BKOPS_EN[0]) must not be set",
>           NULL
Although there is not much sense of setting those both,
I am not sure that the manual and auto bits are mutually exclusive.
Will ask around.

>         },
>         { do_hwreset_en, -1,
> @@ -467,7 +473,7 @@ static int parse_args(int argc, char **argv,
>                         matchcmd->verb, matchcmd->nargs);
>                         return -2;
>         }
> -
> +
Not belong to this patch

> +int do_write_bkops_auto_en(int nargs, char **argv) {
> +       __u8 ext_csd[512], value = 0;
> +       int fd, ret;
> +       char *device;
> +       char *mode;
> +
> +       if (nargs != 3) {
> +              fprintf(stderr, "Usage: mmc bkops auto enable/disable
> </path/to/mmcblkX>\n");
> +              exit(1);
> +       }
> +
> +       mode = argv[1];
> +       device = argv[2];
> +
> +       fd = open(device, O_RDWR);
> +       if (fd < 0) {
> +               perror("open");
> +               exit(1);
> +       }
> +
> +       ret = read_extcsd(fd, ext_csd);
> +       if (ret) {
> +               fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
> +               exit(1);
> +       }
> +
> +       if (ext_csd[EXT_CSD_REV] != EXT_CSD_REV_V5_1) {
ext_csd[EXT_CSD_REV] < EXT_CSD_REV_V5_0

> +               fprintf(stderr, "%s doesn't support eMMC 5.1\n", device);
> +               exit(1);
> +       }
> +
> +       if (!(ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)) {
This is obsolete in 5.1 and need not be checked.

> +               fprintf(stderr, "%s doesn't support BKOPS\n", device);
> +               exit(1);
> +       }
> +
> +       if ((ext_csd[EXT_CSD_BKOPS_EN] & 0x1)) {
Again - not sure about that.
Look in the kerne's driver, its checks for both options.

> +               fprintf(stderr, "%s manual BKOPS already enabled.\n", device);
> +               exit(1);
> +       }
> +
> +       if (strcmp(mode, "enable") == 0) {
> +               ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN,
> BKOPS_ENABLE_AUTO_SET);
> +       } else if (strcmp(mode, "disable") == 0) {
> +               ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN,
> BKOPS_ENABLE_AUTO_CLR);
> +       } else {
> +               fprintf(stderr, "%s invalid mode for BKOPS_EN AUTO_EN requested:
> %s. Valid options: enable or disable\n", mode, device);
> +               exit(1);
> +       }
> +

I even think, as we are not so strictly obliged to zero API changes,
Maybe just change the api to do_write_bkops_en(), and simply allow to set 1, 2, or 3 to that register?


Thanks,
Avri
diff mbox series

Patch

diff --git a/mmc.c b/mmc.c
index 50c9c9e..9e02ba0 100644
--- a/mmc.c
+++ b/mmc.c
@@ -120,9 +120,15 @@  static struct Command commands[] = {
 	  "<boot_bus_width> must be \"x1|x4|x8\"",
 	  NULL
 	},
-	{ do_write_bkops_en, -1,
-	  "bkops enable", "<device>\n"
-		"Enable the eMMC BKOPS feature on <device>.\nNOTE!  This is a one-time programmable (unreversible) change.",
+	{ do_write_bkops_manual_en, -1,
+	  "bkops manual enable", "<device>\n"
+		"Enable eMMC BKOPS MANUAL_EN(BKOPS_EN[0]) on <device>.\nNOTE!  This is a one-time programmable (unreversible) change.",
+	  NULL
+	},
+	{ do_write_bkops_auto_en, -2,
+	  "bkops auto", "<mode> <device>\n"
+		"Enable/disable eMMC BKOPS AUTO_EN feature on <device>.\nThe device may perform background operations while not servicing the host.\n"
+		"NOTE!  Only supported in eMMC-5.1 and MANUAL_EN(BKOPS_EN[0]) must not be set",
 	  NULL
 	},
 	{ do_hwreset_en, -1,
@@ -467,7 +473,7 @@  static int parse_args(int argc, char **argv,
 			matchcmd->verb, matchcmd->nargs);
 			return -2;
 	}
-	
+
         if (prepare_args( nargs_, args_, prgname, matchcmd )){
                 fprintf(stderr, "ERROR: not enough memory\\n");
 		return -20;
diff --git a/mmc.h b/mmc.h
index 648fb26..0596d6c 100644
--- a/mmc.h
+++ b/mmc.h
@@ -126,9 +126,12 @@ 
 #define EN_REL_WR	(1<<2)
 
 /*
- * BKOPS_EN field definition
+ * BKOPS_EN field definitions
  */
-#define BKOPS_ENABLE	(1<<0)
+#define BKOPS_ENABLE_MAN	(1<<0)  /* MANUAL_EN */
+#define BKOPS_ENABLE_AUTO_SET	(1<<1) /* AUTO_EN */
+#define BKOPS_ENABLE_AUTO_CLR	(0)  /* AUTO_EN */
+
 
 /*
  * EXT_CSD field definitions
diff --git a/mmc_cmds.c b/mmc_cmds.c
index fb37189..05a63db 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -729,14 +729,14 @@  int do_hwreset_dis(int nargs, char **argv)
 	return do_hwreset(EXT_CSD_HW_RESET_DIS, nargs, argv);
 }
 
-int do_write_bkops_en(int nargs, char **argv)
+int do_write_bkops_manual_en(int nargs, char **argv)
 {
 	__u8 ext_csd[512], value = 0;
 	int fd, ret;
 	char *device;
 
 	if (nargs != 2) {
-	       fprintf(stderr, "Usage: mmc bkops enable </path/to/mmcblkX>\n");
+	       fprintf(stderr, "Usage: mmc bkops manual enable </path/to/mmcblkX>\n");
 	       exit(1);
 	}
 
@@ -759,7 +759,67 @@  int do_write_bkops_en(int nargs, char **argv)
 		exit(1);
 	}
 
-	ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_ENABLE);
+	ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_ENABLE_MAN);
+	if (ret) {
+		fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
+			value, EXT_CSD_BKOPS_EN, device);
+		exit(1);
+	}
+
+	return ret;
+}
+
+int do_write_bkops_auto_en(int nargs, char **argv)
+{
+	__u8 ext_csd[512], value = 0;
+	int fd, ret;
+	char *device;
+	char *mode;
+
+	if (nargs != 3) {
+	       fprintf(stderr, "Usage: mmc bkops auto enable/disable </path/to/mmcblkX>\n");
+	       exit(1);
+	}
+
+	mode = argv[1];
+	device = argv[2];
+
+	fd = open(device, O_RDWR);
+	if (fd < 0) {
+		perror("open");
+		exit(1);
+	}
+
+	ret = read_extcsd(fd, ext_csd);
+	if (ret) {
+		fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
+		exit(1);
+	}
+
+	if (ext_csd[EXT_CSD_REV] != EXT_CSD_REV_V5_1) {
+		fprintf(stderr, "%s doesn't support eMMC 5.1\n", device);
+		exit(1);
+	}
+
+	if (!(ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1)) {
+		fprintf(stderr, "%s doesn't support BKOPS\n", device);
+		exit(1);
+	}
+
+	if ((ext_csd[EXT_CSD_BKOPS_EN] & 0x1)) {
+		fprintf(stderr, "%s manual BKOPS already enabled.\n", device);
+		exit(1);
+	}
+
+	if (strcmp(mode, "enable") == 0) {
+		ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_ENABLE_AUTO_SET);
+	} else if (strcmp(mode, "disable") == 0) {
+		ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_ENABLE_AUTO_CLR);
+	} else {
+		fprintf(stderr, "%s invalid mode for BKOPS_EN AUTO_EN requested: %s. Valid options: enable or disable\n", mode, device);
+		exit(1);
+	}
+
 	if (ret) {
 		fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
 			value, EXT_CSD_BKOPS_EN, device);
diff --git a/mmc_cmds.h b/mmc_cmds.h
index 9d3246c..7f825ae 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -27,7 +27,8 @@  int do_writeprotect_user_set(int nargs, char **argv);
 int do_disable_512B_emulation(int nargs, char **argv);
 int do_write_boot_en(int nargs, char **argv);
 int do_boot_bus_conditions_set(int nargs, char **argv);
-int do_write_bkops_en(int nargs, char **argv);
+int do_write_bkops_manual_en(int nargs, char **argv);
+int do_write_bkops_auto_en(int nargs, char **argv);
 int do_hwreset_en(int nargs, char **argv);
 int do_hwreset_dis(int nargs, char **argv);
 int do_sanitize(int nargs, char **argv);