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