| Message ID | 1421372666-12288-9-git-send-email-wangyijing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Friday 16 January 2015 09:44:06 Yijing Wang wrote: > Introduce pci_host_assign_domain_nr() to assign domain > number for pci_host_bridge. Later we will remove > pci_bus_assign_domain_nr(). > > Signed-off-by: Yijing Wang <wangyijing@huawei.com> I'm confused: the same code is already part of the PCI tree, but with Lorenzo Pieralisi listed as the patch author. The code is good, and I acked it in the past, but one of you is (probably by accident) misattributing the patch. Assuming that the patch that is already merged in next is the right one, I think you should rebase your series on top of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next to avoid conflicts like this one. Arnd -- 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 Friday 16 January 2015 10:08:45 Arnd Bergmann wrote: > On Friday 16 January 2015 09:44:06 Yijing Wang wrote: > > Introduce pci_host_assign_domain_nr() to assign domain > > number for pci_host_bridge. Later we will remove > > pci_bus_assign_domain_nr(). > > > > Signed-off-by: Yijing Wang <wangyijing@huawei.com> > > I'm confused: the same code is already part of the PCI tree, but with > Lorenzo Pieralisi listed as the patch author. The code is good, > and I acked it in the past, but one of you is (probably by accident) > misattributing the patch. > > Assuming that the patch that is already merged in next is the right > one, I think you should rebase your series on top of > > git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next > > to avoid conflicts like this one. > I think I just got confused because the code duplicates most of pci_bus_assign_domain_nr(). Maybe this can be done in a better way by splitting the existing function into static int pci_assign_domain_nr(struct device *) { ... /* most of pci_bus_assign_domain_nr */ return domain; } void pci_host_assign_domain_nr(struct pci_host_bridge *host) { host->domain = pci_assign_domain_nr(host->dev.parent); } void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) { bus->domain_nr = pci_assign_domain_nr(parent); } -- 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
>> I'm confused: the same code is already part of the PCI tree, but with >> Lorenzo Pieralisi listed as the patch author. The code is good, >> and I acked it in the past, but one of you is (probably by accident) >> misattributing the patch. >> >> Assuming that the patch that is already merged in next is the right >> one, I think you should rebase your series on top of >> >> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next >> >> to avoid conflicts like this one. >> > > I think I just got confused because the code duplicates most of > pci_bus_assign_domain_nr(). Maybe this can be done in a better way > by splitting the existing function into > > static int pci_assign_domain_nr(struct device *) > { > ... /* most of pci_bus_assign_domain_nr */ > > return domain; > } > > void pci_host_assign_domain_nr(struct pci_host_bridge *host) > { > host->domain = pci_assign_domain_nr(host->dev.parent); > } > > void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) > { > bus->domain_nr = pci_assign_domain_nr(parent); > } > Hi Arnd, I kept the almost duplicated pci_host_assign_domain_nr() and pci_bus_assign_domain_nr() here for building happy, because now platform specific pci_domain_nr() still exists which may get domain number from pci_bus. pci_bus_assign_domain_nr() will be removed in the last patch. Thanks! Yijing. > . >
On Monday 19 January 2015 10:14:44 Yijing Wang wrote: > >> I'm confused: the same code is already part of the PCI tree, but with > >> Lorenzo Pieralisi listed as the patch author. The code is good, > >> and I acked it in the past, but one of you is (probably by accident) > >> misattributing the patch. > >> > >> Assuming that the patch that is already merged in next is the right > >> one, I think you should rebase your series on top of > >> > >> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next > >> > >> to avoid conflicts like this one. > >> > > > > I think I just got confused because the code duplicates most of > > pci_bus_assign_domain_nr(). Maybe this can be done in a better way > > by splitting the existing function into > > > > static int pci_assign_domain_nr(struct device *) > > { > > ... /* most of pci_bus_assign_domain_nr */ > > > > return domain; > > } > > > > void pci_host_assign_domain_nr(struct pci_host_bridge *host) > > { > > host->domain = pci_assign_domain_nr(host->dev.parent); > > } > > > > void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) > > { > > bus->domain_nr = pci_assign_domain_nr(parent); > > } > > > > Hi Arnd, > I kept the almost duplicated pci_host_assign_domain_nr() and > pci_bus_assign_domain_nr() here for building happy, because now > platform specific pci_domain_nr() still exists which may get domain > number from pci_bus. pci_bus_assign_domain_nr() will be removed in > the last patch. > I'm not sure I get your point: the approach I showed above seems to have the same effect, except it doesn't duplicate code temporarily, which makes it less error-prone in case your patch gets merged at the same time as another patch that modifies pci_bus_assign_domain_nr. Arnd -- 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 2015/1/19 17:50, Arnd Bergmann wrote: > On Monday 19 January 2015 10:14:44 Yijing Wang wrote: >>>> I'm confused: the same code is already part of the PCI tree, but with >>>> Lorenzo Pieralisi listed as the patch author. The code is good, >>>> and I acked it in the past, but one of you is (probably by accident) >>>> misattributing the patch. >>>> >>>> Assuming that the patch that is already merged in next is the right >>>> one, I think you should rebase your series on top of >>>> >>>> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next >>>> >>>> to avoid conflicts like this one. >>>> >>> >>> I think I just got confused because the code duplicates most of >>> pci_bus_assign_domain_nr(). Maybe this can be done in a better way >>> by splitting the existing function into >>> >>> static int pci_assign_domain_nr(struct device *) >>> { >>> ... /* most of pci_bus_assign_domain_nr */ >>> >>> return domain; >>> } >>> >>> void pci_host_assign_domain_nr(struct pci_host_bridge *host) >>> { >>> host->domain = pci_assign_domain_nr(host->dev.parent); >>> } >>> >>> void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) >>> { >>> bus->domain_nr = pci_assign_domain_nr(parent); >>> } >>> >> >> Hi Arnd, >> I kept the almost duplicated pci_host_assign_domain_nr() and >> pci_bus_assign_domain_nr() here for building happy, because now >> platform specific pci_domain_nr() still exists which may get domain >> number from pci_bus. pci_bus_assign_domain_nr() will be removed in >> the last patch. >> > > I'm not sure I get your point: the approach I showed above seems to have > the same effect, except it doesn't duplicate code temporarily, which > makes it less error-prone in case your patch gets merged at the > same time as another patch that modifies pci_bus_assign_domain_nr. OK, I got it, will update it, thanks! Thanks! Yijing. > > Arnd > > . >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index c419554..8b35e8e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4443,6 +4443,53 @@ int pci_get_new_domain_nr(void) } #ifdef CONFIG_PCI_DOMAINS_GENERIC +void pci_host_assign_domain_nr(struct pci_host_bridge *host) +{ + static int use_dt_domains = -1; + struct device *parent = host->dev.parent; + 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; + } + + host->domain_nr = domain; +} + + void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) { static int use_dt_domains = -1; diff --git a/include/linux/pci.h b/include/linux/pci.h index c771508..1b9c799 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1316,11 +1316,15 @@ static inline int pci_domain_nr(struct pci_bus *bus) return bus->domain_nr; } void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent); +void pci_host_assign_domain_nr(struct pci_host_bridge *host); #else static inline void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) { } +static inline void pci_host_assign_domain_nr(struct pci_host_bridge *host) +{ +} #endif /* some architectures require additional setup to direct VGA traffic */
Introduce pci_host_assign_domain_nr() to assign domain number for pci_host_bridge. Later we will remove pci_bus_assign_domain_nr(). Signed-off-by: Yijing Wang <wangyijing@huawei.com> --- drivers/pci/pci.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 4 ++++ 2 files changed, 51 insertions(+), 0 deletions(-)