[U-Boot] [PATCH v4 1/2][NEXT] davinci_emac: davinci_eth_set_mac_addr to ->write_hwaddr

Nick Thompson
2010-09-23T09:03:21Z

On 22/09/10 19:44, Ben Gardiner wrote:
> This patch proposes to migrate the davinci_emac driver to using the
> eth_device->write_hwaddr function pointer as suggested by Ben Warren.
>
> All the davinci boards had the behaviour, prior to this patch, of
> sync'ing the environment variable enetaddr with the MAC address read
> from non-volatile storage on boot -- when the two locations disagreed,
> the environment variable value took precendence. This patch keeps the
> same behaviour but lets eth_initialize take care of it.
>
> This patch refactors davinci_emac setup in the boards so that the MAC
> address is read from non-volatile storage into the environment variable
> and then the environment variable value is use in eth_intialize. The
> only exception is the direct call to davinci_eth_set_mac_addr made by
> the da830evm board init which was changed into an assignment of the
> enetaddr field.
>
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
> CC: Ben Warren <biggerbadderben@gmail.com>
> CC: Nick Thompson <nick.thompson@ge.com>
>
> --
>
> V4:
> * no changes
> V3:
> * rebased to u-boot/next, removed additional direct call of
> davinci_eth_set_mac_addr added to board_init_r by Heiko's
> relocation patches
> V2:
> * introduced this patch
> ---
> arch/arm/include/asm/arch-davinci/emac_defs.h | 1 -
> arch/arm/lib/board.c | 19 ------
> board/davinci/common/misc.c | 41 +++----------
> board/davinci/common/misc.h | 2 +-
> board/davinci/da8xxevm/da830evm.c | 15 ++++-
> board/davinci/dm365evm/dm365evm.c | 2 +-
> board/davinci/dvevm/dvevm.c | 2 +-
> board/davinci/sffsdr/sffsdr.c | 2 +-
> board/davinci/sonata/sonata.c | 2 +-
> drivers/net/davinci_emac.c | 80 ++++++++++++-------------
> 10 files changed, 64 insertions(+), 102 deletions(-)
>

[snip]

> diff --git a/board/davinci/da8xxevm/da830evm.c b/board/davinci/da8xxevm/da830evm.c
> index 6baa860..d5a228f 100644
> --- a/board/davinci/da8xxevm/da830evm.c
> +++ b/board/davinci/da8xxevm/da830evm.c
> @@ -196,19 +196,17 @@ int board_eth_init(bd_t *bis)
> {
> u_int8_t mac_addr[6];
> u_int8_t switch_start_cmd[2] = { 0x01, 0x23 };
> + struct eth_device *dev;
>
> /* Read Ethernet MAC address from EEPROM */
> if (dvevm_read_mac_address(mac_addr))
> /* set address env if not already set */
> - dv_configure_mac_address(mac_addr);
> + davinci_sync_env_enetaddr(mac_addr);
>
> /* read the address back from env */
> if (!eth_getenv_enetaddr("ethaddr", mac_addr))
> return -1;
>
> - /* provide the resulting addr to the driver */
> - davinci_eth_set_mac_addr(mac_addr);
> -
> /* enable the Ethernet switch in the 3 port PHY */
> if (i2c_write(PHY_SW_I2C_ADDR, 0, 0,
> switch_start_cmd, sizeof(switch_start_cmd))) {
> @@ -222,6 +220,15 @@ int board_eth_init(bd_t *bis)
> return -1;
> }
>
> + dev = eth_get_dev();
> +
> + /* provide the resulting addr to the driver */
> + memcpy(dev->enetaddr, mac_addr, 6);
> + if (!dev->write_hwaddr(dev)) {
> + printf("Error: Could not set MAC address\n");
> + return -1;
> + }
> +
> return 0;
> }
> #endif /* CONFIG_DRIVER_TI_EMAC */

Hi Ben,

davinci_eth_set_mac_addr() always returns zero, so "if (!dev->write_hwaddr(dev))"
always appears to fail here.

In net/eth.c the return code is not checked, so da830 should probably ignore it to:

diff --git a/board/davinci/da8xxevm/da830evm.c b/board/davinci/da8xxevm/da830evm.c
index d5a228f..8a9f988 100644
--- a/board/davinci/da8xxevm/da830evm.c
+++ b/board/davinci/da8xxevm/da830evm.c
@@ -224,10 +224,7 @@ int board_eth_init(bd_t *bis)

/* provide the resulting addr to the driver */
memcpy(dev->enetaddr, mac_addr, 6);
- if (!dev->write_hwaddr(dev)) {
- printf("Error: Could not set MAC address\n");
- return -1;
- }
+ dev->write_hwaddr(dev);

return 0;
}

With this change, this patch works on da830. I'll retest when you resubmit.

Nick.