[U-Boot] [PATCH 2/6] riscv: remove invalid dcache flush implementation

[Auer, Lukas]

Hi Rick,

On Thu, 2019-01-03 at 08:48 +0800, Rick Chen wrote:
> Hi Lukas
> 
> Auer, Lukas <lukas.auer at aisec.fraunhofer.de> 於 2019年1月2日 週三 下午8:22寫道：
> > Hi Rick,
> > 
> > On Wed, 2019-01-02 at 10:54 +0800, Rick Chen wrote:
> > > Hi Lukas
> > > 
> > > > > From: Lukas Auer [mailto:lukas.auer at aisec.fraunhofer.de]
> > > > > Sent: Monday, December 31, 2018 2:28 AM
> > > > > To: u-boot at lists.denx.de
> > > > > Cc: Anup Patel; Lukas Auer; Rick Jian-Zhi Chen(陳建志); Bin
> > > > > Meng;
> > > > > Greentime Hu
> > > > > Subject: [PATCH 2/6] riscv: remove invalid dcache flush
> > > > > implementation
> > > > > 
> > > > > The fence instruction is used to enforce device I/O and
> > > > > memory
> > > > > ordering
> > > > > constraints in RISC-V. It does not directly affect the data
> > > > > cache
> > > > > and particular
> > > > > cannot be used to flush or invalidate it. RISC-V does not
> > > > > have
> > > > > instructions for
> > > > > explicit cache control. Remove the flush_dcache_all
> > > > > implementation and its use
> > > > > in all dcache-specific functions in lib/cache.c.
> > > > > 
> > > 
> > > The privileged mention that fence is for the purposes of
> > > ordering,
> > > but
> > > does not say
> > > it can not be used to flush or invalidate.
> > > 
> > > Andes's ax25 have no coherence agent. So it need fence
> > > instruction to
> > > manipulate
> > > cache flush or invalidate. Or there will be some data
> > > synchronization
> > > error
> > > occurs on ae350 platform.
> > > 
> > 
> > I was not aware of this. Does the ax25 flush the whole dcache on
> > any
> > fence instruction?
> 
> Yes.
> It will flush the whole dcache when execute fence.
> 
> > You are right, we can't remove it in this case. Since it is
> > specific to
> > the ax25, should I move it into cpu/ax25/cache.c instead?
> 
> OK
> It is good to me.
> And thanks for your understanding and improvements.
> 

You are welcome! I just submitted version 2 of the patch series.

Thanks,
Lukas

> Regards
> Rick
> 
> > Thanks,
> > Lukas
> > 
> > > Thanks
> > > Rick
> > > 
> > > 
> > > > > This also adds a missing new line between flush_dcache_all
> > > > > and
> > > > > flush_dcache_range in lib/cache.c.
> > > > > 
> > > > > Signed-off-by: Lukas Auer <lukas.auer at aisec.fraunhofer.de>
> > > > > ---
> > > > > This patch only removes the implementation itself and its use
> > > > > in
> > > > > dcache-specific
> > > > > functions in lib/cache.c. There are more uses of it in
> > > > > arch/riscv/, which this patch
> > > > > does not remove.
> > > > > 
> > > > >  arch/riscv/lib/cache.c | 4 +---
> > > > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c
> > > > > index
> > > > > ae5c60716f..203e287612 100644
> > > > > --- a/arch/riscv/lib/cache.c
> > > > > +++ b/arch/riscv/lib/cache.c
> > > > > @@ -13,11 +13,10 @@ void invalidate_icache_all(void)
> > > > > 
> > > > >  void flush_dcache_all(void)
> > > > >  {
> > > > > -     asm volatile ("fence" :::"memory");
> > > > >  }
> > > > > +
> > > > >  void flush_dcache_range(unsigned long start, unsigned long
> > > > > end)  {
> > > > > -     flush_dcache_all();
> > > > >  }
> > > > > 
> > > > >  void invalidate_icache_range(unsigned long start, unsigned
> > > > > long
> > > > > end) @@ -31,7
> > > > > +30,6 @@ void invalidate_icache_range(unsigned long start,
> > > > > unsigned long end)
> > > > > 
> > > > >  void invalidate_dcache_range(unsigned long start, unsigned
> > > > > long
> > > > > end)  {
> > > > > -     flush_dcache_all();
> > > > >  }
> > > > > 
> > > > >  void cache_flush(void)
> > > > > --
> > > > > 2.20.1