From eeb739a6fde6e926b389b253668b2a8fd09a58c7 Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Thu, 26 Jan 2023 10:24:17 +0100 Subject: [PATCH 1/8] mmc: Check support for TRIM operations When secure/insecure TRIM operations are supported. When used as erase command argument it applies the erase operation to write blocks instead of erase groups. Signed-off-by: Loic Poulain Reviewed-by: Simon Glass Reviewed-by: Jaehoon Chung --- drivers/mmc/mmc.c | 3 +++ include/mmc.h | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index dde251c87bc..1af6af82e6b 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -2432,6 +2432,9 @@ static int mmc_startup_v4(struct mmc *mmc) mmc->wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET]; + mmc->can_trim = + !!(ext_csd[EXT_CSD_SEC_FEATURE] & EXT_CSD_SEC_FEATURE_TRIM_EN); + return 0; error: if (mmc->ext_csd) { diff --git a/include/mmc.h b/include/mmc.h index 36dd841d5d1..b8fbff150de 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -241,6 +241,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx) #define EXT_CSD_HC_WP_GRP_SIZE 221 /* RO */ #define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */ #define EXT_CSD_BOOT_MULT 226 /* RO */ +#define EXT_CSD_SEC_FEATURE 231 /* RO */ #define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */ #define EXT_CSD_BKOPS_SUPPORT 502 /* RO */ @@ -315,6 +316,8 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx) #define EXT_CSD_WR_DATA_REL_USR (1 << 0) /* user data area WR_REL */ #define EXT_CSD_WR_DATA_REL_GP(x) (1 << ((x)+1)) /* GP part (x+1) WR_REL */ +#define EXT_CSD_SEC_FEATURE_TRIM_EN (1 << 4) /* Support secure & insecure trim */ + #define R1_ILLEGAL_COMMAND (1 << 22) #define R1_APP_CMD (1 << 5) @@ -687,6 +690,7 @@ struct mmc { uint tran_speed; uint legacy_speed; /* speed for the legacy mode provided by the card */ uint read_bl_len; + bool can_trim; #if CONFIG_IS_ENABLED(MMC_WRITE) uint write_bl_len; uint erase_grp_size; /* in 512-byte sectors */ From 67642c1254fc7b1392c2745ead609255345f2d25 Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Thu, 26 Jan 2023 10:24:18 +0100 Subject: [PATCH 2/8] mmc: erase: Use TRIM erase when available The default erase command applies on erase group unit, and simply round down to erase group size. When the start block is not aligned to erase group size (e.g. erasing partition) it causes unwanted erasing of the previous blocks, part of the same erase group (e.g. owned by other logical partition, or by the partition table itself). To prevent this issue, a simple solution is to use TRIM as argument of the Erase command, which is usually supported with eMMC > 4.0, and allow to apply erase operation to write blocks instead of erase group Signed-off-by: Loic Poulain Reviewed-by: Simon Glass --- drivers/mmc/mmc_write.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c index 5b7aeeb0121..a6f93380dd0 100644 --- a/drivers/mmc/mmc_write.c +++ b/drivers/mmc/mmc_write.c @@ -15,7 +15,7 @@ #include #include "mmc_private.h" -static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) +static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt, u32 args) { struct mmc_cmd cmd; ulong end; @@ -52,7 +52,7 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt) goto err_out; cmd.cmdidx = MMC_CMD_ERASE; - cmd.cmdarg = MMC_ERASE_ARG; + cmd.cmdarg = args ? args : MMC_ERASE_ARG; cmd.resp_type = MMC_RSP_R1b; err = mmc_send_cmd(mmc, &cmd, NULL); @@ -77,7 +77,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt) #endif int dev_num = block_dev->devnum; int err = 0; - u32 start_rem, blkcnt_rem; + u32 start_rem, blkcnt_rem, erase_args = 0; struct mmc *mmc = find_mmc_device(dev_num); lbaint_t blk = 0, blk_r = 0; int timeout_ms = 1000; @@ -97,13 +97,25 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt) */ err = div_u64_rem(start, mmc->erase_grp_size, &start_rem); err = div_u64_rem(blkcnt, mmc->erase_grp_size, &blkcnt_rem); - if (start_rem || blkcnt_rem) - printf("\n\nCaution! Your devices Erase group is 0x%x\n" - "The erase range would be change to " - "0x" LBAF "~0x" LBAF "\n\n", - mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1), - ((start + blkcnt + mmc->erase_grp_size - 1) - & ~(mmc->erase_grp_size - 1)) - 1); + if (start_rem || blkcnt_rem) { + if (mmc->can_trim) { + /* Trim function applies the erase operation to write + * blocks instead of erase groups. + */ + erase_args = MMC_TRIM_ARG; + } else { + /* The card ignores all LSB's below the erase group + * size, rounding down the addess to a erase group + * boundary. + */ + printf("\n\nCaution! Your devices Erase group is 0x%x\n" + "The erase range would be change to " + "0x" LBAF "~0x" LBAF "\n\n", + mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1), + ((start + blkcnt + mmc->erase_grp_size - 1) + & ~(mmc->erase_grp_size - 1)) - 1); + } + } while (blk < blkcnt) { if (IS_SD(mmc) && mmc->ssr.au) { @@ -113,7 +125,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt) blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ? mmc->erase_grp_size : (blkcnt - blk); } - err = mmc_erase_t(mmc, start + blk, blk_r); + err = mmc_erase_t(mmc, start + blk, blk_r, erase_args); if (err) break; From 94f40b94506397d4927c800f38e96a95e5c50c71 Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Thu, 26 Jan 2023 10:24:19 +0100 Subject: [PATCH 3/8] test: dm: mmc: Check block erasing boundaries Verify that erasing blocks does not impact adjacent ones. - Write four blocks [0 1 2 3] - Erase two blocks [ 1 2 ] - Verify [0 1 2 3 ] Signed-off-by: Loic Poulain --- test/dm/mmc.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/dm/mmc.c b/test/dm/mmc.c index f744452ff24..b1eb8bee2f9 100644 --- a/test/dm/mmc.c +++ b/test/dm/mmc.c @@ -30,7 +30,7 @@ static int dm_test_mmc_blk(struct unit_test_state *uts) struct udevice *dev; struct blk_desc *dev_desc; int i; - char write[1024], read[1024]; + char write[4 * 512], read[4 * 512]; ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev)); ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc)); @@ -39,14 +39,14 @@ static int dm_test_mmc_blk(struct unit_test_state *uts) ut_asserteq(512, dev_desc->blksz); for (i = 0; i < sizeof(write); i++) write[i] = i; - ut_asserteq(2, blk_dwrite(dev_desc, 0, 2, write)); - ut_asserteq(2, blk_dread(dev_desc, 0, 2, read)); + ut_asserteq(4, blk_dwrite(dev_desc, 0, 4, write)); + ut_asserteq(4, blk_dread(dev_desc, 0, 4, read)); ut_asserteq_mem(write, read, sizeof(write)); - /* Now erase them */ - memset(write, '\0', sizeof(write)); - ut_asserteq(2, blk_derase(dev_desc, 0, 2)); - ut_asserteq(2, blk_dread(dev_desc, 0, 2, read)); + /* Now erase two of them [1 - 2] and verify all blocks */ + memset(&write[512], '\0', 2 * 512); + ut_asserteq(2, blk_derase(dev_desc, 1, 2)); + ut_asserteq(4, blk_dread(dev_desc, 0, 4, read)); ut_asserteq_mem(write, read, sizeof(write)); return 0; From 46beaec835d9828ceaafe070401cb4efb80b4fe3 Mon Sep 17 00:00:00 2001 From: Stefan Roese Date: Fri, 10 Feb 2023 13:23:50 +0100 Subject: [PATCH 4/8] mmc: mv_sdhci: Simplify call to sdhci_mvebu_mbus_config() This driver already depends on CONFIG_ARCH_MVEBU, so there is no need to have some checks for this Kconfig symbol in the driver itself. Let's remove these superfluous checks. Signed-off-by: Stefan Roese Cc: Tom Rini Cc: Simon Glass Cc: Peng Fan Cc: Jaehoon Chung Reviewed-by: Jaehoon Chung --- drivers/mmc/mv_sdhci.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c index 336ebf14102..50d03b703ed 100644 --- a/drivers/mmc/mv_sdhci.c +++ b/drivers/mmc/mv_sdhci.c @@ -64,10 +64,8 @@ int mv_sdh_init(unsigned long regbase, u32 max_clk, u32 min_clk, u32 quirks) host->ops = &mv_ops; #endif - if (CONFIG_IS_ENABLED(ARCH_MVEBU)) { - /* Configure SDHCI MBUS mbus bridge windows */ - sdhci_mvebu_mbus_config((void __iomem *)regbase); - } + /* Configure SDHCI MBUS mbus bridge windows */ + sdhci_mvebu_mbus_config((void __iomem *)regbase); return add_sdhci(host, 0, min_clk); } @@ -103,10 +101,8 @@ static int mv_sdhci_probe(struct udevice *dev) if (ret) return ret; - if (CONFIG_IS_ENABLED(ARCH_MVEBU)) { - /* Configure SDHCI MBUS mbus bridge windows */ - sdhci_mvebu_mbus_config(host->ioaddr); - } + /* Configure SDHCI MBUS mbus bridge windows */ + sdhci_mvebu_mbus_config(host->ioaddr); upriv->mmc = host->mmc; From c06a568473d46f5cab64591fde7b56da1facb7c2 Mon Sep 17 00:00:00 2001 From: Stefan Roese Date: Fri, 10 Feb 2023 13:23:51 +0100 Subject: [PATCH 5/8] mmc: mv_sdhci: Remove CONFIG_MMC_SDHCI_IO_ACCESSORS support CONFIG_MMC_SDHCI_IO_ACCESSORS is not supported and/or used by this driver so let's remove these unused parts completely. Signed-off-by: Stefan Roese Cc: Tom Rini Cc: Simon Glass Cc: Peng Fan Cc: Jaehoon Chung Reviewed-by: Jaehoon Chung --- drivers/mmc/mv_sdhci.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c index 50d03b703ed..42fa735f316 100644 --- a/drivers/mmc/mv_sdhci.c +++ b/drivers/mmc/mv_sdhci.c @@ -42,10 +42,6 @@ static void sdhci_mvebu_mbus_config(void __iomem *base) #ifndef CONFIG_DM_MMC -#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS -static struct sdhci_ops mv_ops; -#endif /* CONFIG_MMC_SDHCI_IO_ACCESSORS */ - int mv_sdh_init(unsigned long regbase, u32 max_clk, u32 min_clk, u32 quirks) { struct sdhci_host *host = NULL; @@ -59,10 +55,6 @@ int mv_sdh_init(unsigned long regbase, u32 max_clk, u32 min_clk, u32 quirks) host->ioaddr = (void *)regbase; host->quirks = quirks; host->max_clk = max_clk; -#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS - memset(&mv_ops, 0, sizeof(struct sdhci_ops)); - host->ops = &mv_ops; -#endif /* Configure SDHCI MBUS mbus bridge windows */ sdhci_mvebu_mbus_config((void __iomem *)regbase); From 8af21b094d92f671bd2f6483a15be5b3c33baca6 Mon Sep 17 00:00:00 2001 From: Stefan Roese Date: Fri, 10 Feb 2023 13:23:52 +0100 Subject: [PATCH 6/8] mmc: mv_sdhci: Depend on DM_MMC All build targets using this driver already use DM_MMC. So let's depend this driver on this Kconfig symbol and remove the non-DM driver part. Signed-off-by: Stefan Roese Cc: Tom Rini Cc: Simon Glass Cc: Peng Fan Cc: Jaehoon Chung Reviewed-by: Simon Glass Reviewed-by: Jaehoon Chung --- drivers/mmc/Kconfig | 1 + drivers/mmc/mv_sdhci.c | 39 +++++++-------------------------------- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index 80641e13930..59fb0fb50fb 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -621,6 +621,7 @@ config MMC_SDHCI_MV bool "SDHCI support on Marvell platform" depends on ARCH_MVEBU depends on MMC_SDHCI + depends on DM_MMC help This selects the Secure Digital Host Controller Interface on Marvell platform. diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c index 42fa735f316..dbdd671c88b 100644 --- a/drivers/mmc/mv_sdhci.c +++ b/drivers/mmc/mv_sdhci.c @@ -15,6 +15,13 @@ #define SDHCI_WINDOW_CTRL(win) (0x4080 + ((win) << 4)) #define SDHCI_WINDOW_BASE(win) (0x4084 + ((win) << 4)) +DECLARE_GLOBAL_DATA_PTR; + +struct mv_sdhci_plat { + struct mmc_config cfg; + struct mmc mmc; +}; + static void sdhci_mvebu_mbus_config(void __iomem *base) { const struct mbus_dram_target_info *dram; @@ -40,37 +47,6 @@ static void sdhci_mvebu_mbus_config(void __iomem *base) } } -#ifndef CONFIG_DM_MMC - -int mv_sdh_init(unsigned long regbase, u32 max_clk, u32 min_clk, u32 quirks) -{ - struct sdhci_host *host = NULL; - host = calloc(1, sizeof(*host)); - if (!host) { - printf("sdh_host malloc fail!\n"); - return -ENOMEM; - } - - host->name = MVSDH_NAME; - host->ioaddr = (void *)regbase; - host->quirks = quirks; - host->max_clk = max_clk; - - /* Configure SDHCI MBUS mbus bridge windows */ - sdhci_mvebu_mbus_config((void __iomem *)regbase); - - return add_sdhci(host, 0, min_clk); -} - -#else - -DECLARE_GLOBAL_DATA_PTR; - -struct mv_sdhci_plat { - struct mmc_config cfg; - struct mmc mmc; -}; - static int mv_sdhci_probe(struct udevice *dev) { struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); @@ -123,4 +99,3 @@ U_BOOT_DRIVER(mv_sdhci_drv) = { .priv_auto = sizeof(struct sdhci_host), .plat_auto = sizeof(struct mv_sdhci_plat), }; -#endif /* CONFIG_DM_MMC */ From 8b8820669646ceb08d6ceed4181b53042639f3ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Sat, 11 Mar 2023 11:44:27 +0100 Subject: [PATCH 7/8] mmc: Use EXT_CSD_EXTRACT_BOOT_PART() macro for extracting boot part MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mask macro PART_ACCESS_MASK filter out access bits of emmc register and macro EXT_CSD_EXTRACT_BOOT_PART() extracts boot part bits of emmc register. So use EXT_CSD_EXTRACT_BOOT_PART() when extracting boot partition. Signed-off-by: Pali Rohár Reviewed-by: Simon Glass --- board/purism/librem5/librem5.c | 2 +- cmd/mvebu/bubt.c | 3 +-- common/spl/spl_mmc.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/board/purism/librem5/librem5.c b/board/purism/librem5/librem5.c index caa02655fc4..386ed1b4fb2 100644 --- a/board/purism/librem5/librem5.c +++ b/board/purism/librem5/librem5.c @@ -41,7 +41,7 @@ int board_early_init_f(void) #if IS_ENABLED(CONFIG_LOAD_ENV_FROM_MMC_BOOT_PARTITION) uint board_mmc_get_env_part(struct mmc *mmc) { - uint part = (mmc->part_config >> 3) & PART_ACCESS_MASK; + uint part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); if (part == 7) part = 0; diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c index 49797b23144..37ff9c45522 100644 --- a/cmd/mvebu/bubt.c +++ b/cmd/mvebu/bubt.c @@ -223,8 +223,7 @@ static int mmc_burn_image(size_t image_size) orig_part = mmc->block_dev.hwpart; #endif - part = (mmc->part_config >> 3) & PART_ACCESS_MASK; - + part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); if (part == 7) part = 0; diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index bd5e6adf1ea..a0722167044 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -378,7 +378,7 @@ int default_spl_mmc_emmc_boot_partition(struct mmc *mmc) * 1 and 2 match up to boot0 / boot1 and 7 is user data * which is the first physical partition (0). */ - part = (mmc->part_config >> 3) & PART_ACCESS_MASK; + part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); if (part == 7) part = 0; #endif From fbf368f176641029ac30843d4d3dbf26e384df38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 22 Mar 2023 21:06:53 +0100 Subject: [PATCH 8/8] cmd: mmc: Return CMD_RET_* from commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Numeric return values may cause strange errors line: exit not allowed from main input shell. Signed-off-by: Pali Rohár Reviewed-by: Simon Glass --- cmd/mmc.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/cmd/mmc.c b/cmd/mmc.c index 94deb9a1686..c6bd81cebbc 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -175,7 +175,7 @@ static int do_mmcinfo(struct cmd_tbl *cmdtp, int flag, int argc, curr_device = 0; else { puts("No MMC device available\n"); - return 1; + return CMD_RET_FAILURE; } } @@ -927,7 +927,7 @@ static int mmc_partconf_print(struct mmc *mmc, const char *varname) static int do_mmc_partconf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - int dev; + int ret, dev; struct mmc *mmc; u8 ack, part_num, access; @@ -953,13 +953,17 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int flag, access = dectoul(argv[4], NULL); /* acknowledge to be sent during boot operation */ - return mmc_set_part_conf(mmc, ack, part_num, access); + ret = mmc_set_part_conf(mmc, ack, part_num, access); + if (ret != 0) + return CMD_RET_FAILURE; + + return CMD_RET_SUCCESS; } static int do_mmc_rst_func(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - int dev; + int ret, dev; struct mmc *mmc; u8 enable; @@ -988,7 +992,11 @@ static int do_mmc_rst_func(struct cmd_tbl *cmdtp, int flag, return CMD_RET_FAILURE; } - return mmc_set_rst_n_function(mmc, enable); + ret = mmc_set_rst_n_function(mmc, enable); + if (ret != 0) + return CMD_RET_FAILURE; + + return CMD_RET_SUCCESS; } #endif static int do_mmc_setdsr(struct cmd_tbl *cmdtp, int flag,