From: Ana Bobillo <[email protected]> mmc_startup() sets the block size of the card's block device from the READ_BL_LEN field of its CSD and only caps it at 512 bytes from above. A card reporting a READ_BL_LEN below 9, which the SD specification reserves, gives the block device a block size below 512 bytes. A block that small cannot hold an MBR or a GPT header, and the partition and filesystem code assumes at least 512 bytes, as the patch adding blk_valid_blksz() describes. Nor does the code bound an MMC card's WRITE_BL_LEN from below, though mmc_bwrite() then writes blocks of that length where the block layer passes 512-byte ones.
The check in blk_post_probe() does not catch this. It is left out of SPL, TPL and VPL, which commonly boot from MMC. In U-Boot proper, mmc_startup() runs again whenever the card is initialised again, by 'mmc rescan' or 'mmc dev', or after the card-detect poll has seen it removed, and sets the block size of a block device that is already probed. Use 512-byte blocks whatever READ_BL_LEN and WRITE_BL_LEN say, as the code already did in DDR mode and as Linux does for its transfers; the capacity is still computed with the block length the card reports. Only some old MultiMediaCards could have blocks that really are smaller, and the partition and filesystem code could not use those either. This makes the DDR case redundant, so drop it. The SPL does not grow: imx28_btt3's, close to its size limit, gets 16 bytes smaller. Make the READ_BL_LEN that the sandbox MMC emulator reports settable, and test that a card initialised again with a READ_BL_LEN of 0, 8 or 11 gets 512-byte blocks and can be written and read back. Signed-off-by: Ana Bobillo <[email protected]> --- Changes in v3: - New patch: blk_post_probe() does not see the block size the MMC code sets when it initialises a card again, as the v2 cover letter said arch/sandbox/include/asm/test.h | 10 +++++++++ drivers/mmc/mmc.c | 34 +++++++++------------------- drivers/mmc/sandbox_mmc.c | 11 ++++++++- test/dm/mmc.c | 40 +++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 24 deletions(-) diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h index 76e2f11b059..f73bf51ef35 100644 --- a/arch/sandbox/include/asm/test.h +++ b/arch/sandbox/include/asm/test.h @@ -143,6 +143,16 @@ void sandbox_flash_set_block_size(struct udevice *dev, int block_size); */ void sandbox_scsi_set_block_size(struct udevice *dev, int block_size); +/** + * sandbox_mmc_set_bl_len_shift() - Set the READ_BL_LEN an SD card reports + * + * @dev: Sandbox MMC controller + * @bl_len_shift: log2 of the block length, as the CSD's READ_BL_LEN field + * + * The host reads it when it next initialises the card. + */ +void sandbox_mmc_set_bl_len_shift(struct udevice *dev, int bl_len_shift); + /** * sandbox_osd_get_mem() - get the internal memory of a sandbox OSD * diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 4835ce0c64b..90331823926 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -2555,7 +2555,7 @@ error: static int mmc_startup(struct mmc *mmc) { int err, i; - uint mult, freq; + uint mult, freq, bl_len; u64 cmult, csize; struct mmc_cmd cmd; struct blk_desc *bdesc; @@ -2652,14 +2652,7 @@ static int mmc_startup(struct mmc *mmc) mmc_select_mode(mmc, MMC_LEGACY); mmc->dsr_imp = ((cmd.response[1] >> 12) & 0x1); - mmc->read_bl_len = 1 << ((cmd.response[1] >> 16) & 0xf); -#if CONFIG_IS_ENABLED(MMC_WRITE) - - if (IS_SD(mmc)) - mmc->write_bl_len = mmc->read_bl_len; - else - mmc->write_bl_len = 1 << ((cmd.response[3] >> 22) & 0xf); -#endif + bl_len = 1 << ((cmd.response[1] >> 16) & 0xf); if (mmc->high_capacity) { csize = (mmc->csd[1] & 0x3f) << 16 @@ -2672,18 +2665,21 @@ static int mmc_startup(struct mmc *mmc) } mmc->capacity_user = (csize + 1) << (cmult + 2); - mmc->capacity_user *= mmc->read_bl_len; + mmc->capacity_user *= bl_len; mmc->capacity_boot = 0; mmc->capacity_rpmb = 0; for (i = 0; i < 4; i++) mmc->capacity_gp[i] = 0; - if (mmc->read_bl_len > MMC_MAX_BLOCK_LEN) - mmc->read_bl_len = MMC_MAX_BLOCK_LEN; - + /* + * Use 512-byte blocks whatever READ_BL_LEN and WRITE_BL_LEN say. A + * larger value is the most the card takes in one block, and a smaller + * one, which the SD specification reserves, would give the block + * device a block size the partition and filesystem code cannot use. + */ + mmc->read_bl_len = MMC_MAX_BLOCK_LEN; #if CONFIG_IS_ENABLED(MMC_WRITE) - if (mmc->write_bl_len > MMC_MAX_BLOCK_LEN) - mmc->write_bl_len = MMC_MAX_BLOCK_LEN; + mmc->write_bl_len = MMC_MAX_BLOCK_LEN; #endif if ((mmc->dsr_imp) && (0xffffffff != mmc->dsr)) { @@ -2765,14 +2761,6 @@ static int mmc_startup(struct mmc *mmc) mmc->best_mode = mmc->selected_mode; - /* Fix the block length for DDR mode */ - if (mmc->ddr_mode) { - mmc->read_bl_len = MMC_MAX_BLOCK_LEN; -#if CONFIG_IS_ENABLED(MMC_WRITE) - mmc->write_bl_len = MMC_MAX_BLOCK_LEN; -#endif - } - /* fill in device description */ bdesc = mmc_get_blk_desc(mmc); bdesc->lun = 0; diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c index a24520f2e78..34a5c7506ab 100644 --- a/drivers/mmc/sandbox_mmc.c +++ b/drivers/mmc/sandbox_mmc.c @@ -29,6 +29,7 @@ struct sandbox_mmc_plat { struct sandbox_mmc_priv { char *buf; int csize; /* CSIZE value to report */ + int bl_len_shift; /* READ_BL_LEN value to report */ int size; }; @@ -62,7 +63,7 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, break; case MMC_CMD_SEND_CSD: cmd->response[0] = 0; - cmd->response[1] = (MMC_BL_LEN_SHIFT << 16) | + cmd->response[1] = (priv->bl_len_shift << 16) | ((priv->csize >> 16) & 0x3f); cmd->response[2] = (priv->csize & 0xffff) << 16; cmd->response[3] = 0; @@ -144,6 +145,13 @@ static const struct dm_mmc_ops sandbox_mmc_ops = { .get_cd = sandbox_mmc_get_cd, }; +void sandbox_mmc_set_bl_len_shift(struct udevice *dev, int bl_len_shift) +{ + struct sandbox_mmc_priv *priv = dev_get_priv(dev); + + priv->bl_len_shift = bl_len_shift; +} + static int sandbox_mmc_of_to_plat(struct udevice *dev) { struct sandbox_mmc_plat *plat = dev_get_plat(dev); @@ -169,6 +177,7 @@ static int sandbox_mmc_probe(struct udevice *dev) struct sandbox_mmc_priv *priv = dev_get_priv(dev); int ret; + priv->bl_len_shift = MMC_BL_LEN_SHIFT; if (plat->fname) { ret = os_map_file(plat->fname, OS_O_RDWR | OS_O_CREAT, (void **)&priv->buf, &priv->size); diff --git a/test/dm/mmc.c b/test/dm/mmc.c index cdebb955196..d393883855c 100644 --- a/test/dm/mmc.c +++ b/test/dm/mmc.c @@ -6,6 +6,7 @@ #include <dm.h> #include <mmc.h> #include <part.h> +#include <asm/test.h> #include <dm/test.h> #include <test/test.h> #include <test/ut.h> @@ -51,3 +52,42 @@ static int dm_test_mmc_blk(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_mmc_blk, UTF_SCAN_PDATA | UTF_SCAN_FDT); + +/* Test that a card with a READ_BL_LEN other than 9 gets 512-byte blocks */ +static int dm_test_mmc_bl_len(struct unit_test_state *uts) +{ + static const int shift[] = { 0, 8, 11 }; + struct blk_desc *dev_desc; + char write[512], read[512]; + struct udevice *dev; + struct mmc *mmc; + int i, j; + + ut_assertok(uclass_get_device_by_seq(UCLASS_MMC, 0, &dev)); + ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc)); + mmc = mmc_get_mmc_dev(dev); + + for (i = 0; i < ARRAY_SIZE(shift); i++) { + /* Initialise the card again, as 'mmc rescan' does */ + sandbox_mmc_set_bl_len_shift(dev, shift[i]); + mmc->has_init = 0; + ut_assertok(mmc_init(mmc)); + + ut_asserteq(512, mmc->read_bl_len); + ut_asserteq(512, mmc->write_bl_len); + ut_asserteq(512, dev_desc->blksz); + ut_asserteq(9, dev_desc->log2blksz); + + /* mmc0 reports 1024 blocks of the length in READ_BL_LEN */ + ut_asserteq((1024ULL << shift[i]) / 512, dev_desc->lba); + + for (j = 0; j < sizeof(write); j++) + write[j] = i + j; + ut_asserteq(1, blk_dwrite(dev_desc, 1, 1, write)); + ut_asserteq(1, blk_dread(dev_desc, 1, 1, read)); + ut_asserteq_mem(write, read, sizeof(write)); + } + + return 0; +} +DM_TEST(dm_test_mmc_bl_len, UTF_SCAN_PDATA | UTF_SCAN_FDT);
