diff mbox

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

Message ID 1450096872-16733-3-git-send-email-o.schinagl@ultimaker.com
State Changes Requested
Delegated to: Joe Hershberger
Headers show

Commit Message

Olliver Schinagl Dec. 14, 2015, 12:41 p.m. UTC
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
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(+)

Comments

Joe Hershberger Jan. 26, 2016, 12:32 a.m. UTC | #1
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..."

> 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.

> +
>         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
> --
> 2.6.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Olliver Schinagl Jan. 26, 2016, 4:10 p.m. UTC | #2
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 
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 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.

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.

But then again, I am new to this stuff so I can be gladly very wrong here :)


>
>> +
>>          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
>> --
>> 2.6.2
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
Joe Hershberger Jan. 26, 2016, 4:23 p.m. UTC | #3
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 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
Olliver Schinagl Jan. 26, 2016, 4:35 p.m. UTC | #4
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
diff mbox

Patch

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);
+
 	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