"armv7: integrate cache maintenance support" breaks km_kirkwood ethernet

U-Boot Holger Brunck 5 participants 8 messages Jul 28, 2011 – Aug 7, 2011 Arm (32-bit)

Share & tools

Holger Brunck

author

2011-07-28 13:19 UTC #0 lore raw

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 <[email protected]>
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

Wolfgang Denk

2011-07-29 12:33 UTC #1 lore raw

Dear Aneesh,

Quoted 9 lines · In message <[email protected]> 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:

Quoted 8 lines

c2dd0d45540397704de9b13287417d21049d34c6 is the first bad commit commit c2dd0d45540397704de9b13287417d21049d34c6
Author: Aneesh V <[email protected]>
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

Signature · 5 lines

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.

Jason Liu

2011-07-29 12:51 UTC #2 lore raw

Hi, All,

2011/7/29 Wolfgang Denk <[email protected]>:

Quoted 19 lines

Dear Aneesh,
Quoted 16 lines · In message <[email protected]> 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 <[email protected]>
Date: ? Thu Jun 16 23:30:49 2011 +0000
? ? armv7: integrate cache maintenance support

Quoted 8 lines

Quoted 1 line
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.

arch/arm/lib/board.c +6 -0

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

Quoted 16 lines

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

Aneesh V

2011-07-29 16:00 UTC #3 lore raw

Hi Jason,

Quoted 38 lines · Jason Liu

Hi, All,
2011/7/29 Wolfgang Denk<[email protected]>:
Quoted 19 lines
Dear Aneesh,
Quoted 16 lines · In message<[email protected]> 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<[email protected]>
Date:   Thu Jun 16 23:30:49 2011 +0000
armv7: integrate cache maintenance support
Quoted 8 lines
Quoted 1 line
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.

arch/arm/lib/board.c void board_init_r (gd_t *id, ulong dest_addr) · line 458

Quoted 9 lines

arch/arm/lib/board.c +6 -0

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

Aneesh V

2011-07-29 14:22 UTC #4 lore raw

In reply to Wolfgang Denk

Dear Wolfgang,

Quoted 19 lines · Wolfgang Denk

Dear Aneesh,
Quoted 16 lines · In message<[email protected]> 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<[email protected]>
Date:   Thu Jun 16 23:30:49 2011 +0000
armv7: integrate cache maintenance support

Quoted 8 lines

Quoted 1 line
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

Wolfgang Denk

2011-07-30 20:38 UTC #5 lore raw

Dear Aneesh V,

Quoted 4 lines · In message <[email protected]> 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.

Quoted 5 lines

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.

Quoted 2 lines

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

That would be nice, too. TIA!

Quoted 7 lines

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

Signature · 6 lines

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.

Jens Scharsig

2011-08-07 15:25 UTC #6 lore raw

In reply to Holger Brunck

Am 28.07.2011 15:19, schrieb Holger Brunck:

Quoted 11 lines

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

Quoted 8 lines

Author: Aneesh V<[email protected]>
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

2011-08-07 15:25 UTC #7 lore raw

In reply to Holger Brunck

Am 28.07.2011 15:19, schrieb Holger Brunck:

Quoted 11 lines

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

Quoted 8 lines

Author: Aneesh V<[email protected]>
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