Re: [PATCH] Check mac-address first in fsl_soc.c

[Jerry Van Baren <gerald.vanbaren@smiths-aerospace.com>]

Timur Tabi wrote:
> The mac-address property in the device tree should be checked first,
> before local-mac-address.  This is because mac-address contains the most
> recent MAC address, whereas local-mac-address is the default address.
> Depending on the platform and the version of U-Boot, U-Boot will set
> one or the other, or both.
> 
> This patch updates gfar_of_init() and fs_enet_of_init() to conform to
> this order.  It skips a property if it doesn't exist or if it contains
> an all-zero MAC address.  This patch also adds some NULL-pointer checking
> to make sure there are no panics if no MAC address has been passed.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>  arch/powerpc/sysdev/fsl_soc.c |   39 +++++++++++++++++++++++++++++----------
>  1 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
> index 9f2a9a4..a0586ea 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -233,12 +233,13 @@ static int __init gfar_of_init(void)
>  			goto err;
>  		}
>  
> -		mac_addr = get_property(np, "local-mac-address", NULL);
> -		if (mac_addr == NULL)
> -			mac_addr = get_property(np, "mac-address", NULL);
> -		if (mac_addr == NULL) {
> -			/* Obsolete */
> -			mac_addr = get_property(np, "address", NULL);
> +		mac_addr = get_property(np, "mac-address", NULL);
> +		if (!mac_addr || (memcmp(mac_addr, "\0\0\0\0\0", 6) == 0)) {
> +			mac_addr = get_property(np, "local-mac-address", NULL);
> +			if (!mac_addr || (memcmp(mac_addr, "\0\0\0\0\0", 6) == 0)) {
> +				/* Obsolete */
> +				mac_addr = get_property(np, "address", NULL);
> +			}

If you want to check for common errors, add a check that the first byte 
does not have the multicast flag set.  Note that this will catch the 
error of setting the MAC to the broadcast address of all 0xFFs as well 
as the naive users that set it to 11:22:33:44:55:66 (remarkable number 
of them out there).

if (!mac_addr || (memcmp(mac_addr, "\0\0\0\0\0", 6) == 0)
     || ((*mac_addr & 0x01) == 0x01)) {

or, if you are a terse, excuse me, tight C programmer:

if (!mac_addr || (memcmp(mac_addr, "\0\0\0\0\0", 6) == 0)
     || (*mac_addr & 0x01)) {

gvb