[U-Boot] [PATCH] MMC HW partition switching must also invalidate the cache

Hi Jan,
Subject: [EXT] [U-Boot] [PATCH] MMC HW partition switching must also invalidate the cache
Please add a bit more commit information about why and how.
Thanks, Peng.
Signed-off-by: Jan Šedivý jans@zhinst.com
drivers/mmc/mmc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 456c1b4..3d9a68e 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -954,8 +954,13 @@ int mmc_switch_part(struct mmc *mmc, unsigned int part_num) * to return to representing the raw device. */ if ((ret == 0) || ((ret == -ENODEV) && (part_num == 0))) {
struct blk_desc *desc = mmc_get_blk_desc(mmc);
ret = mmc_set_capacity(mmc, part_num);
mmc_get_blk_desc(mmc)->hwpart = part_num;
if (desc && desc->hwpart != part_num) {
desc->hwpart = part_num;
blkcache_invalidate(desc->if_type,
desc->devnum);
} } return ret;
@@ -2673,7 +2678,12 @@ retry: return err;
/* The internal partition reset to user partition(0) at every CMD0*/
mmc_get_blk_desc(mmc)->hwpart = 0;
struct blk_desc *desc = mmc_get_blk_desc(mmc);
if (desc && desc->hwpart != 0) {
desc->hwpart = 0;
blkcache_invalidate(desc->if_type, desc->devnum);
} /* Test for SD version 2 */ err = mmc_send_if_cond(mmc);
-- 1.7.9.5
U-Boot mailing list U-Boot@lists.denx.de https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d enx.de%2Flistinfo%2Fu-boot&data=02%7C01%7CPeng.Fan%40nxp.com %7Cb99d933608be4817e5f408d6bef7d8a5%7C686ea1d3bc2b4c6fa92cd99c5 c301635%7C0%7C1%7C636906369056326740&sdata=k%2Fo2bgjU1bdl pLyk5nv78yrX3P9fqaVmukcbB2nTvF8%3D&reserved=0

Hi Peng, thank you for the response, I am not experienced with this way of committing change requests. Should I resubmit the patch again with more info, or is it enough, if I post the detailed description here?
PROBLEM: Read access to the eMMC now goes through blk_dread, which tries first to fetch the requested data from block cache using blkcache_read. Unfortunately, only the device type and slot id is used when accessing the cache: blkcache_read(block_dev->if_type, block_dev->devnum, ...) which is correct for many types of devices, but not for the eMMC, where one physical chip (devnum) do contain multiple physical partitions, selectable via "mmc dev devnum hwpart"
As result "mmc read ${my_address} 0 1" does only access the physical device if the sector 0 of the selected devnum was not read before. As a consequence, reading sector 0 from the first boot partition "mmc dev 0 1 && mmc read ${my_address} 0 1" followed by filesystem access to the primary partition "mmc dev 0 0 && mmc part" fails as the MBR at sector 0 is not read from the physical device, but instead from the cache.
SOLUTION: As the cache system is quite simple, any write or erase access (blk_dwrite, blk_derase) invalidates the entire cache for the given device and it should not be a performance limiter if the cache is also invalidated on the hwpart change. In theory, this could be done in blk_select_hwpart, but as this function is called on every filesystem access, the invalidation should be performed just in case the hwpart requested is different, than the one already set. This would be possible, if ops->select_hwpart() would be the only way to change the device HW partition. This is not the case on the mmc access where the hwpart is reset to zero at every CMD0 (see the comment in the original code), in which case, the cache should be invalidated from within the mmc module itself at all places, where the hwpart is modified.
Regards, Jan
On 26.4.2019 8:45, Peng Fan wrote:
Hi Jan,
Subject: [EXT] [U-Boot] [PATCH] MMC HW partition switching must also invalidate the cache
Please add a bit more commit information about why and how.
Thanks, Peng.
Signed-off-by: Jan Šedivý jans@zhinst.com
drivers/mmc/mmc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 456c1b4..3d9a68e 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -954,8 +954,13 @@ int mmc_switch_part(struct mmc *mmc, unsigned int part_num) * to return to representing the raw device. */ if ((ret == 0) || ((ret == -ENODEV) && (part_num == 0))) {
struct blk_desc *desc = mmc_get_blk_desc(mmc);
ret = mmc_set_capacity(mmc, part_num);
mmc_get_blk_desc(mmc)->hwpart = part_num;
if (desc && desc->hwpart != part_num) {
desc->hwpart = part_num;
blkcache_invalidate(desc->if_type,
desc->devnum);
} } return ret;
@@ -2673,7 +2678,12 @@ retry: return err;
/* The internal partition reset to user partition(0) at every CMD0*/
mmc_get_blk_desc(mmc)->hwpart = 0;
struct blk_desc *desc = mmc_get_blk_desc(mmc);
if (desc && desc->hwpart != 0) {
desc->hwpart = 0;
blkcache_invalidate(desc->if_type, desc->devnum);
} /* Test for SD version 2 */ err = mmc_send_if_cond(mmc);
-- 1.7.9.5
U-Boot mailing list U-Boot@lists.denx.de https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d enx.de%2Flistinfo%2Fu-boot&data=02%7C01%7CPeng.Fan%40nxp.com %7Cb99d933608be4817e5f408d6bef7d8a5%7C686ea1d3bc2b4c6fa92cd99c5 c301635%7C0%7C1%7C636906369056326740&sdata=k%2Fo2bgjU1bdl pLyk5nv78yrX3P9fqaVmukcbB2nTvF8%3D&reserved=0
participants (2)
-
Jan Sedivy
-
Peng Fan