Date: Mon, 17 Sep 2018 13:35:14 -0700 From: Mark Millard <marklmi@yahoo.com> To: freebsd-hackers@freebsd.org, freebsd-arm <freebsd-arm@freebsd.org> Subject: An experiment in enabling discard with e.MMC trim on a Pine64+ 2GB (based on head -r338675) Message-ID: <670AF48A-77BF-4246-BD80-9D067CB05DE3@yahoo.com>
next in thread | raw e-mail | index | archive | help
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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- /usr/src/sys/dev/mmc/mmcreg.h (revision 338675) +++ /usr/src/sys/dev/mmc/mmcreg.h (working copy) @@ -463,7 +463,7 @@ =20 #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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- /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 |=3D MMCSD_USE_TRIM; sc->erase_sector =3D 1; + if (6 <=3D ext_csd[EXT_CSD_REV]) // e.MMC 4.5 or later + sc->flags |=3D MMCSD_USE_DISCARD_WITH_MMC_TRIM; } else sc->erase_sector =3D mmc_get_erase_sector(dev); =20 @@ -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; =20 sc =3D part->sc; dev =3D sc->dev; @@ -1261,6 +1264,7 @@ sz =3D part->disk->d_sectorsize; end =3D bp->bio_pblkno + (bp->bio_bcount / sz); use_trim =3D sc->flags & MMCSD_USE_TRIM; + use_discard_with_mmc_trim =3D sc->flags & = MMCSD_USE_DISCARD_WITH_MMC_TRIM; if (use_trim =3D=3D true) { start =3D block; stop =3D end; @@ -1289,8 +1293,10 @@ =20 if ((sc->flags & MMCSD_INAND_CMD38) !=3D 0) { err =3D mmc_switch(mmcbus, dev, sc->rca, = EXT_CSD_CMD_SET_NORMAL, - EXT_CSD_INAND_CMD38, use_trim =3D=3D 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 !=3D 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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- /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) = !=3D 0 && - (host_caps & MMC_CAP_SIGNALING_180) !=3D 0) = { + if ((card_type & = EXT_CSD_CARD_TYPE_DDR_52_3V_OR_1_8V) !=3D 0) { setbit(&ivar->timings, = bus_timing_mmc_ddr52); - setbit(&ivar->vccq_180, = bus_timing_mmc_ddr52); + if ((host_caps & MMC_CAP_SIGNALING_180) = !=3D 0) + setbit(&ivar->vccq_180, = bus_timing_mmc_ddr52); } if ((card_type & EXT_CSD_CARD_TYPE_HS200_1_2V) = !=3D 0 && (host_caps & MMC_CAP_SIGNALING_120) !=3D 0) = { Index: /usr/src/sys/arm/allwinner/aw_mmc.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- /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; =20 - sc->aw_host.caps |=3D MMC_CAP_SIGNALING_330 | = MMC_CAP_SIGNALING_180; + sc->aw_host.caps |=3D MMC_CAP_SIGNALING_330; // | = MMC_CAP_SIGNALING_180; not used on Pine64+ 2GB =20 if (bus_width >=3D 4) sc->aw_host.caps |=3D MMC_CAP_4_BIT_DATA; @@ -1308,17 +1308,20 @@ =20 sc =3D device_get_softc(bus); =20 - if (sc->aw_reg_vqmmc =3D=3D NULL) - return EOPNOTSUPP; - switch (sc->aw_host.ios.vccq) { case vccq_180: + if (sc->aw_reg_vqmmc =3D=3D NULL) + return EOPNOTSUPP; uvolt =3D 1800000; break; case vccq_330: + if (sc->aw_reg_vqmmc =3D=3D NULL) // implicitly already = stuck at vccq_330 + return 0; // avoid calling code treating the = assignment attempt as an error uvolt =3D 3300000; break; default: + if (sc->aw_reg_vqmmc =3D=3D NULL) + return EOPNOTSUPP; return EINVAL; } =20 I wonder if the sc->aw_reg_vqmmc =3D=3D 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 . =3D=3D=3D Mark Millard marklmi at yahoo.com ( dsl-only.net went away in early 2018-Mar)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?670AF48A-77BF-4246-BD80-9D067CB05DE3>