| Message ID | 20200611110321.9574-4-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | 117a52e2387bdfcf7f74c5ec6c4985cdd7c5403e |
| Headers | show |
| Series | arm: Juno board updates and PCIe/SATA enablement | expand |
On Thu, Jun 11, 2020 at 2:05 PM Andre Przywara <andre.przywara at arm.com> wrote: > > When compiled as a DM_ETH driver, the scm911x driver was reading the MAC > address from the optional EEPROM storage, but failed to copy this to the > platdata struct. Since it was also missing a definition of the > read_rom_hwaddr() function, the generic Ethernet code was dismissing > this MAC address, falling back to a random address or denying to start > at all. > > Add an implementation of .read_rom_hwaddr, and refactor the function > reading the ROM address to be called by all interested parties. > > This fixes MAC address issues when using the driver in DM_ETH "mode". > > Signed-off-by: Andre Przywara <andre.przywara at arm.com> > --- > drivers/net/smc911x.c | 60 ++++++++++++++++++++++++++----------------- > 1 file changed, 36 insertions(+), 24 deletions(-) > > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c > index 9d2790e561..053ff9f4ff 100644 > --- a/drivers/net/smc911x.c > +++ b/drivers/net/smc911x.c > @@ -187,6 +187,26 @@ static void smc911x_handle_mac_address(struct smc911x_priv *priv) > printf(DRIVERNAME ": MAC %pM\n", m); > } > > +static bool smc911x_read_mac_address(struct smc911x_priv *priv) > +{ > + u32 addrh, addrl; > + > + /* address is obtained from optional eeprom */ > + addrh = smc911x_get_mac_csr(priv, ADDRH); > + addrl = smc911x_get_mac_csr(priv, ADDRL); > + if (addrl == 0xffffffff && addrh == 0x0000ffff) > + return false; > + > + priv->enetaddr[0] = addrl; > + priv->enetaddr[1] = addrl >> 8; > + priv->enetaddr[2] = addrl >> 16; > + priv->enetaddr[3] = addrl >> 24; > + priv->enetaddr[4] = addrh; > + priv->enetaddr[5] = addrh >> 8; > + > + return true; > +} > + > static int smc911x_eth_phy_read(struct smc911x_priv *priv, > u8 phy, u8 reg, u16 *val) > { > @@ -471,7 +491,6 @@ static int smc911x_recv(struct eth_device *dev) > > int smc911x_initialize(u8 dev_num, int base_addr) > { > - unsigned long addrl, addrh; > struct smc911x_priv *priv; > int ret; > > @@ -489,18 +508,8 @@ int smc911x_initialize(u8 dev_num, int base_addr) > goto err_detect; > } > > - addrh = smc911x_get_mac_csr(priv, ADDRH); > - addrl = smc911x_get_mac_csr(priv, ADDRL); > - if (!(addrl == 0xffffffff && addrh == 0x0000ffff)) { > - /* address is obtained from optional eeprom */ > - priv->enetaddr[0] = addrl; > - priv->enetaddr[1] = addrl >> 8; > - priv->enetaddr[2] = addrl >> 16; > - priv->enetaddr[3] = addrl >> 24; > - priv->enetaddr[4] = addrh; > - priv->enetaddr[5] = addrh >> 8; > + if (smc911x_read_mac_address(priv)) > memcpy(priv->dev.enetaddr, priv->enetaddr, 6); > - } > > priv->dev.init = smc911x_init; > priv->dev.halt = smc911x_halt; > @@ -565,6 +574,19 @@ static int smc911x_recv(struct udevice *dev, int flags, uchar **packetp) > return ret ? ret : -EAGAIN; > } > > +static int smc911x_read_rom_hwaddr(struct udevice *dev) > +{ > + struct smc911x_priv *priv = dev_get_priv(dev); > + struct eth_pdata *pdata = dev_get_platdata(dev); > + > + if (!smc911x_read_mac_address(priv)) > + return -ENODEV; > + > + memcpy(pdata->enetaddr, priv->enetaddr, sizeof(pdata->enetaddr)); > + > + return 0; > +} > + > static int smc911x_bind(struct udevice *dev) > { > return device_set_name(dev, dev->name); > @@ -573,7 +595,6 @@ static int smc911x_bind(struct udevice *dev) > static int smc911x_probe(struct udevice *dev) > { > struct smc911x_priv *priv = dev_get_priv(dev); > - unsigned long addrh, addrl; > int ret; > > /* Try to detect chip. Will fail if not present. */ > @@ -581,17 +602,7 @@ static int smc911x_probe(struct udevice *dev) > if (ret) > return ret; > > - addrh = smc911x_get_mac_csr(priv, ADDRH); > - addrl = smc911x_get_mac_csr(priv, ADDRL); > - if (!(addrl == 0xffffffff && addrh == 0x0000ffff)) { > - /* address is obtained from optional eeprom */ > - priv->enetaddr[0] = addrl; > - priv->enetaddr[1] = addrl >> 8; > - priv->enetaddr[2] = addrl >> 16; > - priv->enetaddr[3] = addrl >> 24; > - priv->enetaddr[4] = addrh; > - priv->enetaddr[5] = addrh >> 8; > - } > + smc911x_read_rom_hwaddr(dev); > > return 0; > } > @@ -612,6 +623,7 @@ static const struct eth_ops smc911x_ops = { > .send = smc911x_send, > .recv = smc911x_recv, > .stop = smc911x_stop, > + .read_rom_hwaddr = smc911x_read_rom_hwaddr, > }; > > static const struct udevice_id smc911x_ids[] = { > -- > 2.17.5 > Reviewed-By: Ramon Fried <rfried.dev at gmail.com>
On Thu, Jun 11, 2020 at 1:04 PM Andre Przywara <andre.przywara at arm.com> wrote: > When compiled as a DM_ETH driver, the scm911x driver was reading the MAC > address from the optional EEPROM storage, but failed to copy this to the > platdata struct. Since it was also missing a definition of the > read_rom_hwaddr() function, the generic Ethernet code was dismissing > this MAC address, falling back to a random address or denying to start > at all. > > Add an implementation of .read_rom_hwaddr, and refactor the function > reading the ROM address to be called by all interested parties. > > This fixes MAC address issues when using the driver in DM_ETH "mode". > > Signed-off-by: Andre Przywara <andre.przywara at arm.com> Oups. That explains why the ethernet addresses were looking so funny at times. Reviewed-by: Linus Walleij <linus.walleij at linaro.org> Yours, Linus Walleij
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 9d2790e561..053ff9f4ff 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -187,6 +187,26 @@ static void smc911x_handle_mac_address(struct smc911x_priv *priv) printf(DRIVERNAME ": MAC %pM\n", m); } +static bool smc911x_read_mac_address(struct smc911x_priv *priv) +{ + u32 addrh, addrl; + + /* address is obtained from optional eeprom */ + addrh = smc911x_get_mac_csr(priv, ADDRH); + addrl = smc911x_get_mac_csr(priv, ADDRL); + if (addrl == 0xffffffff && addrh == 0x0000ffff) + return false; + + priv->enetaddr[0] = addrl; + priv->enetaddr[1] = addrl >> 8; + priv->enetaddr[2] = addrl >> 16; + priv->enetaddr[3] = addrl >> 24; + priv->enetaddr[4] = addrh; + priv->enetaddr[5] = addrh >> 8; + + return true; +} + static int smc911x_eth_phy_read(struct smc911x_priv *priv, u8 phy, u8 reg, u16 *val) { @@ -471,7 +491,6 @@ static int smc911x_recv(struct eth_device *dev) int smc911x_initialize(u8 dev_num, int base_addr) { - unsigned long addrl, addrh; struct smc911x_priv *priv; int ret; @@ -489,18 +508,8 @@ int smc911x_initialize(u8 dev_num, int base_addr) goto err_detect; } - addrh = smc911x_get_mac_csr(priv, ADDRH); - addrl = smc911x_get_mac_csr(priv, ADDRL); - if (!(addrl == 0xffffffff && addrh == 0x0000ffff)) { - /* address is obtained from optional eeprom */ - priv->enetaddr[0] = addrl; - priv->enetaddr[1] = addrl >> 8; - priv->enetaddr[2] = addrl >> 16; - priv->enetaddr[3] = addrl >> 24; - priv->enetaddr[4] = addrh; - priv->enetaddr[5] = addrh >> 8; + if (smc911x_read_mac_address(priv)) memcpy(priv->dev.enetaddr, priv->enetaddr, 6); - } priv->dev.init = smc911x_init; priv->dev.halt = smc911x_halt; @@ -565,6 +574,19 @@ static int smc911x_recv(struct udevice *dev, int flags, uchar **packetp) return ret ? ret : -EAGAIN; } +static int smc911x_read_rom_hwaddr(struct udevice *dev) +{ + struct smc911x_priv *priv = dev_get_priv(dev); + struct eth_pdata *pdata = dev_get_platdata(dev); + + if (!smc911x_read_mac_address(priv)) + return -ENODEV; + + memcpy(pdata->enetaddr, priv->enetaddr, sizeof(pdata->enetaddr)); + + return 0; +} + static int smc911x_bind(struct udevice *dev) { return device_set_name(dev, dev->name); @@ -573,7 +595,6 @@ static int smc911x_bind(struct udevice *dev) static int smc911x_probe(struct udevice *dev) { struct smc911x_priv *priv = dev_get_priv(dev); - unsigned long addrh, addrl; int ret; /* Try to detect chip. Will fail if not present. */ @@ -581,17 +602,7 @@ static int smc911x_probe(struct udevice *dev) if (ret) return ret; - addrh = smc911x_get_mac_csr(priv, ADDRH); - addrl = smc911x_get_mac_csr(priv, ADDRL); - if (!(addrl == 0xffffffff && addrh == 0x0000ffff)) { - /* address is obtained from optional eeprom */ - priv->enetaddr[0] = addrl; - priv->enetaddr[1] = addrl >> 8; - priv->enetaddr[2] = addrl >> 16; - priv->enetaddr[3] = addrl >> 24; - priv->enetaddr[4] = addrh; - priv->enetaddr[5] = addrh >> 8; - } + smc911x_read_rom_hwaddr(dev); return 0; } @@ -612,6 +623,7 @@ static const struct eth_ops smc911x_ops = { .send = smc911x_send, .recv = smc911x_recv, .stop = smc911x_stop, + .read_rom_hwaddr = smc911x_read_rom_hwaddr, }; static const struct udevice_id smc911x_ids[] = {
When compiled as a DM_ETH driver, the scm911x driver was reading the MAC address from the optional EEPROM storage, but failed to copy this to the platdata struct. Since it was also missing a definition of the read_rom_hwaddr() function, the generic Ethernet code was dismissing this MAC address, falling back to a random address or denying to start at all. Add an implementation of .read_rom_hwaddr, and refactor the function reading the ROM address to be called by all interested parties. This fixes MAC address issues when using the driver in DM_ETH "mode". Signed-off-by: Andre Przywara <andre.przywara at arm.com> --- drivers/net/smc911x.c | 60 ++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 24 deletions(-)