Hi Igor,
> On 05/01/11 22:38, Reinhard Meyer wrote:
>
>> Dear Igor Grinberg,
>>
>>> at91 ethernet module used machine_is_cbs337() macro for board specific
>>> Linux compatibility issue.
>>> Use compile time defines instead.
>>>
>>> Signed-off-by: Igor Grinberg<grinb...@compulab.co.il>
>>> ---
>>> arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++---------
>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c
>>> b/arch/arm/cpu/arm920t/at91rm9200/ether.c
>>> index e1cdeba..4aeb883 100644
>>> --- a/arch/arm/cpu/arm920t/at91rm9200/ether.c
>>> +++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c
>>> @@ -201,15 +201,15 @@ int eth_init (bd_t * bd)
>>> * that MicroMonitor behavior so we avoid needing to make such OS code
>>> * care about which bootloader was used.
>>> */
>>> - if (machine_is_csb337()) {
>>> - p_mac->EMAC_SA2H = (enetaddr[0]<< 8) | (enetaddr[1]);
>>> - p_mac->EMAC_SA2L = (enetaddr[2]<< 24) | (enetaddr[3]<< 16)
>>> - | (enetaddr[4]<< 8) | (enetaddr[5]);
>>> - } else {
>>> - p_mac->EMAC_SA2L = (enetaddr[3]<< 24) | (enetaddr[2]<< 16)
>>> - | (enetaddr[1]<< 8) | (enetaddr[0]);
>>> - p_mac->EMAC_SA2H = (enetaddr[5]<< 8) | (enetaddr[4]);
>>> - }
>>> +#ifdef CONFIG_MACH_CSB337
>>> + p_mac->EMAC_SA2H = (enetaddr[0]<< 8) | (enetaddr[1]);
>>> + p_mac->EMAC_SA2L = (enetaddr[2]<< 24) | (enetaddr[3]<< 16)
>>> + | (enetaddr[4]<< 8) | (enetaddr[5]);
>>> +#else
>>> + p_mac->EMAC_SA2L = (enetaddr[3]<< 24) | (enetaddr[2]<< 16)
>>> + | (enetaddr[1]<< 8) | (enetaddr[0]);
>>> + p_mac->EMAC_SA2H = (enetaddr[5]<< 8) | (enetaddr[4]);
>>> +#endif
>>>
>>> p_mac->EMAC_RBQP = (long) (&rbfdt[0]);
>>> p_mac->EMAC_RSR&= ~(AT91C_EMAC_RSR_OVR | AT91C_EMAC_REC |
>>> AT91C_EMAC_BNA);
>>
>> There is nothing wrong with your patch itself, but it let me to take a
>> closer look at the
>> reasoning of why there is a machine dependency. The full code at this
>> section is:
>>
>> eth_getenv_enetaddr("ethaddr", enetaddr);
>>
>> /* The CSB337 originally used a version of the MicroMonitor bootloader
>> * which saved Ethernet addresses in the "wrong" order. Operating
>> * systems (like Linux) know this, and apply a workaround. Replicate
>> * that MicroMonitor behavior so we avoid needing to make such OS code
>> * care about which bootloader was used.
>> */
>> if (machine_is_csb337()) {
>> p_mac->EMAC_SA2H = (enetaddr[0] << 8) | (enetaddr[1]);
>> p_mac->EMAC_SA2L = (enetaddr[2] << 24) | (enetaddr[3] << 16)
>> | (enetaddr[4] << 8) | (enetaddr[5]);
>> } else {
>> p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16)
>> | (enetaddr[1] << 8) | (enetaddr[0]);
>> p_mac->EMAC_SA2H = (enetaddr[5] << 8) | (enetaddr[4]);
>> }
>>
>> So, for the sake of a(nother) broken bootloader and a workaround in Linux we
>> store the MAC address in the wrong order? What if U-Boot itself is used to
>> make
>> LAN accesses?
>
> Well, I've read the comment before preparing the patch.
> Actually, I felt like: "this should be thrown away!".
> Also, I haven't found csb337 board in the tree...
> I didn't want to decide for you (If I'm not mistaken,
> you are the maintainer of Atmel) what to do with it, so I left it.
> Do you think we should remove this?
> I would love to send another patch to remove this completely.
I'd say remove it. Why do I say that?
[dzu@pollux u-boot-testing (master)]$ make csb337_config
make: *** No rule to make target `csb337_config'. Stop.
make: *** [csb337_config] Error 1
[dzu@pollux u-boot-testing (master)]$ grep -i csb337 Makefile
[dzu@pollux u-boot-testing (master)]$ grep -i csb337 boards.cfg
[dzu@pollux u-boot-testing (master)]$
Cheers
Detlev
--
Indeed, the author firmly believes that the best serious work is also
good fun. We needn't apologize if we enjoy doing research.
-- Donald Knuth
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
