[PATCH] switch sdla to initcalls

[PATCH] switch sdla to initcalls

[Christoph Hellwig]

Wow, on driver that had it's dirt fingers in both Space.c and setup.c :)


--- 1.20/drivers/net/Space.c	Wed May 21 03:56:26 2003
+++ edited/drivers/net/Space.c	Tue May 20 11:50:12 2003
@@ -453,17 +453,6 @@
 #define NEXT_DEV	(&tap0_dev)
 #endif
 
-#ifdef CONFIG_SDLA
-extern int sdla_init(struct net_device *);
-static struct net_device sdla0_dev = {
-	.name		=  "sdla0",
-	.next		=  NEXT_DEV,
-	.init		=  sdla_init,
-};
-#undef NEXT_DEV
-#define NEXT_DEV	(&sdla0_dev)
-#endif
-
 #if defined(CONFIG_LTPC)
 extern int ltpc_probe(struct net_device *);
 static struct net_device dev_ltpc = {
--- 1.11/drivers/net/setup.c	Tue May 20 08:57:38 2003
+++ edited/drivers/net/setup.c	Tue May 20 11:56:00 2003
@@ -14,8 +14,6 @@
 extern int arcnet_init(void); 
 extern int scc_enet_init(void); 
 extern int fec_enet_init(void); 
-extern int sdla_setup(void); 
-extern int sdla_c_setup(void); 
 extern int lmc_setup(void);
 
 extern int madgemc_probe(void);
@@ -39,9 +37,6 @@
 #if defined(CONFIG_DMASCC)
 	{dmascc_init, 0},
 #endif	
-#if defined(CONFIG_SDLA)
-	{sdla_c_setup, 0},
-#endif
 #if defined(CONFIG_ARCNET)
 	{arcnet_init, 0},
 #endif
--- 1.10/drivers/net/wan/sdla.c	Sun May 18 17:18:07 2003
+++ edited/drivers/net/wan/sdla.c	Tue May 20 11:52:15 2003
@@ -1655,32 +1655,25 @@
 	return(0);
 }
 
-int __init sdla_c_setup(void)
-{
-	printk("%s.\n", version);
-	register_frad(devname);
-	return 0;
-}
-
-#ifdef MODULE
 static struct net_device sdla0 = {
 	.name = "sdla0",
 	.init = sdla_init
 };
 
-MODULE_LICENSE("GPL");
 
-int init_module(void)
+static int __init init_sdla(void)
 {
 	int result;
 
-	sdla_c_setup();
+	printk("%s.\n", version);
+	register_frad(devname);
+
 	if ((result = register_netdev(&sdla0)) != 0)
 		return result;
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit exit_sdla(void)
 {
 	unregister_netdev(&sdla0);
 	if (sdla0.priv)
@@ -1688,4 +1681,8 @@
 	if (sdla0.irq)
 		free_irq(sdla0.irq, &sdla0);
 }
-#endif /* MODULE */
+
+MODULE_LICENSE("GPL");
+
+module_init(init_sdla);
+module_exit(exit_sdla);

Re: [PATCH] switch sdla to initcalls

[Andi Kleen]

On Wed, May 21, 2003 at 10:11:26AM +0200, Christoph Hellwig wrote:
> Wow, on driver that had it's dirt fingers in both Space.c and setup.c :)

Are you sure this won't change probe order? The normal reason
to keep calls in Space.c is that they're ISA or other non discoverable 
devices and require a specific ISA probe order, otherwise bad things
happen at boot.

The card appears to be non PCI.

-Andi

Re: [PATCH] switch sdla to initcalls

[Christoph Hellwig]

On Wed, May 21, 2003 at 10:19:39AM +0200, Andi Kleen wrote:
> On Wed, May 21, 2003 at 10:11:26AM +0200, Christoph Hellwig wrote:
> > Wow, on driver that had it's dirt fingers in both Space.c and setup.c :)
> 
> Are you sure this won't change probe order? The normal reason
> to keep calls in Space.c is that they're ISA or other non discoverable 
> devices and require a specific ISA probe order, otherwise bad things
> happen at boot.
> 
> The card appears to be non PCI.

Yes, it's a ISA device.  But it has the sdla%d namespace for it's own
and support only a single card anyway.  There's not much chance of
changing any probe order here.

Re: [PATCH] switch sdla to initcalls

[Andi Kleen]

On Wed, May 21, 2003 at 10:25:12AM +0200, Christoph Hellwig wrote:
> On Wed, May 21, 2003 at 10:19:39AM +0200, Andi Kleen wrote:
> > On Wed, May 21, 2003 at 10:11:26AM +0200, Christoph Hellwig wrote:
> > > Wow, on driver that had it's dirt fingers in both Space.c and setup.c :)
> > 
> > Are you sure this won't change probe order? The normal reason
> > to keep calls in Space.c is that they're ISA or other non discoverable 
> > devices and require a specific ISA probe order, otherwise bad things
> > happen at boot.
> > 
> > The card appears to be non PCI.
> 
> Yes, it's a ISA device.  But it has the sdla%d namespace for it's own
> and support only a single card anyway.  There's not much chance of
> changing any probe order here.

ISA probe order has nothing to do with namespaces or single cards. It 
works by reading values from specific registers and completely different
devices can conflict.

-Andi
> 

Re: [PATCH] switch sdla to initcalls

[David S. Miller]

   From: Andi Kleen <ak@suse.de>
   Date: Wed, 21 May 2003 10:26:30 +0200
   
   ISA probe order has nothing to do with namespaces or single
   cards. It works by reading values from specific registers and
   completely different devices can conflict.

Correct.

However, this sdla thing seems to want to be probed after
ethif_probe() runs.  And moving this to initcalls preserves
that relative relationship.

I have to admit that I must feign ignorance here.  And that someone
who really understands the EISA/ISA probe ordering requirements
should document them as best as possible in Space.c

Re: [PATCH] switch sdla to initcalls

[Andi Kleen]

On Wed, 21 May 2003 01:34:53 -0700 (PDT)
"David S. Miller" <davem@redhat.com> wrote:

> I have to admit that I must feign ignorance here.  And that someone
> who really understands the EISA/ISA probe ordering requirements
> should document them as best as possible in Space.c

I doubt anybody understands it. It is just lots of hard earned ad-hoc 
experience distilled in these files.

It's basically untestable/unknown because you'll likely have a hard time
to find such a card anymore. And even if you had you would need the other
ISA cards that could possible conflicts to test all combinations - impossible
task. For such unsolvable problems it is best to leave it untouched because it
cannot be tested. Either that or remove the driver completely.

-Andi

Re: [PATCH] switch sdla to initcalls

[David S. Miller]

   From: Christoph Hellwig <hch@lst.de>
   Date: Wed, 21 May 2003 10:11:26 +0200

   Wow, on driver that had it's dirt fingers in both Space.c and
   setup.c :)

Since I think Andi's arguments are totally valid I'm not
going to apply this, sorry.

Re: [PATCH] switch sdla to initcalls

[Paul Gortmaker]

Andi Kleen wrote:
> 
> On Wed, 21 May 2003 01:34:53 -0700 (PDT)
> "David S. Miller" <davem@redhat.com> wrote:
> 
> > I have to admit that I must feign ignorance here.  And that someone
> > who really understands the EISA/ISA probe ordering requirements
> > should document them as best as possible in Space.c
> 
> I doubt anybody understands it. It is just lots of hard earned ad-hoc
> experience distilled in these files.

This used to be important many years ago when distribution kernels came 
with a bunch of compiled in ISA drivers.  I don't think it is anything
to get too worried about anymore.  Now a distribution is going to have
the ISA drivers as modules, and custom kernels with compiled in drivers
(probing via Space.c) are going to have a very limited set of drivers.

But regardless, the probe ordering was generally "newer before older", 
"more common before less common", and "invasive write before read type 
probes at the bottom".  This was just common-sense ordering, there
weren't that many cases where ordering was critical.  The only one that 
comes to mind now was smc-ultra.c vs wd.c (required "newer before older").
Things like ISA SCSI drivers probing into ne2000 cards was more of a
problem than net drivers screwing each other up back then.

> It's basically untestable/unknown because you'll likely have a hard time
> to find such a card anymore. And even if you had you would need the other
> ISA cards that could possible conflicts to test all combinations - impossible

Again, I wouldn't give too much weight to this issue -- it is safe to say
that all combinations of all cards at all I/O locations with all drivers
was never tested, and was never guaranteed to work either.  In fact if 
you look back, the "reserve=0xNNN" was introduced specifically to handle
corner cases where you had some other probe stomping on an ISA ethercard.
Now this is a totally unused and mostly forgot about boot prompt.

> task. For such unsolvable problems it is best to leave it untouched because it
> cannot be tested. Either that or remove the driver completely.

As an alternative to removal, you could always make the driver in
question as module only, and then the burden of probing order is on 
the installer or end user, as it is with all other modules currently.

Paul.

> 
> -Andi