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

login
register
mail settings
Submitter Jason Liu
Date July 29, 2011, 12:51 p.m.
Message ID <CAB4PhKfkmrG4O11oa7Nfm4O5ZtEpROiDvJfk9iL2iU210Rn=-A@mail.gmail.com>
Download mbox | patch
Permalink /patch/107390/
State Not Applicable
Delegated to: Prafulla Wadaskar
Headers show

Comments

Jason Liu - July 29, 2011, 12:51 p.m.
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.

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
>
Aneesh V - July 29, 2011, 4 p.m.
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

Patch

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