[PATCH 405] Amiga Hydra Ethernet new driver model

[PATCH 405] Amiga Hydra Ethernet new driver model

[Geert Uytterhoeven]

Hydra Ethernet: Convert to the new driver model

--- linux-2.6.3-rc1/drivers/net/hydra.c	2004-02-08 10:19:34.000000000 +0100
+++ linux-m68k-2.6.3-rc1/drivers/net/hydra.c	2004-02-08 11:47:55.000000000 +0100
@@ -44,10 +44,10 @@
 
 #define WORDSWAP(a)     ((((a)>>8)&0xff) | ((a)<<8))
 
-static struct net_device *root_hydra_dev;
 
-static int __init hydra_probe(void);
-static int __init hydra_init(unsigned long board);
+static int __devinit hydra_init_one(struct zorro_dev *z,
+				    const struct zorro_device_id *ent);
+static int __init hydra_init(struct zorro_dev *z);
 static int hydra_open(struct net_device *dev);
 static int hydra_close(struct net_device *dev);
 static void hydra_reset_8390(struct net_device *dev);
@@ -57,34 +57,38 @@
 			      struct sk_buff *skb, int ring_offset);
 static void hydra_block_output(struct net_device *dev, int count,
 			       const unsigned char *buf, int start_page);
-static void __exit hydra_cleanup(void);
+static void __devexit hydra_remove_one(struct zorro_dev *z);
 
-static int __init hydra_probe(void)
-{
-    struct zorro_dev *z = NULL;
-    unsigned long board;
-    int err = -ENODEV;
-
-    while ((z = zorro_find_device(ZORRO_PROD_HYDRA_SYSTEMS_AMIGANET, z))) {
-	board = z->resource.start;
-	if (!request_mem_region(board, 0x10000, "Hydra"))
-	    continue;
-	if ((err = hydra_init(ZTWO_VADDR(board)))) {
-	    release_mem_region(board, 0x10000);
-	    return err;
-	}
-	err = 0;
-    }
+static struct zorro_device_id hydra_zorro_tbl[] = {
+    { ZORRO_PROD_HYDRA_SYSTEMS_AMIGANET },
+    { 0 }
+};
+
+static struct zorro_driver hydra_driver = {
+    .name	= "hydra",
+    .id_table	= hydra_zorro_tbl,
+    .probe	= hydra_init_one,
+    .remove	= __devexit_p(hydra_remove_one),
+};
 
-    if (err == -ENODEV)
-	printk("No Hydra ethernet card found.\n");
+static int __devinit hydra_init_one(struct zorro_dev *z,
+				    const struct zorro_device_id *ent)
+{
+    int err;
 
-    return err;
+    if (!request_mem_region(z->resource.start, 0x10000, "Hydra"))
+	return -EBUSY;
+    if ((err = hydra_init(z))) {
+	release_mem_region(z->resource.start, 0x10000);
+	return -EBUSY;
+    }
+    return 0;
 }
 
-static int __init hydra_init(unsigned long board)
+static int __init hydra_init(struct zorro_dev *z)
 {
     struct net_device *dev;
+    unsigned long board = ZTWO_VADDR(z->resource.start);
     unsigned long ioaddr = board+HYDRA_NIC_BASE;
     const char name[] = "NE2000";
     int start_page, stop_page;
@@ -128,7 +132,7 @@
 	return -ENOMEM;
     }
 
-    printk("%s: hydra at 0x%08lx, address %02x:%02x:%02x:%02x:%02x:%02x (hydra.c " HYDRA_VERSION ")\n", dev->name, ZTWO_PADDR(board),
+    printk("%s: hydra at 0x%08lx, address %02x:%02x:%02x:%02x:%02x:%02x (hydra.c " HYDRA_VERSION ")\n", dev->name, z->resource.start,
 	dev->dev_addr[0], dev->dev_addr[1], dev->dev_addr[2],
 	dev->dev_addr[3], dev->dev_addr[4], dev->dev_addr[5]);
 
@@ -147,19 +151,18 @@
     ei_status.reg_offset = hydra_offsets;
     dev->open = &hydra_open;
     dev->stop = &hydra_close;
-#ifdef MODULE
-    ei_status.priv = (unsigned long)root_hydra_dev;
-    root_hydra_dev = dev;
-#endif
     NS8390_init(dev, 0);
+
     err = register_netdev(dev);
-    if (!err)
-	return 0;
+    if (err) {
+	free_irq(IRQ_AMIGA_PORTS, dev);
+	kfree(dev->priv);
+	free_netdev(dev);
+	return err;
+    }
 
-    free_irq(IRQ_AMIGA_PORTS, dev);
-    kfree(dev->priv);
-    free_netdev(dev);
-    return err;
+    zorro_set_drvdata(z, dev);
+    return 0;
 }
 
 static int hydra_open(struct net_device *dev)
@@ -230,20 +233,27 @@
     z_memcpy_toio(mem_base+((start_page - NESM_START_PG)<<8), buf, count);
 }
 
-static void __exit hydra_cleanup(void)
+static void __devexit hydra_remove_one(struct zorro_dev *z)
 {
-    struct net_device *dev, *next;
+    struct net_device *dev = zorro_get_drvdata(z);
 
-    while ((dev = root_hydra_dev)) {
-	next = (struct net_device *)(ei_status.priv);
-	unregister_netdev(dev);
-	free_irq(IRQ_AMIGA_PORTS, dev);
-	release_mem_region(ZTWO_PADDR(dev->base_addr)-HYDRA_NIC_BASE, 0x10000);
-	free_netdev(dev);
-	root_hydra_dev = next;
-    }
+    unregister_netdev(dev);
+    free_irq(IRQ_AMIGA_PORTS, dev);
+    release_mem_region(ZTWO_PADDR(dev->base_addr)-HYDRA_NIC_BASE, 0x10000);
+    free_netdev(dev);
+}
+
+static int __init hydra_init_module(void)
+{
+    return zorro_module_init(&hydra_driver);
+}
+
+static void __exit hydra_cleanup_module(void)
+{
+    zorro_unregister_driver(&hydra_driver);
 }
 
-module_init(hydra_probe);
-module_exit(hydra_cleanup);
+module_init(hydra_init_module);
+module_exit(hydra_cleanup_module);
+
 MODULE_LICENSE("GPL");

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH 405] Amiga Hydra Ethernet new driver model

[Adrian Bunk]

On Sun, Feb 08, 2004 at 04:28:29PM +0100, Geert Uytterhoeven wrote:
> Hydra Ethernet: Convert to the new driver model
> 
> --- linux-2.6.3-rc1/drivers/net/hydra.c	2004-02-08 10:19:34.000000000 +0100
> +++ linux-m68k-2.6.3-rc1/drivers/net/hydra.c	2004-02-08 11:47:55.000000000 +0100
>...
> -static int __init hydra_init(unsigned long board);
> +static int __devinit hydra_init_one(struct zorro_dev *z,
> +				    const struct zorro_device_id *ent);
> +static int __init hydra_init(struct zorro_dev *z);
>...
> +static int __devinit hydra_init_one(struct zorro_dev *z,
> +				    const struct zorro_device_id *ent)
> +{
> +    int err;
>  
> -    return err;
> +    if (!request_mem_region(z->resource.start, 0x10000, "Hydra"))
> +	return -EBUSY;
> +    if ((err = hydra_init(z))) {
> +	release_mem_region(z->resource.start, 0x10000);
> +	return -EBUSY;
> +    }
> +    return 0;
>  }
>...

__init hydra_init called from __devinit hydra_init_one ?

This will break when compiling the driver statically into a kernel with
CONFIG_HOTPLUG=y .


> Gr{oetje,eeting}s,
> 
> 						Geert

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH 405] Amiga Hydra Ethernet new driver model

[Geert Uytterhoeven]

On Sun, 8 Feb 2004, Adrian Bunk wrote:
> On Sun, Feb 08, 2004 at 04:28:29PM +0100, Geert Uytterhoeven wrote:
> > Hydra Ethernet: Convert to the new driver model
> >
> > --- linux-2.6.3-rc1/drivers/net/hydra.c	2004-02-08 10:19:34.000000000 +0100
> > +++ linux-m68k-2.6.3-rc1/drivers/net/hydra.c	2004-02-08 11:47:55.000000000 +0100
> >...
> > -static int __init hydra_init(unsigned long board);
> > +static int __devinit hydra_init_one(struct zorro_dev *z,
> > +				    const struct zorro_device_id *ent);
> > +static int __init hydra_init(struct zorro_dev *z);
> >...
> > +static int __devinit hydra_init_one(struct zorro_dev *z,
> > +				    const struct zorro_device_id *ent)
> > +{
> > +    int err;
> >
> > -    return err;
> > +    if (!request_mem_region(z->resource.start, 0x10000, "Hydra"))
> > +	return -EBUSY;
> > +    if ((err = hydra_init(z))) {
> > +	release_mem_region(z->resource.start, 0x10000);
> > +	return -EBUSY;
> > +    }
> > +    return 0;
> >  }
> >...
>
> __init hydra_init called from __devinit hydra_init_one ?
>
> This will break when compiling the driver statically into a kernel with
> CONFIG_HOTPLUG=y .

Thanks, you're right!

I checked the other drivers, and zorro8390.c has the same problem with
zorro8390_init(). I'll fix them and will resend after 2.6.3.

BTW, while verifying the rules w.r.t. the various __*it markers, I stumbled
across this in Documentation/pci.txt (while it's not really PCI-related):

| Tips:
|         The module_init()/module_exit() functions (and all initialization
|         functions called only from these) should be marked __init/exit.
|         The struct pci_driver shouldn't be marked with any of these tags.
|         The ID table array should be marked __devinitdata.
|         The probe() and remove() functions (and all initialization
|         functions called only from these) should be marked __devinit/exit.
|         If you are sure the driver is not a hotplug driver then use only
|         __init/exit __initdata/exitdata.
|
|         Pointers to functions marked as __devexit must be created using
|         __devexit_p(function_name).  That will generate the function
|         name or NULL if the __devexit function will be discarded.

But a quick look shows that very few drivers seem to mark their ID table array
__devinitdata...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH 405] Amiga Hydra Ethernet new driver model

[Jeff Garzik]

Geert Uytterhoeven wrote:
> But a quick look shows that very few drivers seem to mark their ID table array
> __devinitdata...


Yes, this highlights a recent problem with CONFIG_HOTPLUG.  These 
markers were removed when the "add PCI ids to your table dynamically" 
patch was added in 2.5.x.  The patch didn't require CONFIG_HOTPLUG, and 
fun ensued.  This got fixed (IIRC), but also many drivers had their 
__devinitdata markers removed.  This is a fuzzy area where Greg loves to 
make the case for removing  CONFIG_HOTPLUG completely :)

	Jeff

[PATCH 405] Amiga Hydra Ethernet new driver model

[Geert Uytterhoeven]

Hydra Ethernet: Convert to the new driver model

--- linux-2.6.3/drivers/net/hydra.c	2004-02-08 10:19:34.000000000 +0100
+++ linux-m68k-2.6.3/drivers/net/hydra.c	2004-02-08 11:47:55.000000000 +0100
@@ -44,10 +44,10 @@
 
 #define WORDSWAP(a)     ((((a)>>8)&0xff) | ((a)<<8))
 
-static struct net_device *root_hydra_dev;
 
-static int __init hydra_probe(void);
-static int __init hydra_init(unsigned long board);
+static int __devinit hydra_init_one(struct zorro_dev *z,
+				    const struct zorro_device_id *ent);
+static int __devinit hydra_init(struct zorro_dev *z);
 static int hydra_open(struct net_device *dev);
 static int hydra_close(struct net_device *dev);
 static void hydra_reset_8390(struct net_device *dev);
@@ -57,34 +57,38 @@
 			      struct sk_buff *skb, int ring_offset);
 static void hydra_block_output(struct net_device *dev, int count,
 			       const unsigned char *buf, int start_page);
-static void __exit hydra_cleanup(void);
+static void __devexit hydra_remove_one(struct zorro_dev *z);
 
-static int __init hydra_probe(void)
-{
-    struct zorro_dev *z = NULL;
-    unsigned long board;
-    int err = -ENODEV;
-
-    while ((z = zorro_find_device(ZORRO_PROD_HYDRA_SYSTEMS_AMIGANET, z))) {
-	board = z->resource.start;
-	if (!request_mem_region(board, 0x10000, "Hydra"))
-	    continue;
-	if ((err = hydra_init(ZTWO_VADDR(board)))) {
-	    release_mem_region(board, 0x10000);
-	    return err;
-	}
-	err = 0;
-    }
+static struct zorro_device_id hydra_zorro_tbl[] __devinitdata = {
+    { ZORRO_PROD_HYDRA_SYSTEMS_AMIGANET },
+    { 0 }
+};
+
+static struct zorro_driver hydra_driver = {
+    .name	= "hydra",
+    .id_table	= hydra_zorro_tbl,
+    .probe	= hydra_init_one,
+    .remove	= __devexit_p(hydra_remove_one),
+};
 
-    if (err == -ENODEV)
-	printk("No Hydra ethernet card found.\n");
+static int __devinit hydra_init_one(struct zorro_dev *z,
+				    const struct zorro_device_id *ent)
+{
+    int err;
 
-    return err;
+    if (!request_mem_region(z->resource.start, 0x10000, "Hydra"))
+	return -EBUSY;
+    if ((err = hydra_init(z))) {
+	release_mem_region(z->resource.start, 0x10000);
+	return -EBUSY;
+    }
+    return 0;
 }
 
-static int __init hydra_init(unsigned long board)
+static int __devinit hydra_init(struct zorro_dev *z)
 {
     struct net_device *dev;
+    unsigned long board = ZTWO_VADDR(z->resource.start);
     unsigned long ioaddr = board+HYDRA_NIC_BASE;
     const char name[] = "NE2000";
     int start_page, stop_page;
@@ -119,7 +123,7 @@
 	return -EAGAIN;
     }
 
-    printk("%s: hydra at 0x%08lx, address %02x:%02x:%02x:%02x:%02x:%02x (hydra.c " HYDRA_VERSION ")\n", dev->name, ZTWO_PADDR(board),
+    printk("%s: hydra at 0x%08lx, address %02x:%02x:%02x:%02x:%02x:%02x (hydra.c " HYDRA_VERSION ")\n", dev->name, z->resource.start,
 	dev->dev_addr[0], dev->dev_addr[1], dev->dev_addr[2],
 	dev->dev_addr[3], dev->dev_addr[4], dev->dev_addr[5]);
 
@@ -138,18 +142,17 @@
     ei_status.reg_offset = hydra_offsets;
     dev->open = &hydra_open;
     dev->stop = &hydra_close;
-#ifdef MODULE
-    ei_status.priv = (unsigned long)root_hydra_dev;
-    root_hydra_dev = dev;
-#endif
     NS8390_init(dev, 0);
+
     err = register_netdev(dev);
-    if (!err)
-	return 0;
+    if (err) {
+	free_irq(IRQ_AMIGA_PORTS, dev);
+	free_netdev(dev);
+	return err;
+    }
 
-    free_irq(IRQ_AMIGA_PORTS, dev);
-    free_netdev(dev);
-    return err;
+    zorro_set_drvdata(z, dev);
+    return 0;
 }
 
 static int hydra_open(struct net_device *dev)
@@ -220,20 +223,27 @@
     z_memcpy_toio(mem_base+((start_page - NESM_START_PG)<<8), buf, count);
 }
 
-static void __exit hydra_cleanup(void)
+static void __devexit hydra_remove_one(struct zorro_dev *z)
 {
-    struct net_device *dev, *next;
+    struct net_device *dev = zorro_get_drvdata(z);
 
-    while ((dev = root_hydra_dev)) {
-	next = (struct net_device *)(ei_status.priv);
-	unregister_netdev(dev);
-	free_irq(IRQ_AMIGA_PORTS, dev);
-	release_mem_region(ZTWO_PADDR(dev->base_addr)-HYDRA_NIC_BASE, 0x10000);
-	free_netdev(dev);
-	root_hydra_dev = next;
-    }
+    unregister_netdev(dev);
+    free_irq(IRQ_AMIGA_PORTS, dev);
+    release_mem_region(ZTWO_PADDR(dev->base_addr)-HYDRA_NIC_BASE, 0x10000);
+    free_netdev(dev);
+}
+
+static int __init hydra_init_module(void)
+{
+    return zorro_module_init(&hydra_driver);
+}
+
+static void __exit hydra_cleanup_module(void)
+{
+    zorro_unregister_driver(&hydra_driver);
 }
 
-module_init(hydra_probe);
-module_exit(hydra_cleanup);
+module_init(hydra_init_module);
+module_exit(hydra_cleanup_module);
+
 MODULE_LICENSE("GPL");

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds