[U-Boot] [u-boot PATCH v5 04/10] ti: common: board_detect: commodify ethaddr environment setting code

[Roger Quadros]

Hi,

On 14/03/17 17:54, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Mar 14, 2017 at 3:05 PM Roger Quadros <rogerq at ti.com <mailto:rogerq at ti.com>> wrote:
> 
>     +void board_ti_set_ethaddr(int index)
>     +{
>     +       uint8_t mac_addr[6];
>     +       int i;
>     +       u64 mac1, mac2;
>     +       u8 mac_addr1[6], mac_addr2[6];
>     +       int num_macs;
>     +       /*
>     +        * Export any Ethernet MAC addresses from EEPROM.
>     +        * The 2 MAC addresses in EEPROM define the address range.
>     +        */
>     +       board_ti_get_eth_mac_addr(0, mac_addr1);
>     +       board_ti_get_eth_mac_addr(1, mac_addr2);
>     +
>     +       if (is_valid_ethaddr(mac_addr1) && is_valid_ethaddr(mac_addr2)) {
>     +               mac1 = mac_to_u64(mac_addr1);
>     +               mac2 = mac_to_u64(mac_addr2);
>     +
>     +               /* must contain an address range */
>     +               num_macs = mac2 - mac1 + 1;
>     +               if (num_macs <= 0)
>     +                       return;
> 
> 
> seems like there's still one minor improvement here:
> 
> If num_macs < 0, then it could be that mac1 and mac2 are swapped.
> 
> Perhaps test for that ?
> 
> if (num_macs == 0)
>      bail();
> 
> if (num_macs < 0) {
>         if ((mac1 - mac2 + 1) < 50) {
>                 num_macs = mac1 - mac2 + 1;
>                 mac1 ^= mac2;
>                 mac2 ^= mac1;
>                 mac1 ^= mac2;
> [...]
> 
> don't know how much this may help, it's a judgment call, I suppose

We don't have any boards with swapped mac1 and mac2. We'd like to
keep it that way and catch any new boards with eeprom errors early so I'd
rather not have this enhancement.

-- 
cheers,
-roger