public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/3] arm: at91: ether: Prepare for mach-types.h changes
Date: Mon, 02 May 2011 12:09:07 +0200	[thread overview]
Message-ID: <m2pqo1h0sc.fsf@ohwell.denx.de> (raw)
In-Reply-To: <4DBE5D69.2020400@compulab.co.il> (Igor Grinberg's message of "Mon, 02 May 2011 10:29:45 +0300")

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<grinberg@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 at pollux u-boot-testing (master)]$ make csb337_config
make: *** No rule to make target `csb337_config'.  Stop.
make: *** [csb337_config] Error 1
[dzu at pollux u-boot-testing (master)]$ grep -i csb337 Makefile 
[dzu at pollux u-boot-testing (master)]$ grep -i csb337 boards.cfg 
[dzu at 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: dzu at denx.de

  reply	other threads:[~2011-05-02 10:09 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-19 12:42 [U-Boot] Update and Cut down mach types Paulraj, Sandeep
2011-04-19 13:39 ` Matthias Weißer
2011-04-19 13:45   ` Paulraj, Sandeep
2011-04-20  8:44     ` Albert ARIBAUD
2011-04-19 14:21   ` Wolfgang Denk
2011-04-19 18:42     ` Matthias Weisser
2011-04-19 18:44     ` Michael Schwingen
2011-04-20  8:15       ` Detlev Zundel
2011-04-20  8:58 ` Igor Grinberg
2011-04-20 17:15   ` Michael Schwingen
2011-04-20 17:49     ` Albert ARIBAUD
2011-04-20 19:26       ` Michael Schwingen
2011-04-21 11:39         ` Albert ARIBAUD
2011-04-26 18:14           ` Michael Schwingen
2011-04-26 19:40             ` Wolfgang Denk
2011-04-26 20:38               ` Albert ARIBAUD
2011-04-26 21:32                 ` Wolfgang Denk
2011-04-26 21:38                   ` Reinhard Meyer
2011-04-27 10:19                     ` Michael Schwingen
2011-04-28  6:20                       ` Igor Grinberg
2011-04-29  8:58                         ` Detlev Zundel
2011-05-01 10:10                           ` [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error Igor Grinberg
2011-05-17 12:40                             ` Igor Grinberg
2011-05-21 21:40                               ` Paulraj, Sandeep
2011-05-01 10:10                           ` [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes Igor Grinberg
2011-05-01 20:28                             ` Alessandro Rubini
2011-05-02  7:18                               ` Igor Grinberg
2011-05-03 10:08                                 ` [U-Boot] [PATCH v2 " Igor Grinberg
2011-05-03 12:29                                   ` Wolfgang Denk
2011-05-03 13:00                                     ` Igor Grinberg
2011-05-04  7:13                                       ` [U-Boot] [PATCH v3 " Igor Grinberg
2011-05-01 10:10                           ` [U-Boot] [PATCH 3/3] arm: at91: ether: " Igor Grinberg
2011-05-01 19:38                             ` Reinhard Meyer
2011-05-02  7:29                               ` Igor Grinberg
2011-05-02 10:09                                 ` Detlev Zundel [this message]
2011-05-02 12:49                                   ` [U-Boot] [PATCH v2 " Igor Grinberg
2011-05-16 13:31                                     ` Igor Grinberg
2011-04-27 11:44                     ` [U-Boot] Update and Cut down mach types Detlev Zundel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2pqo1h0sc.fsf@ohwell.denx.de \
    --to=dzu@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox