[PATCH] smsc95xx: Add module parameter to override MAC address

[PATCH] smsc95xx: Add module parameter to override MAC address

[Sebastien Jan]

Define a new module parameter 'macaddr' to override the MAC address
fetched either from eeprom, or randomly generated.

The expected MAC address shall be in the 01:23:45:67:89:AB format.

Signed-off-by: Sebastien Jan <s-jan@ti.com>
---
 drivers/net/usb/smsc95xx.c |   56 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 3135af6..0ba06d9 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -46,6 +46,7 @@
 #define SMSC95XX_INTERNAL_PHY_ID	(1)
 #define SMSC95XX_TX_OVERHEAD		(8)
 #define SMSC95XX_TX_OVERHEAD_CSUM	(12)
+#define MAC_ADDR_LEN			(6)
 
 struct smsc95xx_priv {
 	u32 mac_cr;
@@ -63,6 +64,10 @@ static int turbo_mode = true;
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 
+static char *macaddr = ":";
+module_param(macaddr, charp, 0);
+MODULE_PARM_DESC(macaddr, "MAC address");
+
 static int smsc95xx_read_reg(struct usbnet *dev, u32 index, u32 *data)
 {
 	u32 *buf = kmalloc(4, GFP_KERNEL);
@@ -637,8 +642,59 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 	return generic_mii_ioctl(&dev->mii, if_mii(rq), cmd, NULL);
 }
 
+/* Check the macaddr module parameter for a MAC address */
+static int smsc95xx_is_macaddr_param(struct usbnet *dev, u8 *dev_mac)
+{
+	int i, j, got_num, num;
+	u8 mtbl[MAC_ADDR_LEN];
+
+	if (macaddr[0] == ':')
+		return 0;
+
+	i = 0;
+	j = 0;
+	num = 0;
+	got_num = 0;
+	while (j < MAC_ADDR_LEN) {
+		if (macaddr[i] && macaddr[i] != ':') {
+			got_num++;
+			if ('0' <= macaddr[i] && macaddr[i] <= '9')
+				num = num * 16 + macaddr[i] - '0';
+			else if ('A' <= macaddr[i] && macaddr[i] <= 'F')
+				num = num * 16 + 10 + macaddr[i] - 'A';
+			else if ('a' <= macaddr[i] && macaddr[i] <= 'f')
+				num = num * 16 + 10 + macaddr[i] - 'a';
+			else
+				break;
+			i++;
+		} else if (got_num == 2) {
+			mtbl[j++] = (u8) num;
+			num = 0;
+			got_num = 0;
+			i++;
+		} else {
+			break;
+		}
+	}
+
+	if (j == MAC_ADDR_LEN && !macaddr[i]) {
+		netif_dbg(dev, ifup, dev->net, "Overriding MAC address with: "
+		"%02x:%02x:%02x:%02x:%02x:%02x\n", mtbl[0], mtbl[1], mtbl[2],
+						mtbl[3], mtbl[4], mtbl[5]);
+		for (i = 0; i < MAC_ADDR_LEN; i++)
+			dev_mac[i] = mtbl[i];
+		return 1;
+	} else {
+		return 0;
+	}
+}
+
 static void smsc95xx_init_mac_address(struct usbnet *dev)
 {
+	/* Check module parameters */
+	if (smsc95xx_is_macaddr_param(dev, dev->net->dev_addr))
+		return;
+
 	/* try reading mac address from EEPROM */
 	if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
 			dev->net->dev_addr) == 0) {
-- 
1.6.3.3

Re: [PATCH] smsc95xx: Add module parameter to override MAC address

[Simon Horman]

On Thu, Jun 24, 2010 at 10:14:14AM +0200, Sebastien Jan wrote:
> Define a new module parameter 'macaddr' to override the MAC address
> fetched either from eeprom, or randomly generated.
> 
> The expected MAC address shall be in the 01:23:45:67:89:AB format.

I'm confused as to why this is desirable when the mac address
can already be configured after module insertion via
smsc95xx_netdev_ops.eth_mac_addr().

Re: [PATCH] smsc95xx: Add module parameter to override MAC address

[Sebastien Jan]

On 06/25/2010 03:25 AM, Simon Horman wrote:
> On Thu, Jun 24, 2010 at 10:14:14AM +0200, Sebastien Jan wrote:
>> Define a new module parameter 'macaddr' to override the MAC address
>> fetched either from eeprom, or randomly generated.
>>
>> The expected MAC address shall be in the 01:23:45:67:89:AB format.
> 
> I'm confused as to why this is desirable when the mac address
> can already be configured after module insertion via
> smsc95xx_netdev_ops.eth_mac_addr().

For example for booting over NFS using a pre-defined MAC address, with 
a minimal setup (no initrd). Or is there another way to force a MAC
address for this use-case?

Re: [PATCH] smsc95xx: Add module parameter to override MAC address

[Steve.Glendinning]

Hi Sebastien,

> > I'm confused as to why this is desirable when the mac address
> > can already be configured after module insertion via
> > smsc95xx_netdev_ops.eth_mac_addr().
> 
> For example for booting over NFS using a pre-defined MAC address, with 
> a minimal setup (no initrd). Or is there another way to force a MAC
> address for this use-case?

I can't see an existing way of specifying this as a kernel parameter
in Documentation/kernel-parameters.txt. netdev= doesn't have a mac
address parameter.

During development I initially had smsc95xx driver using this logic to
select a MAC address:

1. If net->dev_addr has already been set to a valid mac address (i.e. by
an administrator before bringing the device up) then use that address.

2. If the device is already currently set to a valid mac address then
use that address.  This could have been set by either the device's
EEPROM or by a previously running bootloader.

3. Generate a random mac address.

Unfortunately, this doesn't work so well as the usbnet framework sets
net->dev_addr to the USB node_id before calling our bind function, so
we usually matched at step 1 (and not with the desired outcome).  So
I removed step 1 because, as Simon mentioned, it's possible to change
the mac address after the device is brought up.

I can see you have a different use case, but I don't think this specific
driver is the place for this logic.  I'd rather see it added to either
the usbnet framework or (preferably) the netdev framework so *all*
ethernet drivers can do this the same way.  otherwise we could end up
with slight variations of this code in every single driver!

Steve

Re: [PATCH] smsc95xx: Add module parameter to override MAC address

[Sebastien Jan]

Hi Steve,

Thanks for your answer.

On 06/25/2010 10:43 AM, Steve.Glendinning@smsc.com wrote:
[...]
> I can see you have a different use case, but I don't think this specific
> driver is the place for this logic.  I'd rather see it added to either
> the usbnet framework or (preferably) the netdev framework so *all*
> ethernet drivers can do this the same way.  otherwise we could end up
> with slight variations of this code in every single driver!

I perfectly understand your concerns. Unfortunately, I will probably not be
able to implement these changes into the netdev framework. However, I'd be 
happy to make some tests if someone proposes such changes.

Re: [PATCH] smsc95xx: Add module parameter to override MAC address

[David Miller]

From: Sebastien Jan <s-jan@ti.com>
Date: Fri, 25 Jun 2010 16:20:36 +0200

> Hi Steve,
> 
> Thanks for your answer.
> 
> On 06/25/2010 10:43 AM, Steve.Glendinning@smsc.com wrote:
> [...]
>> I can see you have a different use case, but I don't think this specific
>> driver is the place for this logic.  I'd rather see it added to either
>> the usbnet framework or (preferably) the netdev framework so *all*
>> ethernet drivers can do this the same way.  otherwise we could end up
>> with slight variations of this code in every single driver!
> 
> I perfectly understand your concerns. Unfortunately, I will probably not be
> able to implement these changes into the netdev framework. However, I'd be 
> happy to make some tests if someone proposes such changes.

I don't think such logic belongs anywhere other than an initrd.

We have mechanisms by which to handle this already, and contrary to
what many people claim it is not at all so hard to create a custom
initrd.