[U-Boot] ARM v7: Flush icache when executing a program with go

[Tom Rini <trini@ti.com>]

On Thu, May 16, 2013 at 09:14:06AM +0200, Wolfgang Denk wrote:
> Dear Henrik Nordstr??m,
> 
> In message <1368669278.27007.43.camel@localhost> you wrote:
> > 
> > > So my suggestion is to implement the icache_flush in common/bmmt_cmd.c
> > > as follows:
> ...
> > From what I can tell there is no need to theck icache_status(). It's
> > always safe to call invalidate_icache_all().
> > 
> > So it's back to use the compiler define for ARMv7.
> 
> I don't think this would be an acceptable approach.  There are several
> serious issues with the suggested patch.
> 
> 
> First, I think your solution in not complete.   Invalidating the
> instruction cache is only one part of what needs to be done.  You
> should also make sure to flush the data cache.
> 
> Second, flushing _all_ caches may be a time consuming operation on
> some systems, and it is not necessary.  It should be sufficient to
> flush the address range where the loaded code resides.
> 
> Third, Albert is perfectly right:  there is no reason to make this
> architecture specific.  We should NOT add such code here and there on
> a per-arch base; this results only in code fragmentation, duplication
> and a maintenance nightmare.
> 
> Finally, a very similar topic has been discussed here not so long ago;
> please see the thread "command/cache: Add flush command" at [1], [2],
> and [3]; see especially the attempt of a summary at [4].

That this topic keeps coming up is one of the reasons I asked Henrik to
post this patch when I was looking over the Allwinner support queue.  I
thought this was a rather clever fixup.

> [1] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/156449
> [2] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158038
> [3] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158101
> 
> [4] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158101/focus=158559
> 
> 
> In the summary I tried to explain what I think we should do to strive
> for more common code; unfortunately did not follow this suggestion but
> decided to keep his code out-of-tree.  But I still think this is what
> should be done, also in your case.
> 
> We should not add architecture-specific code like you suggested.

The problem is that with go we can't know 'size' for go because it's
just a binary blob, so we need, roughly, flush_cache(gd->ram_top -
gd->reloc_off, gd->ram_size) yes?  That I believe is one of the points
that wasn't solved in the last go-round here.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130516/7a987476/attachment.pgp>