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

[Jan Sedivy]

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 at 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 at 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