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

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

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

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@denx.de In English, every word can be verbed. Would that it were so in our programming languages. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi Jason,
On Friday 29 July 2011 06:21 PM, Jason Liu wrote:
Hi, All,
2011/7/29 Wolfgang Denkwd@denx.de:
Dear Aneesh,
In message4E3161ED.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 Vaneesh@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

Dear Wolfgang,
On Friday 29 July 2011 06:03 PM, Wolfgang Denk wrote:
Dear Aneesh,
In message4E3161ED.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 Vaneesh@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

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:
- Allow boards to define CONFIG_SYS_DCACHE_OFF and/or
- 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

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 Vaneesh@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

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 Vaneesh@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
participants (5)
-
Aneesh V
-
Holger Brunck
-
Jason Liu
-
Jens Scharsig
-
Wolfgang Denk