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?

Apart from that, it feels entirely wrong to do so. Fix the kernel to NOT do a
workaround instead should be the better approach.

Any opinions by Ben or Wolfgang on this?

Best Regards,
Reinhard
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to