On 15.2.2016 12:10, Bin Meng wrote: > Hi Michal, > > On Mon, Feb 15, 2016 at 3:16 PM, Michal Simek <mon...@monstr.eu> wrote: >> Hi Bin, >> >> On 14.2.2016 13:00, Bin Meng wrote: >>> Hi Michal, >>> >>> On Sun, Feb 14, 2016 at 6:03 PM, Michal Simek <mon...@monstr.eu> wrote: >>>> Hi Bin, >>>> >>>> 2016-02-14 3:25 GMT+01:00 Bin Meng <bmeng...@gmail.com>: >>>>> >>>>> Hi Michal, >>>>> >>>>> On Sat, Feb 13, 2016 at 6:39 PM, Michal Simek <mon...@monstr.eu> wrote: >>>>>> Add support for reading MAC address from I2C EEPROM. >>>>>> >>>>> >>>>> Is this a feature provided by the GEM MAC IP? >>>>> >>>>>> Signed-off-by: Michal Simek <mon...@monstr.eu> >>>>>> --- >>>>>> >>>>>> drivers/net/zynq_gem.c | 16 ++++++++++++++++ >>>>>> 1 file changed, 16 insertions(+) >>>>>> >>>>>> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c >>>>>> index b3821c31a91d..ace60c901cb5 100644 >>>>>> --- a/drivers/net/zynq_gem.c >>>>>> +++ b/drivers/net/zynq_gem.c >>>>>> @@ -627,6 +627,21 @@ static int zynq_gem_remove(struct udevice *dev) >>>>>> return 0; >>>>>> } >>>>>> >>>>>> +static int zynq_gem_read_rom_hwaddr(struct udevice *dev) >>>>>> +{ >>>>>> +#if defined(CONFIG_ZYNQ_GEM_EEPROM_ADDR) && \ >>>>>> + defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET) >>>>>> + struct eth_pdata *pdata = dev_get_platdata(dev); >>>>>> + >>>>>> + if (eeprom_read(CONFIG_ZYNQ_GEM_EEPROM_ADDR, >>>>>> + CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET, >>>>>> + pdata->enetaddr, ARRAY_SIZE(pdata->enetaddr))) >>>>> >>>>> This call to eeprom_read() looks to me a board-specific feature, that >>>>> an on-board eeprom is used to store the MAC address for the GEM? >>>>> >>>> >>>> Right. it is board specific feature I can >>>> The question is if this should really go to board specific file >>>> or to be the part of DT binding. I didn't look at the kernel if someone has >>>> some sort of eeprom binding for this case >>>> but I expect local mac addresses via DT are used. Or passing via command >>>> line does it. >>>> >>>> Anyway there is mac_read_from_eeprom() but it is ancient one. >>>> Do you any preference for the name of function? >>> >>> I think the intention of the read_rom_hwaddr op is to read the MAC >>> address via the ethernet controller defined mechanism (eg: e1000 can >>> read its MAC address from either an EEPROM or an SPI flash), or via >>> SoC defined mechanism (eg: the ethernet IP is only seen on a specific >>> SoC, and reading the MAC address only makes sense on that specific >>> SoC). If this is a board-specific mechanism, I don't think we should >>> put the codes into driver's read_rom_hwaddr(). Again, if it is >>> board-specific, we may not come up with a standard DT binding for such >>> stuff, unless we can enumerate all possible ways for storing MAC >>> address on a board (EEPROM, SPI flash, eMMC, SD)? >> >> It is interesting that there is no standard binding for it. Maybe in >> future. Anyway is this better way (look below)? >> (I will send it as regular patch) >> >> Thanks, >> Michal >> >> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c >> index 01bae5d67e3f..ae115245b7ea 100644 >> --- a/board/xilinx/zynq/board.c >> +++ b/board/xilinx/zynq/board.c >> @@ -72,6 +72,18 @@ int board_init(void) >> >> int board_late_init(void) >> { >> +#if defined(CONFIG_ZYNQ_GEM_EEPROM_ADDR) && \ >> + defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET) >> + unsigned char enetaddr[6]; >> + >> + if (eeprom_read(CONFIG_ZYNQ_GEM_EEPROM_ADDR, >> + CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET, >> + enetaddr, ARRAY_SIZE(enetaddr))) >> + printf("I2C EEPROM MAC address read failed\n"); >> + else >> + eth_setenv_enetaddr("ethaddr", enetaddr); >> +#endif >> + > > Looks good, but one comment regarding to the naming: > CONFIG_ZYNQ_GEM_EEPROM_ADDR & CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET. > > Should we include _ZYNQ_GEM_ in the macro name since it may mislead it > has something to do with the GEM controller? Is the on-board EEPROM > only used to store the MAC address? If not, maybe think about some > another generic naming?
Fair enough will fix it and send the patch. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
signature.asc
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot