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
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
