Subject: Re: [PATCH] dm9601: warn on invalid mac address

From: Peter Korsgaard <[email-masked]>
Date: Tue, 06 Jan 2009 12:17:56 +0100

>>>>> "Wu" == Wu Fengguang <[email-masked]> writes:

Hi,

 Wu> if (is_valid_ether_addr(mac))
 Wu> memcpy(dev->net->dev_addr, mac, ETH_ALEN);
 Wu> +	else {
 Wu> +		DECLARE_MAC_BUF(mac_buf);
 Wu> +		print_mac(mac_buf, mac);
 Wu> +		devdbg(dev, "EEPROM reported mac address %s is invalid,"
 Wu> +				" use the randomly generated one.", mac_buf);
 >> 
 >> And this should be a warning.

 Wu> Then let the warning message appear repeatedly for some devices?

This is called at probe time - But yes, I think it makes sense to print
it.

We should print the random address instead of the ff's though.

 Wu> Also dev_warn() won't be able to show the device name at that time,
 Wu> like this:

Ah yes, that's presumably why I used a raw printk just above.

 Wu> [28489.062180] : EEPROM reported mac address ff:ff:ff:ff:ff:ff is
 Wu> invalid, use the randomly generated one.

I would prefer something like:

printk(KERN_WARNING "dm9601: No valid MAC address in EEPROM, using %s\n",
       print_mac(..));

Also, it seems like you're not writing the random address to the
hardware registers, so you won't be able to receive any unicast -
You'll need to add a call to dm9601_set_mac_address() or similar.

-- 
Bye, Peter Korsgaard