diff mbox series

[v4,04/10] ti: common: board_detect: commodify ethaddr environment setting code

Message ID 1489410273-10159-5-git-send-email-rogerq@ti.com
State Superseded
Headers show
Series am57xx-idk LCD and am571x-idk 6 port ethernet pinmux | expand

Commit Message

Roger Quadros March 13, 2017, 1:04 p.m. UTC
Keystone and OMAP platforms will need this to set ethernet
MAC addresses from board EEPROM.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 board/ti/common/board_detect.c | 57 ++++++++++++++++++++++++++++++++++++++++++
 board/ti/common/board_detect.h | 12 +++++++++
 2 files changed, 69 insertions(+)

Comments

Felipe Balbi March 13, 2017, 1:15 p.m. UTC | #1
Hi,

Roger Quadros <rogerq@ti.com> writes:
> +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;

> +		/* <= 50 to protect against user programming error */

> +		if (num_macs > 0 && num_macs <= 50) {


if user programs a range > 50, then you do nothing. How about allowing
up to 50 addresses?

Something like:

	if (num_macs < 0)
        	bail_out();

	if (num_macs > 50) {
		printf("Too many addresses. %d > 50\n", num_macs);
                num_macs = 50;
	}

this also mean you can reduce indentation level for the for loop.

-- 
balbi
Roger Quadros March 14, 2017, 10:52 a.m. UTC | #2
On 13/03/17 15:15, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> +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;
>> +		/* <= 50 to protect against user programming error */
>> +		if (num_macs > 0 && num_macs <= 50) {
> 
> if user programs a range > 50, then you do nothing. How about allowing
> up to 50 addresses?
> 
> Something like:
> 
> 	if (num_macs < 0)
>         	bail_out();
> 
> 	if (num_macs > 50) {
> 		printf("Too many addresses. %d > 50\n", num_macs);
>                 num_macs = 50;
> 	}
> 
> this also mean you can reduce indentation level for the for loop.
> 

Good idea.
diff mbox series

Patch

diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
index a5dba94..23388d0 100644
--- a/board/ti/common/board_detect.c
+++ b/board/ti/common/board_detect.c
@@ -314,3 +314,60 @@  void __maybe_unused set_board_info_env(char *name)
 	else
 		setenv("board_serial", unknown);
 }
+
+static u64 mac_to_u64(u8 mac[6])
+{
+	int i;
+	u64 addr = 0;
+
+	for (i = 0; i < 6; i++) {
+		addr <<= 8;
+		addr |= mac[i];
+	}
+
+	return addr;
+}
+
+static void u64_to_mac(u64 addr, u8 mac[6])
+{
+	mac[5] = addr;
+	mac[4] = addr >> 8;
+	mac[3] = addr >> 16;
+	mac[2] = addr >> 24;
+	mac[1] = addr >> 32;
+	mac[0] = addr >> 40;
+}
+
+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;
+		/* <= 50 to protect against user programming error */
+		if (num_macs > 0 && num_macs <= 50) {
+			for (i = 0; i < num_macs; i++) {
+				u64_to_mac(mac1 + i, mac_addr);
+				if (is_valid_ethaddr(mac_addr)) {
+					eth_setenv_enetaddr_by_index("eth",
+								     i + index,
+								     mac_addr);
+				}
+			}
+		}
+	}
+}
diff --git a/board/ti/common/board_detect.h b/board/ti/common/board_detect.h
index 52b0075..88b0a59 100644
--- a/board/ti/common/board_detect.h
+++ b/board/ti/common/board_detect.h
@@ -193,4 +193,16 @@  u64 board_ti_get_emif2_size(void);
  */
 void set_board_info_env(char *name);
 
+/**
+ * board_ti_set_ethaddr- Sets the ethaddr environment from EEPROM
+ * @index: The first eth<index>addr environment variable to set
+ *
+ * EEPROM should be already read before calling this function.
+ * The EEPROM contains 2 MAC addresses which define the MAC address
+ * range (i.e. first and last MAC address).
+ * This function sets the ethaddr environment variable for all
+ * the available MAC addresses starting from eth<index>addr.
+ */
+void board_ti_set_ethaddr(int index);
+
 #endif	/* __BOARD_DETECT_H */