[PATCH] deprecate pci_module_init

[PATCH] deprecate pci_module_init

[Chris Wedgwood]

A separate pci_module_init doesn't really exist anymore so we should
deprecate this.  Whilst we are at it we should insist any (old) users
of this and also users of pci_register_driver check their return
codes.  Whilst doing this we may as well remove some old (unused)
preprocessor tokens whilst we are at it.

Signed-off-by: Chris Wedgwood <cw@f00f.org>
---

FWIW I have some cleanups for drivers that I'll post in the next day
or so...


===== Documentation/pci.txt 1.15 vs edited =====
--- 1.15/Documentation/pci.txt	2004-10-06 09:47:03 -07:00
+++ edited/Documentation/pci.txt	2004-11-02 11:32:43 -08:00
@@ -230,8 +230,6 @@ pci_find_slot()			Find pci_dev correspon
 pci_set_power_state()		Set PCI Power Management state (0=D0 ... 3=D3)
 pci_find_capability()		Find specified capability in device's capability
 				list.
-pci_module_init()		Inline helper function for ensuring correct
-				pci_driver initialization and error handling.
 pci_resource_start()		Returns bus start address for a given PCI region
 pci_resource_end()		Returns bus end address for a given PCI region
 pci_resource_len()		Returns the byte length of a PCI region
===== drivers/pci/pci-driver.c 1.42 vs edited =====
--- 1.42/drivers/pci/pci-driver.c	2004-10-19 16:56:40 -07:00
+++ edited/drivers/pci/pci-driver.c	2004-11-02 11:32:44 -08:00
@@ -400,7 +400,7 @@ pci_populate_driver_dir(struct pci_drive
  * If no error occured, the driver remains registered even if 
  * no device was claimed during registration.
  */
-int pci_register_driver(struct pci_driver *drv)
+int __must_check pci_register_driver(struct pci_driver *drv)
 {
 	int error;
 
===== include/linux/pci.h 1.139 vs edited =====
--- 1.139/include/linux/pci.h	2004-10-05 15:56:26 -07:00
+++ edited/include/linux/pci.h	2004-11-02 11:32:44 -08:00
@@ -41,13 +41,9 @@
 #define PCI_STATUS		0x06	/* 16 bits */
 #define  PCI_STATUS_CAP_LIST	0x10	/* Support Capability List */
 #define  PCI_STATUS_66MHZ	0x20	/* Support 66 Mhz PCI 2.1 bus */
-#define  PCI_STATUS_UDF		0x40	/* Support User Definable Features [obsolete] */
 #define  PCI_STATUS_FAST_BACK	0x80	/* Accept fast-back to back */
 #define  PCI_STATUS_PARITY	0x100	/* Detected parity error */
 #define  PCI_STATUS_DEVSEL_MASK	0x600	/* DEVSEL timing */
-#define  PCI_STATUS_DEVSEL_FAST	0x000	
-#define  PCI_STATUS_DEVSEL_MEDIUM 0x200
-#define  PCI_STATUS_DEVSEL_SLOW 0x400
 #define  PCI_STATUS_SIG_TARGET_ABORT 0x800 /* Set on target abort */
 #define  PCI_STATUS_REC_TARGET_ABORT 0x1000 /* Master ack of " */
 #define  PCI_STATUS_REC_MASTER_ABORT 0x2000 /* Set on master abort */
@@ -681,7 +677,13 @@ struct pci_driver {
  * pci_module_init is obsolete, this stays here till we fix up all usages of it
  * in the tree.
  */
-#define pci_module_init	pci_register_driver
+static inline __deprecated __must_check int pci_module_init(struct pci_driver *drv)
+{
+	int __must_check pci_register_driver(struct pci_driver *drv);
+
+	return pci_register_driver(drv);
+}
+
 
 /* these external functions are only available when PCI support is enabled */
 #ifdef CONFIG_PCI
@@ -816,7 +818,7 @@ int pci_bus_alloc_resource(struct pci_bu
 void pci_enable_bridges(struct pci_bus *bus);
 
 /* New-style probing supporting hot-pluggable devices */
-int pci_register_driver(struct pci_driver *);
+int __must_check pci_register_driver(struct pci_driver *);
 void pci_unregister_driver(struct pci_driver *);
 void pci_remove_behind_bridge(struct pci_dev *);
 struct pci_driver *pci_dev_driver(const struct pci_dev *);
@@ -907,7 +909,7 @@ static inline void pci_disable_device(st
 static inline int pci_set_dma_mask(struct pci_dev *dev, u64 mask) { return -EIO; }
 static inline int pci_dac_set_dma_mask(struct pci_dev *dev, u64 mask) { return -EIO; }
 static inline int pci_assign_resource(struct pci_dev *dev, int i) { return -EBUSY;}
-static inline int pci_register_driver(struct pci_driver *drv) { return 0;}
+static inline int __must_check pci_register_driver(struct pci_driver *drv) { return 0;}
 static inline void pci_unregister_driver(struct pci_driver *drv) { }
 static inline int pci_find_capability (struct pci_dev *dev, int cap) {return 0; }
 static inline int pci_find_ext_capability (struct pci_dev *dev, int cap) {return 0; }

Re: [PATCH] deprecate pci_module_init

[Jeff Garzik]

Chris Wedgwood wrote:
> A separate pci_module_init doesn't really exist anymore so we should
> deprecate this.  Whilst we are at it we should insist any (old) users


Wrong.  There are way too many __correct__ drivers to do this at present.

	Jeff

Re: [PATCH] deprecate pci_module_init

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 12:46:40PM -0500, Jeff Garzik wrote:

> Wrong.  There are way too many __correct__ drivers to do this at
> present.

i could claim the same is true of MODULE_PARM yet we spew
warning-galore there...

Re: [PATCH] deprecate pci_module_init

[Jeff Garzik]

Chris Wedgwood wrote:
> On Wed, Nov 03, 2004 at 12:46:40PM -0500, Jeff Garzik wrote:
> 
> 
>>Wrong.  There are way too many __correct__ drivers to do this at
>>present.
> 
> 
> i could claim the same is true of MODULE_PARM yet we spew
> warning-galore there...

There is a 2.4 version of module_param().

The semantics of pci_module_init() versus pci_register_driver() are 
different across 2.4/2.6.  If you deprecate pci_module_init(), you are 
breaking drivers which right now can be ported to 2.4 with a simple cp(1).

It's just downright silly to deprecate the API that is used most heavily 
in drivers.

	Jeff

Re: [PATCH] deprecate pci_module_init

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 02:13:39PM -0500, Jeff Garzik wrote:

> There is a 2.4 version of module_param().

2.4 what?

> The semantics of pci_module_init() versus pci_register_driver() are
> different across 2.4/2.6.

I actually didn't realize this... (in truth I didn't think about it).
Is this explained anywhere?

> If you deprecate pci_module_init(), you are breaking drivers which
> right now can be ported to 2.4 with a simple cp(1).

> It's just downright silly to deprecate the API that is used most
> heavily in drivers.

Sure, I'll buy that.

Re: [PATCH] deprecate pci_module_init

[Greg KH]

On Wed, Nov 03, 2004 at 02:13:39PM -0500, Jeff Garzik wrote:
> Chris Wedgwood wrote:
> >On Wed, Nov 03, 2004 at 12:46:40PM -0500, Jeff Garzik wrote:
> >
> >
> >>Wrong.  There are way too many __correct__ drivers to do this at
> >>present.
> >
> >
> >i could claim the same is true of MODULE_PARM yet we spew
> >warning-galore there...
> 
> There is a 2.4 version of module_param().
> 
> The semantics of pci_module_init() versus pci_register_driver() are 
> different across 2.4/2.6.  If you deprecate pci_module_init(), you are 
> breaking drivers which right now can be ported to 2.4 with a simple cp(1).

Yes, but any driver assuming that the pci_module_init() functionality
has not changed from 2.4 to 2.6 will be broken.  I've fixed up all of
the drivers that did this in the 2.6 tree.

> It's just downright silly to deprecate the API that is used most heavily 
> in drivers.

Due to the change in the way the function works, I'm slowly changing
drivers over to the new function.  It's just too dangerous over time to
leave it alone.

Chris, I agree with Christoph, convert more drivers over before marking
this function "obsolete".  In fact, just convert everyone and then
delete the #define all together, that's what I have been working toward.

thanks,

greg k-h

Re: [PATCH] deprecate pci_module_init

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 04:21:39PM -0800, Greg KH wrote:

> Due to the change in the way the function works, I'm slowly changing
> drivers over to the new function.  It's just too dangerous over time
> to leave it alone.

this is what i'm not clear about --- how does it work differently?

Re: [PATCH] deprecate pci_module_init

[Greg KH]

On Wed, Nov 03, 2004 at 04:37:34PM -0800, Chris Wedgwood wrote:
> On Wed, Nov 03, 2004 at 04:21:39PM -0800, Greg KH wrote:
> 
> > Due to the change in the way the function works, I'm slowly changing
> > drivers over to the new function.  It's just too dangerous over time
> > to leave it alone.
> 
> this is what i'm not clear about --- how does it work differently?

Read the code :)

In short, pci_module_init() on 2.4 would return the number of pci
devices bound to the device, on 2.6, it just always returns 0 if the
driver was successfully registered, no knowledge of how many devices
bound are ever returned.

Hope this helps,

greg k-h

Re: [PATCH] deprecate pci_module_init

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 04:51:08PM -0800, Greg KH wrote:

> Read the code :)

fair enough

> In short, pci_module_init() on 2.4 would return the number of pci
> devices bound to the device, on 2.6, it just always returns 0 if the
> driver was successfully registered, no knowledge of how many devices
> bound are ever returned.

ok, this makes perfect sense now -- i have patches to an out-of-tree
driver (madwifi) that broke recently because it does

	 if (pci_register_drive(...) <= 0) {
	    /* error */

sort of thing.

thanks!
  --cw

Re: [PATCH] deprecate pci_module_init

[Jeff Garzik]

Greg KH wrote:
> In short, pci_module_init() on 2.4 would return the number of pci
> devices bound to the device, on 2.6, it just always returns 0 if the
> driver was successfully registered, no knowledge of how many devices
> bound are ever returned.


Incorrect.  pci_register_driver() is the inconsistent one, as I've 
explained before (months ago).

pci_module_init() always returns 0 or an errno-based value, in 2.4 or 
2.6.  Thus is it the portable alternative.

Thus, changing drivers -away from- pci_module_init() makes them less 
portable, for zero apparent gain.  It's just a #define symbol at this 
point, leave it be.

The cost of "#define pci_module_init pci_register_driver" is zero, while 
the cost and impact of changing tons of drivers to the non-portable 
variant is non-zero.

	Jeff

Re: [PATCH] deprecate pci_module_init

[Christoph Hellwig]

On Wed, Nov 03, 2004 at 01:10:39AM -0800, Chris Wedgwood wrote:
> A separate pci_module_init doesn't really exist anymore so we should
> deprecate this.  Whilst we are at it we should insist any (old) users
> of this and also users of pci_register_driver check their return
> codes.  Whilst doing this we may as well remove some old (unused)
> preprocessor tokens whilst we are at it.

There's nothing bad about it, it's a useless indirection but doesn't
cause harm.

If you want it to be removed submit patches to the drivers using it
instead of adding such warnings.  Once your down to very few drivers
you can deprecate it.

Re: [PATCH] deprecate pci_module_init

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 08:33:31PM +0000, Christoph Hellwig wrote:

> There's nothing bad about it, it's a useless indirection but doesn't
> cause harm.

part of the patch is the __much_check, i redo that and submit that
with some driver fixes later

> If you want it to be removed submit patches to the drivers using it
> instead of adding such warnings.  Once your down to very few drivers
> you can deprecate it.

i'm happy to do so --- but it's not clear to me

  (1) is s/pci_module_init/pci_register_driver/ suffices or something
      else needs to be done

  (2) how this will affect drivers also used in 2.4.x, as jeff pointed
      out some drivers are identical (or could be) for both kernels.