All of lore.kernel.org
 help / color / mirror / Atom feed
From: York Sun <york.sun@nxp.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] blk: Remove various places that do flush cache after read
Date: Wed, 23 Aug 2017 15:01:28 +0000	[thread overview]
Message-ID: <VI1PR04MB207859F826DC6702D9E84D349A850@VI1PR04MB2078.eurprd04.prod.outlook.com> (raw)
In-Reply-To: CAEUhbmXxyS1ZLhgoYtOHXAbOtxWwu=eYKuYDDsyZwBWXwRn2DQ@mail.gmail.com

On 08/22/2017 03:53 PM, Bin Meng wrote:
> Hi York,
> 
> On Wed, Aug 23, 2017 at 12:49 AM, York Sun <york.sun@nxp.com> wrote:
>> On 08/21/2017 08:42 PM, 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@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);
>>> -
>>
>> NACK here.
>>
>>
>>>        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
>>
>> Hold on for ppa.c. We need to flush the cache after reading PPA image
>> and it header after loading. These images need to be flushed into main
>> memory. I will need to confirm with our team if we can remove flushing here.
>>
> 
> For the first 3 parts in this ppa.c file, I see they are all reading
> PPA image from MMC while MMC driver is using DMA so this is wrong (as
> I mentioned in the commit message I suspect they are all copy/paste
> from the MMC driver).
> 
> The only questionable change is the last one which is reading
> something from NAND and if NAND driver is not using DMA, that cache
> flush is needed as you pointed out.
> 

Bin,

Our internal team agrees with the removing flushing for MMC.
Give me some time to test NAND.

York

  reply	other threads:[~2017-08-23 15:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-22  3:46 [U-Boot] [PATCH] blk: Remove various places that do flush cache after read Bin Meng
2017-08-22  6:19 ` Stefan Roese
2017-08-22  6:31   ` Bin Meng
2017-08-22 16:49 ` York Sun
2017-08-22 22:53   ` Bin Meng
2017-08-23 15:01     ` York Sun [this message]
2017-08-23  3:14   ` Qiang Zhao
2017-08-23  3:29     ` Y.b. Lu
2017-08-23  5:28       ` Bin Meng
2017-08-24  3:54         ` Y.b. Lu
2017-08-23 16:06 ` York Sun
2017-08-24 16:45 ` Joe Hershberger
2017-08-27 20:09 ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR04MB207859F826DC6702D9E84D349A850@VI1PR04MB2078.eurprd04.prod.outlook.com \
    --to=york.sun@nxp.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.