[PATCH] net #3

[PATCH] net #3

[Andrzej Krzysztofowicz]

The following patch fixes some ISA PnP #ifdefs (enable modular,
disable when non-available) for 3c509 and smc-ultra.

Some other drivers also seem to need fixing (finishing ISA PnP support
inside them).

Andrzej

************************** PATCH 3 *****************************
diff -uNr linux-2.4.5-ac4/drivers/net/3c509.c linux/drivers/net/3c509.c
--- linux-2.4.5-ac4/drivers/net/3c509.c	Wed May 30 01:09:53 2001
+++ linux/drivers/net/3c509.c	Wed May 30 01:10:45 2001
@@ -175,7 +175,7 @@
 };
 #endif /* CONFIG_MCA */
 
-#ifdef CONFIG_ISAPNP
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 static struct isapnp_device_id el3_isapnp_adapters[] __initdata = {
 	{	ISAPNP_ANY_ID, ISAPNP_ANY_ID,
 		ISAPNP_VENDOR('T', 'C', 'M'), ISAPNP_FUNCTION(0x5090),
@@ -201,8 +201,8 @@
 MODULE_DEVICE_TABLE(isapnp, el3_isapnp_adapters);
 
 static u16 el3_isapnp_phys_addr[8][3];
+#endif /* CONFIG_ISAPNP || CONFIG_ISAPNP_MODULE */
 static int nopnp;
-#endif /* CONFIG_ISAPNP */
 
 int __init el3_probe(struct net_device *dev)
 {
@@ -212,9 +212,10 @@
 	u16 phys_addr[3];
 	static int current_tag;
 	int mca_slot = -1;
-#ifdef CONFIG_ISAPNP
+	static int printed_version;
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 	static int pnp_cards;
-#endif /* CONFIG_ISAPNP */
+#endif /* CONFIG_ISAPNP || CONFIG_ISAPNP_MODULE */
 
 	if (dev) SET_MODULE_OWNER(dev);
 
@@ -318,7 +319,7 @@
 	}
 #endif /* CONFIG_MCA */
 
-#ifdef CONFIG_ISAPNP
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 	if (nopnp == 1)
 		goto no_pnp;
 
@@ -354,7 +355,7 @@
 		}
 	}
 no_pnp:
-#endif /* CONFIG_ISAPNP */
+#endif /* CONFIG_ISAPNP || CONFIG_ISAPNP_MODULE */
 
 	/* Select an open I/O location at 0x1*0 to do contention select. */
 	for ( ; id_port < 0x200; id_port += 0x10) {
@@ -400,7 +401,7 @@
 		phys_addr[i] = htons(id_read_eeprom(i));
 	}
 
-#ifdef CONFIG_ISAPNP
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 	if (nopnp == 0) {
 		/* The ISA PnP 3c509 cards respond to the ID sequence.
 		   This check is needed in order not to register them twice. */
@@ -420,7 +421,7 @@
 			}
 		}
 	}
-#endif /* CONFIG_ISAPNP */
+#endif /* CONFIG_ISAPNP || CONFIG_ISAPNP_MODULE */
 
 	{
 		unsigned int iobase = id_read_eeprom(8);
diff -uNr linux-2.4.5-ac4/drivers/net/smc-ultra.c linux/drivers/net/smc-ultra.c
--- linux-2.4.5-ac4/drivers/net/smc-ultra.c	Wed May 30 01:09:53 2001
+++ linux/drivers/net/smc-ultra.c	Wed May 30 01:10:45 2001
@@ -80,7 +80,7 @@
 int ultra_probe(struct net_device *dev);
 static int ultra_probe1(struct net_device *dev, int ioaddr);
 
-#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 static int ultra_probe_isapnp(struct net_device *dev);
 #endif
 
@@ -100,7 +100,7 @@
 							 const unsigned char *buf, const int start_page);
 static int ultra_close_card(struct net_device *dev);
 
-#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 static struct isapnp_device_id ultra_device_ids[] __initdata = {
         {       ISAPNP_VENDOR('S','M','C'), ISAPNP_FUNCTION(0x8416),
                 ISAPNP_VENDOR('S','M','C'), ISAPNP_FUNCTION(0x8416),
@@ -140,7 +140,7 @@
 	else if (base_addr != 0)	/* Don't probe at all. */
 		return -ENXIO;
 
-#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 	/* Look for any installed ISAPnP cards */
 	if (isapnp_present() && (ultra_probe_isapnp(dev) == 0))
 		return 0;
@@ -279,7 +279,7 @@
 	return retval;
 }
 
-#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
 static int __init ultra_probe_isapnp(struct net_device *dev)
 {
         int i;


-- 
=======================================================================
  Andrzej M. Krzysztofowicz               ankry@mif.pg.gda.pl
  phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math.,   Technical University of Gdansk

Re: [PATCH] net #3

[Paul Gortmaker]

Andrzej Krzysztofowicz wrote:
> 
> The following patch fixes some ISA PnP #ifdefs (enable modular,
> disable when non-available) for 3c509 and smc-ultra.

> -#ifdef CONFIG_ISAPNP
> +#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))

Hrrm.  AFAICT the token CONFIG_ISAPNP_MODULE will never be defined.  :-)

Regardless, we can just toss the #ifdefs altogether.  They aren't strictly
required as appropriate stubs exist in linux/isapnp.h and that is what 
they are there for. (I'd recommend this for 2.5, not 2.4 though.)

Granted, the CONFIG_ISAPNP buys you a slight reduction in driver
size (even over the stubs) which somewhat seemed worthwhile in the
past. But most of it is __init anyway, and for 2.5 I'd argue that the
readability and lack of ifdefs outweighs the slight size change.

Paul.

Re: [PATCH] net #3

[David Woodhouse]

ankry@green.mif.pg.gda.pl said:
> -#ifdef CONFIG_ISAPNP 
> +#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE)) 

The result here would be a 3c509 module which differs depending on whether 
the ISAPNP module happened to be compiled at the same time or not. 

The ISAPNP-specific parts of the code aren't large. Please consider
including them unconditionally instead. 

--
dwmw2

Re: [PATCH] net #3

[Andrzej Krzysztofowicz]

"David Woodhouse wrote:"
> 
> ankry@green.mif.pg.gda.pl said:
> > -#ifdef CONFIG_ISAPNP 
> > +#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE)) 
> 
> The result here would be a 3c509 module which differs depending on whether 
> the ISAPNP module happened to be compiled at the same time or not. 

I'm just thinking whether the ISA PnP hardware related modules should depend
on isa-pnp.o at all 
(I mean having different behaviour of a the SAME (compiled) module depending
on whether isa-pnp.o is available or not)

It is just adding some persistent pointers for isa-pnp functions to the
kernel and teaching the modules to use request_module(). Probably also some
hacking to keep away from already used ISA PnP hardware during
initialization...

Also implementing "nopnp" option should be mandatory, IMHO.

> The ISAPNP-specific parts of the code aren't large. Please consider
> including them unconditionally instead. 

I see no objection if __init for modules is implemented...

Andrzej
-- 
=======================================================================
  Andrzej M. Krzysztofowicz               ankry@mif.pg.gda.pl
  phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math.,   Technical University of Gdansk

Re: [PATCH] net #3

[Jeff Garzik]

David Woodhouse wrote:
> 
> ankry@green.mif.pg.gda.pl said:
> > -#ifdef CONFIG_ISAPNP
> > +#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
> 
> The result here would be a 3c509 module which differs depending on whether
> the ISAPNP module happened to be compiled at the same time or not.

In and of itself, that is an acceptable result.  We simply cannot take
into kernel configuration variations between two kernel builds.  If you
build your isapnp module with one configuration, and your 3c509 module
with another config, consider yourself lucky that it works at all.


> The ISAPNP-specific parts of the code aren't large. Please consider
> including them unconditionally instead.

I probably agree with you and Paul, but I'll have to give it a bit more
thought.  Past a certain point, bus support needs to be in its own
driver, similar to drivers/sound/{sb_card,sb_common}.c.  But in this
particular case, that would be overkill.

-- 
Jeff Garzik      | Disbelief, that's why you fail.
Building 1024    |
MandrakeSoft     |

Re: [PATCH] net #3

[Andrzej Krzysztofowicz]

"Paul Gortmaker wrote:"
> Andrzej Krzysztofowicz wrote:
> > 
> > The following patch fixes some ISA PnP #ifdefs (enable modular,
> > disable when non-available) for 3c509 and smc-ultra.
> 
> > -#ifdef CONFIG_ISAPNP
> > +#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
> 
> Hrrm.  AFAICT the token CONFIG_ISAPNP_MODULE will never be defined.  :-)

If IS defined by config subsystem when
  "$CONFIG_ISAPNP" = "m"

Just mark config option and test it.

$ grep ISAPNP include/linux/autoconf.h
#undef  CONFIG_ISAPNP
#define CONFIG_ISAPNP_MODULE 1
#undef  CONFIG_BLK_DEV_ISAPNP

;)

> Regardless, we can just toss the #ifdefs altogether.  They aren't strictly
> required as appropriate stubs exist in linux/isapnp.h and that is what 
> they are there for. (I'd recommend this for 2.5, not 2.4 though.)
> 
> Granted, the CONFIG_ISAPNP buys you a slight reduction in driver
> size (even over the stubs) which somewhat seemed worthwhile in the
> past. But most of it is __init anyway, and for 2.5 I'd argue that the
> readability and lack of ifdefs outweighs the slight size change.

When __init for modules will be implemented ?

Andrzej
-- 
=======================================================================
  Andrzej M. Krzysztofowicz               ankry@mif.pg.gda.pl
  phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math.,   Technical University of Gdansk

Re: [PATCH] net #3

[Andrzej Krzysztofowicz]

> David Woodhouse wrote:
> > 
> > ankry@green.mif.pg.gda.pl said:
> > > -#ifdef CONFIG_ISAPNP
> > > +#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE))
> > 
> > The result here would be a 3c509 module which differs depending on whether
> > the ISAPNP module happened to be compiled at the same time or not.
> 
> In and of itself, that is an acceptable result.  We simply cannot take
> into kernel configuration variations between two kernel builds.  If you
> build your isapnp module with one configuration, and your 3c509 module
> with another config, consider yourself lucky that it works at all.

I think it is better to avoid such a dependency if possible. It will
especially simplify life of people who use precompiled kernels/modules.
And (IMHO) it is possible and quite easy to remove dependency of drivers
like network drivers on isa-pnp. I will try to prepare a test patch for if
when I find some free time...
But it would be probably a 2.5 feature (with possible backport later).

-- 
=======================================================================
  Andrzej M. Krzysztofowicz               ankry@mif.pg.gda.pl
  tel.  (0-58) 347 14 61
Wydz.Fizyki Technicznej i Matematyki Stosowanej Politechniki Gdanskiej

Re: [PATCH] net #3

[Keith Owens]

On Wed, 30 May 2001 12:53:36 +0200 (MET DST), 
Andrzej Krzysztofowicz <ankry@pg.gda.pl> wrote:
>When __init for modules will be implemented ?

When I can persuade myself that discarding code pages but retaining the
associated exception tables and any arch dependent data for the
discarded module pages is safe.  Definitely 2.5 material anyway.