| Message ID | 1411003825-21521-8-git-send-email-Liviu.Dudau@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 18, 2014 at 02:30:22AM +0100, Liviu Dudau wrote: > Add of_pci_get_domain_nr() to retrieve the PCI domain number > of a given device from DT. If the information is not present, > the function can be requested to allocate a new domain number. Is of_pci_get_domain_nr() used somewhere? If the use is in some future series, please mention it explicitly. I'm just trying to avoid merging unused code. Bjorn
On Thursday 18 September 2014, Liviu Dudau wrote: > > Add of_pci_get_domain_nr() to retrieve the PCI domain number > of a given device from DT. If the information is not present, > the function can be requested to allocate a new domain number. > > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Grant Likely <grant.likely@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> This is more elaborate than I remember it, and while the simpler version (which potentially resulted in a conflict for inconsistent DTs) was fine, this one also seems ok. Acked-by: Arnd Bergmann <arnd@arndb.de>
On 09/17/2014 08:30 PM, Liviu Dudau wrote: > Add of_pci_get_domain_nr() to retrieve the PCI domain number > of a given device from DT. If the information is not present, > the function can be requested to allocate a new domain number. > > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Grant Likely <grant.likely@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > --- [...] > +/** > + * This function will try to obtain the host bridge domain number by > + * using of_alias_get_id() call with "pci-domain" as a stem. If that > + * fails, a local allocator will be used. The local allocator can > + * be requested to return a new domain_nr if the information is missing > + * from the device tree. > + * > + * @node: device tree node with the domain information > + * @allocate_if_missing: if DT lacks information about the domain nr, > + * allocate a new number. > + * > + * Returns the associated domain number from DT, or a new domain number > + * if DT information is missing and @allocate_if_missing is true. If > + * @allocate_if_missing is false then the last allocated domain number > + * will be returned. > + */ > +int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing) > +{ > + int domain; > + > + domain = atomic_read(&of_domain_nr); > + if (domain == -1) { > + /* first run, get max defined domain nr in device tree */ > + domain = of_get_max_pci_domain_nr(); > + /* then set the start value for allocator to be max + 1 */ > + atomic_set(&of_domain_nr, domain + 1); atomic_read followed by atomic_set is not an atomic operation. As I previously said, I don't like how this function is a mixture of data retrieval and domian # allocation. I think we need 2 functions. > + } > + domain = of_alias_get_id(node, "pci-domain"); I still do not like using aliases here. Just put pci-domain or linux,pci-domain into the PCI node. I think we should assume all PCI root buses either have a domain property or they don't and a mixture is an error. I'm not sure if that simplifies the code or not though. In the interest of merging, I think you should just do a simple allocation and add the DT domain handling as a second step. You will also need to document the DT part. Rob
On Fri, Sep 19, 2014 at 10:03:13PM +0100, Bjorn Helgaas wrote: > On Thu, Sep 18, 2014 at 02:30:22AM +0100, Liviu Dudau wrote: > > Add of_pci_get_domain_nr() to retrieve the PCI domain number > > of a given device from DT. If the information is not present, > > the function can be requested to allocate a new domain number. > > Is of_pci_get_domain_nr() used somewhere? If the use is in some future > series, please mention it explicitly. I'm just trying to avoid merging > unused code. It is used in the arm64 specific patch that I have dropped out of my pull request. After discussions with Catalin I will add the patch back into the tree that you've pulled from as he is OK with your tree carrying the whole package. I need to ask for some guidance here: for addressing some of your comments and Rob's I can add more patches in my v11 branch and you can pull them when you think they are ready. But one of your comments was requesting splitting a patch into two blocks - one that moves of_pci_range_to_resource() into drivers/of/address.c and one that fixes it's behaviour - and I don't know how you would like that handled. Should I revert the original patch and add the new ones, or should I rebase the whole series into a different branch that you can pull from? Best regards, Liviu > > Bjorn >
On Sat, Sep 20, 2014 at 03:24:20AM +0100, Arnd Bergmann wrote: > On Thursday 18 September 2014, Liviu Dudau wrote: > > > > Add of_pci_get_domain_nr() to retrieve the PCI domain number > > of a given device from DT. If the information is not present, > > the function can be requested to allocate a new domain number. > > > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Grant Likely <grant.likely@linaro.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > > This is more elaborate than I remember it, and while the simpler > version (which potentially resulted in a conflict for inconsistent > DTs) was fine, this one also seems ok. > > Acked-by: Arnd Bergmann <arnd@arndb.de> Thanks Arnd! Best regards, Liviu > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Mon, Sep 22, 2014 at 5:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > On Fri, Sep 19, 2014 at 10:03:13PM +0100, Bjorn Helgaas wrote: >> On Thu, Sep 18, 2014 at 02:30:22AM +0100, Liviu Dudau wrote: >> > Add of_pci_get_domain_nr() to retrieve the PCI domain number >> > of a given device from DT. If the information is not present, >> > the function can be requested to allocate a new domain number. >> >> Is of_pci_get_domain_nr() used somewhere? If the use is in some future >> series, please mention it explicitly. I'm just trying to avoid merging >> unused code. > > It is used in the arm64 specific patch that I have dropped out of my > pull request. After discussions with Catalin I will add the patch back > into the tree that you've pulled from as he is OK with your tree carrying > the whole package. > > I need to ask for some guidance here: for addressing some of your comments > and Rob's I can add more patches in my v11 branch and you can pull them > when you think they are ready. But one of your comments was requesting > splitting a patch into two blocks - one that moves of_pci_range_to_resource() > into drivers/of/address.c and one that fixes it's behaviour - and I don't > know how you would like that handled. Should I revert the original patch > and add the new ones, or should I rebase the whole series into a different > branch that you can pull from? I guess the easiest thing is probably just to send a v12 series. I was hoping we were close enough for me to just hand-integrate minor tweaks into my branch, but I think that will just create more confusion. Bjorn
On Mon, Sep 22, 2014 at 04:25:13PM +0100, Bjorn Helgaas wrote: > On Mon, Sep 22, 2014 at 5:05 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > > On Fri, Sep 19, 2014 at 10:03:13PM +0100, Bjorn Helgaas wrote: > >> On Thu, Sep 18, 2014 at 02:30:22AM +0100, Liviu Dudau wrote: > >> > Add of_pci_get_domain_nr() to retrieve the PCI domain number > >> > of a given device from DT. If the information is not present, > >> > the function can be requested to allocate a new domain number. > >> > >> Is of_pci_get_domain_nr() used somewhere? If the use is in some future > >> series, please mention it explicitly. I'm just trying to avoid merging > >> unused code. > > > > It is used in the arm64 specific patch that I have dropped out of my > > pull request. After discussions with Catalin I will add the patch back > > into the tree that you've pulled from as he is OK with your tree carrying > > the whole package. > > > > I need to ask for some guidance here: for addressing some of your comments > > and Rob's I can add more patches in my v11 branch and you can pull them > > when you think they are ready. But one of your comments was requesting > > splitting a patch into two blocks - one that moves of_pci_range_to_resource() > > into drivers/of/address.c and one that fixes it's behaviour - and I don't > > know how you would like that handled. Should I revert the original patch > > and add the new ones, or should I rebase the whole series into a different > > branch that you can pull from? > > I guess the easiest thing is probably just to send a v12 series. I > was hoping we were close enough for me to just hand-integrate minor > tweaks into my branch, but I think that will just create more > confusion. OK, I will send v12 then with all the acquired ACKs and fixes. Best regards, Liviu > > Bjorn >
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c index 8481996..7eaeac2 100644 --- a/drivers/of/of_pci.c +++ b/drivers/of/of_pci.c @@ -3,6 +3,8 @@ #include <linux/of.h> #include <linux/of_pci.h> +#include "of_private.h" + static inline int __of_pci_pci_compare(struct device_node *node, unsigned int data) { @@ -89,6 +91,66 @@ int of_pci_parse_bus_range(struct device_node *node, struct resource *res) } EXPORT_SYMBOL_GPL(of_pci_parse_bus_range); +static atomic_t of_domain_nr = ATOMIC_INIT(-1); + +/* + * Get the maximum value for a domain number from the device tree + */ +static int of_get_max_pci_domain_nr(void) +{ + struct alias_prop *app; + int max_domain = -1; + + mutex_lock(&of_mutex); + list_for_each_entry(app, &aliases_lookup, link) { + if (strncmp(app->stem, "pci-domain", 10) != 0) + continue; + + max_domain = max(max_domain, app->id); + } + mutex_unlock(&of_mutex); + + return max_domain; +} + +/** + * This function will try to obtain the host bridge domain number by + * using of_alias_get_id() call with "pci-domain" as a stem. If that + * fails, a local allocator will be used. The local allocator can + * be requested to return a new domain_nr if the information is missing + * from the device tree. + * + * @node: device tree node with the domain information + * @allocate_if_missing: if DT lacks information about the domain nr, + * allocate a new number. + * + * Returns the associated domain number from DT, or a new domain number + * if DT information is missing and @allocate_if_missing is true. If + * @allocate_if_missing is false then the last allocated domain number + * will be returned. + */ +int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing) +{ + int domain; + + domain = atomic_read(&of_domain_nr); + if (domain == -1) { + /* first run, get max defined domain nr in device tree */ + domain = of_get_max_pci_domain_nr(); + /* then set the start value for allocator to be max + 1 */ + atomic_set(&of_domain_nr, domain + 1); + } + domain = of_alias_get_id(node, "pci-domain"); + if (domain == -ENODEV) { + domain = atomic_read(&of_domain_nr); + if (allocate_if_missing) + atomic_inc(&of_domain_nr); + } + + return domain; +} +EXPORT_SYMBOL_GPL(of_pci_get_domain_nr); + #ifdef CONFIG_PCI_MSI static LIST_HEAD(of_pci_msi_chip_list); diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h index dde3a4a..3a3824c 100644 --- a/include/linux/of_pci.h +++ b/include/linux/of_pci.h @@ -15,6 +15,7 @@ struct device_node *of_pci_find_child_device(struct device_node *parent, int of_pci_get_devfn(struct device_node *np); int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin); int of_pci_parse_bus_range(struct device_node *node, struct resource *res); +int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing); #else static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq) { @@ -43,6 +44,12 @@ of_pci_parse_bus_range(struct device_node *node, struct resource *res) { return -EINVAL; } + +static inline int +of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing) +{ + return -1; +} #endif #if defined(CONFIG_OF) && defined(CONFIG_PCI_MSI)