| Message ID | 1427179783-985-1-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Mar 24, 2015 at 1:49 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> > > --- > > drivers/net/bcm-sf2-eth.c | 6 ------ > drivers/net/designware.c | 4 ---- > drivers/net/macb.c | 9 --------- > net/eth.c | 15 +++++++++++++-- > 4 files changed, 13 insertions(+), 21 deletions(-) This requires dm/next
Hi Joe, On Tue, Mar 24, 2015 at 2:49 PM, 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> > > --- > > drivers/net/bcm-sf2-eth.c | 6 ------ > drivers/net/designware.c | 4 ---- > drivers/net/macb.c | 9 --------- > net/eth.c | 15 +++++++++++++-- > 4 files changed, 13 insertions(+), 21 deletions(-) > > 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..776fd2f 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -258,13 +258,24 @@ 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_get_ops(current)->write_hwaddr) { > + eth_get_ops(current)->write_hwaddr(current); > + } > > ret = eth_get_ops(current)->start(current); > if (ret >= 0) { > -- I've created a u-boot.rom for Intel Galileo board and found the designware ethernet port does not work any more. Git bisect shows that this commit broke the things. Do you have any idea on what might be the issue? Regards, Bin
On Mon, Jun 15, 2015 at 5:07 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Joe, > > On Tue, Mar 24, 2015 at 2:49 PM, 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> >> >> --- >> >> drivers/net/bcm-sf2-eth.c | 6 ------ >> drivers/net/designware.c | 4 ---- >> drivers/net/macb.c | 9 --------- >> net/eth.c | 15 +++++++++++++-- >> 4 files changed, 13 insertions(+), 21 deletions(-) >> >> 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..776fd2f 100644 >> --- a/net/eth.c >> +++ b/net/eth.c >> @@ -258,13 +258,24 @@ 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_get_ops(current)->write_hwaddr) { >> + eth_get_ops(current)->write_hwaddr(current); >> + } >> >> ret = eth_get_ops(current)->start(current); >> if (ret >= 0) { >> -- > > I've created a u-boot.rom for Intel Galileo board and found the > designware ethernet port does not work any more. Git bisect shows that > this commit broke the things. Do you have any idea on what might be > the issue? > Further investigation shows that we should not remove writing designware MAC address after soft reset. The patch [1] is sent out. [1] http://patchwork.ozlabs.org/patch/484214/ Regards, Bin
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..776fd2f 100644 --- a/net/eth.c +++ b/net/eth.c @@ -258,13 +258,24 @@ 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_get_ops(current)->write_hwaddr) { + eth_get_ops(current)->write_hwaddr(current); + } ret = eth_get_ops(current)->start(current); if (ret >= 0) {
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> --- drivers/net/bcm-sf2-eth.c | 6 ------ drivers/net/designware.c | 4 ---- drivers/net/macb.c | 9 --------- net/eth.c | 15 +++++++++++++-- 4 files changed, 13 insertions(+), 21 deletions(-)