[U-Boot] [PATCH] tegra2: Enable data cache

[Mike Frysinger]

On Thursday, September 15, 2011 12:21:28 Albert ARIBAUD wrote:
> Le 15/09/2011 05:03, Mike Frysinger a écrit :
> > On Wednesday, September 14, 2011 11:13:40 Simon Glass wrote:
> >> On Wed, Sep 14, 2011 at 6:10 AM, Aneesh V wrote:
> >>>> not specific to your patch, but this seems kind of dumb to copy& 
> >>>> paste the same thing between all the arm sub arches/boards.  why cant
> >>>> the default enable_caches() look like this for arm:
> >>>> void enable_caches(void)
> >>>> {
> >>>> #ifndef CONFIG_SYS_DCACHE_OFF
> >>>>        dcache_enable();
> >>>> #endif
> >>>> #ifndef CONFIG_SYS_ICACHE_OFF
> >>>>        icache_enable();
> >>>> #endif
> >>>> }
> >>> 
> >>> That was how it was earlier. But then many boards were not cache ready
> >>> and still didn't define CONFIG_SYS_DCACHE_OFF, so they were broken. So,
> >>> the current situation is that the absence of CONFIG_SYS_DCACHE_OFF
> >>> doesn't mean that the board is cache-ready.
> >>> 
> >>> I like the suggestion made by Jason Liu, that of using
> >>> CONFIG_SYS_DCACHE_ON instead. In the present situation ARM cpus that
> >>> properly support cache handling seems to be in the minority, so
> >>> CONFIG_SYS_DCACHE_ON may be more appropriate. But Wolfgang doesn't seem
> >>> to like this.
> >> 
> >> My concern with this is that the situation will never improve. At
> >> least this way there will be an incentive to sort things out, since
> >> one day the cache enable code will be in arch/arm/lib/board.c. I hope
> >> it is soon, but it will need more driver support first.
> > 
> > right ... this sounds like the sort of thing that takes much longer than
> > it should.  i'm not the arm maintainer, but it seems like it'd have been
> > better to add CONFIG_SYS_{I,D}CACHE_OFF to all the arm board configs and
> > leave it up to the maintainers to turn off.  then there'd be no
> > duplication in core code and no long term migration need.
> 
> Actually, the idea was rather to have the config options control whether
> the code for caches is compiled or not, and the default code for caches
> does nothing but emit a warning on the console, thereby persistently
> nagging board msaintainers and developers to fix the issue -- whereas
> adding CACHE_OFF on config files would not cause any message and may
> cause the board to remain with caches off for as long as nobody
> complains enough.

runtime warnings are useful only if someone actually executes that code.  
build time warnings however are much more prominent.

> > until that happens, CONFIG_SYS_{D,I}CACHE_ON sounds like a reasonable way
> > to get things back under control.
> 
> Nak. The right way is to make sure we have an ARM-wide default
> implementation (in arch/arm/lib) that does nothing and warns on the
> console about caches NOT being actually enabled, and cpu or SoC
> implementations (e.g. in arch/arm/cpu/arm926ejs) that actually enable
> caches on hardware on which it works.

and now we're forced to copy & paste the same chunk of code everywhere for 
people who went through the effort to make things work.  caches not working is 
a matter of the software not handling it, not the hardware being incapable of 
supporting things.

the state of arm systems not supporting cache should be considered legacy 
cruft, not the default starting state.  when people add new SoC's, they should 
be defaulting to using cache, and if they dont want it, they have to manually 
add to their board configs "turn it off".
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110915/3c9656dc/attachment.pgp