diff mbox

smsc95xx: add macaddr module parameter

Message ID 1328030433-12524-1-git-send-email-danny.kukawka@bisect.de
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Danny Kukawka Jan. 31, 2012, 5:20 p.m. UTC
Added smsc95xx.macaddr module parameter to allow the user to
change the MAC address on boot if there was no MAC on the EEPROM.

The parameter take the MAC address in 01:23:45:67:89:ab format and
needs to be locally assigned. The MAC get assigned to the first
smsc95xx device with no MAC on EEPROM (which resulted in a random
MAC before). If there is more than one device without MAC on
EEPROM and the user needs set the MAC to a specific device, it
can be done by attaching the netdev name (e.g. eth0) to the
smsc95xx.macaddr parameter seperated by a ';' as e.g. in
'01:23:45:67:89:ab;eth0'

This allows e.g. u-boot to pass on PandaBoard or BeagleBoard
the by u-boot generated static MAC address to the kernel device
to ensure the MAC stays the same during the whole boot process.

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 drivers/net/usb/smsc95xx.c |   85 ++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 82 insertions(+), 3 deletions(-)

Comments

Alan Cox Jan. 31, 2012, 5:27 p.m. UTC | #1
On Tue, 31 Jan 2012 18:20:33 +0100
Danny Kukawka <danny.kukawka@bisect.de> wrote:

> Added smsc95xx.macaddr module parameter to allow the user to
> change the MAC address on boot if there was no MAC on the EEPROM.

NAK. Please see the discussion on this and ifconfig hw for other devices.

In summary - make the device refuse to ifconfig up without a valid
address, allow it to come into existance without a valid address and then
use ifconfig.


Your patch also seems to break if there are several attached to a board.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Danny Kukawka Jan. 31, 2012, 6:17 p.m. UTC | #2
On Dienstag, 31. Januar 2012, Alan Cox wrote:
> On Tue, 31 Jan 2012 18:20:33 +0100
>
> Danny Kukawka <danny.kukawka@bisect.de> wrote:
> > Added smsc95xx.macaddr module parameter to allow the user to
> > change the MAC address on boot if there was no MAC on the EEPROM.
>
> NAK. Please see the discussion on this and ifconfig hw for other devices.
>
> In summary - make the device refuse to ifconfig up without a valid
> address, allow it to come into existance without a valid address and then
> use ifconfig.
>
>
> Your patch also seems to break if there are several attached to a board.

There are already other kernel driver which allow to override the (hw) MAC 
address via a module parameter (macaddr): 
- KSZ8841/2 PCI network driver (drivers/net/ethernet/micrel/ksz884x.c)
- FEC Ethernet driver (drivers/net/ethernet/freescale/fec.c)
- Sun HappyMealEthernet(HME) 10/100baseT ethernet driver
  (drivers/net/ethernet/sun/sunhme.c)

By default the MAC from the module parameter get only assigned to the first 
device which got a random MAC (caused by missing EEPROM) before. If you have 
more than one device you can simply assign the MAC to a specific interface 
(e.g. use: macaddr=01:23:45:67:89:ab;eth0). If there are only devices with 
hardware MAC nothing will get changed.

Danny
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Jan. 31, 2012, 7:45 p.m. UTC | #3
From: Danny Kukawka <danny.kukawka@bisect.de>
Date: Tue, 31 Jan 2012 19:17:11 +0100

> On Dienstag, 31. Januar 2012, Alan Cox wrote:
>> On Tue, 31 Jan 2012 18:20:33 +0100
>>
>> Danny Kukawka <danny.kukawka@bisect.de> wrote:
>> > Added smsc95xx.macaddr module parameter to allow the user to
>> > change the MAC address on boot if there was no MAC on the EEPROM.
>>
>> NAK. Please see the discussion on this and ifconfig hw for other devices.
>>
>> In summary - make the device refuse to ifconfig up without a valid
>> address, allow it to come into existance without a valid address and then
>> use ifconfig.
>>
>>
>> Your patch also seems to break if there are several attached to a board.
> 
> There are already other kernel driver which allow to override the (hw) MAC 
> address via a module parameter (macaddr): 

Too bad, it's bad precendence and just because we mistakenly let other
drivers do it in the past, it's not an excuse to allow you to make this
mistake too.

This patch will not be applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index d45520e..3198c7d 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -52,6 +52,8 @@  struct smsc95xx_priv {
 	u32 hash_hi;
 	u32 hash_lo;
 	spinlock_t mac_cr_lock;
+	bool mac_set_from_param;
+	bool mac_is_random;
 };
 
 struct usb_context {
@@ -63,6 +65,11 @@  static bool turbo_mode = true;
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
+static char *macaddr = ":";
+static bool set_macaddr = false;
+module_param(macaddr, charp, 0);
+MODULE_PARM_DESC(macaddr, " macaddr=macaddr;[tgt-netdevname] (Set MAC only if there is a device without MAC on EEPROM)");
+
 static int smsc95xx_read_reg(struct usbnet *dev, u32 index, u32 *data)
 {
 	u32 *buf = kmalloc(4, GFP_KERNEL);
@@ -601,8 +608,71 @@  static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 	return generic_mii_ioctl(&dev->mii, if_mii(rq), cmd, NULL);
 }
 
+/* set mac address from the macaddr module parameter */
+static int smsc95xx_init_mac_address_from_param(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	int i, parsed, ret;
+	u8 mtbl[ETH_ALEN];
+	char *input = NULL;
+	char *config_param = NULL;
+	char *netdev_name = NULL;	
+
+	parsed = 0;
+	ret = 0;
+
+	input = kstrdup(macaddr, GFP_KERNEL);
+
+	if (!input)
+		return -ENOMEM;
+
+	if (strlen(input) >= 17) {
+		while ((config_param = strsep(&input, ";"))) {
+			if (parsed == 0) {
+				if (!mac_pton(config_param, mtbl)) {
+					ret = 1;
+					goto parse_err;
+				}
+			} else {
+				netdev_name = config_param;				
+			}
+			parsed ++;
+		}
+
+		if (parsed && is_valid_ether_addr(mtbl)) {
+			if (netdev_name && strlen(netdev_name)) {
+				if (strcmp(netdev_name, dev->net->name) != 0) {
+					netif_dbg(dev, ifup, dev->net, "requested devname (%s) didn't match (%s)\n", netdev_name, dev->net->name);
+					ret = 1;
+					goto out;
+				}
+			}
+
+			for (i = 0; i < ETH_ALEN; i++) {
+				dev->net->dev_addr[i] = mtbl[i];
+			}
+
+			netif_dbg(dev, ifup, dev->net, "set valid MAC address from smsc95xx.macaddr\n");
+			set_macaddr = true;
+			pdata->mac_set_from_param = true;
+			pdata->mac_is_random = false;
+			goto out;
+		}
+	} 
+
+parse_err:
+	netif_dbg(dev, ifup, dev->net, "failed to parse (valid) MAC from smsc95xx.macaddr\n");
+	set_macaddr = true;
+out:
+	if (input)
+		kfree(input);
+	return ret;
+}
+
 static void smsc95xx_init_mac_address(struct usbnet *dev)
 {
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);	
+
 	/* try reading mac address from EEPROM */
 	if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
 			dev->net->dev_addr) == 0) {
@@ -615,16 +685,25 @@  static void smsc95xx_init_mac_address(struct usbnet *dev)
 
 	/* no eeprom, or eeprom values are invalid. generate random MAC */
 	random_ether_addr(dev->net->dev_addr);
+	pdata->mac_is_random = true;
 	netif_dbg(dev, ifup, dev->net, "MAC address set to random_ether_addr\n");
 }
 
 static int smsc95xx_set_mac_address(struct usbnet *dev)
 {
-	u32 addr_lo = dev->net->dev_addr[0] | dev->net->dev_addr[1] << 8 |
-		dev->net->dev_addr[2] << 16 | dev->net->dev_addr[3] << 24;
-	u32 addr_hi = dev->net->dev_addr[4] | dev->net->dev_addr[5] << 8;
+	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	u32 addr_lo, addr_hi;
 	int ret;
 
+	if (pdata->mac_is_random && !pdata->mac_set_from_param && !set_macaddr) {
+		netif_dbg(dev, ifup, dev->net, "random MAC address, not yet set from smsc95xx.macaddr, try to set it ...\n");
+		smsc95xx_init_mac_address_from_param(dev);
+	}
+
+	addr_lo = dev->net->dev_addr[0] | dev->net->dev_addr[1] << 8 |
+		dev->net->dev_addr[2] << 16 | dev->net->dev_addr[3] << 24;
+	addr_hi = dev->net->dev_addr[4] | dev->net->dev_addr[5] << 8;
+
 	ret = smsc95xx_write_reg(dev, ADDRL, addr_lo);
 	if (ret < 0) {
 		netdev_warn(dev->net, "Failed to write ADDRL: %d\n", ret);