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

[Roger Quadros]

On 15/03/17 16:46, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq at ti.com> writes:
>>> 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
> 
> in that case, why would (mac2-mac1+1) ever be < 0?
> 

For incorrectly programmed boards in pre-production probably.

-- 
cheers,
-roger