Re: [PATCH] mmc-utils: write_reliability set: Fix partition handling, dry run (Was: Possible bug in mmc-utils: Is MMC 'partition setting completed' required for 'write reliability'?)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello all,

this is the result of my experimentation with enabling 'reliable write'
on our card. Some of the old behaviour of mmc-utils is in clear
contradiction to that given in the standard and the observed behaviour.
Enabling 'reliable write' in the "User Data Area" without a power cycle
may be a peculiarity of our card, but being able to do so cleanly may be
useful for setting up cards that behave that way. Other than dropping
the '-c'-Option for 'write_reliability set', which does not make sense,
the existing behaviour is unchanged. I updated the man page to give more
details.

Here is the output of git format-patch:

--------8<-----------8<-----------8<-----------8<-----------8<---
From: Harald Brinkmann <harald.brinkmann@xxxxxxxxxxxxxxxxxxxxx>
Date: Tue, 28 Mar 2017 11:01:31 +0200
Subject: [PATCH] mmc-utils: write_reliability set: Fix partition handling, dry
 run
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Extended CSD register WR_REL_SET [167] is strictly write once and
unlike the partitioning is not implicitly validated by setting
PARTITION_SETTING_COMPLETED later. This version clarifies the
help and manual texts and fixes theses problems:

* The write_reliability set "dry-run" (option -n) permanently
  set this register.
* Setting write reliability was only possible for one partition
  at a time and the card refuses to add write reliability for
  more partitions in later calls.
* Setting PARTITION_SETTING_COMPLETED when setting write
  reliability with option -y. This call failed on our card if
  no partition or enhanced user area configuration was given,
  but write reliability was activated.

Although the "Embedded Multi-Media Card (e•MMC) Electrical
Standard (5.1)" (JESD84-B51) says in Chapter 6.6.8 "Data Write":
"The changes made to the WR_REL_SET register will not have an
impact until the partitioning process is complete (i.e., after
the power cycle has occurred and the partitioning has completed
successfully)." on our test card (Micron MTFC4GMDEA-4M) write
reliability on the "User Data Area" was enabled rightaway without
finalising the partition setting, any power cycle or data loss.

Signed-off-by: Harald Brinkmann <harald.brinkmann@xxxxxxxxxxxxxxxxxxxxx>
---
 man/mmc.1  | 31 ++++++++++++++++++++++++------
 mmc.c      |  8 ++++++--
 mmc_cmds.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++----------------
 3 files changed, 80 insertions(+), 24 deletions(-)

diff --git a/man/mmc.1 b/man/mmc.1
index 971feae..4aa8681 100644
--- a/man/mmc.1
+++ b/man/mmc.1
@@ -30,21 +30,40 @@ This sets the eMMC to be write-protected until next boot.
 Set the eMMC data sector size to 4KB by disabling emulation on
 <device>.
 .TP
-.BR "gp create <-y|-n> <length KiB> <partition> <enh_attr> <ext_attr> <device>"
+.BR "gp create <-y|-n|-c> <length KiB> <partition> <enh_attr> <ext_attr> <device>"
 create general purpose partition for the <device>.
-Dry-run only unless -y is passed.
+.IP
+Dry-run only unless -y or -c is passed.
+Use -c if more partitions are to be created or an enhanced area (see "enh_area set")
+is to be set. Using -y finalises the partitions, enhanced area and reliable write setup
+if the card regards it as valid.
 To set enhanced attribute to general partition being created set <enh_attr> to 1 else set it to 0.
 To set extended attribute to general partition set <ext_attr> to 1,2 else set it to 0.
+.IP
 NOTE!  This is a one-time programmable (unreversible) change.
 .TP
-.BR "enh_area set <-y|-n> <start KiB> <length KiB> <device>"
+.BR "enh_area set <-y|-n|-c> <start KiB> <length KiB> <device>"
 Enable the enhanced user area for the <device>.
-Dry-run only unless -y is passed.
+.IP
+Dry-run only unless -y or -c is passed.
+Use -c if general partitions are to be created (see "gp create").
+Using -y finalises the partitions, enhanced area and reliable write setup
+if the card regards it as valid.
+.IP
 NOTE!  This is a one-time programmable (unreversible) change.
 .TP
-.BR "write_reliability set <-y|-n> <partition> <device>"
+.BR "write_reliability set <-y|-n> [0] [1] [2] [3] [4] <device>"
 Enable write reliability per partition for the <device>.
-Dry-run only unless -y is passed.
+.IP
+Dry-run only unless -y is passed. This must be done for all selected partitions in one call,
+where 0 refers to the "User Data Area" and 1 to 4 refer to "General Purpose Partitions" 1 to 4.
+.IP
+NOTE!  Embedded Multi-Media Card eMMC Electrical Standard (5.1) (JESD84-B51) specifies
+in Chapter 6.6.8 "Data Write" that this must be set before the partitioning (see "gp create"
+and "enh_area set") is complete and will not have an impact before the partitioning has been
+completed successfully. However, some cards will enable write reliability on the "User Data
+Area" rightaway and without a power cycle or data loss.
+.IP
 NOTE!  This is a one-time programmable (unreversible) change.
 .TP
 .BR "status get <device>"
diff --git a/mmc.c b/mmc.c
index 50c9c9e..7874a06 100644
--- a/mmc.c
+++ b/mmc.c
@@ -98,8 +98,12 @@ static struct Command commands[] = {
          NULL
        },
        { do_write_reliability_set, -2,
-         "write_reliability set", "<-y|-n|-c> " "<partition> " "<device>\n"
-               "Enable write reliability per partition for the <device>.\nDry-run only unless -y or -c is passed.\nUse -c if more partitioning settings are still to come.\nNOTE!  This is a one-time programmable (unreversible) change.",
+         "write_reliability set", "<-y|-n> " "[0] " "[1] " "[2] " "[3] " "[4] " "<device>\n"
+               "Enable write reliability for the given partition(s) for the <device>.\n"
+           "Where 0 is the \"User Data Area\" and 1 to 4 refer to the \"General Purpose Partitions\".\n"
+               "Dry-run only unless -y is passed.\n"
+               "NOTE!  This is a one-time programmable (unreversible) change and all bits must be set in one operation.\n"
+               "NOTE!  This setting may only become active after partitioning and its power cycle is complete.",
          NULL
        },
        { do_status_get, -1,
diff --git a/mmc_cmds.c b/mmc_cmds.c
index d7215bb..194f3cc 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -1228,23 +1228,34 @@ int do_write_reliability_set(int nargs, char **argv)
 {
        __u8 value;
        __u8 ext_csd[512];
-       int fd, ret;
+       int fd, ret, i;
+       long int part;

        int dry_run = 1;
-       int partition;
+       int wr_rel_already_set = 0;
+       int wr_rel_to_set = 0;
+       int wr_rel_set_bitmask = 0x1f;
+       int warn = 0;
        char *device;

-       CHECK(nargs != 4, "Usage: mmc write_reliability set <-y|-n|-c> "
-                       "<partition> </path/to/mmcblkX>\n", exit(1));
+       CHECK((nargs < 4 || nargs > 8), "Usage: mmc write_reliability set <-y|-n> "
+                       "[0] [1] [2] [3] [4] </path/to/mmcblkX> giving at least one partition\n", exit(1));

        if (!strcmp("-y", argv[1])) {
                dry_run = 0;
-       } else if (!strcmp("-c", argv[1])) {
-               dry_run = 2;
        }

-       partition = strtol(argv[2], NULL, 10);
-       device = argv[3];
+       for (i = 2; i < nargs - 1; i++) {
+               part = strtol(argv[i], NULL, 10);
+               if (part < 0 || part > 4) {
+                       fprintf(stderr, "Specified partition '%s' is not in range"
+                               " of 0 to 4\n", argv[i]);
+                       exit(1);
+               }
+               wr_rel_to_set |= (1 << part);
+       }
+
+       device = argv[nargs - 1];

        fd = open(device, O_RDWR);
        if (fd < 0) {
@@ -1268,25 +1279,47 @@ int do_write_reliability_set(int nargs, char **argv)
        /* assert HS_CTRL_REL */
        if (!(ext_csd[EXT_CSD_WR_REL_PARAM] & HS_CTRL_REL)) {
                printf("Cannot set write reliability parameters, WR_REL_SET is "
-                               "read-only\n");
+                       "read-only\n");
+               exit(1);
+       }
+
+       wr_rel_already_set = ext_csd[EXT_CSD_WR_REL_SET] & wr_rel_set_bitmask;
+
+       if ((wr_rel_already_set & wr_rel_to_set) == wr_rel_to_set) {
+               printf("All given partitions are already set to use reliable write\n");
+               return 0;
+       }
+
+       if (wr_rel_already_set) {
+               fprintf(stderr, "Some write reliability registers are already set.\n"
+                       "Setting some more will probably not work!\n");
+               warn = 1;
+       }
+
+       value = wr_rel_already_set | wr_rel_to_set;
+
+       if (dry_run) {
+               /* Contrary to the partitioning itself, this value is permanent
+                * after writing this register, so we skip that for the dry run
+                */
+               printf("NOT writing 0x%02x to WR_REL_SET! (Dry run is specified)\n",
+                       value);
                exit(1);
        }

-       value = ext_csd[EXT_CSD_WR_REL_SET] | (1<<partition);
        ret = write_extcsd_value(fd, EXT_CSD_WR_REL_SET, value);
        if (ret) {
                fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
-                               value, EXT_CSD_WR_REL_SET, device);
+                       value, EXT_CSD_WR_REL_SET, device);
                exit(1);
        }

-       printf("Done setting EXT_CSD_WR_REL_SET to 0x%02x on %s\n",
+       printf("Done setting EXT_CSD_WR_REL_SET to 0x%02x on %s\n"
+               "Confirm that the new value is bit is correctly set "
+               "using 'extcsd read' after power cycle\n",
                value, device);

-       if (set_partitioning_setting_completed(dry_run, device, fd))
-               exit(1);
-
-       return 0;
+       return warn;
 }

 int do_read_extcsd(int nargs, char **argv)
--
2.1.4

------->8---------->8---------->8---------->8---------->8---------->8---


--

i.A. Harald Brinkmann

BST eltromat International GmbH
Werk Leopoldshöhe
Herforder Straße 249-251
D-33818 Leopoldshöhe

T:      +49 (5208) 987-513

E:      harald.brinkmann@xxxxxxxxxxxxxxxxxxxxx
W:      http://www.bst-eltromat.com




_______________________________________________________
Amtsgericht Bielefeld, HRB Nr. 30830
Geschäftsführer Kristian Jünke, Dr. Johann-Carsten Kipp, Dr. Gunter
Tautorus
Sitz der Gesellschaft: Bielefeld
Vertrauliche E-Mail von BST eltromat International GmbH

��.n��������+%������w��{.n�����{��i��)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥





[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux