Jeff Garzik wrote:
>
> Attached is a patch against 2.4.3-pre6, which adds alloc_etherdev and
> variants to the driver API.
Looks good. Few minor things:
In register_netdev():
if (strchr(dev->name, '%'))
{
err = -EBUSY;
if(dev_alloc_name(dev, dev->name)<0)
goto out;
}
We should propagate dev_alloc_name's return value:
err = dev_alloc_name()
if (err < 0)
goto out;
And register_netdevice's, so
err = -EIO;
if (register_netdevice(dev))
goto out;
err = 0;
out:
becomes simply
err = register_netdevice(dev);
out:
More significantly, the driver probe functions now become:
xxx_probe()
{
dev = alloc_etherdev(sizeof(xxx_private));
...
printk(KERN_INFO "%s: stuff\n", dev->name);
...
ret = register_netdev(dev);
if (ret < 0)
kfree(ret);
return ret;
}
yes?
And the printk() will say "eth%d: stuff", so we'll need to
change the messages:
- printk(KERN_INFO "%s: stuff\n", dev->name);
+ printk(KERN_INFO "xxx: stuff\n");
Correct?
My quibble with this is things like wait_for_completion(),
which are called from both the probe() function and
the mainline driver code. These also print dev->name,
and there's no obvious fix for that.
For this reason I think I'd prefer it if alloc_etherdev()
was passed some probe-time identifier:
alloc_etherdev(sizeof(xxx_private), "xxx");
which goes into dev->name, and gets overwritten by
register_netdev().
So, against your current patch:
--- drivers/net/net_init.c.orig Thu Mar 22 02:11:47 2001
+++ drivers/net/net_init.c Thu Mar 22 02:17:46 2001
@@ -71,7 +71,7 @@
static struct net_device *alloc_netdev(int sizeof_priv, const char *mask,
- void (*setup)(struct net_device *))
+ const char *driver_name, void (*setup)(struct net_device *))
{
struct net_device *dev;
int alloc_size;
@@ -92,7 +92,8 @@
dev->priv = (void *) (((long)(dev + 1) + 31) & ~31);
setup(dev);
- strcpy(dev->name, mask);
+ strcpy(dev->ifname, mask);
+ strcpy(dev->name, driver_name);
return dev;
}
@@ -216,9 +217,9 @@
* this private data area.
*/
-struct net_device *alloc_etherdev(int sizeof_priv)
+struct net_device *alloc_etherdev(int sizeof_priv, const char *driver_name)
{
- return alloc_netdev(sizeof_priv, "eth%d", ether_setup);
+ return alloc_netdev(sizeof_priv, "eth%d", driver_name, ether_setup);
}
EXPORT_SYMBOL(init_etherdev);
@@ -532,7 +533,7 @@
int register_netdev(struct net_device *dev)
{
- int err;
+ int err = 0;
rtnl_lock();
@@ -540,13 +541,14 @@
* If the name is a format string the caller wants us to
* do a name allocation
*/
-
- if (strchr(dev->name, '%'))
- {
- err = -EBUSY;
- if(dev_alloc_name(dev, dev->name)<0)
- goto out;
- }
+
+ /* Insert comment here */
+ if (strchr(dev->ifname, '%'))
+ err = dev_alloc_name(dev, dev->ifname);
+ else if (strchr(dev->name, '%'))
+ err = dev_alloc_name(dev, dev->name);
+ if (err < 0)
+ goto out;
/*
* Back compatibility hook. Kill this one in 2.5
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Mar 23 2001 - 21:00:15 EST