lkml.org 
[lkml]   [2013]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v9 11/11] PCI: Put pci dev to device tree as early as possible
    Date
    On Thursday, January 17, 2013 11:53:22 PM Yinghai Lu wrote:
    > We want to put created pci device in the device tree as soon as possible.
    > - just after we find it and create pci_dev struct for it.
    > so for_pci_dev iteration will not miss them.
    >
    > But at that time, we can not load driver for them yet. Need to be after
    > pci_assign_unsigned_resources() etc to make sure all pci devices get
    > resource allocated at first.
    >
    > Move out device registering out of pci_bus_add_devices, and
    > new pci_bus_add_devices() will do the device_attach work to load pci drivers
    >
    > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    > ---
    > drivers/pci/bus.c | 47 +++--------------------------------------------
    > drivers/pci/iov.c | 7 -------
    > drivers/pci/probe.c | 34 +++++++++++++++++++++++++++-------
    > 3 files changed, 30 insertions(+), 58 deletions(-)
    >
    > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
    > index 18c1c6d..0a55845 100644
    > --- a/drivers/pci/bus.c
    > +++ b/drivers/pci/bus.c
    > @@ -178,22 +178,9 @@ static void pci_bus_attach_device(struct pci_dev *dev)
    > */
    > int pci_bus_add_device(struct pci_dev *dev)
    > {
    > - int retval;
    > -
    > - pci_fixup_device(pci_fixup_final, dev);
    > -
    > - retval = pcibios_add_device(dev);
    > - if (retval)
    > - return retval;
    > -
    > - retval = device_add(&dev->dev);
    > - if (retval)
    > - return retval;
    > -
    > pci_bus_attach_device(dev);
    > dev->is_added = 1;
    > - pci_proc_attach_device(dev);
    > - pci_create_sysfs_dev_files(dev);
    > +
    > return 0;
    > }
    >
    > @@ -205,21 +192,9 @@ int pci_bus_add_device(struct pci_dev *dev)
    > */
    > int pci_bus_add_child(struct pci_bus *bus)
    > {
    > - int retval;
    > -
    > - if (bus->bridge)
    > - bus->dev.parent = bus->bridge;
    > -
    > - retval = device_register(&bus->dev);
    > - if (retval)
    > - return retval;
    > -
    > bus->is_added = 1;
    >
    > - /* Create legacy_io and legacy_mem files for this bus */
    > - pci_create_legacy_files(bus);
    > -
    > - return retval;
    > + return 0;
    > }

    Well, what sense does this make to keep that function as is after removing
    almost all of the code from it?

    >
    > /**
    > @@ -245,36 +220,20 @@ void pci_bus_add_devices(const struct pci_bus *bus)
    > if (dev->is_added)
    > continue;
    > retval = pci_bus_add_device(dev);
    > - if (retval)
    > - dev_err(&dev->dev, "Error adding device, continuing\n");
    > }
    >
    > list_for_each_entry(dev, &bus->devices, bus_list) {
    > BUG_ON(!dev->is_added);
    >
    > child = dev->subordinate;
    > - /*
    > - * If there is an unattached subordinate bus, attach
    > - * it and then scan for unattached PCI devices.
    > - */
    > +
    > if (!child)
    > continue;
    > - if (list_empty(&child->node)) {
    > - down_write(&pci_bus_sem);
    > - list_add_tail(&child->node, &dev->bus->children);
    > - up_write(&pci_bus_sem);
    > - }

    This doesn't seem to have a replacement. Why isn't it necessary any more?

    > pci_bus_add_devices(child);
    >
    > - /*
    > - * register the bus with sysfs as the parent is now
    > - * properly registered.
    > - */
    > if (child->is_added)
    > continue;
    > retval = pci_bus_add_child(child);
    > - if (retval)
    > - dev_err(&dev->dev, "Error adding bus, continuing\n");
    > }
    > }
    >
    > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
    > index bafd2bb..dbad72e 100644
    > --- a/drivers/pci/iov.c
    > +++ b/drivers/pci/iov.c
    > @@ -48,12 +48,7 @@ static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
    > return NULL;
    >
    > pci_bus_insert_busn_res(child, busnr, busnr);
    > - child->dev.parent = bus->bridge;
    > rc = pci_bus_add_child(child);
    > - if (rc) {
    > - pci_remove_bus(child);
    > - return NULL;
    > - }
    >
    > return child;
    > }
    > @@ -123,8 +118,6 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
    > virtfn->is_virtfn = 1;
    >
    > rc = pci_bus_add_device(virtfn);
    > - if (rc)
    > - goto failed1;
    > sprintf(buf, "virtfn%u", id);
    > rc = sysfs_create_link(&dev->dev.kobj, &virtfn->dev.kobj, buf);
    > if (rc)
    > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
    > index 9588207..6392dac 100644
    > --- a/drivers/pci/probe.c
    > +++ b/drivers/pci/probe.c
    > @@ -623,6 +623,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
    > {
    > struct pci_bus *child;
    > int i;
    > + int ret;
    >
    > /*
    > * Allocate a new bus, and inherit stuff from the parent..
    > @@ -637,8 +638,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
    > child->bus_flags = parent->bus_flags;
    >
    > /* initialize some portions of the bus device, but don't register it
    > - * now as the parent is not properly set up yet. This device will get
    > - * registered later in pci_bus_add_devices()
    > + * now as the parent is not properly set up yet.
    > */
    > child->dev.class = &pcibus_class;
    > dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
    > @@ -651,11 +651,14 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
    > child->primary = parent->busn_res.start;
    > child->busn_res.end = 0xff;
    >
    > - if (!bridge)
    > - return child;
    > + if (!bridge) {
    > + child->dev.parent = parent->bridge;
    > + goto add_dev;
    > + }
    >
    > child->self = bridge;
    > child->bridge = get_device(&bridge->dev);
    > + child->dev.parent = child->bridge;
    > pci_set_bus_of_node(child);
    > pci_set_bus_speed(child);
    >
    > @@ -666,6 +669,13 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
    > }
    > bridge->subordinate = child;
    >
    > +add_dev:
    > + ret = device_register(&child->dev);
    > + WARN_ON(ret < 0);
    > +
    > + /* Create legacy_io and legacy_mem files for this bus */
    > + pci_create_legacy_files(child);
    > +
    > return child;
    > }
    >
    > @@ -1296,6 +1306,8 @@ static void pci_init_capabilities(struct pci_dev *dev)
    >
    > void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
    > {
    > + int ret;
    > +
    > device_initialize(&dev->dev);
    > dev->dev.release = pci_release_dev;
    >
    > @@ -1326,6 +1338,16 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
    > down_write(&pci_bus_sem);
    > list_add_tail(&dev->bus_list, &bus->devices);
    > up_write(&pci_bus_sem);
    > +
    > + pci_fixup_device(pci_fixup_final, dev);
    > + ret = pcibios_add_device(dev);
    > + WARN_ON(ret < 0);
    > + /* notifier could use pci capabilities */
    > + ret = device_add(&dev->dev);
    > + WARN_ON(ret < 0);
    > +
    > + pci_proc_attach_device(dev);
    > + pci_create_sysfs_dev_files(dev);
    > }
    >
    > struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
    > @@ -1656,13 +1678,13 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
    > char bus_addr[64];
    > char *fmt;
    >
    > -
    > b = pci_alloc_bus();
    > if (!b)
    > return NULL;
    >
    > b->sysdata = sysdata;
    > b->ops = ops;
    > + b->number = b->busn_res.start = bus;
    > b2 = pci_find_bus(pci_domain_nr(b), bus);
    > if (b2) {
    > /* If we already got to this bus through a different bridge, ignore it */
    > @@ -1701,8 +1723,6 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
    > /* Create legacy_io and legacy_mem files for this bus */
    > pci_create_legacy_files(b);
    >
    > - b->number = b->busn_res.start = bus;
    > -
    > if (parent)
    > dev_info(parent, "PCI host bridge to bus %s\n", dev_name(&b->dev));
    > else
    >

    Thanks,
    Rafael


    --
    I speak only for myself.
    Rafael J. Wysocki, Intel Open Source Technology Center.


    \
     
     \ /
      Last update: 2013-01-21 00:41    [W:3.462 / U:0.120 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site