Re: [PATCH 5/8] PCI: add generic device into pci_host_bridge struct

Bjorn Helgaas
2012-02-27T19:17:59Z

On Mon, Feb 27, 2012 at 12:08 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Feb 27, 2012 at 9:57 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Sun, Feb 26, 2012 at 4:33 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> use that device for pci_root_bus bridge pointer.
>>>
>>> With that make code more simple.
>>>
>>> Also we can use pci_release_bus_bridge_dev() to release allocated
>>> pci_host_bridge during removing path.
>>>
>>> At last, we can use root bus bridge pointer to get host bridge pointer instead
>>> of going over host bridge list, so could kill that host bridge list.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>
>>> ---
>>>  drivers/pci/host-bridge.c |   14 ---------
>>>  drivers/pci/pci.h         |    2 -
>>>  drivers/pci/probe.c       |   65 ++++++++++++++++++++++++----------------------
>>>  include/linux/pci.h       |    4 ++
>>>  4 files changed, 38 insertions(+), 47 deletions(-)
>>>
>>> Index: linux-2.6/drivers/pci/host-bridge.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/pci/host-bridge.c
>>> +++ linux-2.6/drivers/pci/host-bridge.c
>>> -       dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>>> -       if (!dev)
>>> -               goto err_dev;
>>> +               return NULL;
>>>
> ...
>
> here old code allocate dummy struct device.
>
>>> +       bridge = pci_alloc_host_bridge(b);
>>> +       if (!bridge)
>>> +               goto err_out;
>>> +
>>> +       bridge->dev.parent = parent;
>>> +       bridge->dev.release = pci_release_bus_bridge_dev;
>>> +       dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus);
>>> +       error = device_register(&bridge->dev);
>>>        if (error)
>>> -               goto dev_reg_err;
>>> -       b->bridge = get_device(dev);
>>> +               goto bridge_dev_reg_err;
>>> +       b->bridge = get_device(&bridge->dev);
> ...
>>> --- linux-2.6.orig/include/linux/pci.h
>>> +++ linux-2.6/include/linux/pci.h
>>> @@ -375,11 +375,13 @@ struct pci_host_bridge_window {
>>>  };
>>>
>>>  struct pci_host_bridge {
>>> -       struct list_head list;
>>> +       struct device dev;
>>>        struct pci_bus *bus;            /* root bus */
>>>        struct list_head windows;       /* pci_host_bridge_windows */
>>>  };
>>
>> This doesn't feel right to me.  You're allocating a new struct device
>> here, but the arch likely already has one.  In fact, I think we even
>> passed it in to pci_create_root_bus().
>
> no.
>
> original pci_create_root_bus() will create one dummy struct device and
> use it as parent for bus->dev.
>
>>
>> On x86, we already have an ACPI device for the host bridge, and now
>> we'll have a second one.  (Actually a *third* one, because PNPACPI
>> also has one, but that's a long-standing problem.)
>
> not related.

OK. Can you educate me? What is the device we pass in, what does the
dummy one correspond to, why do we have two, what's the difference,
etc., etc.?

Bjorn