Linus,
Here's a patch that fixes a bug that can cause PCI driver list
corruption. If parport_pc's init_module fails after it calls
pci_register_driver, cleanup_module isn't called and so it's still
registered when it gets unloaded.
Tim.
*/
2001-01-13 Tim Waugh <[email protected]>
* parport_pc.c: Fix PCI driver list corruption on
unsuccessful module load (Andrew Morton).
--- linux/drivers/parport/parport_pc.c.init Tue Feb 13 23:31:25 2001
+++ linux/drivers/parport/parport_pc.c Tue Feb 13 23:35:56 2001
@@ -89,6 +89,7 @@
} superios[NR_SUPERIOS] __devinitdata = { {0,},};
static int user_specified __devinitdata = 0;
+static int registered_parport;
/* frob_control, but for ECR */
static void frob_econtrol (struct parport *pb, unsigned char m,
@@ -2605,6 +2606,7 @@
count += parport_pc_find_nonpci_ports (autoirq, autodma);
r = pci_register_driver (&parport_pc_pci_driver);
+ registered_parport = 1;
if (r > 0)
count += r;
@@ -2667,6 +2669,7 @@
/* Work out how many ports we have, then get parport_share to parse
the irq values. */
unsigned int i;
+ int ret;
for (i = 0; i < PARPORT_PC_MAX_PORTS && io[i]; i++);
if (i) {
if (parport_parse_irqs(i, irq, irqval)) return 1;
@@ -2691,7 +2694,11 @@
}
}
- return !parport_pc_init (io, io_hi, irqval, dmaval);
+ ret = !parport_pc_init (io, io_hi, irqval, dmaval);
+ if (ret && registered_parport)
+ pci_unregister_driver (&parport_pc_pci_driver);
+
+ return ret;
}
void cleanup_module(void)
*** linux/drivers/parport/ChangeLog.init Fri Jan 5 10:41:52 2001
--- linux/drivers/parport/ChangeLog Tue Feb 13 23:32:02 2001
***************
*** 0 ****
--- 1,7 ----
+ 2001-02-13 Andrew Morton <[email protected]>
+
+ * parport_pc.c (registered_parport): New static variable.
+ (parport_pc_find_ports): Set it when we register PCI driver.
+ (init_module): Unregister PCI driver if necessary when we
+ fail.
+
On Tue, 13 Feb 2001, Tim Waugh wrote:
> Here's a patch that fixes a bug that can cause PCI driver list
> corruption. If parport_pc's init_module fails after it calls
> pci_register_driver, cleanup_module isn't called and so it's still
> registered when it gets unloaded.
> --- linux/drivers/parport/parport_pc.c.init Tue Feb 13 23:31:25 2001
> +++ linux/drivers/parport/parport_pc.c Tue Feb 13 23:35:56 2001
> @@ -89,6 +89,7 @@
> } superios[NR_SUPERIOS] __devinitdata = { {0,},};
>
> static int user_specified __devinitdata = 0;
> +static int registered_parport;
>
> /* frob_control, but for ECR */
> static void frob_econtrol (struct parport *pb, unsigned char m,
> @@ -2605,6 +2606,7 @@
> count += parport_pc_find_nonpci_ports (autoirq, autodma);
>
> r = pci_register_driver (&parport_pc_pci_driver);
> + registered_parport = 1;
> if (r > 0)
> count += r;
Bad patch. It should be
if (r >= 0) {
registered_parport = 1;
if (r > 0)
count += r;
}
If pci_register_driver returns < 0, the driver is not registered with
the system.
Jeff
On Wed, 14 Feb 2001, Jeff Garzik wrote:
> On Tue, 13 Feb 2001, Tim Waugh wrote:
> > Here's a patch that fixes a bug that can cause PCI driver list
> > corruption. If parport_pc's init_module fails after it calls
> > pci_register_driver, cleanup_module isn't called and so it's still
> > registered when it gets unloaded.
>
> > --- linux/drivers/parport/parport_pc.c.init Tue Feb 13 23:31:25 2001
> > +++ linux/drivers/parport/parport_pc.c Tue Feb 13 23:35:56 2001
> > @@ -89,6 +89,7 @@
> > } superios[NR_SUPERIOS] __devinitdata = { {0,},};
> >
> > static int user_specified __devinitdata = 0;
> > +static int registered_parport;
> >
> > /* frob_control, but for ECR */
> > static void frob_econtrol (struct parport *pb, unsigned char m,
> > @@ -2605,6 +2606,7 @@
> > count += parport_pc_find_nonpci_ports (autoirq, autodma);
> >
> > r = pci_register_driver (&parport_pc_pci_driver);
> > + registered_parport = 1;
> > if (r > 0)
> > count += r;
>
> Bad patch. It should be
>
> if (r >= 0) {
> registered_parport = 1;
> if (r > 0)
> count += r;
> }
The second "if" is redundant here: if r==0, count+=r has no effect. I
don't think gcc would spot that on optimization, would it??
James.
On Wed, Feb 14, 2001 at 02:03:07AM -0600, Jeff Garzik wrote:
> If pci_register_driver returns < 0, the driver is not registered with
> the system.
Thanks. Okay, second try:
2001-01-14 Tim Waugh <[email protected]>
* parport_pc.c: Fix PCI driver list corruption on
unsuccessful module load (Andrew Morton).
--- linux/drivers/parport/parport_pc.c.init Wed Feb 14 10:49:28 2001
+++ linux/drivers/parport/parport_pc.c Wed Feb 14 10:50:31 2001
@@ -89,6 +89,7 @@
} superios[NR_SUPERIOS] __devinitdata = { {0,},};
static int user_specified __devinitdata = 0;
+static int registered_parport;
/* frob_control, but for ECR */
static void frob_econtrol (struct parport *pb, unsigned char m,
@@ -2605,8 +2606,10 @@
count += parport_pc_find_nonpci_ports (autoirq, autodma);
r = pci_register_driver (&parport_pc_pci_driver);
- if (r > 0)
+ if (r >= 0) {
+ registered_parport = 1;
count += r;
+ }
return count;
}
@@ -2667,6 +2670,7 @@
/* Work out how many ports we have, then get parport_share to parse
the irq values. */
unsigned int i;
+ int ret;
for (i = 0; i < PARPORT_PC_MAX_PORTS && io[i]; i++);
if (i) {
if (parport_parse_irqs(i, irq, irqval)) return 1;
@@ -2691,7 +2695,11 @@
}
}
- return !parport_pc_init (io, io_hi, irqval, dmaval);
+ ret = !parport_pc_init (io, io_hi, irqval, dmaval);
+ if (ret && registered_parport)
+ pci_unregister_driver (&parport_pc_pci_driver);
+
+ return ret;
}
void cleanup_module(void)
*** linux/drivers/parport/ChangeLog.init Fri Jan 5 10:41:52 2001
--- linux/drivers/parport/ChangeLog Wed Feb 14 10:50:00 2001
***************
*** 0 ****
--- 1,7 ----
+ 2001-02-14 Andrew Morton <[email protected]>
+
+ * parport_pc.c (registered_parport): New static variable.
+ (parport_pc_find_ports): Set it when we register PCI driver.
+ (init_module): Unregister PCI driver if necessary when we
+ fail.
+
Tim.
*/
Jeff Garzik wrote:
>
> Bad patch. It should be
>
> if (r >= 0) {
> registered_parport = 1;
> if (r > 0)
> count += r;
> }
>
> If pci_register_driver returns < 0, the driver is not registered with
> the system.
eh?
pci_register_driver(struct pci_driver *drv)
{
struct pci_dev *dev;
int count = 0;
list_add_tail(&drv->node, &pci_drivers);
pci_for_each_dev(dev) {
if (!pci_dev_driver(dev))
count += pci_announce_device(drv, dev);
}
return count;
}
Maybe you're thinking of pci_module_init?
Now, if there were some actual COMMENTS (scary, I know) in the pci
code which described the API, stuff like this wouldn't happen.
-
On Wed, 14 Feb 2001, Tim Waugh wrote:
> On Wed, Feb 14, 2001 at 02:03:07AM -0600, Jeff Garzik wrote:
>
> > If pci_register_driver returns < 0, the driver is not registered with
> > the system.
>
> Thanks. Okay, second try:
>
> 2001-01-14 Tim Waugh <[email protected]>
>
> * parport_pc.c: Fix PCI driver list corruption on
> unsuccessful module load (Andrew Morton).
>
> --- linux/drivers/parport/parport_pc.c.init Wed Feb 14 10:49:28 2001
> +++ linux/drivers/parport/parport_pc.c Wed Feb 14 10:50:31 2001
> @@ -89,6 +89,7 @@
> } superios[NR_SUPERIOS] __devinitdata = { {0,},};
>
> static int user_specified __devinitdata = 0;
> +static int registered_parport;
>
> /* frob_control, but for ECR */
> static void frob_econtrol (struct parport *pb, unsigned char m,
> @@ -2605,8 +2606,10 @@
> count += parport_pc_find_nonpci_ports (autoirq, autodma);
>
> r = pci_register_driver (&parport_pc_pci_driver);
> - if (r > 0)
> + if (r >= 0) {
> + registered_parport = 1;
> count += r;
> + }
>
> return count;
> }
Should the call to pci_unregister_driver in cleanup_module be
conditional on registered_parport as well? I didn't check...
Also, is it possible to convert parport_pc to new-style module_init()?
Jeff
On Wed, 14 Feb 2001, Andrew Morton wrote:
> Jeff Garzik wrote:
> >
> > Bad patch. It should be
> >
> > if (r >= 0) {
> > registered_parport = 1;
> > if (r > 0)
> > count += r;
> > }
> >
> > If pci_register_driver returns < 0, the driver is not registered with
> > the system.
>
> eh?
>
> pci_register_driver(struct pci_driver *drv)
> {
> struct pci_dev *dev;
> int count = 0;
>
> list_add_tail(&drv->node, &pci_drivers);
> pci_for_each_dev(dev) {
> if (!pci_dev_driver(dev))
> count += pci_announce_device(drv, dev);
> }
> return count;
> }
>
> Maybe you're thinking of pci_module_init?
Apparently :) Oops, sorry Tim.
Oh well, the new patch is a better one anyway ;) Guards against me
changing pci_register_driver as such. :)
Jeff
On Wed, Feb 14, 2001 at 05:14:19AM -0600, Jeff Garzik wrote:
> Should the call to pci_unregister_driver in cleanup_module be
> conditional on registered_parport as well? I didn't check...
No. (cleanup_module is only called if init_module succeeded.)
> Also, is it possible to convert parport_pc to new-style module_init()?
Certainly, in 2.5, or when it's needed to fix a bug.
Tim.
*/
On Wed, Feb 14, 2001 at 10:21:38PM +1100, Andrew Morton wrote:
> Now, if there were some actual COMMENTS (scary, I know) in the pci
> code which described the API, stuff like this wouldn't happen.
Oh yeah, that reminds me: if someone would like to make sure that the
following changes are accurate, and amend them where they're not, that
would be great.
Tim.
*/
2001-02-04 Jani Monoses <[email protected]>
* drivers/pci/pci.c: Inline documentation.
--- linux/drivers/pci/pci.c.pcidoc Tue Dec 12 13:03:27 2000
+++ linux/drivers/pci/pci.c Sun Feb 4 10:02:08 2001
@@ -40,10 +40,12 @@
/**
* pci_find_slot - locate PCI device from a given PCI slot
* @bus: number of PCI bus on which desired PCI device resides
- * @devfn: number of PCI slot in which desired PCI device resides
+ * @devfn: encodes number of PCI slot in which the desired PCI
+ * device resides and the logical device number within that slot
+ * in case of multi-function devices.
*
- * Given a PCI bus and slot number, the desired PCI device is
- * located in system global list of PCI devices. If the device
+ * Given a PCI bus and slot/function number, the desired PCI device
+ * is located in system global list of PCI devices. If the device
* is found, a pointer to its data structure is returned. If no
* device is found, %NULL is returned.
*/
@@ -59,7 +61,20 @@
return NULL;
}
-
+/**
+ * pci_find_subsys - begin or continue searching for a PCI device by vendor/subvendor/device/subdevice id
+ * @vendor: PCI vendor id to match, or %PCI_ANY_ID to match all vendor ids
+ * @device: PCI device id to match, or %PCI_ANY_ID to match all vendor ids
+ * @ss_vendor: PCI subsystem vendor id to match, or %PCI_ANY_ID to match all vendor ids
+ * @ss_device: PCI subsystem device id to match, or %PCI_ANY_ID to match all vendor ids
+ * @from: Previous PCI device found in search, or %NULL for new search.
+ *
+ * Iterates through the list of known PCI devices. If a PCI device is
+ * found with a matching @vendor, @device, @ss_vendor and @ss_device, a pointer to its
+ * device structure is returned. Otherwise, %NULL is returned.
+ * A new search is initiated by passing %NULL to the @from argument.
+ * Otherwise if @from is not %NULL, searches continue from next device on the global list.
+ */
struct pci_dev *
pci_find_subsys(unsigned int vendor, unsigned int device,
unsigned int ss_vendor, unsigned int ss_device,
@@ -89,9 +104,8 @@
* Iterates through the list of known PCI devices. If a PCI device is
* found with a matching @vendor and @device, a pointer to its device structure is
* returned. Otherwise, %NULL is returned.
- *
* A new search is initiated by passing %NULL to the @from argument.
- * Otherwise if @from is not null, searches continue from that point.
+ * Otherwise if @from is not %NULL, searches continue from next device on the global list.
*/
struct pci_dev *
pci_find_device(unsigned int vendor, unsigned int device, const struct pci_dev *from)
@@ -108,9 +122,8 @@
* Iterates through the list of known PCI devices. If a PCI device is
* found with a matching @class, a pointer to its device structure is
* returned. Otherwise, %NULL is returned.
- *
* A new search is initiated by passing %NULL to the @from argument.
- * Otherwise if @from is not null, searches continue from that point.
+ * Otherwise if @from is not %NULL, searches continue from next device on the global list.
*/
struct pci_dev *
pci_find_class(unsigned int class, const struct pci_dev *from)
@@ -126,7 +139,28 @@
return NULL;
}
-
+/**
+ * pci_find_capability - query for devices' capabilities
+ * @dev: PCI device to query
+ * @cap: capability code
+ *
+ * Tell if a device supports a given PCI capability.
+ * Returns the address of the requested capability structure within the device's PCI
+ * configuration space or 0 in case the device does not support it.
+ * Possible values for @flags:
+ *
+ * %PCI_CAP_ID_PM Power Management
+ *
+ * %PCI_CAP_ID_AGP Accelerated Graphics Port
+ *
+ * %PCI_CAP_ID_VPD Vital Product Data
+ *
+ * %PCI_CAP_ID_SLOTID Slot Identification
+ *
+ * %PCI_CAP_ID_MSI Message Signalled Interrupts
+ *
+ * %PCI_CAP_ID_CHSWP CompactPCI HotSwap
+ */
int
pci_find_capability(struct pci_dev *dev, int cap)
{
@@ -281,6 +315,15 @@
static LIST_HEAD(pci_drivers);
+/**
+ * pci_match_device - Tell if a PCI device structure has a matching PCI device id structure
+ * @ids: array of PCI device id structures to search in
+ * @dev: the PCI device structure to match against
+ *
+ * Used by a driver to check whether a PCI device present in the
+ * system is in its list of supported devices.Returns the matching
+ * pci_device_id structure or %NULL if there is no match.
+ */
const struct pci_device_id *
pci_match_device(const struct pci_device_id *ids, const struct pci_dev *dev)
{
@@ -295,7 +338,7 @@
}
return NULL;
}
-
+
static int
pci_announce_device(struct pci_driver *drv, struct pci_dev *dev)
{
@@ -321,6 +364,14 @@
return ret;
}
+/**
+ * pci_register_driver - register a new pci driver
+ * @drv: the driver structure to register
+ *
+ * Adds the driver structure to the list of registered drivers
+ * Returns the number of pci devices which were claimed by the driver
+ * during registration.
+ */
int
pci_register_driver(struct pci_driver *drv)
{
@@ -335,6 +386,15 @@
return count;
}
+/**
+ * pci_unregister_driver - unregister a pci driver
+ * @drv: the driver structure to unregister
+ *
+ * Deletes the driver structure from the list of registered PCI drivers,
+ * gives it a chance to clean up and marks the devices for which it
+ * was responsible as driverless.
+ */
+
void
pci_unregister_driver(struct pci_driver *drv)
{
@@ -396,6 +456,13 @@
call_usermodehelper (argv [0], argv, envp);
}
+/**
+ * pci_insert_device - insert a hotplug device
+ * @dev: the device to insert
+ * @bus: where to insert it
+ *
+ * Add a new device to the device lists and notify userspace (/sbin/hotplug).
+ */
void
pci_insert_device(struct pci_dev *dev, struct pci_bus *bus)
{
@@ -428,6 +495,13 @@
}
}
+/**
+ * pci_remove_device - remove a hotplug device
+ * @dev: the device to remove
+ *
+ * Delete the device structure from the device lists and
+ * notify userspace (/sbin/hotplug).
+ */
void
pci_remove_device(struct pci_dev *dev)
{
@@ -453,6 +527,13 @@
name: "compat"
};
+/**
+ * pci_dev_driver - get the pci_driver of a device
+ * @dev: the device to query
+ *
+ * Returns the appropriate pci_driver structure or %NULL if there is no
+ * registered driver for the device.
+ */
struct pci_driver *
pci_dev_driver(const struct pci_dev *dev)
{
@@ -504,7 +585,13 @@
PCI_OP(write, word, u16)
PCI_OP(write, dword, u32)
-
+/**
+ * pci_set_master - enables bus-mastering for device dev
+ * @dev: the PCI device to enable
+ *
+ * Enables bus-mastering on the device and calls pcibios_set_master()
+ * to do the needed arch specific settings.
+ */
void
pci_set_master(struct pci_dev *dev)
{
@@ -838,8 +925,15 @@
dev->irq = irq;
}
-/*
- * Fill in class and map information of a device
+/**
+ * pci_setup_device - fill in class and map information of a device
+ * @dev: the device structure to fill
+ *
+ * Initialize the device structure with information about the device's
+ * vendor,class,memory and IO-space addresses,IRQ lines etc.
+ * Called at initialisation of the PCI subsystem and by CardBus services.
+ * Returns 0 on success and -1 if unknown type of device (not normal, bridge
+ * or CardBus).
*/
int pci_setup_device(struct pci_dev * dev)
{
Philipp Rumpf wrote:
> Jeff Garzik wrote:
> > Looks ok, but I wonder if we should include this list in the docs.
> > These is stuff defined by the PCI spec, and this list could potentially
> > get longer... (opinions either way wanted...)
Having people look things up in the spec isn't very user friendly.
Finding a copy of the PCI 2.1 or 2.2 spec I could pass on to others
(outside of HP) was a problem last year. The best I could do then
(legally) was point them to "PCI Systems Architecture" published
by MindShare.
> I'm not sure whether the
> plan is to have drivers handle MSIs or do it in the generic PCI code.
> Grant ?
Generic PCI code can d very little by itself with MSI since not all
platforms provide support for it - even within the same arch.
Support for MSI is very platform specific.
Eg for parisc: I expect everything but V-class to support MSI.
There are some subtle differences between transaction based
interrupts used by parisc CPU and MSI. But I don't recall what
they are at the moment - I'd have to look it up again.
I thought any x86 with SAPIC (not sure about APIC) could support
MSI as well. But I don't know the x86 arch nearly as well.
It's also possible for the driver to just ignore MSI and not use it.
ie use regular PCI IRQ lines for interrupts.
grant
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253
On Wed, 14 Feb 2001, Grant Grundler wrote:
> Philipp Rumpf wrote:
> > Jeff Garzik wrote:
> > > Looks ok, but I wonder if we should include this list in the docs.
> > > These is stuff defined by the PCI spec, and this list could potentially
> > > get longer... (opinions either way wanted...)
>
> Having people look things up in the spec isn't very user friendly.
Having the constants in some well-known header file should be sufficient,
shouldn't it ?
> > I'm not sure whether the
> > plan is to have drivers handle MSIs or do it in the generic PCI code.
> > Grant ?
>
> Generic PCI code can d very little by itself with MSI since not all
> platforms provide support for it - even within the same arch.
It depends on the platform and maybe the exact PCI slot used, but I don't
think it depends on the driver (unless MSI support is broken in which case
you would want to fix it up in the driver). At least I can't find
anything in the PCI 2.2 spec that would suggest we need to consult the
driver before enabling MSIs with one message only.
> It's also possible for the driver to just ignore MSI and not use it.
> ie use regular PCI IRQ lines for interrupts.
.. at least until someone comes out with a PCI board that supports MSI
interrupts only.
Philipp Rumpf wrote:
> On Wed, 14 Feb 2001, Grant Grundler wrote:
> > Having people look things up in the spec isn't very user friendly.
>
> Having the constants in some well-known header file should be sufficient,
> shouldn't it ?
I would hope anyone bothering to include the constants in a document would
spend a few minutes explaining them as well. Perhaps a bad assumption
on my part...
> It depends on the platform and maybe the exact PCI slot used, but I don't
> think it depends on the driver (unless MSI support is broken in which case
> you would want to fix it up in the driver).
correct.
> At least I can't find
> anything in the PCI 2.2 spec that would suggest we need to consult the
> driver before enabling MSIs with one message only.
I don't know how BIOS's treat this (if at all). Need to know this first.
If they manage this resource and pre-assign everything, ok.
That's how it goes.
But if generic PCI manages this, I prefer to avoid allocating resources
that may not get used. The host platform may have a limited pool of
usable MSI data values (think parisc EIRR bits) and some cards may want
to use more than one MSI.
grant
ps. seems this thread has gotten a bit far off from the original subject. :^/
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253
On Wed, 14 Feb 2001, Grant Grundler wrote:
> Philipp Rumpf wrote:
> > Jeff Garzik wrote:
> > > Looks ok, but I wonder if we should include this list in the docs.
> > > These is stuff defined by the PCI spec, and this list could potentially
> > > get longer... (opinions either way wanted...)
>
> Having people look things up in the spec isn't very user friendly.
> Finding a copy of the PCI 2.1 or 2.2 spec I could pass on to others
> (outside of HP) was a problem last year. The best I could do then
> (legally) was point them to "PCI Systems Architecture" published
> by MindShare.
_PCI Systems Architecture_ is an awesome book, definitely.
AFAIK there are two avenues to go, when getting the PCI specs.
Become a PCI SIG member (much $$$), or buy a CD-ROM.
For US$50, a non-member can purchase a CD-ROM from the PCI SIG
which includes the latest versions of all the PCI specifications,
in PDF format, as well as a hardcopy of the PCI 2.2 spec itself.
Great deal, I recommend it for anybody intersted in hacking PCI.
Jeff
On Wed, 14 Feb 2001, Tim Waugh wrote:
> }
>
> -
> +/**
> + * pci_find_capability - query for devices' capabilities
> + * @dev: PCI device to query
> + * @cap: capability code
> + *
> + * Tell if a device supports a given PCI capability.
> + * Returns the address of the requested capability structure within the device's PCI
> + * configuration space or 0 in case the device does not support it.
> + * Possible values for @flags:
> + *
> + * %PCI_CAP_ID_PM Power Management
> + *
> + * %PCI_CAP_ID_AGP Accelerated Graphics Port
> + *
> + * %PCI_CAP_ID_VPD Vital Product Data
> + *
> + * %PCI_CAP_ID_SLOTID Slot Identification
> + *
> + * %PCI_CAP_ID_MSI Message Signalled Interrupts
> + *
> + * %PCI_CAP_ID_CHSWP CompactPCI HotSwap
> + */
Looks ok, but I wonder if we should include this list in the docs.
These is stuff defined by the PCI spec, and this list could potentially
get longer... (opinions either way wanted...)
> +/**
> + * pci_match_device - Tell if a PCI device structure has a matching PCI device id structure
> + * @ids: array of PCI device id structures to search in
> + * @dev: the PCI device structure to match against
> + *
> + * Used by a driver to check whether a PCI device present in the
> + * system is in its list of supported devices.Returns the matching
> + * pci_device_id structure or %NULL if there is no match.
> + */
Maybe note that the return value comes from @ids (in case that isn't
abundantly clear to everybody)?
> }
>
> +/**
> + * pci_register_driver - register a new pci driver
> + * @drv: the driver structure to register
> + *
> + * Adds the driver structure to the list of registered drivers
> + * Returns the number of pci devices which were claimed by the driver
> + * during registration.
> + */
Definitely mention that the driver remains registered even if the return
value is zero. That trips a lot of people up (as we saw.. :))
> +/**
> + * pci_unregister_driver - unregister a pci driver
> + * @drv: the driver structure to unregister
> + *
> + * Deletes the driver structure from the list of registered PCI drivers,
> + * gives it a chance to clean up and marks the devices for which it
> + * was responsible as driverless.
> + */
Clarify "chance to clean up" as calls each driver's remove() method...
> +/**
> + * pci_dev_driver - get the pci_driver of a device
> + * @dev: the device to query
> + *
> + * Returns the appropriate pci_driver structure or %NULL if there is no
> + * registered driver for the device.
> + */
Mention the case where pci_compat_driver is returned...
Jeff
On Wed, 14 Feb 2001, Tim Waugh wrote:
> + * pci_find_subsys - begin or continue searching for a PCI device by vendor/subvendor/device/subdevice id
> + * @vendor: PCI vendor id to match, or %PCI_ANY_ID to match all vendor ids
> + * @device: PCI device id to match, or %PCI_ANY_ID to match all vendor ids
"match all device ids", surely ?
> + * @ss_device: PCI subsystem device id to match, or %PCI_ANY_ID to match all vendor ids
ditto.
(the pci_find_device documentation has "all vendor ids" as well)
On Wed, 14 Feb 2001, Jeff Garzik wrote:
> On Wed, 14 Feb 2001, Tim Waugh wrote:
> > +/**
> > + * pci_find_capability - query for devices' capabilities
> > + * @dev: PCI device to query
> > + * @cap: capability code
> > + *
> > + * Tell if a device supports a given PCI capability.
> > + * Returns the address of the requested capability structure within the device's PCI
> > + * configuration space or 0 in case the device does not support it.
> > + * Possible values for @flags:
^^^^^^
@cap would make more sense, no ?
> > + * %PCI_CAP_ID_AGP Accelerated Graphics Port
> > + *
> > + * %PCI_CAP_ID_VPD Vital Product Data
> > + *
> > + * %PCI_CAP_ID_SLOTID Slot Identification
> > + *
> > + * %PCI_CAP_ID_MSI Message Signalled Interrupts
> > + *
> > + * %PCI_CAP_ID_CHSWP CompactPCI HotSwap
> > + */
>
> Looks ok, but I wonder if we should include this list in the docs.
> These is stuff defined by the PCI spec, and this list could potentially
> get longer... (opinions either way wanted...)
I would vote for including those capabilities which are most likely to be
used by drivers (PCI_CAP_ID_PM, and maybe _VPD). I'm not sure whether the
plan is to have drivers handle MSIs or do it in the generic PCI code.
Grant ?