From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Date: Wed, 15 Mar 2017 16:46:36 +0200 Subject: [U-Boot] [u-boot PATCH v5 04/10] ti: common: board_detect: commodify ethaddr environment setting code In-Reply-To: References: <1489410273-10159-1-git-send-email-rogerq@ti.com> <1489410273-10159-5-git-send-email-rogerq@ti.com> Message-ID: <871stywqb7.fsf@linux.intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, Roger Quadros writes: >> On Tue, Mar 14, 2017 at 3:05 PM Roger Quadros > 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? -- balbi