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

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

[Simon Glass]

This enables the data cache on Tegra2 boards.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/nvidia/common/board.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index 160dac8..031cc68 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -307,3 +307,11 @@ int board_mmc_getcd(u8 *cd, struct mmc *mmc)
 	return 0;
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
-- 
1.7.3.1

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

[Mike Frysinger]

On Thursday, September 08, 2011 18:20:16 Simon Glass wrote:
> --- a/board/nvidia/common/board.c
> +++ b/board/nvidia/common/board.c
> @@ -307,3 +307,11 @@ int board_mmc_getcd(u8 *cd, struct mmc *mmc)
>  	return 0;
>  }
>  #endif
> +
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +void enable_caches(void)
> +{
> +	/* Enable D-cache. I-cache is already enabled in start.S */
> +	dcache_enable();
> +}
> +#endif

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
}
-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/20110908/1d7cee40/attachment.pgp 

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

[Marek Vasut]

On Friday, September 09, 2011 02:25:46 AM Mike Frysinger wrote:
> On Thursday, September 08, 2011 18:20:16 Simon Glass wrote:
> > --- a/board/nvidia/common/board.c
> > +++ b/board/nvidia/common/board.c
> > @@ -307,3 +307,11 @@ int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> > 
> >  	return 0;
> >  
> >  }
> >  #endif
> > 
> > +
> > +#ifndef CONFIG_SYS_DCACHE_OFF
> > +void enable_caches(void)
> > +{
> > +	/* Enable D-cache. I-cache is already enabled in start.S */
> > +	dcache_enable();
> > +}
> > +#endif
> 
> 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
> }
> -mike

Makes sense ... and also, we won't eventually need this "enable_caches()" at all 
in cpu-directories. It can then be moved to lib/.

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

[Simon Glass]

Hi,

On Thu, Sep 8, 2011 at 5:30 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On Friday, September 09, 2011 02:25:46 AM Mike Frysinger wrote:
>> On Thursday, September 08, 2011 18:20:16 Simon Glass wrote:
>> > --- a/board/nvidia/common/board.c
>> > +++ b/board/nvidia/common/board.c
>> > @@ -307,3 +307,11 @@ int board_mmc_getcd(u8 *cd, struct mmc *mmc)
>> >
>> > ? ? return 0;
>> >
>> > ?}
>> > ?#endif
>> >
>> > +
>> > +#ifndef CONFIG_SYS_DCACHE_OFF
>> > +void enable_caches(void)
>> > +{
>> > + ? /* Enable D-cache. I-cache is already enabled in start.S */
>> > + ? dcache_enable();
>> > +}
>> > +#endif
>>
>> 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
>> }
>> -mike

See the patch here:

http://lists.denx.de/pipermail/u-boot/2011-August/098435.html

There was some discussion about this and apparently we need the dcache
to default off on ARM even though CONFIG_SYS_DCACHE_OFF is not defined
in the board config header. This is because many ARM boards are still
broken. It is a little unfortunate.

I could modify my patch to be in arch/arm/cpu/armv7/tegra2 if that
will help? I hope that this is only a temporary situation.

>
> Makes sense ... and also, we won't eventually need this "enable_caches()" at all
> in cpu-directories. It can then be moved to lib/.

Yes that's where it was at first. I was quite happy with it there. It
actually got called :-)

BTW note that the icache is enabled much earlier, normally as one of
the first things after boot, certainly prior to relocation.

Regards,
Simon

> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

[Aneesh V]

Hi Mike,

On Friday 09 September 2011 05:55 AM, Mike Frysinger wrote:
> On Thursday, September 08, 2011 18:20:16 Simon Glass wrote:
>> --- a/board/nvidia/common/board.c
>> +++ b/board/nvidia/common/board.c
>> @@ -307,3 +307,11 @@ int board_mmc_getcd(u8 *cd, struct mmc *mmc)
>>  	return 0;
>>  }
>>  #endif
>> +
>> +#ifndef CONFIG_SYS_DCACHE_OFF
>> +void enable_caches(void)
>> +{
>> +	/* Enable D-cache. I-cache is already enabled in start.S */
>> +	dcache_enable();
>> +}
>> +#endif
> 
> 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.

best regards,
Aneesh

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

[Simon Glass]

Hi Aneesh,

On Wed, Sep 14, 2011 at 6:10 AM, Aneesh V <aneesh@ti.com> wrote:
[snip]
>> 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.

Regards,
Simon

>
> best regards,
> Aneesh
>

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

[Mike Frysinger]

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.

until that happens, CONFIG_SYS_{D,I}CACHE_ON sounds like a reasonable way to 
get things back under control.
-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/20110914/47ce99ef/attachment.pgp 

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

[Albert ARIBAUD]

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.

> 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.

> -mike

Amicalement,
-- 
Albert.

[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 

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

[Albert ARIBAUD]

Le 15/09/2011 18:31, Mike Frysinger a ?crit :
> 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.

We are not forced to copy and paste -- see below (or see the RFC directly)

> 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".

I agree on the need and interest to support cache. I disagree on the way 
to get there. Turning cache off indiscriminately and hoping that board 
developers/maintainer turn their caches off will only lead to U-Boot 
failing in interesting ways and cluttering the mailing list with 
requests for help until someone figures out the cache is the culprit.

We have started outlining a solution (weak generic cache (non-)functions 
in arch/arm/lib/cache.c which only warn about cache not being enabled, 
and cpu/SoC/board specific versions that enable cache where it works.

We also have started discussing minimizing cache code copy -- admittedly 
spinned off a patch submission discussion, so it may have been missed, 
and I will repost the RFC in its own thread -- whereby the cache 
functions  would be defined as weak function in the lib, and could be 
overriden by ISA-level function, which themselves could be overriden by 
CPU-level functions, and SoC (or even board) level functions could 
override them all.

So for those part of ARM boards which are all based on, say, an ISA for 
which almost all CPUs share the same cache functionality, that 
functionality would be in a single copy at the ISA level, with 
exceptions for the relevant CPUs. And if a SoC is an exception to its 
CPU, then only this SoC would have to override part or all of the 
CPU-provided cache functions.

> -mike

Amicalement,
-- 
Albert.

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

[Mike Frysinger]

On Thursday, September 15, 2011 13:30:09 Albert ARIBAUD wrote:
> I agree on the need and interest to support cache. I disagree on the way
> to get there. Turning cache off indiscriminately and hoping that board
> developers/maintainer turn their caches off will only lead to U-Boot
> failing in interesting ways and cluttering the mailing list with
> requests for help until someone figures out the cache is the culprit.

if code only works when cache is turned on, that usually means the code is 
broken and is missing appropriate barriers/flushes.  turning cache on doesnt 
fix the underlying issue, it ignores it.  ive seen this in the past with 
Blackfin parts where our testers will do some operation with caches on, and 
then try things with caches off, and trigger bugs in drivers due to missing 
syncs but things work due to implicit timings with cache on.

> We have started outlining a solution (weak generic cache (non-)functions
> in arch/arm/lib/cache.c which only warn about cache not being enabled,
> and cpu/SoC/board specific versions that enable cache where it works.

which is causing the copy & paste i'm whining about

> We also have started discussing minimizing cache code copy -- admittedly
> spinned off a patch submission discussion, so it may have been missed,
> and I will repost the RFC in its own thread -- whereby the cache
> functions  would be defined as weak function in the lib, and could be
> overriden by ISA-level function, which themselves could be overriden by
> CPU-level functions, and SoC (or even board) level functions could
> override them all.

i guess i did miss it.  it doesnt matter to me how we get to the ultimate 
destination ({I,D}CACHE_ON/etc...), just that ultimately the crappy/dead SoC's 
are not bogging down the modern/maintained ones by forcing the common 
implementation to suck.
-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/ce5bdfdf/attachment.pgp 

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

[Simon Glass]

Hi,

On Thu, Sep 15, 2011 at 10:41 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday, September 15, 2011 13:30:09 Albert ARIBAUD wrote:
>> I agree on the need and interest to support cache. I disagree on the way
>> to get there. Turning cache off indiscriminately and hoping that board
>> developers/maintainer turn their caches off will only lead to U-Boot
>> failing in interesting ways and cluttering the mailing list with
>> requests for help until someone figures out the cache is the culprit.
>
> if code only works when cache is turned on, that usually means the code is
> broken and is missing appropriate barriers/flushes. ?turning cache on doesnt
> fix the underlying issue, it ignores it. ?ive seen this in the past with
> Blackfin parts where our testers will do some operation with caches on, and
> then try things with caches off, and trigger bugs in drivers due to missing
> syncs but things work due to implicit timings with cache on.
>
>> We have started outlining a solution (weak generic cache (non-)functions
>> in arch/arm/lib/cache.c which only warn about cache not being enabled,
>> and cpu/SoC/board specific versions that enable cache where it works.
>
> which is causing the copy & paste i'm whining about
>
>> We also have started discussing minimizing cache code copy -- admittedly
>> spinned off a patch submission discussion, so it may have been missed,
>> and I will repost the RFC in its own thread -- whereby the cache
>> functions ?would be defined as weak function in the lib, and could be
>> overriden by ISA-level function, which themselves could be overriden by
>> CPU-level functions, and SoC (or even board) level functions could
>> override them all.
>
> i guess i did miss it. ?it doesnt matter to me how we get to the ultimate
> destination ({I,D}CACHE_ON/etc...), just that ultimately the crappy/dead SoC's
> are not bogging down the modern/maintained ones by forcing the common
> implementation to suck.
> -mike
>

Right, I am going to resubmit this patch in the
arch/arm/cpu/armv7/tegra2 directory and see how that looks.

Regards,
Simon