[U-Boot] Setting up MAC address for eth drivers
Hi,
I have a question regarding setting mac address for drivers. Drivers setting up write_hwaddr via eth_write_hwaddr via eth_initialize which is called from common/board_r.c.
But then there are some drivers(macb, designware, altera_tse) which also calls mac setup from dev->init which has side effect for example when you setup CONFIG_ENV_OVERWRITE and change mac address you can directly use it.
It also means if there is intention to call hwaddr from dev->init that for the first packet mac address is setup twice - in eth core init and then before every driver use.
I am asking this question because I would like to know the right flow for eth mac setup. If it is set ethaddr xx.... saveenv reset eth with new mac
or if set ethaddr eth with new mac should work
The second approach looks reasonable when ethaddr is not set at all but then at least our driver needs to be fixed to support this feature.
Thanks, Michal
Hi,
any update on this one?
Thanks, Michal
On 03/13/2015 01:25 PM, Michal Simek wrote:
Hi,
I have a question regarding setting mac address for drivers. Drivers setting up write_hwaddr via eth_write_hwaddr via eth_initialize which is called from common/board_r.c.
But then there are some drivers(macb, designware, altera_tse) which also calls mac setup from dev->init which has side effect for example when you setup CONFIG_ENV_OVERWRITE and change mac address you can directly use it.
It also means if there is intention to call hwaddr from dev->init that for the first packet mac address is setup twice - in eth core init and then before every driver use.
I am asking this question because I would like to know the right flow for eth mac setup. If it is set ethaddr xx.... saveenv reset eth with new mac
or if set ethaddr eth with new mac should work
The second approach looks reasonable when ethaddr is not set at all but then at least our driver needs to be fixed to support this feature.
Thanks, Michal _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Michal,
On Fri, Mar 13, 2015 at 7:25 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi,
I have a question regarding setting mac address for drivers. Drivers setting up write_hwaddr via eth_write_hwaddr via eth_initialize which is called from common/board_r.c.
This is because on at least ARM kernels, the MAC is passed via the MAC's filter registers. Because of this, we need to write hwaddr even before the MAC is started.
But then there are some drivers(macb, designware, altera_tse) which also
calls
mac setup from dev->init which has side effect for example when you setup
CONFIG_ENV_OVERWRITE
and change mac address you can directly use it.
This is probably a work-around for a shortcoming of the net/eth.c not calling write_hwaddr() when the env MAC changes. It already updates the registers used by the network stack, so presumably if those MACs are filtering incoming traffic based on the register as expected, changing the MAC after boot without rewriting that register would cause all traffic to not be returned.
It also means if there is intention to call hwaddr from dev->init that for the first packet mac address is setup twice - in eth core init and then before every driver use.
The design of the network stack assumes the MAC is started each time a network operation is requested and stopped when done.
As for the setting of the hwaddr, we should check if it changed.
I am asking this question because I would like to know the right flow for eth mac setup. If it is set ethaddr xx.... saveenv reset eth with new mac
or if set ethaddr eth with new mac should work
The second approach looks reasonable when ethaddr is not set at all but then at least our driver needs to be fixed to support this feature.
I think the second should work ideally, but we need to add a call to eth_init() if the "ethaddr" in the env changes and set it again if so. We should also remove the calls from the drivers you identified since the framework will now do it.
Cheers, -Joe
Hi Joe,
On 03/17/2015 04:56 PM, Joe Hershberger wrote:
Hi Michal,
On Fri, Mar 13, 2015 at 7:25 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi,
I have a question regarding setting mac address for drivers. Drivers setting up write_hwaddr via eth_write_hwaddr via eth_initialize which is called from common/board_r.c.
This is because on at least ARM kernels, the MAC is passed via the MAC's filter registers. Because of this, we need to write hwaddr even before the MAC is started.
But then there are some drivers(macb, designware, altera_tse) which also
calls
mac setup from dev->init which has side effect for example when you setup
CONFIG_ENV_OVERWRITE
and change mac address you can directly use it.
This is probably a work-around for a shortcoming of the net/eth.c not calling write_hwaddr() when the env MAC changes. It already updates the registers used by the network stack, so presumably if those MACs are filtering incoming traffic based on the register as expected, changing the MAC after boot without rewriting that register would cause all traffic to not be returned.
It also means if there is intention to call hwaddr from dev->init that for the first packet mac address is setup twice - in eth core init and then before every driver use.
The design of the network stack assumes the MAC is started each time a network operation is requested and stopped when done.
As for the setting of the hwaddr, we should check if it changed.
I am asking this question because I would like to know the right flow for eth mac setup. If it is set ethaddr xx.... saveenv reset eth with new mac
or if set ethaddr eth with new mac should work
The second approach looks reasonable when ethaddr is not set at all but then at least our driver needs to be fixed to support this feature.
I think the second should work ideally, but we need to add a call to eth_init() if the "ethaddr" in the env changes and set it again if so. We should also remove the calls from the drivers you identified since the framework will now do it.
Does it mean that you are going to write that code for it?
Thanks, Michal
Hi Michal,
On Tue, Mar 17, 2015 at 11:57 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi Joe,
On 03/17/2015 04:56 PM, Joe Hershberger wrote:
Hi Michal,
On Fri, Mar 13, 2015 at 7:25 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi,
I have a question regarding setting mac address for drivers. Drivers setting up write_hwaddr via eth_write_hwaddr via eth_initialize which is called from common/board_r.c.
This is because on at least ARM kernels, the MAC is passed via the MAC's filter registers. Because of this, we need to write hwaddr even before
the
MAC is started.
But then there are some drivers(macb, designware, altera_tse) which
also
calls
mac setup from dev->init which has side effect for example when you
setup
CONFIG_ENV_OVERWRITE
and change mac address you can directly use it.
This is probably a work-around for a shortcoming of the net/eth.c not calling write_hwaddr() when the env MAC changes. It already updates the registers used by the network stack, so presumably if those MACs are filtering incoming traffic based on the register as expected, changing
the
MAC after boot without rewriting that register would cause all traffic
to
not be returned.
It also means if there is intention to call hwaddr from dev->init that for the first packet mac address is setup twice - in eth core init and then before every driver use.
The design of the network stack assumes the MAC is started each time a network operation is requested and stopped when done.
As for the setting of the hwaddr, we should check if it changed.
I am asking this question because I would like to know the right flow for eth mac setup. If it is set ethaddr xx.... saveenv reset eth with new mac
or if set ethaddr eth with new mac should work
The second approach looks reasonable when ethaddr is not set at all but then at least our driver needs to be fixed to support this feature.
I think the second should work ideally, but we need to add a call to eth_init() if the "ethaddr" in the env changes and set it again if so.
We
should also remove the calls from the drivers you identified since the framework will now do it.
Does it mean that you are going to write that code for it?
Yes... I'll write it, but it needs to be on top of the DM_ETH patches, so I'll probably wait until that's pulled.
-Joe
Hi Joe,
On 03/17/2015 06:21 PM, Joe Hershberger wrote:
Hi Michal,
On Tue, Mar 17, 2015 at 11:57 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi Joe,
On 03/17/2015 04:56 PM, Joe Hershberger wrote:
Hi Michal,
On Fri, Mar 13, 2015 at 7:25 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi,
I have a question regarding setting mac address for drivers. Drivers setting up write_hwaddr via eth_write_hwaddr via eth_initialize which is called from common/board_r.c.
This is because on at least ARM kernels, the MAC is passed via the MAC's filter registers. Because of this, we need to write hwaddr even before
the
MAC is started.
But then there are some drivers(macb, designware, altera_tse) which
also
calls
mac setup from dev->init which has side effect for example when you
setup
CONFIG_ENV_OVERWRITE
and change mac address you can directly use it.
This is probably a work-around for a shortcoming of the net/eth.c not calling write_hwaddr() when the env MAC changes. It already updates the registers used by the network stack, so presumably if those MACs are filtering incoming traffic based on the register as expected, changing
the
MAC after boot without rewriting that register would cause all traffic
to
not be returned.
It also means if there is intention to call hwaddr from dev->init that for the first packet mac address is setup twice - in eth core init and then before every driver use.
The design of the network stack assumes the MAC is started each time a network operation is requested and stopped when done.
As for the setting of the hwaddr, we should check if it changed.
I am asking this question because I would like to know the right flow for eth mac setup. If it is set ethaddr xx.... saveenv reset eth with new mac
or if set ethaddr eth with new mac should work
The second approach looks reasonable when ethaddr is not set at all but then at least our driver needs to be fixed to support this feature.
I think the second should work ideally, but we need to add a call to eth_init() if the "ethaddr" in the env changes and set it again if so.
We
should also remove the calls from the drivers you identified since the framework will now do it.
Does it mean that you are going to write that code for it?
Yes... I'll write it, but it needs to be on top of the DM_ETH patches, so I'll probably wait until that's pulled.
That's fine.
Thanks, Michal
Hi,
On 17 March 2015 at 11:47, Michal Simek michal.simek@xilinx.com wrote:
Hi Joe,
On 03/17/2015 06:21 PM, Joe Hershberger wrote:
Hi Michal,
On Tue, Mar 17, 2015 at 11:57 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi Joe,
On 03/17/2015 04:56 PM, Joe Hershberger wrote:
Hi Michal,
On Fri, Mar 13, 2015 at 7:25 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi,
I have a question regarding setting mac address for drivers. Drivers setting up write_hwaddr via eth_write_hwaddr via eth_initialize which is called from common/board_r.c.
This is because on at least ARM kernels, the MAC is passed via the MAC's filter registers. Because of this, we need to write hwaddr even before
the
MAC is started.
But then there are some drivers(macb, designware, altera_tse) which
also
calls
mac setup from dev->init which has side effect for example when you
setup
CONFIG_ENV_OVERWRITE
and change mac address you can directly use it.
This is probably a work-around for a shortcoming of the net/eth.c not calling write_hwaddr() when the env MAC changes. It already updates the registers used by the network stack, so presumably if those MACs are filtering incoming traffic based on the register as expected, changing
the
MAC after boot without rewriting that register would cause all traffic
to
not be returned.
It also means if there is intention to call hwaddr from dev->init that for the first packet mac address is setup twice - in eth core init and then before every driver use.
The design of the network stack assumes the MAC is started each time a network operation is requested and stopped when done.
As for the setting of the hwaddr, we should check if it changed.
I am asking this question because I would like to know the right flow for eth mac setup. If it is set ethaddr xx.... saveenv reset eth with new mac
or if set ethaddr eth with new mac should work
The second approach looks reasonable when ethaddr is not set at all but then at least our driver needs to be fixed to support this feature.
I think the second should work ideally, but we need to add a call to eth_init() if the "ethaddr" in the env changes and set it again if so.
We
should also remove the calls from the drivers you identified since the framework will now do it.
Does it mean that you are going to write that code for it?
Yes... I'll write it, but it needs to be on top of the DM_ETH patches, so I'll probably wait until that's pulled.
That's fine.
This is in u-boot-dm/next now.
Regards, Simon
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) {
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
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(-)
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);
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
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 &ð_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 &ð_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
participants (6)
-
Bin Meng -
Joe Hershberger -
Joe Hershberger -
Michal Simek -
Michal Simek -
Simon Glass