[U-Boot] [PATCH] sunxi: Add support for eth1addr

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Hans de Goede]

Currently we will already fill ethaddr with a fixed unique address
based on the SoCs serial (from the sid) to make sure that boards which
use the integrated emac / gmac get a fixed mac rather then a random one.

On some boards (observed on 2 tablets using sdio rtl8703as wifi chips)
the wifi does not come with a fixed mac either, so also set eth1addr,
so that dts files can set an ethernet1 alias to get mac-address and
local-mac-address filled for dt nodes describing the wifi controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/board.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index b5a50f4..41d796c 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -641,6 +641,18 @@ int misc_init_r(void)
 			eth_setenv_enetaddr("ethaddr", mac_addr);
 		}
 
+		if (!getenv("eth1addr")) {
+			/* Non OUI / registered MAC address */
+			mac_addr[0] = 0x12;
+			mac_addr[1] = (sid[0] >>  0) & 0xff;
+			mac_addr[2] = (sid[3] >> 24) & 0xff;
+			mac_addr[3] = (sid[3] >> 16) & 0xff;
+			mac_addr[4] = (sid[3] >>  8) & 0xff;
+			mac_addr[5] = (sid[3] >>  0) & 0xff;
+
+			eth_setenv_enetaddr("eth1addr", mac_addr);
+		}
+
 		if (!getenv("serial#")) {
 			snprintf(serial_string, sizeof(serial_string),
 				"%08x%08x", sid[0], sid[3]);
-- 
2.7.4

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Ian Campbell]

On Sun, 2016-06-26 at 13:54 +0200, Hans de Goede wrote:
> Currently we will already fill ethaddr with a fixed unique address
> based on the SoCs serial (from the sid) to make sure that boards which
> use the integrated emac / gmac get a fixed mac rather then a random one.
> 
> On some boards (observed on 2 tablets using sdio rtl8703as wifi chips)
> the wifi does not come with a fixed mac either, so also set eth1addr,
> so that dts files can set an ethernet1 alias to get mac-address and
> local-mac-address filled for dt nodes describing the wifi controller.

This does it unconditionally, won't having eth1addr show up for boards
which only have one network device (WIFI or otherwise) be potentially
confusing for users? i.e. lacking it would be a sign that the online
guide you are following might not exactly be relevant to your board, or
people seeing it and then wasting time trying to figure out how to use
the second device. Of secondary concern (since I think it is far less
liklely) would be confusing some software somewhere.

Also what is the impact on boards which have a second network device
which does have a proper MAC address, either one which is part of the
board or e.g. on a USB dongle? I think we don't want to override any
burnt in MAC addresses.

Ian.

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> ?board/sunxi/board.c | 12 ++++++++++++
> ?1 file changed, 12 insertions(+)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index b5a50f4..41d796c 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -641,6 +641,18 @@ int misc_init_r(void)
> ?			eth_setenv_enetaddr("ethaddr", mac_addr);
> ?		}
> ?
> +		if (!getenv("eth1addr")) {
> +			/* Non OUI / registered MAC address */
> +			mac_addr[0] = 0x12;
> +			mac_addr[1] = (sid[0] >>??0) & 0xff;
> +			mac_addr[2] = (sid[3] >> 24) & 0xff;
> +			mac_addr[3] = (sid[3] >> 16) & 0xff;
> +			mac_addr[4] = (sid[3] >>??8) & 0xff;
> +			mac_addr[5] = (sid[3] >>??0) & 0xff;
> +
> +			eth_setenv_enetaddr("eth1addr", mac_addr);
> +		}
> +
> ?		if (!getenv("serial#")) {
> ?			snprintf(serial_string,
> sizeof(serial_string),
> ?				"%08x%08x", sid[0], sid[3]);

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Hans de Goede]

Hi,

On 30-06-16 12:50, Ian Campbell wrote:
> On Sun, 2016-06-26 at 13:54 +0200, Hans de Goede wrote:
>> Currently we will already fill ethaddr with a fixed unique address
>> based on the SoCs serial (from the sid) to make sure that boards which
>> use the integrated emac / gmac get a fixed mac rather then a random one.
>>
>> On some boards (observed on 2 tablets using sdio rtl8703as wifi chips)
>> the wifi does not come with a fixed mac either, so also set eth1addr,
>> so that dts files can set an ethernet1 alias to get mac-address and
>> local-mac-address filled for dt nodes describing the wifi controller.
>
> This does it unconditionally, won't having eth1addr show up for boards
> which only have one network device (WIFI or otherwise) be potentially
> confusing for users? i.e. lacking it would be a sign that the online
> guide you are following might not exactly be relevant to your board, or
> people seeing it and then wasting time trying to figure out how to use
> the second device. Of secondary concern (since I think it is far less
> liklely) would be confusing some software somewhere.
>
> Also what is the impact on boards which have a second network device
> which does have a proper MAC address, either one which is part of the
> board or e.g. on a USB dongle? I think we don't want to override any
> burnt in MAC addresses.

This just sets eth1addr in the u-boot env, it will only actually
do something to the devicetree if there is an ethernet1 alias in
there (which there should not be for e.g an usb dongle).

So this can only cause any issues if people add a ethernet1 alias
when they should not.

As for overriding a burnt-in mac-address, if the ethernet1
alias is present then u-boot will only modify a pre-existing
"mac-address" dt property, if none is present it will not
touch it. It will unconditional modify / add a "local-mac-address"
property. Drivers should only honor "local-mac-address" if
there is no burnt-in mac-address, see e.g. :

https://github.com/jwrdegoede/rtl8723bs/commit/be5e87ea71677120ede457213208902ed9942653

Now if users have a driver which honors mac-address to
override the burnt-in mac, and the add an alias ethernet1
and they add an "mac-address" property to the dt node
the ethernet1 alias points to, then they get exactly
what they ask for.

Regards,

Hans





>
> Ian.
>
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  board/sunxi/board.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index b5a50f4..41d796c 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -641,6 +641,18 @@ int misc_init_r(void)
>>  			eth_setenv_enetaddr("ethaddr", mac_addr);
>>  		}
>>
>> +		if (!getenv("eth1addr")) {
>> +			/* Non OUI / registered MAC address */
>> +			mac_addr[0] = 0x12;
>> +			mac_addr[1] = (sid[0] >>  0) & 0xff;
>> +			mac_addr[2] = (sid[3] >> 24) & 0xff;
>> +			mac_addr[3] = (sid[3] >> 16) & 0xff;
>> +			mac_addr[4] = (sid[3] >>  8) & 0xff;
>> +			mac_addr[5] = (sid[3] >>  0) & 0xff;
>> +
>> +			eth_setenv_enetaddr("eth1addr", mac_addr);
>> +		}
>> +
>>  		if (!getenv("serial#")) {
>>  			snprintf(serial_string,
>> sizeof(serial_string),
>>  				"%08x%08x", sid[0], sid[3]);

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Ian Campbell]

On Thu, 2016-06-30 at 13:15 +0200, Hans de Goede wrote:
> Hi,
> 
> On 30-06-16 12:50, Ian Campbell wrote:
> > On Sun, 2016-06-26 at 13:54 +0200, Hans de Goede wrote:
> > > Currently we will already fill ethaddr with a fixed unique address
> > > based on the SoCs serial (from the sid) to make sure that boards which
> > > use the integrated emac / gmac get a fixed mac rather then a random one.
> > > 
> > > On some boards (observed on 2 tablets using sdio rtl8703as wifi chips)
> > > the wifi does not come with a fixed mac either, so also set eth1addr,
> > > so that dts files can set an ethernet1 alias to get mac-address and
> > > local-mac-address filled for dt nodes describing the wifi controller.
> > 
> > This does it unconditionally, won't having eth1addr show up for boards
> > which only have one network device (WIFI or otherwise) be potentially
> > confusing for users? i.e. lacking it would be a sign that the online
> > guide you are following might not exactly be relevant to your board, or
> > people seeing it and then wasting time trying to figure out how to use
> > the second device. Of secondary concern (since I think it is far less
> > liklely) would be confusing some software somewhere.
> > 
[...]
> This just sets eth1addr in the u-boot env,

It's this which I worried might confuse people, people who notice
eth1addr (perhaps due to tab completion on "printenv eth"?) will wonder
where the eth1 device is and/or why it is not working for them.

The rest of what you say regarding how this goes on to interact with
Linux makes sense to me, although even then having an eth1addr which
does't correspond to the MAC used by the device under Linux (because it
is burnt in and therefore eth1addr is ignored) still seems potentially
confusing to me.

I see at least some platforms use a?CONFIG_HAS_ETH1 to control this. It
could also perhaps be gated on the DTB used by u-boot itself by looking
at the mac-address for the ethernet1 alias (and ethernet2 too I
suppose).

Ian.

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Hans de Goede]

Hi,

On 30-06-16 15:52, Ian Campbell wrote:
> On Thu, 2016-06-30 at 13:15 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 30-06-16 12:50, Ian Campbell wrote:
>>> On Sun, 2016-06-26 at 13:54 +0200, Hans de Goede wrote:
>>>> Currently we will already fill ethaddr with a fixed unique address
>>>> based on the SoCs serial (from the sid) to make sure that boards which
>>>> use the integrated emac / gmac get a fixed mac rather then a random one.
>>>>
>>>> On some boards (observed on 2 tablets using sdio rtl8703as wifi chips)
>>>> the wifi does not come with a fixed mac either, so also set eth1addr,
>>>> so that dts files can set an ethernet1 alias to get mac-address and
>>>> local-mac-address filled for dt nodes describing the wifi controller.
>>>
>>> This does it unconditionally, won't having eth1addr show up for boards
>>> which only have one network device (WIFI or otherwise) be potentially
>>> confusing for users? i.e. lacking it would be a sign that the online
>>> guide you are following might not exactly be relevant to your board, or
>>> people seeing it and then wasting time trying to figure out how to use
>>> the second device. Of secondary concern (since I think it is far less
>>> liklely) would be confusing some software somewhere.
>>>
> [...]
>> This just sets eth1addr in the u-boot env,
>
> It's this which I worried might confuse people, people who notice
> eth1addr (perhaps due to tab completion on "printenv eth"?) will wonder
> where the eth1 device is and/or why it is not working for them.

People who use the u-boot cmdline at all really are experienced users
so TBH I'm not all that worried about this.

> The rest of what you say regarding how this goes on to interact with
> Linux makes sense to me, although even then having an eth1addr which
> does't correspond to the MAC used by the device under Linux (because it
> is burnt in and therefore eth1addr is ignored) still seems potentially
> confusing to me.
>
> I see at least some platforms use a CONFIG_HAS_ETH1 to control this. It
> could also perhaps be gated on the DTB used by u-boot itself by looking
> at the mac-address for the ethernet1 alias (and ethernet2 too I
> suppose).

Adding a Kconfig option just to have a cleaner u-boot env seems a bit
like overkill to me, and parsing the dtb even more so.

Regards,

Hans

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Ian Campbell]

On Thu, 2016-06-30 at 17:07 +0200, Hans de Goede wrote:
> Hi,
> 
> On 30-06-16 15:52, Ian Campbell wrote:
> > On Thu, 2016-06-30 at 13:15 +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 30-06-16 12:50, Ian Campbell wrote:
> > > > On Sun, 2016-06-26 at 13:54 +0200, Hans de Goede wrote:
> > > > > Currently we will already fill ethaddr with a fixed unique
> > > > > address
> > > > > based on the SoCs serial (from the sid) to make sure that
> > > > > boards which
> > > > > use the integrated emac / gmac get a fixed mac rather then a
> > > > > random one.
> > > > > 
> > > > > On some boards (observed on 2 tablets using sdio rtl8703as
> > > > > wifi chips)
> > > > > the wifi does not come with a fixed mac either, so also set
> > > > > eth1addr,
> > > > > so that dts files can set an ethernet1 alias to get mac-
> > > > > address and
> > > > > local-mac-address filled for dt nodes describing the wifi
> > > > > controller.
> > > > 
> > > > This does it unconditionally, won't having eth1addr show up for
> > > > boards
> > > > which only have one network device (WIFI or otherwise) be
> > > > potentially
> > > > confusing for users? i.e. lacking it would be a sign that the
> > > > online
> > > > guide you are following might not exactly be relevant to your
> > > > board, or
> > > > people seeing it and then wasting time trying to figure out how
> > > > to use
> > > > the second device. Of secondary concern (since I think it is
> > > > far less
> > > > liklely) would be confusing some software somewhere.
> > > > 
> > [...]
> > > This just sets eth1addr in the u-boot env,
> > 
> > It's this which I worried might confuse people, people who notice
> > eth1addr (perhaps due to tab completion on "printenv eth"?) will
> > wonder
> > where the eth1 device is and/or why it is not working for them.
> 
> People who use the u-boot cmdline at all really are experienced users
> so TBH I'm not all that worried about this.

I know that I personally once wasted quite a bit of time (on arndale,
but still) being mislead/confused by different eth*addr variables
(arndale has another possible prefix too, usbaddr IIRC, which makes it
doubly confusing) and what applies to what and when. So IME having
irrelevant envvars like that floating around in the default env really
does lead to confusion even for people who (supposedly) know what they
are doing.

Even naive users will find random guides online which don't quite apply
to their particular board (especially likely with the vast number of
sunxi variants in existence) and get lead down the wrong path (which
should be "fail early since the envvar discussed doesn't exist)

Ian.

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Hans de Goede]

Currently we will already fill ethaddr with a fixed unique address
based on the SoCs serial (from the sid) to make sure that boards which
use the integrated emac / gmac get a fixed mac rather then a random one.

On some boards (observed on 2 tablets using sdio rtl8703as wifi chips)
the wifi does not come with a fixed mac either, so also set eth1addr,
so that dts files can set an ethernet1 alias to get mac-address and
local-mac-address filled for dt nodes describing the wifi controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/board.c | 59 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 16 deletions(-)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 78dfda5..955a44f 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -26,6 +26,7 @@
 #endif
 #include <asm/gpio.h>
 #include <asm/io.h>
+#include <libfdt.h>
 #include <nand.h>
 #include <net.h>
 #include <sy8106a.h>
@@ -609,36 +610,38 @@ static void parse_spl_header(const uint32_t spl_addr)
 }
 #endif
 
-#ifdef CONFIG_MISC_INIT_R
-int misc_init_r(void)
+static void setup_environment(const void *fdt)
 {
 	char serial_string[17] = { 0 };
 	unsigned int sid[4];
 	uint8_t mac_addr[6];
-	int ret;
-
-#if !defined(CONFIG_SPL_BUILD)
-	setenv("fel_booted", NULL);
-	setenv("fel_scriptaddr", NULL);
-	/* determine if we are running in FEL mode */
-	if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
-		setenv("fel_booted", "1");
-		parse_spl_header(SPL_ADDR);
-	}
-#endif
+	char ethaddr[16];
+	int i, ret;
 
 	ret = sunxi_get_sid(sid);
 	if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
-		if (!getenv("ethaddr")) {
+		for (i = 0; i < 4; i++) {
+			sprintf(ethaddr, "ethernet%d", i);
+			if (!fdt_get_alias(fdt, ethaddr))
+				continue;
+
+			if (i == 0)
+				strcpy(ethaddr, "ethaddr");
+			else
+				sprintf(ethaddr, "eth%daddr", i);
+
+			if (getenv(ethaddr))
+				continue;
+
 			/* Non OUI / registered MAC address */
-			mac_addr[0] = 0x02;
+			mac_addr[0] = (i << 4) | 0x02;
 			mac_addr[1] = (sid[0] >>  0) & 0xff;
 			mac_addr[2] = (sid[3] >> 24) & 0xff;
 			mac_addr[3] = (sid[3] >> 16) & 0xff;
 			mac_addr[4] = (sid[3] >>  8) & 0xff;
 			mac_addr[5] = (sid[3] >>  0) & 0xff;
 
-			eth_setenv_enetaddr("ethaddr", mac_addr);
+			eth_setenv_enetaddr(ethaddr, mac_addr);
 		}
 
 		if (!getenv("serial#")) {
@@ -648,6 +651,24 @@ int misc_init_r(void)
 			setenv("serial#", serial_string);
 		}
 	}
+}
+
+#ifdef CONFIG_MISC_INIT_R
+int misc_init_r(void)
+{
+	int ret;
+
+#if !defined(CONFIG_SPL_BUILD)
+	setenv("fel_booted", NULL);
+	setenv("fel_scriptaddr", NULL);
+	/* determine if we are running in FEL mode */
+	if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
+		setenv("fel_booted", "1");
+		parse_spl_header(SPL_ADDR);
+	}
+#endif
+
+	setup_environment(gd->fdt_blob);
 
 #ifndef CONFIG_MACH_SUN9I
 	ret = sunxi_usb_phy_probe();
@@ -664,6 +685,12 @@ int ft_board_setup(void *blob, bd_t *bd)
 {
 	int __maybe_unused r;
 
+	/*
+	 * Call setup_environment again in case the boot fdt has
+	 * ethernet aliases the u-boot copy does not have.
+	 */
+	setup_environment(blob);
+
 #ifdef CONFIG_VIDEO_DT_SIMPLEFB
 	r = sunxi_simplefb_setup(blob);
 	if (r)
-- 
2.7.4

[U-Boot] [PATCH] sunxi: Add support for eth1addr

[Hans de Goede]

Hi,

On 09-07-16 15:05, Hans de Goede wrote:
> Currently we will already fill ethaddr with a fixed unique address
> based on the SoCs serial (from the sid) to make sure that boards which
> use the integrated emac / gmac get a fixed mac rather then a random one.
>
> On some boards (observed on 2 tablets using sdio rtl8703as wifi chips)
> the wifi does not come with a fixed mac either, so also set eth1addr,
> so that dts files can set an ethernet1 alias to get mac-address and
> local-mac-address filled for dt nodes describing the wifi controller.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Oops, I forgot to update the commit msg and mark this as v2, I will
send a new version instead.

Regards,

Hans


> ---
>  board/sunxi/board.c | 59 ++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 78dfda5..955a44f 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -26,6 +26,7 @@
>  #endif
>  #include <asm/gpio.h>
>  #include <asm/io.h>
> +#include <libfdt.h>
>  #include <nand.h>
>  #include <net.h>
>  #include <sy8106a.h>
> @@ -609,36 +610,38 @@ static void parse_spl_header(const uint32_t spl_addr)
>  }
>  #endif
>
> -#ifdef CONFIG_MISC_INIT_R
> -int misc_init_r(void)
> +static void setup_environment(const void *fdt)
>  {
>  	char serial_string[17] = { 0 };
>  	unsigned int sid[4];
>  	uint8_t mac_addr[6];
> -	int ret;
> -
> -#if !defined(CONFIG_SPL_BUILD)
> -	setenv("fel_booted", NULL);
> -	setenv("fel_scriptaddr", NULL);
> -	/* determine if we are running in FEL mode */
> -	if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
> -		setenv("fel_booted", "1");
> -		parse_spl_header(SPL_ADDR);
> -	}
> -#endif
> +	char ethaddr[16];
> +	int i, ret;
>
>  	ret = sunxi_get_sid(sid);
>  	if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
> -		if (!getenv("ethaddr")) {
> +		for (i = 0; i < 4; i++) {
> +			sprintf(ethaddr, "ethernet%d", i);
> +			if (!fdt_get_alias(fdt, ethaddr))
> +				continue;
> +
> +			if (i == 0)
> +				strcpy(ethaddr, "ethaddr");
> +			else
> +				sprintf(ethaddr, "eth%daddr", i);
> +
> +			if (getenv(ethaddr))
> +				continue;
> +
>  			/* Non OUI / registered MAC address */
> -			mac_addr[0] = 0x02;
> +			mac_addr[0] = (i << 4) | 0x02;
>  			mac_addr[1] = (sid[0] >>  0) & 0xff;
>  			mac_addr[2] = (sid[3] >> 24) & 0xff;
>  			mac_addr[3] = (sid[3] >> 16) & 0xff;
>  			mac_addr[4] = (sid[3] >>  8) & 0xff;
>  			mac_addr[5] = (sid[3] >>  0) & 0xff;
>
> -			eth_setenv_enetaddr("ethaddr", mac_addr);
> +			eth_setenv_enetaddr(ethaddr, mac_addr);
>  		}
>
>  		if (!getenv("serial#")) {
> @@ -648,6 +651,24 @@ int misc_init_r(void)
>  			setenv("serial#", serial_string);
>  		}
>  	}
> +}
> +
> +#ifdef CONFIG_MISC_INIT_R
> +int misc_init_r(void)
> +{
> +	int ret;
> +
> +#if !defined(CONFIG_SPL_BUILD)
> +	setenv("fel_booted", NULL);
> +	setenv("fel_scriptaddr", NULL);
> +	/* determine if we are running in FEL mode */
> +	if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
> +		setenv("fel_booted", "1");
> +		parse_spl_header(SPL_ADDR);
> +	}
> +#endif
> +
> +	setup_environment(gd->fdt_blob);
>
>  #ifndef CONFIG_MACH_SUN9I
>  	ret = sunxi_usb_phy_probe();
> @@ -664,6 +685,12 @@ int ft_board_setup(void *blob, bd_t *bd)
>  {
>  	int __maybe_unused r;
>
> +	/*
> +	 * Call setup_environment again in case the boot fdt has
> +	 * ethernet aliases the u-boot copy does not have.
> +	 */
> +	setup_environment(blob);
> +
>  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
>  	r = sunxi_simplefb_setup(blob);
>  	if (r)
>