lkml.org 
[lkml]   [2006]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 3/7] Check root chipset no_msi flag instead of all parent busses flags
    On Sun, Jul 02, 2006 at 08:40:02PM -0400, Brice Goglin wrote:
    > MSI only requires support in the root chipset, which may not even have
    > a subordinate bus.

    Brice,
    The above comment and the main part of this patch don't look right to me.
    Maybe it's the glass of wine I had two hours ago and it's around midnight
    now...

    If the "root chipset" is not a PCI device and the architecture doesn't
    fake it, the root chipset (aka "PCI Host bus controller") is not visible
    to PCI subsystem. Some of the arch code (e.g. drivers/parisc/dino.c)
    uses "bus->self == NULL" to differentiate between PCI-PCI secondary busses
    and PCI bus below a "root chipset". ISTR alpha and sparc doing the same.

    Can you confirm I'm remembering/understanding this bit correctly?

    > pci_msi_supported() now checks the no_msi flag in the root_chipset pci_dev
    > struct instead of the PCI_BUS_FLAGS_NO_MSI flag of all its parent busses.
    > The MSI quirk now sets the no_msi flag accordingly.

    I don't see how this could work for alpha/parisc/sparc (IIRC) or any other
    architecture that doesn't create "fake" PCI Root busses.
    I think your previous patch was correct in this regard.

    > Index: linux-git/drivers/pci/msi.c
    > ===================================================================
    > --- linux-git.orig/drivers/pci/msi.c 2006-07-02 20:28:28.000000000 -0400
    > +++ linux-git/drivers/pci/msi.c 2006-07-02 20:28:58.000000000 -0400
    > @@ -905,19 +905,24 @@
    > * @dev: pointer to the pci_dev data structure of MSI device function
    > *
    > * MSI must be globally enabled and supported by the device and
    > - * its parent busses.
    > + * its root chipset.
    > **/
    > static
    > int pci_msi_supported(struct pci_dev * dev)
    > {
    > - struct pci_bus *bus;
    > + struct pci_dev *pdev;
    >
    > if (!pci_msi_enable || !dev || dev->no_msi)
    > return -1;
    >
    > - for (bus = dev->bus; bus; bus = bus->parent)
    > - if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
    > - return -1;
    > + /* find root complex for our device */
    > + pdev = dev;
    > + while (pdev->bus && pdev->bus->self)
    > + pdev = pdev->bus->self;

    In particular, I'm thinking this loop isn't correct though I'm not
    in a particular, erm, "certain" frame of mind right now.
    Under a parisc root device, pdev->bus->self would be NULL.

    Maybe that's why parisc uses pci_scan_bus_parented()
    instead of pci_scan_bus(). (I've forgot the background
    behind that change).

    thanks,
    grant
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-07-04 09:09    [W:0.026 / U:30.184 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site