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ý <[email protected]>
>> ---
>>
>> 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
>> [email protected]
>> 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
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot
