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 <[email protected]>
---
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; }
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
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...
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
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.
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.
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.
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
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?
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
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
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