diff mbox

[U-Boot,5/6] arm: sunxi: Use board hooks to obtain MAC address

Message ID 20161125153838.15366-6-oliver@schinagl.nl
State Changes Requested
Delegated to: Joe Hershberger
Headers show

Commit Message

Olliver Schinagl Nov. 25, 2016, 3:38 p.m. UTC
Add board hooks allowing to get ethernet addresses in a board specific
manner. Currently this is done by generating a MAC address from
the SID and injecting the ethernet device number in the first octet.

This usually happens as a fallback, if either the eeprom fails to set a
MAC address or the FDT forces an override.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
 board/sunxi/board.c                         | 161 +++++++++++++++-------------
 net/eth_legacy.c                            |   1 +
 3 files changed, 98 insertions(+), 75 deletions(-)

Comments

Joe Hershberger Nov. 30, 2016, 9:36 p.m. UTC | #1
On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Add board hooks allowing to get ethernet addresses in a board specific
> manner. Currently this is done by generating a MAC address from
> the SID and injecting the ethernet device number in the first octet.
>
> This usually happens as a fallback, if either the eeprom fails to set a
> MAC address or the FDT forces an override.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>  board/sunxi/board.c                         | 161 +++++++++++++++-------------
>  net/eth_legacy.c                            |   1 +
>  3 files changed, 98 insertions(+), 75 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> index a373319..fad7c48 100644
> --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
> +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> @@ -30,4 +30,15 @@ void eth_init_board(void);
>  static inline void eth_init_board(void) {}
>  #endif
>
> +int board_get_enetaddr(const int i, unsigned char *mac_addr);
> +
> +#if CONFIG_SUNXI_EMAC
> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
> +#endif
> +
> +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
> +#endif
> +
> +
>  #endif
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 5365638..4aeab51 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -21,6 +21,7 @@
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/mmc.h>
>  #include <asm/arch/spl.h>
> +#include <asm/arch/sys_proto.h>
>  #include <asm/arch/usb_phy.h>
>  #ifndef CONFIG_ARM64
>  #include <asm/armv7.h>
> @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>  }
>  #endif
>
> +int sunxi_get_board_serial(unsigned int *serial)
> +{
> +       int ret;
> +
> +       ret = sunxi_get_sid(serial);
> +       if (!ret || serial[0])
> +               return -ENOSYS;
> +
> +       /*
> +        * The single words 1 - 3 of the SID have quite a few bits
> +        * which are the same on many models, so we take a crc32
> +        * of all 3 words, to get a more unique value.
> +        *
> +        * Note we only do this on newer SoCs as we cannot change
> +        * the algorithm on older SoCs since those have been using
> +        * fixed mac-addresses/serial based on only using word 3 for a
> +        * long time and changing a fixed mac-address/serial with an
> +        * u-boot update is not good.
> +        */
> +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
> +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
> +    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
> +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
> +#endif
> +
> +       return 0;
> +}
> +
>  #ifdef CONFIG_SERIAL_TAG
>  void get_board_serial(struct tag_serialnr *serialnr)
>  {
> @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr *serialnr)
>  #endif
>
>  /*
> + * Generate a MAC address based on device index and the serial number.
> + * The first half of the of the first octet holds the eth index.
> + *
> + * In the second octet we forcefully mark the MAC address to a locally
> + * administered MAC address.
> + *
> + */
> +int board_get_enetaddr(const int index, unsigned char *enetaddr)

This would be part of a board-specific eth driver.

> +{
> +       uint8_t mac_addr[ARP_HLEN] = { 0x00 };
> +       unsigned int serial[4];
> +       int ret;
> +
> +       if ((index < 0) || !enetaddr)
> +               return -ENOSYS;
> +
> +       ret = sunxi_get_board_serial(serial);
> +       if (!ret)
> +               return ret;
> +
> +       /* Ensure the NIC specific bytes of the mac are not all 0 */
> +       if ((serial[3] & 0xffffff) == 0)
> +               serial[3] |= 0x800000;
> +
> +       mac_addr[0] = (index << 4);
> +       mac_addr[1] = (serial[0] >>  0) & 0xff;
> +       mac_addr[2] = (serial[3] >> 24) & 0xff;
> +       mac_addr[3] = (serial[3] >> 16) & 0xff;
> +       mac_addr[4] = (serial[3] >>  8) & 0xff;
> +       mac_addr[5] = (serial[3] >>  0) & 0xff;
> +
> +       set_local_ethaddr(mac_addr);
> +       memcpy(enetaddr, mac_addr, ARP_HLEN);
> +
> +       return 0;
> +}
> +
> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> +{
> +       return board_get_enetaddr(id, enetaddr);
> +}
> +
> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> +{
> +       return board_get_enetaddr(id, enetaddr);
> +}
> +
> +/*
>   * Check the SPL header for the "sunxi" variant. If found: parse values
>   * that might have been passed by the loader ("fel" utility), and update
>   * the environment accordingly.
> @@ -617,77 +694,10 @@ static void parse_spl_header(const uint32_t spl_addr)
>         setenv_hex("fel_scriptaddr", spl->fel_script_address);
>  }
>
> -/*
> - * Note this function gets called multiple times.
> - * It must not make any changes to env variables which already exist.
> - */
> -static void setup_environment(const void *fdt)
> -{
> -       char serial_string[17] = { 0 };
> -       unsigned int sid[4];
> -       uint8_t mac_addr[6];
> -       char ethaddr[16];
> -       int i, ret;
> -
> -       ret = sunxi_get_sid(sid);
> -       if (ret == 0 && sid[0] != 0) {
> -               /*
> -                * The single words 1 - 3 of the SID have quite a few bits
> -                * which are the same on many models, so we take a crc32
> -                * of all 3 words, to get a more unique value.
> -                *
> -                * Note we only do this on newer SoCs as we cannot change
> -                * the algorithm on older SoCs since those have been using
> -                * fixed mac-addresses based on only using word 3 for a
> -                * long time and changing a fixed mac-address with an
> -                * u-boot update is not good.
> -                */
> -#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
> -    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
> -    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
> -               sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
> -#endif
> -
> -               /* Ensure the NIC specific bytes of the mac are not all 0 */
> -               if ((sid[3] & 0xffffff) == 0)
> -                       sid[3] |= 0x800000;
> -
> -               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] = (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);
> -               }
> -
> -               if (!getenv("serial#")) {
> -                       snprintf(serial_string, sizeof(serial_string),
> -                               "%08x%08x", sid[0], sid[3]);
> -
> -                       setenv("serial#", serial_string);
> -               }
> -       }
> -}
> -
>  int misc_init_r(void)
>  {
>         __maybe_unused int ret;
> +       unsigned int serial[4];
>
>         setenv("fel_booted", NULL);
>         setenv("fel_scriptaddr", NULL);
> @@ -697,7 +707,14 @@ int misc_init_r(void)
>                 parse_spl_header(SPL_ADDR);
>         }
>
> -       setup_environment(gd->fdt_blob);
> +       if (sunxi_get_board_serial(serial)) {
> +               char serial_string[17] = { 0 };
> +
> +               snprintf(serial_string, sizeof(serial_string),
> +                        "%08x%08x", serial[0], serial[3]);
> +
> +               setenv("serial#", serial_string);
> +       }
>
>  #ifndef CONFIG_MACH_SUN9I
>         ret = sunxi_usb_phy_probe();
> @@ -713,12 +730,6 @@ 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)
> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> index d6d7cee..b8b1e3b 100644
> --- a/net/eth_legacy.c
> +++ b/net/eth_legacy.c
> @@ -10,6 +10,7 @@
>  #include <command.h>
>  #include <environment.h>
>  #include <net.h>
> +#include <i2c.h>
>  #include <phy.h>
>  #include <linux/errno.h>
>  #include "eth_internal.h"
> --
> 2.10.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Olliver Schinagl April 7, 2017, 1:45 p.m. UTC | #2
Hey Joe,

On 30-11-16 22:36, Joe Hershberger wrote:
> On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
>> Add board hooks allowing to get ethernet addresses in a board specific
>> manner. Currently this is done by generating a MAC address from
>> the SID and injecting the ethernet device number in the first octet.
>>
>> This usually happens as a fallback, if either the eeprom fails to set a
>> MAC address or the FDT forces an override.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>>  board/sunxi/board.c                         | 161 +++++++++++++++-------------
>>  net/eth_legacy.c                            |   1 +
>>  3 files changed, 98 insertions(+), 75 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> index a373319..fad7c48 100644
>> --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> @@ -30,4 +30,15 @@ void eth_init_board(void);
>>  static inline void eth_init_board(void) {}
>>  #endif
>>
>> +int board_get_enetaddr(const int i, unsigned char *mac_addr);
>> +
>> +#if CONFIG_SUNXI_EMAC
>> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>> +#endif
>> +
>> +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
>> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>> +#endif
>> +
>> +
>>  #endif
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index 5365638..4aeab51 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -21,6 +21,7 @@
>>  #include <asm/arch/gpio.h>
>>  #include <asm/arch/mmc.h>
>>  #include <asm/arch/spl.h>
>> +#include <asm/arch/sys_proto.h>
>>  #include <asm/arch/usb_phy.h>
>>  #ifndef CONFIG_ARM64
>>  #include <asm/armv7.h>
>> @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>>  }
>>  #endif
>>
>> +int sunxi_get_board_serial(unsigned int *serial)
>> +{
>> +       int ret;
>> +
>> +       ret = sunxi_get_sid(serial);
>> +       if (!ret || serial[0])
>> +               return -ENOSYS;
>> +
>> +       /*
>> +        * The single words 1 - 3 of the SID have quite a few bits
>> +        * which are the same on many models, so we take a crc32
>> +        * of all 3 words, to get a more unique value.
>> +        *
>> +        * Note we only do this on newer SoCs as we cannot change
>> +        * the algorithm on older SoCs since those have been using
>> +        * fixed mac-addresses/serial based on only using word 3 for a
>> +        * long time and changing a fixed mac-address/serial with an
>> +        * u-boot update is not good.
>> +        */
>> +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
>> +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
>> +    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
>> +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
>> +#endif
>> +
>> +       return 0;
>> +}
>> +
>>  #ifdef CONFIG_SERIAL_TAG
>>  void get_board_serial(struct tag_serialnr *serialnr)
>>  {
>> @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr *serialnr)
>>  #endif
>>
>>  /*
>> + * Generate a MAC address based on device index and the serial number.
>> + * The first half of the of the first octet holds the eth index.
>> + *
>> + * In the second octet we forcefully mark the MAC address to a locally
>> + * administered MAC address.
>> + *
>> + */
>> +int board_get_enetaddr(const int index, unsigned char *enetaddr)
>
> This would be part of a board-specific eth driver.

this is being called now from sunxi_gmac.c and sunxi_emac.c and supplies 
these board specific drivers with a mac address based on the serial 
number of the board. I could move this logic over, but then i'd have to 
add it to both eth drivers. By having it in the board.c file, we have 2 
simple functions in the board-specific eth driver:


static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
{
	struct eth_pdata *pdata = dev_get_platdata(dev);

	return board_get_enetaddr(dev->seq, pdata->enetaddr);
}

So do you propose to dupilicate the code into both board specific 
drivers, have it named differently or that the shared code live elsewhere?

Olliver
>
>> +{
>> +       uint8_t mac_addr[ARP_HLEN] = { 0x00 };
>> +       unsigned int serial[4];
>> +       int ret;
>> +
>> +       if ((index < 0) || !enetaddr)
>> +               return -ENOSYS;
>> +
>> +       ret = sunxi_get_board_serial(serial);
>> +       if (!ret)
>> +               return ret;
>> +
>> +       /* Ensure the NIC specific bytes of the mac are not all 0 */
>> +       if ((serial[3] & 0xffffff) == 0)
>> +               serial[3] |= 0x800000;
>> +
>> +       mac_addr[0] = (index << 4);
>> +       mac_addr[1] = (serial[0] >>  0) & 0xff;
>> +       mac_addr[2] = (serial[3] >> 24) & 0xff;
>> +       mac_addr[3] = (serial[3] >> 16) & 0xff;
>> +       mac_addr[4] = (serial[3] >>  8) & 0xff;
>> +       mac_addr[5] = (serial[3] >>  0) & 0xff;
>> +
>> +       set_local_ethaddr(mac_addr);
>> +       memcpy(enetaddr, mac_addr, ARP_HLEN);
>> +
>> +       return 0;
>> +}
>> +
>> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>> +{
>> +       return board_get_enetaddr(id, enetaddr);
>> +}
>> +
>> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>> +{
>> +       return board_get_enetaddr(id, enetaddr);
>> +}
>> +
>> +/*
>>   * Check the SPL header for the "sunxi" variant. If found: parse values
>>   * that might have been passed by the loader ("fel" utility), and update
>>   * the environment accordingly.
>> @@ -617,77 +694,10 @@ static void parse_spl_header(const uint32_t spl_addr)
>>         setenv_hex("fel_scriptaddr", spl->fel_script_address);
>>  }
>>
>> -/*
>> - * Note this function gets called multiple times.
>> - * It must not make any changes to env variables which already exist.
>> - */
>> -static void setup_environment(const void *fdt)
>> -{
>> -       char serial_string[17] = { 0 };
>> -       unsigned int sid[4];
>> -       uint8_t mac_addr[6];
>> -       char ethaddr[16];
>> -       int i, ret;
>> -
>> -       ret = sunxi_get_sid(sid);
>> -       if (ret == 0 && sid[0] != 0) {
>> -               /*
>> -                * The single words 1 - 3 of the SID have quite a few bits
>> -                * which are the same on many models, so we take a crc32
>> -                * of all 3 words, to get a more unique value.
>> -                *
>> -                * Note we only do this on newer SoCs as we cannot change
>> -                * the algorithm on older SoCs since those have been using
>> -                * fixed mac-addresses based on only using word 3 for a
>> -                * long time and changing a fixed mac-address with an
>> -                * u-boot update is not good.
>> -                */
>> -#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
>> -    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
>> -    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
>> -               sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
>> -#endif
>> -
>> -               /* Ensure the NIC specific bytes of the mac are not all 0 */
>> -               if ((sid[3] & 0xffffff) == 0)
>> -                       sid[3] |= 0x800000;
>> -
>> -               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] = (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);
>> -               }
>> -
>> -               if (!getenv("serial#")) {
>> -                       snprintf(serial_string, sizeof(serial_string),
>> -                               "%08x%08x", sid[0], sid[3]);
>> -
>> -                       setenv("serial#", serial_string);
>> -               }
>> -       }
>> -}
>> -
>>  int misc_init_r(void)
>>  {
>>         __maybe_unused int ret;
>> +       unsigned int serial[4];
>>
>>         setenv("fel_booted", NULL);
>>         setenv("fel_scriptaddr", NULL);
>> @@ -697,7 +707,14 @@ int misc_init_r(void)
>>                 parse_spl_header(SPL_ADDR);
>>         }
>>
>> -       setup_environment(gd->fdt_blob);
>> +       if (sunxi_get_board_serial(serial)) {
>> +               char serial_string[17] = { 0 };
>> +
>> +               snprintf(serial_string, sizeof(serial_string),
>> +                        "%08x%08x", serial[0], serial[3]);
>> +
>> +               setenv("serial#", serial_string);
>> +       }
>>
>>  #ifndef CONFIG_MACH_SUN9I
>>         ret = sunxi_usb_phy_probe();
>> @@ -713,12 +730,6 @@ 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)
>> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
>> index d6d7cee..b8b1e3b 100644
>> --- a/net/eth_legacy.c
>> +++ b/net/eth_legacy.c
>> @@ -10,6 +10,7 @@
>>  #include <command.h>
>>  #include <environment.h>
>>  #include <net.h>
>> +#include <i2c.h>
>>  #include <phy.h>
>>  #include <linux/errno.h>
>>  #include "eth_internal.h"
>> --
>> 2.10.2
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
Olliver Schinagl April 7, 2017, 1:57 p.m. UTC | #3
On Fri, 2017-04-07 at 15:45 +0200, Olliver Schinagl wrote:
> Hey Joe,
> 
> On 30-11-16 22:36, Joe Hershberger wrote:
> > On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.
> > nl> wrote:
> > > Add board hooks allowing to get ethernet addresses in a board
> > > specific
> > > manner. Currently this is done by generating a MAC address from
> > > the SID and injecting the ethernet device number in the first
> > > octet.
> > > 
> > > This usually happens as a fallback, if either the eeprom fails to
> > > set a
> > > MAC address or the FDT forces an override.
> > > 
> > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> > > ---
> > >  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
> > >  board/sunxi/board.c                         | 161
> > > +++++++++++++++-------------
> > >  net/eth_legacy.c                            |   1 +
> > >  3 files changed, 98 insertions(+), 75 deletions(-)
> > > 
> > > diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > index a373319..fad7c48 100644
> > > --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > @@ -30,4 +30,15 @@ void eth_init_board(void);
> > >  static inline void eth_init_board(void) {}
> > >  #endif
> > > 
> > > +int board_get_enetaddr(const int i, unsigned char *mac_addr);
> > > +
> > > +#if CONFIG_SUNXI_EMAC
> > > +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
> > > int id);
> > > +#endif
> > > +
> > > +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
> > > +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
> > > +#endif
> > > +
> > > +
> > >  #endif
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index 5365638..4aeab51 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -21,6 +21,7 @@
> > >  #include <asm/arch/gpio.h>
> > >  #include <asm/arch/mmc.h>
> > >  #include <asm/arch/spl.h>
> > > +#include <asm/arch/sys_proto.h>
> > >  #include <asm/arch/usb_phy.h>
> > >  #ifndef CONFIG_ARM64
> > >  #include <asm/armv7.h>
> > > @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
> > >  }
> > >  #endif
> > > 
> > > +int sunxi_get_board_serial(unsigned int *serial)
> > > +{
> > > +       int ret;
> > > +
> > > +       ret = sunxi_get_sid(serial);
> > > +       if (!ret || serial[0])
> > > +               return -ENOSYS;
> > > +
> > > +       /*
> > > +        * The single words 1 - 3 of the SID have quite a few
> > > bits
> > > +        * which are the same on many models, so we take a crc32
> > > +        * of all 3 words, to get a more unique value.
> > > +        *
> > > +        * Note we only do this on newer SoCs as we cannot change
> > > +        * the algorithm on older SoCs since those have been
> > > using
> > > +        * fixed mac-addresses/serial based on only using word 3
> > > for a
> > > +        * long time and changing a fixed mac-address/serial with
> > > an
> > > +        * u-boot update is not good.
> > > +        */
> > > +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
> > > && \
> > > +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
> > > && \
> > > +    !defined(CONFIG_MACH_SUN8I_A23) &&
> > > !defined(CONFIG_MACH_SUN8I_A33)
> > > +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
> > > +#endif
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  #ifdef CONFIG_SERIAL_TAG
> > >  void get_board_serial(struct tag_serialnr *serialnr)
> > >  {
> > > @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr
> > > *serialnr)
> > >  #endif
> > > 
> > >  /*
> > > + * Generate a MAC address based on device index and the serial
> > > number.
> > > + * The first half of the of the first octet holds the eth index.
> > > + *
> > > + * In the second octet we forcefully mark the MAC address to a
> > > locally
> > > + * administered MAC address.
> > > + *
> > > + */
> > > +int board_get_enetaddr(const int index, unsigned char *enetaddr)
> > 
> > This would be part of a board-specific eth driver.
> 
> this is being called now from sunxi_gmac.c and sunxi_emac.c and
> supplies 
> these board specific drivers with a mac address based on the serial 
> number of the board. I could move this logic over, but then i'd have
> to 
> add it to both eth drivers. By having it in the board.c file, we have
> 2 
> simple functions in the board-specific eth driver:
> 
> 
> static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
> {
> 	struct eth_pdata *pdata = dev_get_platdata(dev);
> 
> 	return board_get_enetaddr(dev->seq, pdata->enetaddr);
> }
> 
> So do you propose to dupilicate the code into both board specific 
> drivers, have it named differently or that the shared code live
> elsewhere?
Replying to myself here,

I just realized, while this bit was not accepted, the overal
implementation has changed in the set. So before, I did things wrong :)
As Simon explained last time.

To clarify, I now have added the logic to the sunxi gmac and emac board
specific drivers. But afaik they share no common code. (like
sunxi_common.c in drivers/net)

With that in mind, how we did things up until now, was to have a
fallback scenario where we use the SoC serial number to generate a MAC
address.

If this is go be done with the board specific driver, we'd need to
still however call board specific functions (sunxi_get_board_serial).

The solution I think is still one of the previously mentioned, a
sunxi_common.c which does the serial -> MAC conversion according to the
previous logic, using sunxi_get_board_serial() (which really is a SoC
specific function, rather board) or have (sunxi)_board_get_enetaddr()
in the same spot where it is now.

Writing this, I realize the sunxi_common.c approach may not be half
bad, even if it only contains a single function for now.

Olliver

> 
> Olliver
> > 
> > > +{
> > > +       uint8_t mac_addr[ARP_HLEN] = { 0x00 };
> > > +       unsigned int serial[4];
> > > +       int ret;
> > > +
> > > +       if ((index < 0) || !enetaddr)
> > > +               return -ENOSYS;
> > > +
> > > +       ret = sunxi_get_board_serial(serial);
> > > +       if (!ret)
> > > +               return ret;
> > > +
> > > +       /* Ensure the NIC specific bytes of the mac are not all 0
> > > */
> > > +       if ((serial[3] & 0xffffff) == 0)
> > > +               serial[3] |= 0x800000;
> > > +
> > > +       mac_addr[0] = (index << 4);
> > > +       mac_addr[1] = (serial[0] >>  0) & 0xff;
> > > +       mac_addr[2] = (serial[3] >> 24) & 0xff;
> > > +       mac_addr[3] = (serial[3] >> 16) & 0xff;
> > > +       mac_addr[4] = (serial[3] >>  8) & 0xff;
> > > +       mac_addr[5] = (serial[3] >>  0) & 0xff;
> > > +
> > > +       set_local_ethaddr(mac_addr);
> > > +       memcpy(enetaddr, mac_addr, ARP_HLEN);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
> > > int id)
> > > +{
> > > +       return board_get_enetaddr(id, enetaddr);
> > > +}
> > > +
> > > +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> > > +{
> > > +       return board_get_enetaddr(id, enetaddr);
> > > +}
> > > +
> > > +/*
> > >   * Check the SPL header for the "sunxi" variant. If found: parse
> > > values
> > >   * that might have been passed by the loader ("fel" utility),
> > > and update
> > >   * the environment accordingly.
> > > @@ -617,77 +694,10 @@ static void parse_spl_header(const uint32_t
> > > spl_addr)
> > >         setenv_hex("fel_scriptaddr", spl->fel_script_address);
> > >  }
> > > 
> > > -/*
> > > - * Note this function gets called multiple times.
> > > - * It must not make any changes to env variables which already
> > > exist.
> > > - */
> > > -static void setup_environment(const void *fdt)
> > > -{
> > > -       char serial_string[17] = { 0 };
> > > -       unsigned int sid[4];
> > > -       uint8_t mac_addr[6];
> > > -       char ethaddr[16];
> > > -       int i, ret;
> > > -
> > > -       ret = sunxi_get_sid(sid);
> > > -       if (ret == 0 && sid[0] != 0) {
> > > -               /*
> > > -                * The single words 1 - 3 of the SID have quite a
> > > few bits
> > > -                * which are the same on many models, so we take
> > > a crc32
> > > -                * of all 3 words, to get a more unique value.
> > > -                *
> > > -                * Note we only do this on newer SoCs as we
> > > cannot change
> > > -                * the algorithm on older SoCs since those have
> > > been using
> > > -                * fixed mac-addresses based on only using word 3
> > > for a
> > > -                * long time and changing a fixed mac-address
> > > with an
> > > -                * u-boot update is not good.
> > > -                */
> > > -#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
> > > && \
> > > -    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
> > > && \
> > > -    !defined(CONFIG_MACH_SUN8I_A23) &&
> > > !defined(CONFIG_MACH_SUN8I_A33)
> > > -               sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
> > > -#endif
> > > -
> > > -               /* Ensure the NIC specific bytes of the mac are
> > > not all 0 */
> > > -               if ((sid[3] & 0xffffff) == 0)
> > > -                       sid[3] |= 0x800000;
> > > -
> > > -               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] = (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);
> > > -               }
> > > -
> > > -               if (!getenv("serial#")) {
> > > -                       snprintf(serial_string,
> > > sizeof(serial_string),
> > > -                               "%08x%08x", sid[0], sid[3]);
> > > -
> > > -                       setenv("serial#", serial_string);
> > > -               }
> > > -       }
> > > -}
> > > -
> > >  int misc_init_r(void)
> > >  {
> > >         __maybe_unused int ret;
> > > +       unsigned int serial[4];
> > > 
> > >         setenv("fel_booted", NULL);
> > >         setenv("fel_scriptaddr", NULL);
> > > @@ -697,7 +707,14 @@ int misc_init_r(void)
> > >                 parse_spl_header(SPL_ADDR);
> > >         }
> > > 
> > > -       setup_environment(gd->fdt_blob);
> > > +       if (sunxi_get_board_serial(serial)) {
> > > +               char serial_string[17] = { 0 };
> > > +
> > > +               snprintf(serial_string, sizeof(serial_string),
> > > +                        "%08x%08x", serial[0], serial[3]);
> > > +
> > > +               setenv("serial#", serial_string);
> > > +       }
> > > 
> > >  #ifndef CONFIG_MACH_SUN9I
> > >         ret = sunxi_usb_phy_probe();
> > > @@ -713,12 +730,6 @@ 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)
> > > diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> > > index d6d7cee..b8b1e3b 100644
> > > --- a/net/eth_legacy.c
> > > +++ b/net/eth_legacy.c
> > > @@ -10,6 +10,7 @@
> > >  #include <command.h>
> > >  #include <environment.h>
> > >  #include <net.h>
> > > +#include <i2c.h>
> > >  #include <phy.h>
> > >  #include <linux/errno.h>
> > >  #include "eth_internal.h"
> > > --
> > > 2.10.2
> > > 
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot@lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Joe Hershberger April 10, 2017, 10:56 p.m. UTC | #4
On Fri, Apr 7, 2017 at 8:57 AM,  <o.schinagl@ultimaker.com> wrote:
> On Fri, 2017-04-07 at 15:45 +0200, Olliver Schinagl wrote:
>> Hey Joe,
>>
>> On 30-11-16 22:36, Joe Hershberger wrote:
>> > On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.
>> > nl> wrote:
>> > > Add board hooks allowing to get ethernet addresses in a board
>> > > specific
>> > > manner. Currently this is done by generating a MAC address from
>> > > the SID and injecting the ethernet device number in the first
>> > > octet.
>> > >
>> > > This usually happens as a fallback, if either the eeprom fails to
>> > > set a
>> > > MAC address or the FDT forces an override.
>> > >
>> > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> > > ---
>> > >  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>> > >  board/sunxi/board.c                         | 161
>> > > +++++++++++++++-------------
>> > >  net/eth_legacy.c                            |   1 +
>> > >  3 files changed, 98 insertions(+), 75 deletions(-)
>> > >
>> > > diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > index a373319..fad7c48 100644
>> > > --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > @@ -30,4 +30,15 @@ void eth_init_board(void);
>> > >  static inline void eth_init_board(void) {}
>> > >  #endif
>> > >
>> > > +int board_get_enetaddr(const int i, unsigned char *mac_addr);
>> > > +
>> > > +#if CONFIG_SUNXI_EMAC
>> > > +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
>> > > int id);
>> > > +#endif
>> > > +
>> > > +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
>> > > +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>> > > +#endif
>> > > +
>> > > +
>> > >  #endif
>> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> > > index 5365638..4aeab51 100644
>> > > --- a/board/sunxi/board.c
>> > > +++ b/board/sunxi/board.c
>> > > @@ -21,6 +21,7 @@
>> > >  #include <asm/arch/gpio.h>
>> > >  #include <asm/arch/mmc.h>
>> > >  #include <asm/arch/spl.h>
>> > > +#include <asm/arch/sys_proto.h>
>> > >  #include <asm/arch/usb_phy.h>
>> > >  #ifndef CONFIG_ARM64
>> > >  #include <asm/armv7.h>
>> > > @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>> > >  }
>> > >  #endif
>> > >
>> > > +int sunxi_get_board_serial(unsigned int *serial)
>> > > +{
>> > > +       int ret;
>> > > +
>> > > +       ret = sunxi_get_sid(serial);
>> > > +       if (!ret || serial[0])
>> > > +               return -ENOSYS;
>> > > +
>> > > +       /*
>> > > +        * The single words 1 - 3 of the SID have quite a few
>> > > bits
>> > > +        * which are the same on many models, so we take a crc32
>> > > +        * of all 3 words, to get a more unique value.
>> > > +        *
>> > > +        * Note we only do this on newer SoCs as we cannot change
>> > > +        * the algorithm on older SoCs since those have been
>> > > using
>> > > +        * fixed mac-addresses/serial based on only using word 3
>> > > for a
>> > > +        * long time and changing a fixed mac-address/serial with
>> > > an
>> > > +        * u-boot update is not good.
>> > > +        */
>> > > +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
>> > > && \
>> > > +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
>> > > && \
>> > > +    !defined(CONFIG_MACH_SUN8I_A23) &&
>> > > !defined(CONFIG_MACH_SUN8I_A33)
>> > > +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
>> > > +#endif
>> > > +
>> > > +       return 0;
>> > > +}
>> > > +
>> > >  #ifdef CONFIG_SERIAL_TAG
>> > >  void get_board_serial(struct tag_serialnr *serialnr)
>> > >  {
>> > > @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr
>> > > *serialnr)
>> > >  #endif
>> > >
>> > >  /*
>> > > + * Generate a MAC address based on device index and the serial
>> > > number.
>> > > + * The first half of the of the first octet holds the eth index.
>> > > + *
>> > > + * In the second octet we forcefully mark the MAC address to a
>> > > locally
>> > > + * administered MAC address.
>> > > + *
>> > > + */
>> > > +int board_get_enetaddr(const int index, unsigned char *enetaddr)
>> >
>> > This would be part of a board-specific eth driver.
>>
>> this is being called now from sunxi_gmac.c and sunxi_emac.c and
>> supplies
>> these board specific drivers with a mac address based on the serial
>> number of the board. I could move this logic over, but then i'd have
>> to
>> add it to both eth drivers. By having it in the board.c file, we have
>> 2
>> simple functions in the board-specific eth driver:
>>
>>
>> static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
>> {
>>       struct eth_pdata *pdata = dev_get_platdata(dev);
>>
>>       return board_get_enetaddr(dev->seq, pdata->enetaddr);
>> }
>>
>> So do you propose to dupilicate the code into both board specific
>> drivers, have it named differently or that the shared code live
>> elsewhere?
> Replying to myself here,
>
> I just realized, while this bit was not accepted, the overal
> implementation has changed in the set. So before, I did things wrong :)
> As Simon explained last time.
>
> To clarify, I now have added the logic to the sunxi gmac and emac board
> specific drivers. But afaik they share no common code. (like
> sunxi_common.c in drivers/net)
>
> With that in mind, how we did things up until now, was to have a
> fallback scenario where we use the SoC serial number to generate a MAC
> address.
>
> If this is go be done with the board specific driver, we'd need to
> still however call board specific functions (sunxi_get_board_serial).
>
> The solution I think is still one of the previously mentioned, a
> sunxi_common.c which does the serial -> MAC conversion according to the
> previous logic, using sunxi_get_board_serial() (which really is a SoC
> specific function, rather board) or have (sunxi)_board_get_enetaddr()
> in the same spot where it is now.
>
> Writing this, I realize the sunxi_common.c approach may not be half
> bad, even if it only contains a single function for now.

Sounds like the common layout has promise. Care to send an RFC for how
it would look?

Thanks,
-Joe
Olliver Schinagl April 11, 2017, 8:14 p.m. UTC | #5
Hey Joe,

On 11-04-17 00:56, Joe Hershberger wrote:
> On Fri, Apr 7, 2017 at 8:57 AM,  <o.schinagl@ultimaker.com> wrote:
>> On Fri, 2017-04-07 at 15:45 +0200, Olliver Schinagl wrote:
>>> Hey Joe,
>>>
>>> On 30-11-16 22:36, Joe Hershberger wrote:
>>>> On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.
>>>> nl> wrote:
>>>>> Add board hooks allowing to get ethernet addresses in a board
>>>>> specific
>>>>> manner. Currently this is done by generating a MAC address from
>>>>> the SID and injecting the ethernet device number in the first
>>>>> octet.
>>>>>
>>>>> This usually happens as a fallback, if either the eeprom fails to
>>>>> set a
>>>>> MAC address or the FDT forces an override.
>>>>>
>>>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>>>>> ---
>>>>>   arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>>>>>   board/sunxi/board.c                         | 161
>>>>> +++++++++++++++-------------
>>>>>   net/eth_legacy.c                            |   1 +
>>>>>   3 files changed, 98 insertions(+), 75 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> index a373319..fad7c48 100644
>>>>> --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> @@ -30,4 +30,15 @@ void eth_init_board(void);
>>>>>   static inline void eth_init_board(void) {}
>>>>>   #endif
>>>>>
>>>>> +int board_get_enetaddr(const int i, unsigned char *mac_addr);
>>>>> +
>>>>> +#if CONFIG_SUNXI_EMAC
>>>>> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
>>>>> int id);
>>>>> +#endif
>>>>> +
>>>>> +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
>>>>> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>>>>> +#endif
>>>>> +
>>>>> +
>>>>>   #endif
>>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>>>> index 5365638..4aeab51 100644
>>>>> --- a/board/sunxi/board.c
>>>>> +++ b/board/sunxi/board.c
>>>>> @@ -21,6 +21,7 @@
>>>>>   #include <asm/arch/gpio.h>
>>>>>   #include <asm/arch/mmc.h>
>>>>>   #include <asm/arch/spl.h>
>>>>> +#include <asm/arch/sys_proto.h>
>>>>>   #include <asm/arch/usb_phy.h>
>>>>>   #ifndef CONFIG_ARM64
>>>>>   #include <asm/armv7.h>
>>>>> @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>>>>>   }
>>>>>   #endif
>>>>>
>>>>> +int sunxi_get_board_serial(unsigned int *serial)
>>>>> +{
>>>>> +       int ret;
>>>>> +
>>>>> +       ret = sunxi_get_sid(serial);
>>>>> +       if (!ret || serial[0])
>>>>> +               return -ENOSYS;
>>>>> +
>>>>> +       /*
>>>>> +        * The single words 1 - 3 of the SID have quite a few
>>>>> bits
>>>>> +        * which are the same on many models, so we take a crc32
>>>>> +        * of all 3 words, to get a more unique value.
>>>>> +        *
>>>>> +        * Note we only do this on newer SoCs as we cannot change
>>>>> +        * the algorithm on older SoCs since those have been
>>>>> using
>>>>> +        * fixed mac-addresses/serial based on only using word 3
>>>>> for a
>>>>> +        * long time and changing a fixed mac-address/serial with
>>>>> an
>>>>> +        * u-boot update is not good.
>>>>> +        */
>>>>> +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
>>>>> && \
>>>>> +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
>>>>> && \
>>>>> +    !defined(CONFIG_MACH_SUN8I_A23) &&
>>>>> !defined(CONFIG_MACH_SUN8I_A33)
>>>>> +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
>>>>> +#endif
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>>   #ifdef CONFIG_SERIAL_TAG
>>>>>   void get_board_serial(struct tag_serialnr *serialnr)
>>>>>   {
>>>>> @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr
>>>>> *serialnr)
>>>>>   #endif
>>>>>
>>>>>   /*
>>>>> + * Generate a MAC address based on device index and the serial
>>>>> number.
>>>>> + * The first half of the of the first octet holds the eth index.
>>>>> + *
>>>>> + * In the second octet we forcefully mark the MAC address to a
>>>>> locally
>>>>> + * administered MAC address.
>>>>> + *
>>>>> + */
>>>>> +int board_get_enetaddr(const int index, unsigned char *enetaddr)
>>>> This would be part of a board-specific eth driver.
>>> this is being called now from sunxi_gmac.c and sunxi_emac.c and
>>> supplies
>>> these board specific drivers with a mac address based on the serial
>>> number of the board. I could move this logic over, but then i'd have
>>> to
>>> add it to both eth drivers. By having it in the board.c file, we have
>>> 2
>>> simple functions in the board-specific eth driver:
>>>
>>>
>>> static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
>>> {
>>>        struct eth_pdata *pdata = dev_get_platdata(dev);
>>>
>>>        return board_get_enetaddr(dev->seq, pdata->enetaddr);
>>> }
>>>
>>> So do you propose to dupilicate the code into both board specific
>>> drivers, have it named differently or that the shared code live
>>> elsewhere?
>> Replying to myself here,
>>
>> I just realized, while this bit was not accepted, the overal
>> implementation has changed in the set. So before, I did things wrong :)
>> As Simon explained last time.
>>
>> To clarify, I now have added the logic to the sunxi gmac and emac board
>> specific drivers. But afaik they share no common code. (like
>> sunxi_common.c in drivers/net)
>>
>> With that in mind, how we did things up until now, was to have a
>> fallback scenario where we use the SoC serial number to generate a MAC
>> address.
>>
>> If this is go be done with the board specific driver, we'd need to
>> still however call board specific functions (sunxi_get_board_serial).
>>
>> The solution I think is still one of the previously mentioned, a
>> sunxi_common.c which does the serial -> MAC conversion according to the
>> previous logic, using sunxi_get_board_serial() (which really is a SoC
>> specific function, rather board) or have (sunxi)_board_get_enetaddr()
>> in the same spot where it is now.
>>
>> Writing this, I realize the sunxi_common.c approach may not be half
>> bad, even if it only contains a single function for now.
> Sounds like the common layout has promise. Care to send an RFC for how
> it would look?
Not sure if we are on the same page :) but I've sent a patch set which 
includes the change. Find it on patchwork [0].

We should discuss it there I reccon, but for now, it is a single 
function for the read_rom_hwaddr hook for sunxi_emac, sunxi_gmac and 
sun8i_emac. I saw more similar code paths, such as the write_hwaddr, 
which could move here as well. I think in time with some effort a few 
more functions can go into this shared file.

The only thing I haven't figured out yet, where I did continue the 
original thread [1], is how to solve the fdt_fixup scenario.

Olliver

[0] https://patchwork.ozlabs.org/patch/749103/
[1] https://patchwork.ozlabs.org/patch/699288/
>
> Thanks,
> -Joe
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h b/arch/arm/include/asm/arch-sunxi/sys_proto.h
index a373319..fad7c48 100644
--- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
+++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
@@ -30,4 +30,15 @@  void eth_init_board(void);
 static inline void eth_init_board(void) {}
 #endif
 
+int board_get_enetaddr(const int i, unsigned char *mac_addr);
+
+#if CONFIG_SUNXI_EMAC
+int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
+#endif
+
+#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
+#endif
+
+
 #endif
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 5365638..4aeab51 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -21,6 +21,7 @@ 
 #include <asm/arch/gpio.h>
 #include <asm/arch/mmc.h>
 #include <asm/arch/spl.h>
+#include <asm/arch/sys_proto.h>
 #include <asm/arch/usb_phy.h>
 #ifndef CONFIG_ARM64
 #include <asm/armv7.h>
@@ -564,6 +565,34 @@  int g_dnl_board_usb_cable_connected(void)
 }
 #endif
 
+int sunxi_get_board_serial(unsigned int *serial)
+{
+	int ret;
+
+	ret = sunxi_get_sid(serial);
+	if (!ret || serial[0])
+		return -ENOSYS;
+
+	/*
+	 * The single words 1 - 3 of the SID have quite a few bits
+	 * which are the same on many models, so we take a crc32
+	 * of all 3 words, to get a more unique value.
+	 *
+	 * Note we only do this on newer SoCs as we cannot change
+	 * the algorithm on older SoCs since those have been using
+	 * fixed mac-addresses/serial based on only using word 3 for a
+	 * long time and changing a fixed mac-address/serial with an
+	 * u-boot update is not good.
+	 */
+#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
+    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
+    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
+	serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
+#endif
+
+	return 0;
+}
+
 #ifdef CONFIG_SERIAL_TAG
 void get_board_serial(struct tag_serialnr *serialnr)
 {
@@ -585,6 +614,54 @@  void get_board_serial(struct tag_serialnr *serialnr)
 #endif
 
 /*
+ * Generate a MAC address based on device index and the serial number.
+ * The first half of the of the first octet holds the eth index.
+ *
+ * In the second octet we forcefully mark the MAC address to a locally
+ * administered MAC address.
+ *
+ */
+int board_get_enetaddr(const int index, unsigned char *enetaddr)
+{
+	uint8_t mac_addr[ARP_HLEN] = { 0x00 };
+	unsigned int serial[4];
+	int ret;
+
+	if ((index < 0) || !enetaddr)
+		return -ENOSYS;
+
+	ret = sunxi_get_board_serial(serial);
+	if (!ret)
+		return ret;
+
+	/* Ensure the NIC specific bytes of the mac are not all 0 */
+	if ((serial[3] & 0xffffff) == 0)
+		serial[3] |= 0x800000;
+
+	mac_addr[0] = (index << 4);
+	mac_addr[1] = (serial[0] >>  0) & 0xff;
+	mac_addr[2] = (serial[3] >> 24) & 0xff;
+	mac_addr[3] = (serial[3] >> 16) & 0xff;
+	mac_addr[4] = (serial[3] >>  8) & 0xff;
+	mac_addr[5] = (serial[3] >>  0) & 0xff;
+
+	set_local_ethaddr(mac_addr);
+	memcpy(enetaddr, mac_addr, ARP_HLEN);
+
+	return 0;
+}
+
+int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
+{
+	return board_get_enetaddr(id, enetaddr);
+}
+
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
+{
+	return board_get_enetaddr(id, enetaddr);
+}
+
+/*
  * Check the SPL header for the "sunxi" variant. If found: parse values
  * that might have been passed by the loader ("fel" utility), and update
  * the environment accordingly.
@@ -617,77 +694,10 @@  static void parse_spl_header(const uint32_t spl_addr)
 	setenv_hex("fel_scriptaddr", spl->fel_script_address);
 }
 
-/*
- * Note this function gets called multiple times.
- * It must not make any changes to env variables which already exist.
- */
-static void setup_environment(const void *fdt)
-{
-	char serial_string[17] = { 0 };
-	unsigned int sid[4];
-	uint8_t mac_addr[6];
-	char ethaddr[16];
-	int i, ret;
-
-	ret = sunxi_get_sid(sid);
-	if (ret == 0 && sid[0] != 0) {
-		/*
-		 * The single words 1 - 3 of the SID have quite a few bits
-		 * which are the same on many models, so we take a crc32
-		 * of all 3 words, to get a more unique value.
-		 *
-		 * Note we only do this on newer SoCs as we cannot change
-		 * the algorithm on older SoCs since those have been using
-		 * fixed mac-addresses based on only using word 3 for a
-		 * long time and changing a fixed mac-address with an
-		 * u-boot update is not good.
-		 */
-#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
-    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
-    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
-		sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
-#endif
-
-		/* Ensure the NIC specific bytes of the mac are not all 0 */
-		if ((sid[3] & 0xffffff) == 0)
-			sid[3] |= 0x800000;
-
-		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] = (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);
-		}
-
-		if (!getenv("serial#")) {
-			snprintf(serial_string, sizeof(serial_string),
-				"%08x%08x", sid[0], sid[3]);
-
-			setenv("serial#", serial_string);
-		}
-	}
-}
-
 int misc_init_r(void)
 {
 	__maybe_unused int ret;
+	unsigned int serial[4];
 
 	setenv("fel_booted", NULL);
 	setenv("fel_scriptaddr", NULL);
@@ -697,7 +707,14 @@  int misc_init_r(void)
 		parse_spl_header(SPL_ADDR);
 	}
 
-	setup_environment(gd->fdt_blob);
+	if (sunxi_get_board_serial(serial)) {
+		char serial_string[17] = { 0 };
+
+		snprintf(serial_string, sizeof(serial_string),
+			 "%08x%08x", serial[0], serial[3]);
+
+		setenv("serial#", serial_string);
+	}
 
 #ifndef CONFIG_MACH_SUN9I
 	ret = sunxi_usb_phy_probe();
@@ -713,12 +730,6 @@  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)
diff --git a/net/eth_legacy.c b/net/eth_legacy.c
index d6d7cee..b8b1e3b 100644
--- a/net/eth_legacy.c
+++ b/net/eth_legacy.c
@@ -10,6 +10,7 @@ 
 #include <command.h>
 #include <environment.h>
 #include <net.h>
+#include <i2c.h>
 #include <phy.h>
 #include <linux/errno.h>
 #include "eth_internal.h"