[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Holger Brunck]

Hi Aneesh,
today I did a rebase of my development branch to current u-boot master. And I
saw on our km_kirkwood board that our egiga0 interface isn't working anymore.

The CPU is a:
SoC:   Kirkwood 88F6281_A0

After bisecting the current tree I got:

c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit
commit c2dd0d45540397704de9b13287417d21049d34c6
Author: Aneesh V <aneesh@ti.com>
Date:   Thu Jun 16 23:30:49 2011 +0000

    armv7: integrate cache maintenance support

And indeed after reverting this commit on current HEAD my board is usable again.

Any ideas how to fix this problem?

Best regards
Holger Brunck

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Wolfgang Denk]

Dear Aneesh,

In message <4E3161ED.5030109@keymile.com> Holger Brunck wrote:
>
> today I did a rebase of my development branch to current u-boot master. And I
> saw on our km_kirkwood board that our egiga0 interface isn't working anymore.
> 
> The CPU is a:
> SoC:   Kirkwood 88F6281_A0
> 
> After bisecting the current tree I got:
> 
> c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit
> commit c2dd0d45540397704de9b13287417d21049d34c6
> Author: Aneesh V <aneesh@ti.com>
> Date:   Thu Jun 16 23:30:49 2011 +0000
> 
>     armv7: integrate cache maintenance support
> 
> And indeed after reverting this commit on current HEAD my board is usable again.

The same is true for iMX27 (and probably other boards / SoCs):

I verified for both the "imx27lite" and "magnesium" boards that above
patch breaks Ethernet on these iMX27 boards.

Seems we have a bigger problem here...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In English, every word can be verbed.  Would that it were  so  in our
programming languages.

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Jason Liu]

Hi, All,

2011/7/29 Wolfgang Denk <wd@denx.de>:
> Dear Aneesh,
>
> In message <4E3161ED.5030109@keymile.com> Holger Brunck wrote:
>>
>> today I did a rebase of my development branch to current u-boot master. And I
>> saw on our km_kirkwood board that our egiga0 interface isn't working anymore.
>>
>> The CPU is a:
>> SoC: ? Kirkwood 88F6281_A0
>>
>> After bisecting the current tree I got:
>>
>> c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit
>> commit c2dd0d45540397704de9b13287417d21049d34c6
>> Author: Aneesh V <aneesh@ti.com>
>> Date: ? Thu Jun 16 23:30:49 2011 +0000
>>
>> ? ? armv7: integrate cache maintenance support
>>
>> And indeed after reverting this commit on current HEAD my board is usable again.
>
> The same is true for iMX27 (and probably other boards / SoCs):
>
> I verified for both the "imx27lite" and "magnesium" boards that above
> patch breaks Ethernet on these iMX27 boards.
>
> Seems we have a bigger problem here...

The root cause is that: This commit enable d-cache for all the ARM
platform silently,
Not just ARMV7.

--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -450,6 +450,12 @@ void board_init_r (gd_t *id, ulong dest_addr)
        gd->flags |= GD_FLG_RELOC;      /* tell others: relocation done */

        monitor_flash_len = _end_ofs;
+       /*
+        * Enable D$:
+        * I$, if needed, must be already enabled in start.S
+        */
+       dcache_enable();
+

And for such kind of big change, it should broadcast to everyone by
ML, otherwise, many
people will pay much to checkout what's going on. It did cost me half
day to find it out.

Jason

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> In English, every word can be verbed. ?Would that it were ?so ?in our
> programming languages.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Aneesh V]

Hi Jason,

On Friday 29 July 2011 06:21 PM, Jason Liu wrote:
> Hi, All,
>
> 2011/7/29 Wolfgang Denk<wd@denx.de>:
>> Dear Aneesh,
>>
>> In message<4E3161ED.5030109@keymile.com>  Holger Brunck wrote:
>>>
>>> today I did a rebase of my development branch to current u-boot master. And I
>>> saw on our km_kirkwood board that our egiga0 interface isn't working anymore.
>>>
>>> The CPU is a:
>>> SoC:   Kirkwood 88F6281_A0
>>>
>>> After bisecting the current tree I got:
>>>
>>> c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit
>>> commit c2dd0d45540397704de9b13287417d21049d34c6
>>> Author: Aneesh V<aneesh@ti.com>
>>> Date:   Thu Jun 16 23:30:49 2011 +0000
>>>
>>>      armv7: integrate cache maintenance support
>>>
>>> And indeed after reverting this commit on current HEAD my board is usable again.
>>
>> The same is true for iMX27 (and probably other boards / SoCs):
>>
>> I verified for both the "imx27lite" and "magnesium" boards that above
>> patch breaks Ethernet on these iMX27 boards.
>>
>> Seems we have a bigger problem here...
>
> The root cause is that: This commit enable d-cache for all the ARM
> platform silently,
> Not just ARMV7.
>
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -450,6 +450,12 @@ void board_init_r (gd_t *id, ulong dest_addr)
>          gd->flags |= GD_FLG_RELOC;      /* tell others: relocation done */
>
>          monitor_flash_len = _end_ofs;
> +       /*
> +        * Enable D$:
> +        * I$, if needed, must be already enabled in start.S
> +        */
> +       dcache_enable();
> +


The rationale for doing that is explained in the following thread:

http://marc.info/?l=u-boot&m=131107645915931&w=2

The idea was to enable it immediately after relocation. But I agree
that the patch title probably doesn't indicate this.

best regards,
Aneesh

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Aneesh V]

Dear Wolfgang,

On Friday 29 July 2011 06:03 PM, Wolfgang Denk wrote:
> Dear Aneesh,
>
> In message<4E3161ED.5030109@keymile.com>  Holger Brunck wrote:
>>
>> today I did a rebase of my development branch to current u-boot master. And I
>> saw on our km_kirkwood board that our egiga0 interface isn't working anymore.
>>
>> The CPU is a:
>> SoC:   Kirkwood 88F6281_A0
>>
>> After bisecting the current tree I got:
>>
>> c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit
>> commit c2dd0d45540397704de9b13287417d21049d34c6
>> Author: Aneesh V<aneesh@ti.com>
>> Date:   Thu Jun 16 23:30:49 2011 +0000
>>
>>      armv7: integrate cache maintenance support
>>
>> And indeed after reverting this commit on current HEAD my board is usable again.
>
> The same is true for iMX27 (and probably other boards / SoCs):
>
> I verified for both the "imx27lite" and "magnesium" boards that above
> patch breaks Ethernet on these iMX27 boards.
>
> Seems we have a bigger problem here...

I had written a small write-up on this earlier today in the below
thread:
http://marc.info/?l=u-boot&m=131193466800729&w=2

I had done extensive testing on the armv7 cache-maintenance APIs by
creating coherency issues and solving them using the APIs. I believe
the problems are due to the APIs not being appropriately used where
they need to be used, also coupled with the fact that dcache_enable()
is now called from board_init_r().

Anyway, I will test my APIs once again on Cortex-A8 and Cortex-A9.

Maybe, I should also write a README with guidelines for correct cache
usage.

Also, I would suggest the following to solve the problem with breaking
boards:
1. Allow boards to define CONFIG_SYS_DCACHE_OFF and/or
2. Instead of calling dcache_enable() from board_init_r() let's call a
new function like dcache_init(). The default implementation of this
function shall do nothing. The real implementation in platform code may
enable or disable d_cache() on a per SoC/board basis.

best regards,
Aneesh

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Wolfgang Denk]

Dear Aneesh V,

In message <4E32C223.2050309@ti.com> you wrote:
> 
> I had written a small write-up on this earlier today in the below
> thread:
> http://marc.info/?l=u-boot&m=131193466800729&w=2

Thanks.

> I had done extensive testing on the armv7 cache-maintenance APIs by
> creating coherency issues and solving them using the APIs. I believe
> the problems are due to the APIs not being appropriately used where
> they need to be used, also coupled with the fact that dcache_enable()
> is now called from board_init_r().

Yes, this is the problem - now suddenly a large number of drivers
which are not prepared to run with caches on break.

> Maybe, I should also write a README with guidelines for correct cache
> usage.

That would be nice, too. TIA!

> Also, I would suggest the following to solve the problem with breaking
> boards:
> 1. Allow boards to define CONFIG_SYS_DCACHE_OFF and/or
> 2. Instead of calling dcache_enable() from board_init_r() let's call a
> new function like dcache_init(). The default implementation of this
> function shall do nothing. The real implementation in platform code may
> enable or disable d_cache() on a per SoC/board basis.

I think approach 2. is the better one (we should try to avoid
unnecessary #ifdef's).  But instead of doing nothing, the default
implementation should print a warning "dcache: not enabled", so users
and board maintainers are permanently aware which boards have not been
adapted / fixed yet :-)

Thanks!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
ATTENTION: Despite Any Other Listing of Product Contents Found  Here-
on, the Consumer is Advised That, in Actuality, This Product Consists
Of 99.9999999999% Empty Space.

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Jens Scharsig]

Am 28.07.2011 15:19, schrieb Holger Brunck:
> Hi Aneesh,
> today I did a rebase of my development branch to current u-boot master. And I
> saw on our km_kirkwood board that our egiga0 interface isn't working anymore.
>
> The CPU is a:
> SoC:   Kirkwood 88F6281_A0
>
> After bisecting the current tree I got:
>
> c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit
> commit c2dd0d45540397704de9b13287417d21049d34c6
> Author: Aneesh V<aneesh@ti.com>
> Date:   Thu Jun 16 23:30:49 2011 +0000
>
>      armv7: integrate cache maintenance support
>
> And indeed after reverting this commit on current HEAD my board is usable again.
>
> Any ideas how to fix this problem?

At91 boards are also affected.


Best regards

Jens Scharsig

[U-Boot] "armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

[Jens Scharsig]

Am 28.07.2011 15:19, schrieb Holger Brunck:
> Hi Aneesh,
> today I did a rebase of my development branch to current u-boot master. And I
> saw on our km_kirkwood board that our egiga0 interface isn't working anymore.
>
> The CPU is a:
> SoC:   Kirkwood 88F6281_A0
>
> After bisecting the current tree I got:
>
> c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit
> commit c2dd0d45540397704de9b13287417d21049d34c6
> Author: Aneesh V<aneesh@ti.com>
> Date:   Thu Jun 16 23:30:49 2011 +0000
>
>      armv7: integrate cache maintenance support
>
> And indeed after reverting this commit on current HEAD my board is usable again.
>
> Any ideas how to fix this problem?

At91 boards are also affected.


Best regards

Jens Scharsig