On Wed, Nov 19, 2014 at 09:16:34AM +0000, Yijing Wang wrote: > Hi Lorenzo, > You should send this to Bjorn instead of cc. Other, why put the OF related > function in PCI core. Why not move it in drivers/of/of_pci.c ? I did, you missed v1, and the problem is that with ACPI forthcoming we do not want to have domain assignment scattered in different places, but part of core code (ie a single function that prevents mixed initialization from DT/counter and so on). Bjorn, are you fine with this patch ? Do you want me to resend it as part of the ARM 32 PCI domain refactoring [1] ? [1] depends on this patch going in first. Thanks, Lorenzo [1] http://www.spinics.net/lists/arm-kernel/msg375423.html > On 2014/11/11 0:41, Lorenzo Pieralisi wrote: > > The current logic used for PCI domain assignment in arm64 > > pci_bus_assign_domain_nr() is flawed in that, depending on the host > > controllers configuration for a platform and the respective initialization > > sequence, core code may end up allocating PCI domain numbers from both DT and > > the core code generic domain counter, which would result in PCI domain > > allocation aliases/errors. > > > > This patch fixes the logic behind the PCI domain number assignment and > > moves the resulting code to generic PCI core code so that the same domain > > allocation logic is used on all platforms selecting > > > > CONFIG_PCI_DOMAINS_GENERIC > > > > instead of resorting to an arch specific implementation that might end up > > duplicating the PCI domain assignment logic wrongly. > > > > Cc: Arnd Bergmann <arnd@xxxxxxxx> > > Cc: Liviu Dudau <liviu.dudau@xxxxxxx> > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > Cc: Rob Herring <robh+dt@xxxxxxxxxx> > > Cc: Catalin Marinas <catalin.marinas@xxxxxxx> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> > > --- > > v1 => v2: > > > > - Moved generic pci_bus_assign_domain_nr() code to PCI core instead of > > adding an OF layer API > > - Updated commit log and code comments > > > > arch/arm64/kernel/pci.c | 22 ---------------------- > > drivers/pci/pci.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 50 insertions(+), 22 deletions(-) > > > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c > > index ce5836c..6f93c24 100644 > > --- a/arch/arm64/kernel/pci.c > > +++ b/arch/arm64/kernel/pci.c > > @@ -46,25 +46,3 @@ int pcibios_add_device(struct pci_dev *dev) > > > > return 0; > > } > > - > > - > > -#ifdef CONFIG_PCI_DOMAINS_GENERIC > > -static bool dt_domain_found = false; > > - > > -void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) > > -{ > > - int domain = of_get_pci_domain_nr(parent->of_node); > > - > > - if (domain >= 0) { > > - dt_domain_found = true; > > - } else if (dt_domain_found == true) { > > - dev_err(parent, "Node %s is missing \"linux,pci-domain\" property in DT\n", > > - parent->of_node->full_name); > > - return; > > - } else { > > - domain = pci_get_new_domain_nr(); > > - } > > - > > - bus->domain_nr = domain; > > -} > > -#endif > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 625a4ac..2279414 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -10,6 +10,8 @@ > > #include <linux/kernel.h> > > #include <linux/delay.h> > > #include <linux/init.h> > > +#include <linux/of.h> > > +#include <linux/of_pci.h> > > #include <linux/pci.h> > > #include <linux/pm.h> > > #include <linux/slab.h> > > @@ -4447,6 +4449,54 @@ int pci_get_new_domain_nr(void) > > { > > return atomic_inc_return(&__domain_nr); > > } > > + > > +#ifdef CONFIG_PCI_DOMAINS_GENERIC > > + > > +void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) > > +{ > > + static int use_dt_domains = -1; > > + int domain = of_get_pci_domain_nr(parent->of_node); > > + > > + /* > > + * Check DT domain and use_dt_domains values. > > + * > > + * If DT domain property is valid (domain >= 0) and > > + * use_dt_domains != 0, the DT assignment is valid since this means > > + * we have not previously allocated a domain number by using > > + * pci_get_new_domain_nr(); we should also update use_dt_domains to > > + * 1, to indicate that we have just assigned a domain number from > > + * DT. > > + * > > + * If DT domain property value is not valid (ie domain < 0), and we > > + * have not previously assigned a domain number from DT > > + * (use_dt_domains != 1) we should assign a domain number by > > + * using the: > > + * > > + * pci_get_new_domain_nr() > > + * > > + * API and update the use_dt_domains value to keep track of method we > > + * are using to assign domain numbers (use_dt_domains = 0). > > + * > > + * All other combinations imply we have a platform that is trying > > + * to mix domain numbers obtained from DT and pci_get_new_domain_nr(), > > + * which is a recipe for domain mishandling and it is prevented by > > + * invalidating the domain value (domain = -1) and printing a > > + * corresponding error. > > + */ > > + if (domain >= 0 && use_dt_domains) { > > + use_dt_domains = 1; > > + } else if (domain < 0 && use_dt_domains != 1) { > > + use_dt_domains = 0; > > + domain = pci_get_new_domain_nr(); > > + } else { > > + dev_err(parent, "Node %s has inconsistent \"linux,pci-domain\" property in DT\n", > > + parent->of_node->full_name); > > + domain = -1; > > + } > > + > > + bus->domain_nr = domain; > > +} > > +#endif > > #endif > > > > /** > > > > > -- > Thanks! > Yijing > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
