| Message ID | 1427182909-14529-1-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | Accepted |
| Delegated to: | Joe Hershberger |
| Headers | show |
Hi Michal, On Tue, Mar 24, 2015 at 2:41 AM, Joe Hershberger <joe.hershberger@ni.com> wrote: > > When the ethaddr changes in the env, the hardware should also be updated > so that MAC filtering will work properly without resetting U-Boot. > > Also remove the manual calls to set the hwaddr that was included in a > few drivers as a result of the framework not doing it. > > Reported-by: Michal Simek <michal.simek@xilinx.com> > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > > --- > > Changes in v2: > -Implement the check in non-DM eth_initialize also > -Use static helper function in DM case > > drivers/net/bcm-sf2-eth.c | 6 ---- > drivers/net/designware.c | 4 --- > drivers/net/macb.c | 9 ------ > net/eth.c | 76 ++++++++++++++++++++++++++++++----------------- > 4 files changed, 48 insertions(+), 47 deletions(-) Does this sufficiently resolve the issue you have? Thanks, -Joe
On 03/30/2015 08:08 PM, Joe Hershberger wrote: > Hi Michal, > > On Tue, Mar 24, 2015 at 2:41 AM, Joe Hershberger <joe.hershberger@ni.com> > wrote: >> >> When the ethaddr changes in the env, the hardware should also be updated >> so that MAC filtering will work properly without resetting U-Boot. >> >> Also remove the manual calls to set the hwaddr that was included in a >> few drivers as a result of the framework not doing it. >> >> Reported-by: Michal Simek <michal.simek@xilinx.com> >> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >> >> --- >> >> Changes in v2: >> -Implement the check in non-DM eth_initialize also >> -Use static helper function in DM case >> >> drivers/net/bcm-sf2-eth.c | 6 ---- >> drivers/net/designware.c | 4 --- >> drivers/net/macb.c | 9 ------ >> net/eth.c | 76 > ++++++++++++++++++++++++++++++----------------- >> 4 files changed, 48 insertions(+), 47 deletions(-) > > Does this sufficiently resolve the issue you have? I wouldn't say it was my issue. It was more about asking what's the right thing to do. But anyway I have tested it with our ethernet driver and it is working fine. I have applied it on the top of u-boot-dm/next. Tested-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
On Tue, Mar 31, 2015 at 6:41 AM, Michal Simek <michal.simek@xilinx.com> wrote: > On 03/30/2015 08:08 PM, Joe Hershberger wrote: >> Hi Michal, >> >> On Tue, Mar 24, 2015 at 2:41 AM, Joe Hershberger <joe.hershberger@ni.com> >> wrote: >>> >>> When the ethaddr changes in the env, the hardware should also be updated >>> so that MAC filtering will work properly without resetting U-Boot. >>> >>> Also remove the manual calls to set the hwaddr that was included in a >>> few drivers as a result of the framework not doing it. >>> >>> Reported-by: Michal Simek <michal.simek@xilinx.com> >>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >>> >>> --- >>> >>> Changes in v2: >>> -Implement the check in non-DM eth_initialize also >>> -Use static helper function in DM case >>> >>> drivers/net/bcm-sf2-eth.c | 6 ---- >>> drivers/net/designware.c | 4 --- >>> drivers/net/macb.c | 9 ------ >>> net/eth.c | 76 >> ++++++++++++++++++++++++++++++----------------- >>> 4 files changed, 48 insertions(+), 47 deletions(-) >> >> Does this sufficiently resolve the issue you have? > > I wouldn't say it was my issue. It was more about asking what's the > right thing to do. > But anyway I have tested it with our ethernet driver and it is working fine. > > I have applied it on the top of u-boot-dm/next. > > Tested-by: Michal Simek <michal.simek@xilinx.com> Applied to u-boot-net/next! -Joe
diff --git a/drivers/net/bcm-sf2-eth.c b/drivers/net/bcm-sf2-eth.c index 5252d49..9ae88d3 100644 --- a/drivers/net/bcm-sf2-eth.c +++ b/drivers/net/bcm-sf2-eth.c @@ -154,12 +154,6 @@ static int bcm_sf2_eth_open(struct eth_device *dev, bd_t *bt) debug("Enabling BCM SF2 Ethernet.\n"); - /* Set MAC address from env */ - if (bcm_sf2_eth_write_hwaddr(dev) != 0) { - error("%s: MAC set error when opening !\n", __func__); - return -1; - } - eth->enable_mac(); /* enable tx and rx DMA */ diff --git a/drivers/net/designware.c b/drivers/net/designware.c index cc01604..d35236b 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -244,10 +244,6 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis) mdelay(100); }; - /* Soft reset above clears HW address registers. - * So we have to set it here once again */ - dw_write_hwaddr(dev); - rx_descs_init(dev); tx_descs_init(dev); diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 170ff06..7d5b36b 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -525,7 +525,6 @@ static int macb_phy_init(struct macb_device *macb) return 1; } -static int macb_write_hwaddr(struct eth_device *dev); static int macb_init(struct eth_device *netdev, bd_t *bd) { struct macb_device *macb = to_macb(netdev); @@ -594,14 +593,6 @@ static int macb_init(struct eth_device *netdev, bd_t *bd) #endif /* CONFIG_RMII */ } - /* update the ethaddr */ - if (is_valid_ether_addr(netdev->enetaddr)) { - macb_write_hwaddr(netdev); - } else { - printf("%s: mac address is not valid\n", netdev->name); - return -1; - } - if (!macb_phy_init(macb)) return -1; diff --git a/net/eth.c b/net/eth.c index 13b7723..ed2d52a 100644 --- a/net/eth.c +++ b/net/eth.c @@ -239,6 +239,31 @@ int eth_get_dev_index(void) return -1; } +static int eth_write_hwaddr(struct udevice *dev) +{ + struct eth_pdata *pdata = dev->platdata; + int ret = 0; + + if (!dev || !device_active(dev)) + return -EINVAL; + + /* seq is valid since the device is active */ + if (eth_get_ops(dev)->write_hwaddr && !eth_mac_skip(dev->seq)) { + if (!is_valid_ether_addr(pdata->enetaddr)) { + printf("\nError: %s address %pM illegal value\n", + dev->name, pdata->enetaddr); + return -EINVAL; + } + + ret = eth_get_ops(dev)->write_hwaddr(dev); + if (ret) + printf("\nWarning: %s failed to set MAC address\n", + dev->name); + } + + return ret; +} + int eth_init(void) { struct udevice *current; @@ -258,13 +283,22 @@ int eth_init(void) if (device_active(current)) { uchar env_enetaddr[6]; struct eth_pdata *pdata = current->platdata; + int enetaddr_changed = 0; /* Sync environment with network device */ if (eth_getenv_enetaddr_by_index("eth", current->seq, - env_enetaddr)) + env_enetaddr)) { + enetaddr_changed = memcmp(pdata->enetaddr, + env_enetaddr, 6); memcpy(pdata->enetaddr, env_enetaddr, 6); - else + } else { + memset(env_enetaddr, 0, 6); + enetaddr_changed = memcmp(pdata->enetaddr, + env_enetaddr, 6); memset(pdata->enetaddr, 0, 6); + } + if (enetaddr_changed) + eth_write_hwaddr(current); ret = eth_get_ops(current)->start(current); if (ret >= 0) { @@ -356,31 +390,6 @@ int eth_rx(void) return ret; } -static int eth_write_hwaddr(struct udevice *dev) -{ - struct eth_pdata *pdata = dev->platdata; - int ret = 0; - - if (!dev || !device_active(dev)) - return -EINVAL; - - /* seq is valid since the device is active */ - if (eth_get_ops(dev)->write_hwaddr && !eth_mac_skip(dev->seq)) { - if (!is_valid_ether_addr(pdata->enetaddr)) { - printf("\nError: %s address %pM illegal value\n", - dev->name, pdata->enetaddr); - return -EINVAL; - } - - ret = eth_get_ops(dev)->write_hwaddr(dev); - if (ret) - printf("\nWarning: %s failed to set MAC address\n", - dev->name); - } - - return ret; -} - int eth_initialize(void) { int num_devices = 0; @@ -820,10 +829,21 @@ int eth_init(void) dev = eth_devices; do { uchar env_enetaddr[6]; + int enetaddr_changed = 0; if (eth_getenv_enetaddr_by_index("eth", dev->index, - env_enetaddr)) + env_enetaddr)) { + enetaddr_changed = memcmp(dev->enetaddr, + env_enetaddr, 6); memcpy(dev->enetaddr, env_enetaddr, 6); + } else { + memset(env_enetaddr, 0, 6); + enetaddr_changed = memcmp(dev->enetaddr, + env_enetaddr, 6); + memset(dev->enetaddr, 0, 6); + } + if (enetaddr_changed) + eth_write_hwaddr(dev, "eth", dev->index); dev = dev->next; } while (dev != eth_devices);
When the ethaddr changes in the env, the hardware should also be updated so that MAC filtering will work properly without resetting U-Boot. Also remove the manual calls to set the hwaddr that was included in a few drivers as a result of the framework not doing it. Reported-by: Michal Simek <michal.simek@xilinx.com> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> --- Changes in v2: -Implement the check in non-DM eth_initialize also -Use static helper function in DM case drivers/net/bcm-sf2-eth.c | 6 ---- drivers/net/designware.c | 4 --- drivers/net/macb.c | 9 ------ net/eth.c | 76 ++++++++++++++++++++++++++++++----------------- 4 files changed, 48 insertions(+), 47 deletions(-)