[U-Boot] [PATCH v2 2/5] sunxi: net: Allow the sunxi to set the MAC from an EEPROM

[Olliver Schinagl <o.schinagl@ultimaker.com>]

Hey Joe,

On 26-01-16 17:23, Joe Hershberger wrote:
> On Tue, Jan 26, 2016 at 10:10 AM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>> Hey Joe
>>
>> On 26-01-16 01:32, Joe Hershberger wrote:
>>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>>> <o.schinagl@ultimaker.com> wrote:
>>>> This patch uses the newly introduced Kconfig options to set the MAC
>>>> address from an EEPROM, this will be especially useful for the Olimex
>>>> OLinuXino series of sunxi boards as they all have an 2k i2c eeprom chip.
>>>>
>>>> The MAC address is in the eeprom is ignored if there is already a MAC
>>> "The MAC address in the eeprom is ignored..."
>> oops :) fixed for v3
>>
>>>> address in the environment or (if enabled) the CRC8 check fails.
>>>>
>>>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>>>> ---
>>>>    board/sunxi/Kconfig |  4 ++++
>>>>    board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>    net/Kconfig         |  1 +
>>>>    3 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>>> index 2dd9d3b..a2da3a6 100644
>>>> --- a/board/sunxi/Kconfig
>>>> +++ b/board/sunxi/Kconfig
>>>> @@ -339,18 +339,21 @@ config I2C0_ENABLE
>>>>
>>>>    config I2C1_ENABLE
>>>>           bool "Enable I2C/TWI controller 1"
>>>> +       default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
>>>>           default n
>>>>           ---help---
>>>>           See I2C0_ENABLE help text.
>>>>
>>>>    config I2C2_ENABLE
>>>>           bool "Enable I2C/TWI controller 2"
>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
>>>>           default n
>>>>           ---help---
>>>>           See I2C0_ENABLE help text.
>>>>
>>>>    if MACH_SUN6I || MACH_SUN7I
>>>>    config I2C3_ENABLE
>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
>>>>           bool "Enable I2C/TWI controller 3"
>>>>           default n
>>>>           ---help---
>>>> @@ -359,6 +362,7 @@ endif
>>>>
>>>>    if MACH_SUN7I
>>>>    config I2C4_ENABLE
>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
>>>>           bool "Enable I2C/TWI controller 4"
>>>>           default n
>>>>           ---help---
>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>>> index 6ac398c..28310a2 100644
>>>> --- a/board/sunxi/board.c
>>>> +++ b/board/sunxi/board.c
>>>> @@ -12,6 +12,7 @@
>>>>     */
>>>>
>>>>    #include <common.h>
>>>> +#include <i2c.h>
>>>>    #include <mmc.h>
>>>>    #include <axp_pmic.h>
>>>>    #include <asm/arch/clock.h>
>>>> @@ -23,6 +24,7 @@
>>>>    #include <asm/arch/usb_phy.h>
>>>>    #include <asm/gpio.h>
>>>>    #include <asm/io.h>
>>>> +#include <linux/crc8.h>
>>>>    #include <nand.h>
>>>>    #include <net.h>
>>>>
>>>> @@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr *serialnr)
>>>>    }
>>>>    #endif
>>>>
>>>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) &&
>>>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
>>>> +static int read_mac_from_eeprom(uint8_t *mac_addr)
>>>> +{
>>>> +       uint8_t eeprom[7];
>>>> +       int old_i2c_bus;
>>>> +
>>>> +       old_i2c_bus = i2c_get_bus_num();
>>>> +       i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
>>>> +       if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
>>>> +                    CONFIG_NET_ETHADDR_EEPROM_OFFSET,
>>>> +                    CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
>>>> +                    eeprom, 7)) {
>>>> +               i2c_set_bus_num(old_i2c_bus);
>>>> +               puts("Could not read the EEPROM; EEPROM missing?\n");
>>>> +               return -1;
>>>> +       }
>>>> +       i2c_set_bus_num(old_i2c_bus);
>>>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
>>>> +       if (crc8(eeprom, 6) != eeprom[6]) {
>>>> +               puts("CRC error on MAC address from EEPROM.\n");
>>>> +               return -1;
>>>> +       }
>>>> +#endif
>>>> +       memcpy(mac_addr, eeprom, 6);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +#else
>>>> +static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; }
>>>> +#endif
>>>> +
>>>>    #if !defined(CONFIG_SPL_BUILD)
>>>>    #include <asm/arch/spl.h>
>>>>
>>>> @@ -553,6 +586,9 @@ int misc_init_r(void)
>>>>           }
>>>>    #endif
>>>>
>>>> +       if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
>>>> +               eth_setenv_enetaddr("ethaddr", mac_addr);
>>> It seems a bit odd to actually write this to the environment instead
>>> of it being a read_rom_hwaddr operation for the Ethernet driver that
>>> this board uses.
>>>
>>> Do we need a different board-level ROM callback? Maybe the drivers
>>> used in such situations can be the only ones to support it.
>> I'm hoping your thinking out-loud, as I'm not too familiar with all the
> Indeed. I was thinking out loud.
>
>> backing code. But if I understand you correctly, you suggest to implement a
>> new callback for ethernet drivers to get mac's from eeprom? Anyhow, I just
> Not for now... at least not in the core code.
>
>> followed suit with how the address gets set in case of when it is not in the
>> environment and we need to generate a random one.
> Sure. I don't think it's the same case though. We already have the
> entry for the driver to read the rom ethaddr. I think you should use
> that.
I think I lost you here, what function/example can you point me to that 
uses this already so I can spy/learn from?

olliver
>
>> I was reading the u-boot documentation and though the assumed method was to
>> use the environment and to only use other methods (such as the eeprom
>> method) where it is absolutely a must. In this case the eeprom is not
>> directly related to the ethernet, it's a generic eeprom.
> Sure. I think you should add support to whatever driver your board
> uses (it's a driver model driver, right?) for a weak board function.
> That way that one driver is responsible for that behavior because of
> your board's need. Any other boards that use that driver just don't
> supply that function and it is a no-op. I'd rather be able to easily
> follow where that ethaddr is getting configured instead of using
> misc_init_r() and having the env populate magically. This also means
> you'll be in a context to write the ethaddr to the pdata struct
> instead of the env.
>
>> But then again, I am new to this stuff so I can be gladly very wrong here :)
> Seems it's not a case that comes up, so it's fairly new territory.
>
>>
>>>> +
>>>>           ret = sunxi_get_sid(sid);
>>>>           if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>>>>                   if (!getenv("ethaddr")) {
>>>> diff --git a/net/Kconfig b/net/Kconfig
>>>> index aced51e..0f4cc5a 100644
>>>> --- a/net/Kconfig
>>>> +++ b/net/Kconfig
>>>> @@ -8,6 +8,7 @@ menuconfig NET
>>>>    if NET
>>>>
>>>>    config NET_ETHADDR_EEPROM
>>>> +       depends on ARCH_SUNXI
>>>>           bool "Get ethaddr from eeprom"
>>>>           help
>>>>             Selecting this will try to get the Ethernet address from an
>>>> onboard
> Thanks,
> -Joe

-- 
Met vriendelijke groeten, Kind regards, ??????

Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.