[U-Boot] [PATCH] blk: Remove various places that do flush cache after read

Bin Meng bmeng.cn at gmail.com
Tue Aug 22 06:31:05 UTC 2017


Hi Stefan,

On Tue, Aug 22, 2017 at 2:19 PM, Stefan Roese <sr at denx.de> wrote:
> On 22.08.2017 05:46, Bin Meng wrote:
>>
>> All these places seem to inherit the codes from the MMC driver where
>> a FIXME was put in the comment. However the correct operation after
>> read should be cache invalidate, not flush.
>>
>> The underlying drivers should be responsible for the cache operation.
>> Remove these codes completely.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> ---
>>
>>   arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 15 ---------------
>>   board/toradex/common/tdx-cfg-block.c    |  2 --
>>   cmd/mmc.c                               |  2 --
>>   drivers/block/blk-uclass.c              |  3 ---
>>   drivers/block/blk_legacy.c              |  3 ---
>>   drivers/net/fm/fm.c                     |  2 --
>>   drivers/net/phy/cortina.c               |  2 --
>>   drivers/qe/qe.c                         |  2 --
>>   8 files changed, 31 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
>> index 24ddb5d..bbf8bba 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
>> @@ -107,9 +107,6 @@ int ppa_init(void)
>>                 return -EIO;
>>         }
>>   -     /* flush cache after read */
>> -       flush_cache((ulong)fitp, cnt * 512);
>> -
>>         ret = fdt_check_header(fitp);
>>         if (ret) {
>>                 free(fitp);
>> @@ -134,9 +131,6 @@ int ppa_init(void)
>>         }
>>         debug("Read PPA header to 0x%p\n", ppa_hdr_ddr);
>>   -     /* flush cache after read */
>> -       flush_cache((ulong)ppa_hdr_ddr, cnt * 512);
>> -
>>         ppa_esbc_hdr = (uintptr_t)ppa_hdr_ddr;
>>   #endif
>>   @@ -164,9 +158,6 @@ int ppa_init(void)
>>                 return -EIO;
>>         }
>>   -     /* flush cache after read */
>> -       flush_cache((ulong)ppa_fit_addr, cnt * 512);
>> -
>>   #elif defined(CONFIG_SYS_LS_PPA_FW_IN_NAND)
>>         struct fdt_header fit;
>>   @@ -208,9 +199,6 @@ int ppa_init(void)
>>         }
>>         debug("Read PPA header to 0x%p\n", ppa_hdr_ddr);
>>   -     /* flush cache after read */
>> -       flush_cache((ulong)ppa_hdr_ddr, fw_length);
>> -
>>         ppa_esbc_hdr = (uintptr_t)ppa_hdr_ddr;
>>   #endif
>>   @@ -232,9 +220,6 @@ int ppa_init(void)
>>                        CONFIG_SYS_LS_PPA_FW_ADDR);
>>                 return -EIO;
>>         }
>> -
>> -       /* flush cache after read */
>> -       flush_cache((ulong)ppa_fit_addr, fw_length);
>>   #else
>>   #error "No CONFIG_SYS_LS_PPA_FW_IN_xxx defined"
>>   #endif
>> diff --git a/board/toradex/common/tdx-cfg-block.c
>> b/board/toradex/common/tdx-cfg-block.c
>> index 328c4c0..f850a3c 100644
>> --- a/board/toradex/common/tdx-cfg-block.c
>> +++ b/board/toradex/common/tdx-cfg-block.c
>> @@ -129,8 +129,6 @@ static int tdx_cfg_block_mmc_storage(u8 *config_block,
>> int write)
>>                         ret = -EIO;
>>                         goto out;
>>                 }
>> -               /* Flush cache after read */
>> -               flush_cache((ulong)(unsigned char *)config_block, 512);
>>         } else {
>>                 /* Just writing one 512 byte block */
>>                 if (blk_dwrite(mmc_get_blk_desc(mmc), blk_start, 1,
>> diff --git a/cmd/mmc.c b/cmd/mmc.c
>> index 00697fc..5def4ea 100644
>> --- a/cmd/mmc.c
>> +++ b/cmd/mmc.c
>> @@ -293,8 +293,6 @@ static int do_mmc_read(cmd_tbl_t *cmdtp, int flag,
>>                curr_device, blk, cnt);
>>         n = blk_dread(mmc_get_blk_desc(mmc), blk, cnt, addr);
>> -       /* flush cache after read */
>> -       flush_cache((ulong)addr, cnt * 512); /* FIXME */
>>         printf("%d blocks read: %s\n", n, (n == cnt) ? "OK" : "ERROR");
>>         return (n == cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
>> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
>> index 3aec569..e5f00dc 100644
>> --- a/drivers/block/blk-uclass.c
>> +++ b/drivers/block/blk-uclass.c
>> @@ -294,9 +294,6 @@ ulong blk_read_devnum(enum if_type if_type, int
>> devnum, lbaint_t start,
>>         if (IS_ERR_VALUE(n))
>>                 return n;
>>   -     /* flush cache after read */
>> -       flush_cache((ulong)buffer, blkcnt * desc->blksz);
>> -
>>         return n;
>>   }
>>   diff --git a/drivers/block/blk_legacy.c b/drivers/block/blk_legacy.c
>> index 981872e..16d3bfe 100644
>> --- a/drivers/block/blk_legacy.c
>> +++ b/drivers/block/blk_legacy.c
>> @@ -232,9 +232,6 @@ ulong blk_read_devnum(enum if_type if_type, int
>> devnum, lbaint_t start,
>>         if (IS_ERR_VALUE(n))
>>                 return n;
>>   -     /* flush cache after read */
>> -       flush_cache((ulong)buffer, blkcnt * desc->blksz);
>> -
>>         return n;
>>   }
>>   diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c
>> index 451dfde..261f1b9 100644
>> --- a/drivers/net/fm/fm.c
>> +++ b/drivers/net/fm/fm.c
>> @@ -405,8 +405,6 @@ int fm_init_common(int index, struct ccsr_fman *reg)
>>                 mmc_init(mmc);
>>                 (void)mmc->block_dev.block_read(&mmc->block_dev, blk, cnt,
>>                                                 addr);
>> -               /* flush cache after read */
>> -               flush_cache((ulong)addr, cnt * 512);
>>         }
>>   #elif defined(CONFIG_SYS_QE_FMAN_FW_IN_REMOTE)
>>         void *addr = (void *)CONFIG_SYS_FMAN_FW_ADDR;
>> diff --git a/drivers/net/phy/cortina.c b/drivers/net/phy/cortina.c
>> index e0e9ed9..637d89a 100644
>> --- a/drivers/net/phy/cortina.c
>> +++ b/drivers/net/phy/cortina.c
>> @@ -177,8 +177,6 @@ void cs4340_upload_firmware(struct phy_device *phydev)
>>                 mmc_init(mmc);
>>                 (void)mmc->block_dev.block_read(&mmc->block_dev, blk, cnt,
>>                                                 addr);
>> -               /* flush cache after read */
>> -               flush_cache((ulong)addr, cnt * 512);
>>         }
>>   #endif
>>   diff --git a/drivers/qe/qe.c b/drivers/qe/qe.c
>> index 24e764d..931c9d9 100644
>> --- a/drivers/qe/qe.c
>> +++ b/drivers/qe/qe.c
>> @@ -221,8 +221,6 @@ void u_qe_init(void)
>>                 mmc_init(mmc);
>>                 (void)mmc->block_dev.block_read(&mmc->block_dev, blk, cnt,
>>                                                 addr);
>> -               /* flush cache after read */
>> -               flush_cache((ulong)addr, cnt * 512);
>>         }
>>   #endif
>>         u_qe_upload_firmware(addr);
>>
>
> Did you experience any problems with the "incorrect" code in the
> MMC driver?
>

No, since I have been mostly working on x86, and on x86 cache
flush/invalidate is a nop, so no problems were seen.

> Without looking into the underlying driver, your explanation makes
> perfect sense and flushing is wrong after a read of course. So:
>

Yes, I doubt how these codes were working on other platforms where
cache operations are really needed. I just noticed that when I was
working on NVMe stuff.

> Reviewed-by: Stefan Roese <sr at denx.de>

Regards,
Bin


More information about the U-Boot mailing list