PCI: pci_host_bridge related cleanup

x86 does not need to offset the address. So we can skip that costing offset searching.

---
 arch/x86/pci/i386.c       |   14 ++++++++++++++
 drivers/pci/host-bridge.c |    9 +++++----

 2 files changed, 19 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/x86/pci/i386.c
===================================================================

arch/x86/pci/i386.c +14 -0

@@ -335,6 +335,20 @@ void __init pcibios_resource_survey(void
  */
  fs_initcall(pcibios_assign_resources);
 
+void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
+                            struct resource *res)
+{
+       region->start = res->start;
+       region->end = res->end;
+}
+
+void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+                            struct pci_bus_region *region)
+{
+       res->start = region->start;
+       res->end = region->end;
+}
+
  static const struct vm_operations_struct pci_mmap_ops = {
        .access = generic_access_phys,
  };

Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================

drivers/pci/host-bridge.c +5 -4

@@ -38,8 +38,9 @@ static bool resource_contains(struct res
        return res1->start <= res2->start && res1->end >= res2->end;
  }
 
-void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
-                            struct resource *res)
+void __weak pcibios_resource_to_bus(struct pci_dev *dev,
+                                   struct pci_bus_region *region,
+                                   struct resource *res)
  {
        struct pci_host_bridge *bridge = pci_host_bridge(dev);
        struct pci_host_bridge_window *window;

@@ -66,8 +67,8 @@ static bool region_contains(struct pci_b
        return region1->start <= region2->start && region1->end >= region2->end;
  }
 
-void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
-                            struct pci_bus_region *region)
+void __weak pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+                                   struct pci_bus_region *region)
  {
        struct pci_host_bridge *bridge = pci_host_bridge(dev);
        struct pci_host_bridge_window *window;

1. change pci_host_bridge to find_pci_root_bridge.
2. separate find_pci_root_bus().
3. on any possible path return NULL.

Signed-off-by Yinghai Lu <[email protected]>

---
 drivers/pci/host-bridge.c |   35 ++++++++++++++++++++++++++++-------

 1 file changed, 28 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================

drivers/pci/host-bridge.c +1 -1

@@ -16,19 +16,31 @@ void add_to_pci_host_bridges(struct pci_
        list_add_tail(&bridge->list, &pci_host_bridges);
  }
 
-static struct pci_host_bridge *pci_host_bridge(struct pci_dev *dev)
+static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
  {

drivers/pci/host-bridge.c +13 -4

+       list_for_each_entry(bridge, &pci_host_bridges, list)
                if (bridge->bus == bus)
                        return bridge;
-       }
 
        return NULL;
  }

@@ -42,10 +54,13 @@ void __weak pcibios_resource_to_bus(stru
                                    struct pci_bus_region *region,
                                    struct resource *res)
  {
-       struct pci_host_bridge *bridge = pci_host_bridge(dev);
+       struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
        struct pci_host_bridge_window *window;
        resource_size_t offset = 0;
 
+       if (!bridge)
+               goto no_bridge;
+
        list_for_each_entry(window, &bridge->windows, list) {
                if (resource_type(res) != resource_type(window->res))
                        continue;

@@ -56,6 +71,7 @@ void __weak pcibios_resource_to_bus(stru
                }
        }
 
+no_bridge:
        region->start = res->start - offset;
        region->end = res->end - offset;
  }

@@ -70,12 +86,16 @@ static bool region_contains(struct pci_b
  void __weak pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
                                    struct pci_bus_region *region)
  {
-       struct pci_host_bridge *bridge = pci_host_bridge(dev);
+       struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
        struct pci_host_bridge_window *window;
-       struct pci_bus_region bus_region;
        resource_size_t offset = 0;
 
+       if (!bridge)
+               goto no_bridge;
+
        list_for_each_entry(window, &bridge->windows, list) {
+               struct pci_bus_region bus_region;
+
                if (resource_type(res) != resource_type(window->res))
                        continue;
 

@@ -88,6 +108,7 @@ void __weak pcibios_bus_to_resource(stru
                }
        }
 
+no_bridge:
        res->start = region->start + offset;
        res->end = region->end + offset;
  }

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 <[email protected]>

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

drivers/pci/host-bridge.c +1 -13

@@ -9,13 +9,6 @@
 
  #include "pci.h"
 
-static LIST_HEAD(pci_host_bridges);
-
-void add_to_pci_host_bridges(struct pci_host_bridge *bridge)
-{
-       list_add_tail(&bridge->list, &pci_host_bridges);
-}
-
  static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
  {
        struct pci_bus *bus;

@@ -33,16 +26,11 @@ static struct pci_bus *find_pci_root_bus
  static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
  {
        struct pci_bus *bus = find_pci_root_bus(dev);
-       struct pci_host_bridge *bridge;
 
        if (!bus)
                return NULL;
 
-       list_for_each_entry(bridge, &pci_host_bridges, list)
-               if (bridge->bus == bus)
-                       return bridge;
-
-       return NULL;
+       return to_pci_host_bridge(bus->bridge);
  }
 
  static bool resource_contains(struct resource *res1, struct resource *res2)

Index: linux-2.6/drivers/pci/pci.h
===================================================================

drivers/pci/pci.h +0 -2

@@ -231,8 +231,6 @@ static inline int pci_ari_enabled(struct
  void pci_reassigndev_resource_alignment(struct pci_dev *dev);
  extern void pci_disable_bridge_window(struct pci_dev *dev);
 
-void add_to_pci_host_bridges(struct pci_host_bridge *bridge);
-
  /* Single Root I/O Virtualization */
  struct pci_sriov {

       int pos;                /* capability position */
Index: linux-2.6/drivers/pci/probe.c
===================================================================

drivers/pci/probe.c +13 -0

@@ -421,6 +421,19 @@ static struct pci_bus * pci_alloc_bus(vo
        return b;
  }
 
+static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b)
+{
+       struct pci_host_bridge *bridge;
+
+       bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
+       if (bridge) {
+               INIT_LIST_HEAD(&bridge->windows);
+               bridge->bus = b;
+       }
+
+       return bridge;
+}
+
  static unsigned char pcix_bus_speed[] = {

       PCI_SPEED_UNKNOWN,              /* 0 */
       PCI_SPEED_66MHz_PCIX,           /* 1 */

drivers/pci/probe.c +18 -20

@@ -1121,7 +1134,13 @@ int pci_cfg_space_size(struct pci_dev *d
 
  static void pci_release_bus_bridge_dev(struct device *dev)
  {
-       kfree(dev);
+       struct pci_host_bridge *bridge = to_pci_host_bridge(dev);
+
+       /* TODO: need to free window->res */
+
+       pci_free_resource_list(&bridge->windows);
+
+       kfree(bridge);
  }
 
  struct pci_dev *alloc_pci_dev(void)

@@ -1570,28 +1589,19 @@ struct pci_bus *pci_create_root_bus(stru
        int error;
        struct pci_host_bridge *bridge;
        struct pci_bus *b, *b2;
-       struct device *dev;
        struct pci_host_bridge_window *window, *n;
        struct resource *res;
        resource_size_t offset;
        char bus_addr[64];
        char *fmt;
 
-       bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
-       if (!bridge)
-               return NULL;
 
        b = pci_alloc_bus();
        if (!b)
-               goto err_bus;
-
-       dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-       if (!dev)
-               goto err_dev;
+               return NULL;
 
        b->sysdata = sysdata;
        b->ops = ops;
-
        b2 = pci_find_bus(pci_domain_nr(b), bus);
        if (b2) {
                /* If we already got to this bus through a different bridge, ignore it */

@@ -1599,13 +1609,17 @@ struct pci_bus *pci_create_root_bus(stru
                goto err_out;
        }
 
-       dev->parent = parent;
-       dev->release = pci_release_bus_bridge_dev;
-       dev_set_name(dev, "pci%04x:%02x", pci_domain_nr(b), bus);
-       error = device_register(dev);
+       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);
        device_enable_async_suspend(b->bridge);
        pci_set_bus_of_node(b);
 

@@ -1624,9 +1638,6 @@ struct pci_bus *pci_create_root_bus(stru
 
        b->number = b->secondary = bus;
 
-       bridge->bus = b;
-       INIT_LIST_HEAD(&bridge->windows);
-
        if (parent)
                dev_info(parent, "PCI host bridge to bus %s\n", dev_name(&b->dev));

       else

drivers/pci/probe.c +0 -1

@@ -1652,25 +1663,17 @@ struct pci_bus *pci_create_root_bus(stru
        }
 
        down_write(&pci_bus_sem);
-       add_to_pci_host_bridges(bridge);
        list_add_tail(&b->node, &pci_root_buses);
        up_write(&pci_bus_sem);
 
        return b;
 

 class_dev_reg_err:
-       device_unregister(dev);

drivers/pci/probe.c +3 -5

-dev_reg_err:
-       down_write(&pci_bus_sem);
-       list_del(&bridge->list);
-       list_del(&b->node);
-       up_write(&pci_bus_sem);
+       device_unregister(&bridge->dev);
+bridge_dev_reg_err:
+       kfree(bridge);

 err_out:
-       kfree(dev);
-err_dev:
       kfree(b);
-err_bus:
-       kfree(bridge);
       return NULL;
 }

Index: linux-2.6/include/linux/pci.h
===================================================================

include/linux/pci.h +1 -1

@@ -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 */ };

include/linux/pci.h +2 -0

+#define        to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, dev)
+

 /*
 * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
 * to P2P or CardBus bridge windows) go in a table.  Additional ones (for

Quoted 19 lines · Yinghai Lu

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 <[email protected]>

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

drivers/pci/host-bridge.c

Quoted 9 lines

Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================

drivers/pci/host-bridge.c +1 -3

-       dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-       if (!dev)
-               goto err_dev;
+               return NULL;

Quoted 13 lines

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

Quoted 5 lines

include/linux/pci.h +0 -0

@@ -375,11 +375,13 @@ struct pci_host_bridge_window {
  };
 

Quoted 6 lines

include/linux/pci.h +1 -1

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

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

Quoted 20 lines · Bjorn Helgaas

Quoted 19 lines · Yinghai Lu

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 <[email protected]>

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

Quoted 9 lines

drivers/pci/host-bridge.c

Quoted 9 lines

Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================

drivers/pci/host-bridge.c +1 -3

-       dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-       if (!dev)
-               goto err_dev;
+               return NULL;

...

here old code allocate dummy struct device.

Quoted 13 lines

Quoted 13 lines

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

...

Quoted 5 lines

Quoted 5 lines

include/linux/pci.h +0 -0

@@ -375,11 +375,13 @@ struct pci_host_bridge_window {
  };
 

Quoted 10 lines

Quoted 6 lines

include/linux/pci.h +1 -1

  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.

Quoted 1 line

Quoted 3 lines

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.

Quoted 14 lines · Yinghai Lu

Quoted 3 lines

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.

Quoted 4 lines

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

Quoted 1 line

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

Quoted 16 lines · Bjorn Helgaas

Quoted 14 lines · Yinghai Lu

Quoted 3 lines

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.

Quoted 4 lines

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

Quoted 5 lines

Quoted 1 line

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

for every bus will have ->bridge, and that is device pointer. to  pci bridge dev.

for root bus, there is not pci bridge for that. So create_root_bus allocate local dummy one.

that dummy device will be bus->dev's parent.

Now pci_host_bridge is added, and it is allocated, so just put the struct device into that pci_host_bridge.

and use pci_host_bridge->dev as parent of root bus->dev's parent.

Quoted 9 lines

for every bus will have ->bridge, and that is device pointer. to  pci bridge dev.

for root bus, there is not pci bridge for that. So create_root_bus allocate local dummy one.

that dummy device will be bus->dev's parent.

Quoted 4 lines

Now pci_host_bridge is added, and it is allocated, so just put the struct device into that pci_host_bridge.

and use pci_host_bridge->dev as parent of root bus->dev's parent.

What's the device passed in to pci_create_root_bus()?  Why is that different than the root bus dev's parent?

Quoted 9 lines · Bjorn Helgaas

Quoted 9 lines

for every bus will have ->bridge, and that is device pointer. to  pci bridge dev.

for root bus, there is not pci bridge for that. So create_root_bus allocate local dummy one.

that dummy device will be bus->dev's parent.

Quoted 7 lines

Quoted 4 lines

Now pci_host_bridge is added, and it is allocated, so just put the struct device into that pci_host_bridge.

and use pci_host_bridge->dev as parent of root bus->dev's parent.

What's the device passed in to pci_create_root_bus()?  Why is that different than the root bus dev's parent?

ia64 and x86 doesn't pass that.

arch/ia64/pci/pci.c:    pbus = pci_create_root_bus(NULL, bus, &pci_root_ops, controller, arch/powerpc/kernel/pci-common.c:       bus = pci_create_root_bus(hose->parent, hose->first_busno, arch/sparc/kernel/pci.c:        bus = pci_create_root_bus(parent, pbm->pci_first_busno, pbm->pci_ops,
arch/x86/pci/acpi.c:            bus = pci_create_root_bus(NULL, busnum, &pci_root_ops, sd,
drivers/parisc/dino.c:  dino_dev->hba.hba_bus = bus = pci_create_root_bus(&dev->dev,
drivers/parisc/lba_pci.c:               pci_create_root_bus(&dev->dev, lba_dev->hba.bus_num.start,

sparc, powerpc, parisc have their local ...

so local hose etc's dev ---> allocated dev ---> root bus's dev

this patch change to

local hose etc's dev ---> allocated host bridge's dev ---> root bus's dev