[U-Boot] [PATCH 1/6] net: gem: Add support for reading MAC from I2C EEPROM

[Michal Simek]

Hi,

2016-02-15 18:53 GMT+01:00 Joe Hershberger <joe.hershberger at gmail.com>:

> Hi Michal,
>
> On Mon, Feb 15, 2016 at 11:41 AM, Michal Simek <monstr at monstr.eu> wrote:
> > Hi Joe,
> >
> > 2016-02-15 17:01 GMT+01:00 Joe Hershberger <joe.hershberger at gmail.com>:
> >>
> >> Hi Bin,
> >>
> >> On Sun, Feb 14, 2016 at 6:00 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> >> > Hi Michal,
> >> >
> >> > On Sun, Feb 14, 2016 at 6:03 PM, Michal Simek <monstr at monstr.eu>
> wrote:
> >> >> Hi Bin,
> >> >>
> >> >> 2016-02-14 3:25 GMT+01:00 Bin Meng <bmeng.cn at gmail.com>:
> >> >>>
> >> >>> Hi Michal,
> >> >>>
> >> >>> On Sat, Feb 13, 2016 at 6:39 PM, Michal Simek <monstr at 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 <monstr at 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)?
> >>
> >> Did you see this: https://patchwork.ozlabs.org/patch/573373/
> >>
> >> This is the concept I had in mind for handling this sort of thing.
> >
> >
> > This also works for me. I have no problem with both ways. It means this
> one
> > or setting
> > up variable in board init file.
> > Also is there any hook that mac address will be updated in DTS file
> > (local-mac-address)
> > to be the same in u-boot and in Linux.
>
> I'm not aware of a common way.
>

Anyway you are network custodian and I believe you will tell me which way
to go to implement
this mac address reading from eeprom. :-)

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