Discussion:
An experiment in enabling discard with e.MMC trim on a Pine64+ 2GB (based on head -r338675)
Mark Millard via freebsd-hackers
2018-09-17 20:35:14 UTC
Permalink
As stands this is just investigatory material. Also, I first
had to enable using an e.MMC on a microsd card adapter on
the Pine64+ 2GB, so those changes are listed too.

The goal was for e.MMC 4.5+ to enable the discard bit
with the trim bit to allow an e.MMC to not necessarily
force the trim'ed data to become all zeros or all ones:
the performance command trim variant that does not
require data removal (but allows it).

(I did nothing about discard from the modern sd card
material. I doubt that I have any test context for that.)

Unfortunately I have a very narrow range as far as test
environments go currently: just Pine64+ 2GB and just one
type of e.MMC in use. But for the context that I have
it seems to be working.

The discard-enabling related changes are in:

/usr/src/sys/dev/mmc/mmcreg.h (an additional #define)
/usr/src/sys/dev/mmc/mmcsd.c

As stands I have nothing for overriding e.MMC 4.5+
using discard with trim (when trim is enabled via
the historical criteria). If trim without discard
worked but with discard did not for some device this
could be a problem. Also: if trim without discard is
desired for security reasons it would be a problem.
But what I have here is sufficient for my basic
testing.


The Pine64+ 2GB e.MMC-on-adapter enabling changes are in:

/usr/src/sys/dev/mmc/mmcreg.h (a renamed #define as a form of commentary)
/usr/src/sys/dev/mmc/mmc.c (not really Pine64+ 2GB or A64 specific)
/usr/src/sys/arm/allwinner/aw_mmc.c (MMC_CAP_SIGNALING_180 part: Pine64+ 2GB or A64 specific)

(So mmcreg.h has something for each part.)

I expect the mmc.c change is a generic correction to
more accurately follow the e.MMC DDR52 definition
as far as voltage requirements go. But it was driven
by the Pine64+ 2GB properties leading to wanting the
change.


The changes for discard ( and mmcreg.h ) are:


Index: /usr/src/sys/dev/mmc/mmcreg.h
===================================================================
--- /usr/src/sys/dev/mmc/mmcreg.h (revision 338675)
+++ /usr/src/sys/dev/mmc/mmcreg.h (working copy)
@@ -463,7 +463,7 @@

#define EXT_CSD_CARD_TYPE_HS_26 0x0001
#define EXT_CSD_CARD_TYPE_HS_52 0x0002
-#define EXT_CSD_CARD_TYPE_DDR_52_1_8V 0x0004
+#define EXT_CSD_CARD_TYPE_DDR_52_3V_OR_1_8V 0x0004
#define EXT_CSD_CARD_TYPE_DDR_52_1_2V 0x0008
#define EXT_CSD_CARD_TYPE_HS200_1_8V 0x0010
#define EXT_CSD_CARD_TYPE_HS200_1_2V 0x0020
@@ -493,6 +493,7 @@
#define EXT_CSD_INAND_CMD38 113
#define EXT_CSD_INAND_CMD38_ERASE 0x00
#define EXT_CSD_INAND_CMD38_TRIM 0x01
+#define EXT_CSD_INAND_CMD38_DISCARD_WITH_MMC_TRIM 0x03
#define EXT_CSD_INAND_CMD38_SECURE_ERASE 0x80
#define EXT_CSD_INAND_CMD38_SECURE_TRIM1 0x81
#define EXT_CSD_INAND_CMD38_SECURE_TRIM2 0x82
Index: /usr/src/sys/dev/mmc/mmcsd.c
===================================================================
--- /usr/src/sys/dev/mmc/mmcsd.c (revision 338675)
+++ /usr/src/sys/dev/mmc/mmcsd.c (working copy)
@@ -136,6 +136,7 @@
#define MMCSD_USE_TRIM 0x0002
#define MMCSD_FLUSH_CACHE 0x0004
#define MMCSD_DIRTY 0x0008
+#define MMCSD_USE_DISCARD_WITH_MMC_TRIM 0x0010
uint32_t cmd6_time; /* Generic switch timeout [us] */
uint32_t part_time; /* Partition switch timeout [us] */
off_t enh_base; /* Enhanced user data area slice base ... */
@@ -297,6 +298,8 @@
device_printf(dev, "taking advantage of TRIM\n");
sc->flags |= MMCSD_USE_TRIM;
sc->erase_sector = 1;
+ if (6 <= ext_csd[EXT_CSD_REV]) // e.MMC 4.5 or later
+ sc->flags |= MMCSD_USE_DISCARD_WITH_MMC_TRIM;
} else
sc->erase_sector = mmc_get_erase_sector(dev);

@@ -1251,7 +1254,7 @@
device_t dev, mmcbus;
u_int erase_sector, sz;
int err;
- bool use_trim;
+ bool use_trim, use_discard_with_mmc_trim;

sc = part->sc;
dev = sc->dev;
@@ -1261,6 +1264,7 @@
sz = part->disk->d_sectorsize;
end = bp->bio_pblkno + (bp->bio_bcount / sz);
use_trim = sc->flags & MMCSD_USE_TRIM;
+ use_discard_with_mmc_trim = sc->flags & MMCSD_USE_DISCARD_WITH_MMC_TRIM;
if (use_trim == true) {
start = block;
stop = end;
@@ -1289,8 +1293,10 @@

if ((sc->flags & MMCSD_INAND_CMD38) != 0) {
err = mmc_switch(mmcbus, dev, sc->rca, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_INAND_CMD38, use_trim == true ?
- EXT_CSD_INAND_CMD38_TRIM : EXT_CSD_INAND_CMD38_ERASE,
+ EXT_CSD_INAND_CMD38,
+ use_discard_with_mmc_trim ? EXT_CSD_INAND_CMD38_DISCARD_WITH_MMC_TRIM
+ : use_trim ? EXT_CSD_INAND_CMD38_TRIM
+ : EXT_CSD_INAND_CMD38_ERASE,
sc->cmd6_time, true);
if (err != MMC_ERR_NONE) {
device_printf(dev,


As for enabling the Pine64+ 2GB e.MMC use
(parts of which may be more general):


Index: /usr/src/sys/dev/mmc/mmc.c
===================================================================
--- /usr/src/sys/dev/mmc/mmc.c (revision 338675)
+++ /usr/src/sys/dev/mmc/mmc.c (working copy)
@@ -1814,10 +1814,10 @@
setbit(&ivar->timings, bus_timing_mmc_ddr52);
setbit(&ivar->vccq_120, bus_timing_mmc_ddr52);
}
- if ((card_type & EXT_CSD_CARD_TYPE_DDR_52_1_8V) != 0 &&
- (host_caps & MMC_CAP_SIGNALING_180) != 0) {
+ if ((card_type & EXT_CSD_CARD_TYPE_DDR_52_3V_OR_1_8V) != 0) {
setbit(&ivar->timings, bus_timing_mmc_ddr52);
- setbit(&ivar->vccq_180, bus_timing_mmc_ddr52);
+ if ((host_caps & MMC_CAP_SIGNALING_180) != 0)
+ setbit(&ivar->vccq_180, bus_timing_mmc_ddr52);
}
if ((card_type & EXT_CSD_CARD_TYPE_HS200_1_2V) != 0 &&
(host_caps & MMC_CAP_SIGNALING_120) != 0) {
Index: /usr/src/sys/arm/allwinner/aw_mmc.c
===================================================================
--- /usr/src/sys/arm/allwinner/aw_mmc.c (revision 338675)
+++ /usr/src/sys/arm/allwinner/aw_mmc.c (working copy)
@@ -509,7 +509,7 @@
MMC_CAP_UHS_SDR25 | MMC_CAP_UHS_SDR50 |
MMC_CAP_UHS_DDR50 | MMC_CAP_MMC_DDR52;

- sc->aw_host.caps |= MMC_CAP_SIGNALING_330 | MMC_CAP_SIGNALING_180;
+ sc->aw_host.caps |= MMC_CAP_SIGNALING_330; // | MMC_CAP_SIGNALING_180; not used on Pine64+ 2GB

if (bus_width >= 4)
sc->aw_host.caps |= MMC_CAP_4_BIT_DATA;
@@ -1308,17 +1308,20 @@

sc = device_get_softc(bus);

- if (sc->aw_reg_vqmmc == NULL)
- return EOPNOTSUPP;
-
switch (sc->aw_host.ios.vccq) {
case vccq_180:
+ if (sc->aw_reg_vqmmc == NULL)
+ return EOPNOTSUPP;
uvolt = 1800000;
break;
case vccq_330:
+ if (sc->aw_reg_vqmmc == NULL) // implicitly already stuck at vccq_330
+ return 0; // avoid calling code treating the assignment attempt as an error
uvolt = 3300000;
break;
default:
+ if (sc->aw_reg_vqmmc == NULL)
+ return EOPNOTSUPP;
return EINVAL;
}


I wonder if the sc->aw_reg_vqmmc == NULL handling change
may be more general than the Pine64+ 2GB (A64) context.

Anyway that is what I have so far. See any problems?

Would FreeBSD even want to support e.MMC 4.5+'s discard
with trim (probably with more control over if it was
used)? If yes, I'd presume not until 13.0 .


===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)

Loading...