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

[o.schinagl at ultimaker.com <o.schinagl@ultimaker.com>]

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 at lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot