2.4.2-pre3: parport_pc init_module bug

2.4.2-pre3: parport_pc init_module bug

Post by Tim Waug » Thu, 15 Feb 2001 08:43:49



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.
*/


        * 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

 } superios[NR_SUPERIOS] __devinitdata = { {0,},};

 static int user_specified __devinitdata = 0;
+static int registered_parport;

 /* frob_control, but for ECR */

        count += parport_pc_find_nonpci_ports (autoirq, autodma);

        r = pci_register_driver (&parport_pc_pci_driver);
+       registered_parport = 1;
        if (r > 0)
                count += r;

        /* 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) {

                        }
        }

-       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 ----

+
+       * 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.
+
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Jeff Garzi » Thu, 15 Feb 2001 17:03:07



> 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

>  } 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,

>    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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by James Sutherlan » Thu, 15 Feb 2001 17:53:06




> > 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

> >  } 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,

> >       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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Tim Waug » Thu, 15 Feb 2001 19:53:32



> If pci_register_driver returns < 0, the driver is not registered with
> the system.

Thanks.  Okay, second try:


        * 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

 } superios[NR_SUPERIOS] __devinitdata = { {0,},};

 static int user_specified __devinitdata = 0;
+static int registered_parport;

 /* frob_control, but for ECR */

        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;

        /* 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) {

                        }
        }

-       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 ----

+
+       * 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.
*/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Andrew Morto » Thu, 15 Feb 2001 20:21:38



> 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;

Quote:}

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.

-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Jeff Garzi » Thu, 15 Feb 2001 20:14:19




> > If pci_register_driver returns < 0, the driver is not registered with
> > the system.

> Thanks.  Okay, second try:


>    * 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

>  } 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,

>    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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Tim Waug » Thu, 15 Feb 2001 20:17:00


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  <j...@virtualro.ic.ro>

        * 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)
 {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Jeff Garzi » Thu, 15 Feb 2001 20:17:19




> > 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Tim Waug » Thu, 15 Feb 2001 20:18:06



> 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.)

Quote:> 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.
*/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Jeff Garzi » Thu, 15 Feb 2001 20:24:12



>  }

> -
> +/**
> + * pci_find_capability - query for devices' capabilities


> + *
> + * 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.

> + *
> + *  %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


> + *
> + * 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.
> + */


abundantly clear to everybody)?

>  }

> +/**
> + * pci_register_driver - register a new pci driver

> + *
> + * 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

> + *
> + * 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

> + *
> + * 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Philipp Rump » Thu, 15 Feb 2001 20:31:47



> + * pci_find_subsys - begin or continue searching for a PCI device by vendor/subvendor/device/subdevice id



"match all device ids", surely ?

ditto.

(the pci_find_device documentation has "all vendor ids" as well)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Philipp Rump » Thu, 15 Feb 2001 20:40:03




> > +/**
> > + * pci_find_capability - query for devices' capabilities


> > + *
> > + * 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.



Quote:> > + *  %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 ?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Grant Grundle » Fri, 16 Feb 2001 02:25:20




> > 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.

Quote:> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Philipp Rump » Fri, 16 Feb 2001 06:12:22





> > > 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 ?

Quote:> > 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.

Quote:> 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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

2.4.2-pre3: parport_pc init_module bug

Post by Grant Grundle » Fri, 16 Feb 2001 11:26:55




> > 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...

Quote:> 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.

Quote:> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/