| Message ID | 20161108155437.1085-9-oliver@schinagl.nl |
|---|---|
| State | Superseded |
| Delegated to: | Joe Hershberger |
| Headers | show |
On 8.11.2016 16:54, Olliver Schinagl wrote: > This patch uses the newly introduced Kconfig options to use the net_op > read_rom_hwaddr to retrieve the MAC 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 in the eeprom is ignored (if enabled) if the CRC8 check > fails. > > This new functionality allows for querying multiple MAC addresses. The > first (supported) device being probed gets the first address, the second > the second etc. If a generated MAC address is desired, set it to all 0 > (and if crc8 is configured also add that) for the adapter. > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> > --- > board/sunxi/Kconfig | 4 ++++ > board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++- > 2 files changed, 42 insertions(+), 1 deletion(-) > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > index e1d4ab1..6b8ac99 100644 > --- a/board/sunxi/Kconfig > +++ b/board/sunxi/Kconfig > @@ -414,6 +414,7 @@ config I2C0_ENABLE > > config I2C1_ENABLE > bool "Enable I2C/TWI controller 1" > + default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1) > default n > select CMD_I2C > ---help--- > @@ -421,6 +422,7 @@ config I2C1_ENABLE > > config I2C2_ENABLE > bool "Enable I2C/TWI controller 2" > + default y if NET_ETHADDR_EEPROM_I2C_BUS = 2 > default n > select CMD_I2C > ---help--- > @@ -428,6 +430,7 @@ config I2C2_ENABLE > > 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 > select CMD_I2C > @@ -447,6 +450,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 > select CMD_I2C > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index 71124f4..f1e64cd 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> > @@ -31,6 +32,7 @@ > #include <crc.h> > #include <environment.h> > #include <libfdt.h> > +#include <linux/crc8.h> > #include <nand.h> > #include <net.h> > #include <sy8106a.h> > @@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char *enetaddr, uint8_t cnt) > memcpy(enetaddr, mac_addr, ARP_HLEN); > } > > +static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t cnt) > +{ > + uint8_t eeprom[ARP_HLEN + 1] = { 0x00 }; > +#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C) > + int old_i2c_bus; > + > + old_i2c_bus = i2c_get_bus_num(); > + if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS) > + i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS); > + /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */ > + if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR, > + CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN + 2)), > + CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN, > + eeprom, ARP_HLEN + 1)) { > + i2c_set_bus_num(old_i2c_bus); > + puts("Could not read the EEPROM; EEPROM missing?\n"); > + return; > + } > + i2c_set_bus_num(old_i2c_bus); > +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8 > + if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) { > + puts("CRC error on MAC address from EEPROM.\n"); > + return; > + } > +#endif > +#endif > + > + memcpy(enetaddr, eeprom, ARP_HLEN); > +} > + ok. I have briefly looked at the whole series and I think that this should be done in the core because this should be shared across all drivers. Because what you have above make in general sense for every board which contain mac address in eeprom. That's why I would create eeprom_read_rom_etheaddr() in core which will do stuff as you have above and in driver we will simply assign it to read_rom_hwaddr in drivers or by default for all with options to rewrite it. This function will be empty when !NET_ETHADDR_EEPROM. By this or similar way you open this to all ethernet drivers to read mac just through enabling Kconfig. IMHO doesn't make sense to c&p the same logic over all ethernet drivers. Thanks, Michal
Hi Michal, On 10-11-16 12:51, Michal Simek wrote: > On 8.11.2016 16:54, Olliver Schinagl wrote: >> This patch uses the newly introduced Kconfig options to use the net_op >> read_rom_hwaddr to retrieve the MAC 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 in the eeprom is ignored (if enabled) if the CRC8 check >> fails. >> >> This new functionality allows for querying multiple MAC addresses. The >> first (supported) device being probed gets the first address, the second >> the second etc. If a generated MAC address is desired, set it to all 0 >> (and if crc8 is configured also add that) for the adapter. >> >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >> --- >> board/sunxi/Kconfig | 4 ++++ >> board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 42 insertions(+), 1 deletion(-) >> >> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig >> index e1d4ab1..6b8ac99 100644 >> --- a/board/sunxi/Kconfig >> +++ b/board/sunxi/Kconfig >> @@ -414,6 +414,7 @@ config I2C0_ENABLE >> >> config I2C1_ENABLE >> bool "Enable I2C/TWI controller 1" >> + default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1) >> default n >> select CMD_I2C >> ---help--- >> @@ -421,6 +422,7 @@ config I2C1_ENABLE >> >> config I2C2_ENABLE >> bool "Enable I2C/TWI controller 2" >> + default y if NET_ETHADDR_EEPROM_I2C_BUS = 2 >> default n >> select CMD_I2C >> ---help--- >> @@ -428,6 +430,7 @@ config I2C2_ENABLE >> >> 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 >> select CMD_I2C >> @@ -447,6 +450,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 >> select CMD_I2C >> diff --git a/board/sunxi/board.c b/board/sunxi/board.c >> index 71124f4..f1e64cd 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> >> @@ -31,6 +32,7 @@ >> #include <crc.h> >> #include <environment.h> >> #include <libfdt.h> >> +#include <linux/crc8.h> >> #include <nand.h> >> #include <net.h> >> #include <sy8106a.h> >> @@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char *enetaddr, uint8_t cnt) >> memcpy(enetaddr, mac_addr, ARP_HLEN); >> } >> >> +static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t cnt) >> +{ >> + uint8_t eeprom[ARP_HLEN + 1] = { 0x00 }; >> +#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C) >> + int old_i2c_bus; >> + >> + old_i2c_bus = i2c_get_bus_num(); >> + if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS) >> + i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS); >> + /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */ >> + if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR, >> + CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN + 2)), >> + CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN, >> + eeprom, ARP_HLEN + 1)) { >> + i2c_set_bus_num(old_i2c_bus); >> + puts("Could not read the EEPROM; EEPROM missing?\n"); >> + return; >> + } >> + i2c_set_bus_num(old_i2c_bus); >> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8 >> + if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) { >> + puts("CRC error on MAC address from EEPROM.\n"); >> + return; >> + } >> +#endif >> +#endif >> + >> + memcpy(enetaddr, eeprom, ARP_HLEN); >> +} >> + > ok. I have briefly looked at the whole series and I think that this > should be done in the core because this should be shared across all > drivers. > Because what you have above make in general sense for every board which > contain mac address in eeprom. > That's why I would create > > eeprom_read_rom_etheaddr() in core which will do stuff as you have above > and in driver we will simply assign it to read_rom_hwaddr in drivers or > by default for all with options to rewrite it. > This function will be empty when !NET_ETHADDR_EEPROM. > > By this or similar way you open this to all ethernet drivers to read mac > just through enabling Kconfig. > > IMHO doesn't make sense to c&p the same logic over all ethernet drivers. Initially, I do agree very much. But when I first wrote this last year, there was no other driver yet etc. It is very very generic so maybe make this a weak function up one level, and let the driver override it even? Makes using the eeprom framework later easier too. I'll cook something up. Good idea! Olliver > > Thanks, > Michal > > >
On Thu, Nov 10, 2016 at 6:11 AM, Olliver Schinagl <oliver@schinagl.nl> wrote: > Hi Michal, > > > On 10-11-16 12:51, Michal Simek wrote: >> >> On 8.11.2016 16:54, Olliver Schinagl wrote: >>> >>> This patch uses the newly introduced Kconfig options to use the net_op >>> read_rom_hwaddr to retrieve the MAC 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 in the eeprom is ignored (if enabled) if the CRC8 check >>> fails. >>> >>> This new functionality allows for querying multiple MAC addresses. The >>> first (supported) device being probed gets the first address, the second >>> the second etc. If a generated MAC address is desired, set it to all 0 >>> (and if crc8 is configured also add that) for the adapter. >>> >>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>> --- >>> board/sunxi/Kconfig | 4 ++++ >>> board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++- >>> 2 files changed, 42 insertions(+), 1 deletion(-) >>> >>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig >>> index e1d4ab1..6b8ac99 100644 >>> --- a/board/sunxi/Kconfig >>> +++ b/board/sunxi/Kconfig >>> @@ -414,6 +414,7 @@ config I2C0_ENABLE >>> config I2C1_ENABLE >>> bool "Enable I2C/TWI controller 1" >>> + default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1) >>> default n >>> select CMD_I2C >>> ---help--- >>> @@ -421,6 +422,7 @@ config I2C1_ENABLE >>> config I2C2_ENABLE >>> bool "Enable I2C/TWI controller 2" >>> + default y if NET_ETHADDR_EEPROM_I2C_BUS = 2 >>> default n >>> select CMD_I2C >>> ---help--- >>> @@ -428,6 +430,7 @@ config I2C2_ENABLE >>> 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 >>> select CMD_I2C >>> @@ -447,6 +450,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 >>> select CMD_I2C >>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c >>> index 71124f4..f1e64cd 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> >>> @@ -31,6 +32,7 @@ >>> #include <crc.h> >>> #include <environment.h> >>> #include <libfdt.h> >>> +#include <linux/crc8.h> >>> #include <nand.h> >>> #include <net.h> >>> #include <sy8106a.h> >>> @@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char >>> *enetaddr, uint8_t cnt) >>> memcpy(enetaddr, mac_addr, ARP_HLEN); >>> } >>> +static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t >>> cnt) >>> +{ >>> + uint8_t eeprom[ARP_HLEN + 1] = { 0x00 }; >>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) && >>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C) >>> + int old_i2c_bus; >>> + >>> + old_i2c_bus = i2c_get_bus_num(); >>> + if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS) >>> + i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS); >>> + /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */ >>> + if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR, >>> + CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN >>> + 2)), >>> + CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN, >>> + eeprom, ARP_HLEN + 1)) { >>> + i2c_set_bus_num(old_i2c_bus); >>> + puts("Could not read the EEPROM; EEPROM missing?\n"); >>> + return; >>> + } >>> + i2c_set_bus_num(old_i2c_bus); >>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8 >>> + if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) { >>> + puts("CRC error on MAC address from EEPROM.\n"); >>> + return; >>> + } >>> +#endif >>> +#endif >>> + >>> + memcpy(enetaddr, eeprom, ARP_HLEN); >>> +} >>> + >> >> ok. I have briefly looked at the whole series and I think that this >> should be done in the core because this should be shared across all >> drivers. >> Because what you have above make in general sense for every board which >> contain mac address in eeprom. >> That's why I would create >> >> eeprom_read_rom_etheaddr() in core which will do stuff as you have above >> and in driver we will simply assign it to read_rom_hwaddr in drivers or >> by default for all with options to rewrite it. >> This function will be empty when !NET_ETHADDR_EEPROM. >> >> By this or similar way you open this to all ethernet drivers to read mac >> just through enabling Kconfig. >> >> IMHO doesn't make sense to c&p the same logic over all ethernet drivers. > > > Initially, I do agree very much. But when I first wrote this last year, > there was no other driver yet etc. It is very very generic so maybe make > this a weak function up one level, and let the driver override it even? > > Makes using the eeprom framework later easier too. I'll cook something up. > Good idea! Do you think it is valuable enough to apply as is and retrofit later, or just wait on this series? Thanks, -Joe
On 15.11.2016 04:27, Joe Hershberger wrote: > On Thu, Nov 10, 2016 at 6:11 AM, Olliver Schinagl <oliver@schinagl.nl> wrote: >> Hi Michal, >> >> >> On 10-11-16 12:51, Michal Simek wrote: >>> >>> On 8.11.2016 16:54, Olliver Schinagl wrote: >>>> >>>> This patch uses the newly introduced Kconfig options to use the net_op >>>> read_rom_hwaddr to retrieve the MAC 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 in the eeprom is ignored (if enabled) if the CRC8 check >>>> fails. >>>> >>>> This new functionality allows for querying multiple MAC addresses. The >>>> first (supported) device being probed gets the first address, the second >>>> the second etc. If a generated MAC address is desired, set it to all 0 >>>> (and if crc8 is configured also add that) for the adapter. >>>> >>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>>> --- >>>> board/sunxi/Kconfig | 4 ++++ >>>> board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++- >>>> 2 files changed, 42 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig >>>> index e1d4ab1..6b8ac99 100644 >>>> --- a/board/sunxi/Kconfig >>>> +++ b/board/sunxi/Kconfig >>>> @@ -414,6 +414,7 @@ config I2C0_ENABLE >>>> config I2C1_ENABLE >>>> bool "Enable I2C/TWI controller 1" >>>> + default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1) >>>> default n >>>> select CMD_I2C >>>> ---help--- >>>> @@ -421,6 +422,7 @@ config I2C1_ENABLE >>>> config I2C2_ENABLE >>>> bool "Enable I2C/TWI controller 2" >>>> + default y if NET_ETHADDR_EEPROM_I2C_BUS = 2 >>>> default n >>>> select CMD_I2C >>>> ---help--- >>>> @@ -428,6 +430,7 @@ config I2C2_ENABLE >>>> 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 >>>> select CMD_I2C >>>> @@ -447,6 +450,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 >>>> select CMD_I2C >>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c >>>> index 71124f4..f1e64cd 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> >>>> @@ -31,6 +32,7 @@ >>>> #include <crc.h> >>>> #include <environment.h> >>>> #include <libfdt.h> >>>> +#include <linux/crc8.h> >>>> #include <nand.h> >>>> #include <net.h> >>>> #include <sy8106a.h> >>>> @@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char >>>> *enetaddr, uint8_t cnt) >>>> memcpy(enetaddr, mac_addr, ARP_HLEN); >>>> } >>>> +static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t >>>> cnt) >>>> +{ >>>> + uint8_t eeprom[ARP_HLEN + 1] = { 0x00 }; >>>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) && >>>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C) >>>> + int old_i2c_bus; >>>> + >>>> + old_i2c_bus = i2c_get_bus_num(); >>>> + if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS) >>>> + i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS); >>>> + /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */ >>>> + if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR, >>>> + CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN >>>> + 2)), >>>> + CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN, >>>> + eeprom, ARP_HLEN + 1)) { >>>> + i2c_set_bus_num(old_i2c_bus); >>>> + puts("Could not read the EEPROM; EEPROM missing?\n"); >>>> + return; >>>> + } >>>> + i2c_set_bus_num(old_i2c_bus); >>>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8 >>>> + if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) { >>>> + puts("CRC error on MAC address from EEPROM.\n"); >>>> + return; >>>> + } >>>> +#endif >>>> +#endif >>>> + >>>> + memcpy(enetaddr, eeprom, ARP_HLEN); >>>> +} >>>> + >>> >>> ok. I have briefly looked at the whole series and I think that this >>> should be done in the core because this should be shared across all >>> drivers. >>> Because what you have above make in general sense for every board which >>> contain mac address in eeprom. >>> That's why I would create >>> >>> eeprom_read_rom_etheaddr() in core which will do stuff as you have above >>> and in driver we will simply assign it to read_rom_hwaddr in drivers or >>> by default for all with options to rewrite it. >>> This function will be empty when !NET_ETHADDR_EEPROM. >>> >>> By this or similar way you open this to all ethernet drivers to read mac >>> just through enabling Kconfig. >>> >>> IMHO doesn't make sense to c&p the same logic over all ethernet drivers. >> >> >> Initially, I do agree very much. But when I first wrote this last year, >> there was no other driver yet etc. It is very very generic so maybe make >> this a weak function up one level, and let the driver override it even? >> >> Makes using the eeprom framework later easier too. I'll cook something up. >> Good idea! > > Do you think it is valuable enough to apply as is and retrofit later, > or just wait on this series? TBH I would split this series to two to get Sunxi part in and this get later. Thanks, Michal
Hey Joe, Michal, On 15-11-16 08:22, Michal Simek wrote: > On 15.11.2016 04:27, Joe Hershberger wrote: >> On Thu, Nov 10, 2016 at 6:11 AM, Olliver Schinagl <oliver@schinagl.nl> wrote: >>> Hi Michal, >>> >>> >>> On 10-11-16 12:51, Michal Simek wrote: >>>> On 8.11.2016 16:54, Olliver Schinagl wrote: >>>>> This patch uses the newly introduced Kconfig options to use the net_op >>>>> read_rom_hwaddr to retrieve the MAC 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 in the eeprom is ignored (if enabled) if the CRC8 check >>>>> fails. >>>>> >>>>> This new functionality allows for querying multiple MAC addresses. The >>>>> first (supported) device being probed gets the first address, the second >>>>> the second etc. If a generated MAC address is desired, set it to all 0 >>>>> (and if crc8 is configured also add that) for the adapter. >>>>> >>>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>>>> --- >>>>> board/sunxi/Kconfig | 4 ++++ >>>>> board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++- >>>>> 2 files changed, 42 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig >>>>> index e1d4ab1..6b8ac99 100644 >>>>> --- a/board/sunxi/Kconfig >>>>> +++ b/board/sunxi/Kconfig >>>>> @@ -414,6 +414,7 @@ config I2C0_ENABLE >>>>> config I2C1_ENABLE >>>>> bool "Enable I2C/TWI controller 1" >>>>> + default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1) >>>>> default n >>>>> select CMD_I2C >>>>> ---help--- >>>>> @@ -421,6 +422,7 @@ config I2C1_ENABLE >>>>> config I2C2_ENABLE >>>>> bool "Enable I2C/TWI controller 2" >>>>> + default y if NET_ETHADDR_EEPROM_I2C_BUS = 2 >>>>> default n >>>>> select CMD_I2C >>>>> ---help--- >>>>> @@ -428,6 +430,7 @@ config I2C2_ENABLE >>>>> 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 >>>>> select CMD_I2C >>>>> @@ -447,6 +450,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 >>>>> select CMD_I2C >>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c >>>>> index 71124f4..f1e64cd 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> >>>>> @@ -31,6 +32,7 @@ >>>>> #include <crc.h> >>>>> #include <environment.h> >>>>> #include <libfdt.h> >>>>> +#include <linux/crc8.h> >>>>> #include <nand.h> >>>>> #include <net.h> >>>>> #include <sy8106a.h> >>>>> @@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char >>>>> *enetaddr, uint8_t cnt) >>>>> memcpy(enetaddr, mac_addr, ARP_HLEN); >>>>> } >>>>> +static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t >>>>> cnt) >>>>> +{ >>>>> + uint8_t eeprom[ARP_HLEN + 1] = { 0x00 }; >>>>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) && >>>>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C) >>>>> + int old_i2c_bus; >>>>> + >>>>> + old_i2c_bus = i2c_get_bus_num(); >>>>> + if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS) >>>>> + i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS); >>>>> + /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */ >>>>> + if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR, >>>>> + CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN >>>>> + 2)), >>>>> + CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN, >>>>> + eeprom, ARP_HLEN + 1)) { >>>>> + i2c_set_bus_num(old_i2c_bus); >>>>> + puts("Could not read the EEPROM; EEPROM missing?\n"); >>>>> + return; >>>>> + } >>>>> + i2c_set_bus_num(old_i2c_bus); >>>>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8 >>>>> + if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) { >>>>> + puts("CRC error on MAC address from EEPROM.\n"); >>>>> + return; >>>>> + } >>>>> +#endif >>>>> +#endif >>>>> + >>>>> + memcpy(enetaddr, eeprom, ARP_HLEN); >>>>> +} >>>>> + >>>> ok. I have briefly looked at the whole series and I think that this >>>> should be done in the core because this should be shared across all >>>> drivers. >>>> Because what you have above make in general sense for every board which >>>> contain mac address in eeprom. >>>> That's why I would create >>>> >>>> eeprom_read_rom_etheaddr() in core which will do stuff as you have above >>>> and in driver we will simply assign it to read_rom_hwaddr in drivers or >>>> by default for all with options to rewrite it. >>>> This function will be empty when !NET_ETHADDR_EEPROM. >>>> >>>> By this or similar way you open this to all ethernet drivers to read mac >>>> just through enabling Kconfig. >>>> >>>> IMHO doesn't make sense to c&p the same logic over all ethernet drivers. >>> >>> Initially, I do agree very much. But when I first wrote this last year, >>> there was no other driver yet etc. It is very very generic so maybe make >>> this a weak function up one level, and let the driver override it even? >>> >>> Makes using the eeprom framework later easier too. I'll cook something up. >>> Good idea! >> Do you think it is valuable enough to apply as is and retrofit later, >> or just wait on this series? > TBH I would split this series to two to get Sunxi part in and this get > later. I have both series ready and they just need to be tested. I'll test it hopefully later today, and send the 2 seperate patch series very soon (within a day or so, not a year :p) Olliver > > Thanks, > Michal > >
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index e1d4ab1..6b8ac99 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -414,6 +414,7 @@ config I2C0_ENABLE config I2C1_ENABLE bool "Enable I2C/TWI controller 1" + default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1) default n select CMD_I2C ---help--- @@ -421,6 +422,7 @@ config I2C1_ENABLE config I2C2_ENABLE bool "Enable I2C/TWI controller 2" + default y if NET_ETHADDR_EEPROM_I2C_BUS = 2 default n select CMD_I2C ---help--- @@ -428,6 +430,7 @@ config I2C2_ENABLE 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 select CMD_I2C @@ -447,6 +450,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 select CMD_I2C diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 71124f4..f1e64cd 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> @@ -31,6 +32,7 @@ #include <crc.h> #include <environment.h> #include <libfdt.h> +#include <linux/crc8.h> #include <nand.h> #include <net.h> #include <sy8106a.h> @@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char *enetaddr, uint8_t cnt) memcpy(enetaddr, mac_addr, ARP_HLEN); } +static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t cnt) +{ + uint8_t eeprom[ARP_HLEN + 1] = { 0x00 }; +#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C) + int old_i2c_bus; + + old_i2c_bus = i2c_get_bus_num(); + if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS) + i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS); + /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */ + if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR, + CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN + 2)), + CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN, + eeprom, ARP_HLEN + 1)) { + i2c_set_bus_num(old_i2c_bus); + puts("Could not read the EEPROM; EEPROM missing?\n"); + return; + } + i2c_set_bus_num(old_i2c_bus); +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8 + if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) { + puts("CRC error on MAC address from EEPROM.\n"); + return; + } +#endif +#endif + + memcpy(enetaddr, eeprom, ARP_HLEN); +} + static int sunxi_read_rom_hwaddr(unsigned char *enetaddr) { static unsigned int cnt; - _sunxi_gen_sid_hwaddr(enetaddr, cnt); + _sunxi_read_rom_hwaddr(enetaddr, cnt); + + if (is_zero_ethaddr(enetaddr)) { + _sunxi_gen_sid_hwaddr(enetaddr, cnt); + puts("Serial# based "); + } /* First call, first stored MAC address, increase for next call */ cnt++;
This patch uses the newly introduced Kconfig options to use the net_op read_rom_hwaddr to retrieve the MAC 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 in the eeprom is ignored (if enabled) if the CRC8 check fails. This new functionality allows for querying multiple MAC addresses. The first (supported) device being probed gets the first address, the second the second etc. If a generated MAC address is desired, set it to all 0 (and if crc8 is configured also add that) for the adapter. Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> --- board/sunxi/Kconfig | 4 ++++ board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-)