On Tue, Jul 01, 2014 at 07:43:31PM +0100, Liviu Dudau wrote:
> Make it easier to discover the domain number of a bus by storing
> the number in pci_host_bridge for the root bus. Several architectures
> have their own way of storing this information, so it makes sense
> to try to unify the code. While at this, add a new function that
> creates a root bus in a given domain and make pci_create_root_bus()
> a wrapper around this function.

"While at this" is always a good clue that maybe something should be
split into a separate patch :)  This is a very good example, since it
adds a new interface that deserves its own changelog.

> Signed-off-by: Liviu Dudau <[email protected]>
> Tested-by: Tanmay Inamdar <[email protected]>
> ---
>  drivers/pci/probe.c | 41 +++++++++++++++++++++++++++++++++--------
>  include/linux/pci.h |  4 ++++
>  2 files changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 2c92662..abf5e82 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1748,8 +1748,9 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
>  {
>  }
>  
> -struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> -             struct pci_ops *ops, void *sysdata, struct list_head *resources)
> +struct pci_bus *pci_create_root_bus_in_domain(struct device *parent,
> +             int domain, int bus, struct pci_ops *ops, void *sysdata,
> +             struct list_head *resources)

I don't think we should do it this way; this makes it possible to have a
host bridge where "bridge->domain_nr != pci_domain_nr(bridge->bus)".

I wonder if it would help to make a weak pci_domain_nr() function that
returns "bridge->domain_nr".  Then each arch could individually drop its
pci_domain_nr() definition as it was converted, e.g., something like this:

  - Convert every arch pci_domain_nr() from a #define to a non-inline
    function
  - Add bridge.domain_nr, initialized from pci_domain_nr()
  - Add a weak generic pci_domain_nr() that returns bridge.domain_nr
  - Add a way to create a host bridge in a specified domain, so we can
    initialize bridge.domain_nr without using pci_domain_nr()
  - Convert each arch to use the new creation mechanism and drop its
    pci_domain_nr() implementation

>  {
>       int error;
>       struct pci_host_bridge *bridge;
> @@ -1762,27 +1763,31 @@ struct pci_bus *pci_create_root_bus(struct device 
> *parent, int bus,
>  
>       bridge = pci_alloc_host_bridge();
>       if (!bridge)
> -             return NULL;
> +             return ERR_PTR(-ENOMEM);
>  
>       bridge->dev.parent = parent;
>       bridge->dev.release = pci_release_host_bridge_dev;
> +     bridge->domain_nr = domain;
>  
>       b = pci_alloc_bus();
> -     if (!b)
> +     if (!b) {
> +             error = -ENOMEM;
>               goto err_out;
> +     }
>  
>       b->sysdata = sysdata;
>       b->ops = ops;
>       b->number = b->busn_res.start = bus;
> -     b2 = pci_find_bus(pci_domain_nr(b), bus);
> +     b2 = pci_find_bus(bridge->domain_nr, bus);
>       if (b2) {
>               /* If we already got to this bus through a different bridge, 
> ignore it */
>               dev_dbg(&b2->dev, "bus already known\n");
> +             error = -EEXIST;
>               goto err_bus_out;
>       }
>  
>       bridge->bus = b;
> -     dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus);
> +     dev_set_name(&bridge->dev, "pci%04x:%02x", bridge->domain_nr, bus);
>       error = pcibios_root_bridge_prepare(bridge);
>       if (error)
>               goto err_out;
> @@ -1801,7 +1806,7 @@ struct pci_bus *pci_create_root_bus(struct device 
> *parent, int bus,
>  
>       b->dev.class = &pcibus_class;
>       b->dev.parent = b->bridge;
> -     dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
> +     dev_set_name(&b->dev, "%04x:%02x", bridge->domain_nr, bus);
>       error = device_register(&b->dev);
>       if (error)
>               goto class_dev_reg_err;
> @@ -1851,7 +1856,27 @@ err_bus_out:
>       kfree(b);
>  err_out:
>       kfree(bridge);
> -     return NULL;
> +     return ERR_PTR(error);
> +}
> +
> +struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> +             struct pci_ops *ops, void *sysdata, struct list_head *resources)
> +{
> +     int domain_nr;
> +     struct pci_bus *b = pci_alloc_bus();
> +     if (!b)
> +             return NULL;
> +
> +     b->sysdata = sysdata;
> +     domain_nr = pci_domain_nr(b);
> +     kfree(b);
> +
> +     b = pci_create_root_bus_in_domain(parent, domain_nr, bus,
> +                             ops, sysdata, resources);
> +     if (IS_ERR(b))
> +             return NULL;
> +
> +     return b;
>  }
>  
>  int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 466bcd1..7e7b939 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -401,6 +401,7 @@ struct pci_host_bridge_window {
>  struct pci_host_bridge {
>       struct device dev;
>       struct pci_bus *bus;            /* root bus */
> +     int domain_nr;
>       struct list_head windows;       /* pci_host_bridge_windows */
>       void (*release_fn)(struct pci_host_bridge *);
>       void *release_data;
> @@ -769,6 +770,9 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops 
> *ops, void *sysdata);
>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>                                   struct pci_ops *ops, void *sysdata,
>                                   struct list_head *resources);
> +struct pci_bus *pci_create_root_bus_in_domain(struct device *parent,
> +                     int domain, int bus, struct pci_ops *ops,
> +                     void *sysdata, struct list_head *resources);
>  int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
>  int pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
>  void pci_bus_release_busn_res(struct pci_bus *b);
> -- 
> 2.0.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to