Hi Stefan, On Tue, Aug 22, 2017 at 2:19 PM, Stefan Roese <s...@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...@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 <s...@denx.de> Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot