From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Warren <bwarren@qstreams.com>
Date: Tue, 07 Aug 2007 12:07:47 -0400
Subject: [U-Boot-Users] [PATCH] Add ability to take MAC address from the
 environment to DM9000 driver
In-Reply-To: <46B84A47.3030908@compulab.co.il>
References: <46B84A47.3030908@compulab.co.il>
Message-ID: <46B898D3.40107@qstreams.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Mike,

Mike Rapoport wrote:
> Signed-off-by: Mike Rapoport <mike@compulab.co.il>
>
>   drivers/dm9000x.c |   14 ++++++++++++++
>   include/net.h     |   39 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/dm9000x.c b/drivers/dm9000x.c
> index 6877076..6486699 100644
> --- a/drivers/dm9000x.c
> +++ b/drivers/dm9000x.c
> @@ -302,6 +302,20 @@ eth_init(bd_t * bd)
>   	/* Set Node address */
>   	for (i = 0; i < 6; i++)
>   		((u16 *) bd->bi_enetaddr)[i] = read_srom_word(i);
> +
> +	if (!is_valid_ether_addr(bd->bi_enetaddr)) {
> +		/* try reading from environment */
> +		u8 i;
> +		char *s, *e;
> +		s = getenv ("ethaddr");
>   
Could this ever be other than the first Ethernet controller on a board? 
If so, 'ethaddr' won't cut it.
> +		for (i = 0; i < 6; ++i) {
> +			bd->bi_enetaddr[i] = s ?
> +				simple_strtoul (s, &e, 16) : 0;
> +			if (s)
> +				s = (*e) ? e + 1 : e;
> +		}
> +	}
>   
While this is probably fine, it looks scary to me. My little brain is 
going to need a few more passes...
> +
>   	printf("MAC: %02x:%02x:%02x:%02x:%02x:%02x\n", bd->bi_enetaddr[0],
>   	       bd->bi_enetaddr[1], bd->bi_enetaddr[2], bd->bi_enetaddr[3],
>   	       bd->bi_enetaddr[4], bd->bi_enetaddr[5]);
> diff --git a/include/net.h b/include/net.h
> index 461e038..34967e4 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -435,6 +435,45 @@ static inline void NetCopyLong(ulong *to, ulong *from)
>   	memcpy((void*)to, (void*)from, sizeof(ulong));
>   }
>
> +/**
> + * is_zero_ether_addr - Determine if give Ethernet address is all zeros.
> + * @addr: Pointer to a six-byte array containing the Ethernet address
> + *
> + * Return true if the address is all zeroes.
> + */
> +static inline int is_zero_ether_addr(const u8 *addr)
> +{
> +	return !(addr[0] | addr[1] | addr[2] | addr[3] | addr[4] | addr[5]);
> +}
> +
> +/**
> + * is_multicast_ether_addr - Determine if the Ethernet address is a multicast.
> + * @addr: Pointer to a six-byte array containing the Ethernet address
> + *
> + * Return true if the address is a multicast address.
> + * By definition the broadcast address is also a multicast address.
> + */
> +static inline int is_multicast_ether_addr(const u8 *addr)
> +{
> +	return (0x01 & addr[0]);
> +}
> +
> +/**
> + * is_valid_ether_addr - Determine if the given Ethernet address is valid
> + * @addr: Pointer to a six-byte array containing the Ethernet address
> + *
> + * Check that the Ethernet address (MAC) is not 00:00:00:00:00:00, is not
> + * a multicast address, and is not FF:FF:FF:FF:FF:FF.
> + *
> + * Return true if the address is valid.
> + */
> +static inline int is_valid_ether_addr(const u8 *addr)
> +{
>   
Please choose a better name for this function. While multicast addresses 
are poor choices for source, they're perfectly fine for destination.

thanks,
Ben