[TRIVIAL] Fix initialization sequence in SunGEM driver

[TRIVIAL] Fix initialization sequence in SunGEM driver

[David Gibson]

The patch below alters the SunGEM driver so that the net_device
callback pointers are initialized before calling register_netdev(),
rather than after.

I think that would always have been a bug, but its effects have become
worse in recent kernels:  the fact that get_stats is NULL when
register_netdev() is called means that the netstat_group sysfs
attributes are not registered.  That in turn means that we get an oops
on module remove as it attempts to deregister those attributes
(dir->d_inode is NULL in sysfs_hash_and_remove()).

diff -urN linuxppc-2.5-benh/drivers/net/sungem.c linux-zax/drivers/net/sungem.c
--- linuxppc-2.5-benh/drivers/net/sungem.c	2003-09-29 18:30:43.000000000 +1000
+++ linux-zax/drivers/net/sungem.c	2003-10-09 16:18:19.024186984 +1000
@@ -2757,6 +2757,19 @@
 	if (gem_get_device_address(gp))
 		goto err_out_free_consistent;
 
+	dev->open = gem_open;
+	dev->stop = gem_close;
+	dev->hard_start_xmit = gem_start_xmit;
+	dev->get_stats = gem_get_stats;
+	dev->set_multicast_list = gem_set_multicast;
+	dev->do_ioctl = gem_ioctl;
+	dev->ethtool_ops = &gem_ethtool_ops;
+	dev->tx_timeout = gem_tx_timeout;
+	dev->watchdog_timeo = 5 * HZ;
+	dev->change_mtu = gem_change_mtu;
+	dev->irq = pdev->irq;
+	dev->dma = 0;
+
 	if (register_netdev(dev)) {
 		printk(KERN_ERR PFX "Cannot register net device, "
 		       "aborting.\n");
@@ -2785,19 +2798,6 @@
 
 	pci_set_drvdata(pdev, dev);
 
-	dev->open = gem_open;
-	dev->stop = gem_close;
-	dev->hard_start_xmit = gem_start_xmit;
-	dev->get_stats = gem_get_stats;
-	dev->set_multicast_list = gem_set_multicast;
-	dev->do_ioctl = gem_ioctl;
-	dev->ethtool_ops = &gem_ethtool_ops;
-	dev->tx_timeout = gem_tx_timeout;
-	dev->watchdog_timeo = 5 * HZ;
-	dev->change_mtu = gem_change_mtu;
-	dev->irq = pdev->irq;
-	dev->dma = 0;
-
 	/* GEM can do it all... */
 	dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM;
 	if (pci_using_dac)


-- 
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

Re: [TRIVIAL] Fix initialization sequence in SunGEM driver

[David S. Miller]

Applied, thanks David.

But in the future please send net driver fixes to netdev@oss.sgi.com
with either Jeff Garzik or myself CC:'d?  Thanks.

I also would absolutely not classify your patch as "trivial".
It's not like a missing semi-colon or a comment typo, someone
with real understanding of how this stuff works would need to
review this patch.

Based just upon the kinds of networking patches Linus has forwarded
to me today, I can duly say that far too much stuff is being sent
as "[TRIVIAL]" which is not.

Re: [TRIVIAL] Fix initialization sequence in SunGEM driver

[Rusty Russell]

In message <20031009235653.26e509c1.davem@redhat.com> you write:
> Based just upon the kinds of networking patches Linus has forwarded
> to me today, I can duly say that far too much stuff is being sent
> as "[TRIVIAL]" which is not.

Perhaps.  Certainly I give more slack to a trusted source, and since I
end up just sending it to the maintainer anyway, I don't usually see
too much harm in holding it.  Perhaps I need a non-trivial patch
monkey 8)

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.