lkml.org 
[lkml]   [2011]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC PATCH 2/3] PCI: Pass available resources into pci_create_bus()
    On Wed, Aug 24, 2011 at 12:24 AM, Deng-Cheng Zhu
    <dengcheng.zhu@gmail.com> wrote:
    > Currently, after pci_create_bus(), resources available on the bus could be
    > handled by pci_scan_child_bus(). The problem is that, in
    > pci_scan_child_bus(), before calling arch-dependent pcibios_fixup_bus(),
    > PCI quirks will probably conflict (while doing pci_claim_resource()) with
    > resources like system controller's I/O resource that have not yet been
    > added to the bus. So, add those resources right before returning the newly
    > created bus in pci_create_bus().

    I like this approach a lot. Thanks for working it up. It's a nice
    small change with very little impact to other architectures, and you
    have a nice clear changelog. You might mention something about the
    fact that by default, the bus starts out with all of ioport_resource
    and iomem_resource -- that will mean something to people who know how
    host bridges work.

    > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
    > index 8473727..7735fe7 100644
    > --- a/drivers/pci/probe.c
    > +++ b/drivers/pci/probe.c
    > @@ -1516,7 +1516,8 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
    >  }
    >
    >  struct pci_bus * pci_create_bus(struct device *parent,
    > -               int bus, struct pci_ops *ops, void *sysdata)
    > +               int bus, struct pci_ops *ops, void *sysdata,
    > +               struct pci_bus_resource *bus_res)
    >  {
    >        int error;
    >        struct pci_bus *b, *b2;
    > @@ -1570,8 +1571,14 @@ struct pci_bus * pci_create_bus(struct device *parent,
    >        pci_create_legacy_files(b);
    >
    >        b->number = b->secondary = bus;
    > -       b->resource[0] = &ioport_resource;
    > -       b->resource[1] = &iomem_resource;
    > +
    > +       /* Add initial resources to the bus */
    > +       if (bus_res != NULL) {
    > +               list_add_tail(&b->resources, &bus_res->list);
    > +       } else {
    > +               pci_bus_add_resource(b, &ioport_resource, 0);
    > +               pci_bus_add_resource(b, &iomem_resource, 0);
    > +       }

    Using pci_bus_add_resource() here *seems* like it should be the right
    thing, but I don't think it will work correctly.

    The problem is that struct pci_bus has both a table of resources
    (bus->resource[]) *and* a list (bus->resources).
    pci_bus_add_resource() always puts the new resource on the list, but
    various arch code still references the table directly, e.g., sparc has
    "pbus->resource[0] = &pbm->io_space" in pcibios_fixup_bus().

    As written, I think this patch will break sparc because the host
    bridge will end up with both pbm->io_space (in the table) and
    ioport_resource (in the list).

    I think something like this would work, though:

    if (bus_res)
    list_add_tail(&b->resources, &bus_res->list);
    else {
    b->resource[0] = &ioport_resource;
    b->resource[1] = &iomem_resource;
    }

    Bjorn
    --
    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: 2011-08-24 16:39    [W:0.029 / U:58.252 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site