| Message ID | 1414669490-1217-3-git-send-email-lorenzo.pieralisi@arm.com |
|---|---|
| State | New |
| Headers | show |
On Thursday 30 October 2014 11:44:48 Lorenzo Pieralisi wrote: > Most if not all ARM PCI host controller device drivers either ignore the > domain field in the pci_sys_data structure or just increment it every > time a host controller is probed, using it as a domain counter. > > Therefore, instead of relying on pci_sys_data to stash the domain number > in a standard location, ARM pcibios code can be moved to the newly > introduced generic PCI domains code, implemented in commits: > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > ("PCI: Add generic domain handling") > > In order to assign a domain number dynamically, the ARM pcibios defines > the function, called by core PCI code: > > void pci_bus_assign_domain_nr(...) > > that relies on a DT property to define the domain number or falls back to > a counter; its usage replaces the current domain assignment code in PCI > host controllers present in the kernel. > > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Phil Edworthy <phil.edworthy@renesas.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Jingoo Han <jg1.han@samsung.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Mohit Kumar <mohit.kumar@st.com> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Acked-by: Arnd Bergmann <arnd@arndb.de> Very nice!
On 2014/10/30 19:44, Lorenzo Pieralisi wrote: > Most if not all ARM PCI host controller device drivers either ignore the > domain field in the pci_sys_data structure or just increment it every > time a host controller is probed, using it as a domain counter. > > Therefore, instead of relying on pci_sys_data to stash the domain number > in a standard location, ARM pcibios code can be moved to the newly > introduced generic PCI domains code, implemented in commits: > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > ("PCI: Add generic domain handling") > > In order to assign a domain number dynamically, the ARM pcibios defines > the function, called by core PCI code: > > void pci_bus_assign_domain_nr(...) > > that relies on a DT property to define the domain number or falls back to > a counter; its usage replaces the current domain assignment code in PCI > host controllers present in the kernel. Nice clean up. Reviewed-by: Yijing Wang <wangyijing@huawei.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Phil Edworthy <phil.edworthy@renesas.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Jingoo Han <jg1.han@samsung.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Mohit Kumar <mohit.kumar@st.com> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > --- > arch/arm/Kconfig | 3 +++ > arch/arm/include/asm/mach/pci.h | 3 --- > arch/arm/include/asm/pci.h | 7 ------- > arch/arm/kernel/bios32.c | 26 +++++++++++++++++++++++--- > drivers/pci/host/pci-mvebu.c | 10 ++-------- > drivers/pci/host/pcie-designware.c | 3 --- > drivers/pci/host/pcie-rcar.c | 3 --- > 7 files changed, 28 insertions(+), 27 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 89c4b5c..29544f0 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1292,6 +1292,9 @@ config PCI_DOMAINS > bool > depends on PCI > > +config PCI_DOMAINS_GENERIC > + def_bool PCI_DOMAINS > + > config PCI_NANOENGINE > bool "BSE nanoEngine PCI support" > depends on SA1100_NANOENGINE > diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h > index 7fc4278..f19f627 100644 > --- a/arch/arm/include/asm/mach/pci.h > +++ b/arch/arm/include/asm/mach/pci.h > @@ -44,9 +44,6 @@ struct hw_pci { > * Per-controller structure > */ > struct pci_sys_data { > -#ifdef CONFIG_PCI_DOMAINS > - int domain; > -#endif > struct list_head node; > int busnr; /* primary bus number */ > u64 mem_offset; /* bus->cpu memory mapping offset */ > diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h > index 7e95d85..585dc33 100644 > --- a/arch/arm/include/asm/pci.h > +++ b/arch/arm/include/asm/pci.h > @@ -18,13 +18,6 @@ static inline int pcibios_assign_all_busses(void) > } > > #ifdef CONFIG_PCI_DOMAINS > -static inline int pci_domain_nr(struct pci_bus *bus) > -{ > - struct pci_sys_data *root = bus->sysdata; > - > - return root->domain; > -} > - > static inline int pci_proc_domain(struct pci_bus *bus) > { > return pci_domain_nr(bus); > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index 17a26c1..d8c2b4e 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -11,6 +11,8 @@ > #include <linux/slab.h> > #include <linux/init.h> > #include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_pci.h> > > #include <asm/mach-types.h> > #include <asm/mach/map.h> > @@ -468,9 +470,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > if (!sys) > panic("PCI: unable to allocate sys data!"); > > -#ifdef CONFIG_PCI_DOMAINS > - sys->domain = hw->domain; > -#endif > sys->busnr = busnr; > sys->swizzle = hw->swizzle; > sys->map_irq = hw->map_irq; > @@ -511,6 +510,27 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > } > } > > +#ifdef CONFIG_PCI_DOMAINS_GENERIC > +static bool dt_domain_found; > + > +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 > + > void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > { > struct pci_sys_data *sys; > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index b1315e1..2b4c1f2 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -722,17 +722,11 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > { > struct mvebu_pcie *pcie = sys_to_pcie(sys); > int i; > - int domain = 0; > > -#ifdef CONFIG_PCI_DOMAINS > - domain = sys->domain; > -#endif > - > - snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", > - domain); > + snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", nr); > pcie->mem.name = pcie->mem_name; > > - snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", domain); > + snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", nr); > pcie->realio.name = pcie->io_name; > > if (request_resource(&iomem_resource, &pcie->mem)) > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index dfed00a..6790b87 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -502,9 +502,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > dw_pci.private_data = (void **)&pp; > > pci_common_init_dev(pp->dev, &dw_pci); > -#ifdef CONFIG_PCI_DOMAINS > - dw_pci.domain++; > -#endif > > return 0; > } > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 61158e0..b6b859e 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -404,9 +404,6 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie) > rcar_pci.private_data = (void **)&pcie; > > pci_common_init_dev(&pdev->dev, &rcar_pci); > -#ifdef CONFIG_PCI_DOMAINS > - rcar_pci.domain++; > -#endif > } > > static int phy_wait_for_ack(struct rcar_pcie *pcie) >
On Thu, Oct 30, 2014 at 11:55:31AM +0000, Arnd Bergmann wrote: > On Thursday 30 October 2014 11:44:48 Lorenzo Pieralisi wrote: > > Most if not all ARM PCI host controller device drivers either ignore the > > domain field in the pci_sys_data structure or just increment it every > > time a host controller is probed, using it as a domain counter. > > > > Therefore, instead of relying on pci_sys_data to stash the domain number > > in a standard location, ARM pcibios code can be moved to the newly > > introduced generic PCI domains code, implemented in commits: > > > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > ("PCI: Add generic domain handling") > > > > In order to assign a domain number dynamically, the ARM pcibios defines > > the function, called by core PCI code: > > > > void pci_bus_assign_domain_nr(...) > > > > that relies on a DT property to define the domain number or falls back to > > a counter; its usage replaces the current domain assignment code in PCI > > host controllers present in the kernel. > > > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Phil Edworthy <phil.edworthy@renesas.com> > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Cc: Jingoo Han <jg1.han@samsung.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Russell King <linux@arm.linux.org.uk> > > Cc: Mohit Kumar <mohit.kumar@st.com> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > Very nice! Thank you very much, I would be grateful if Jason, Jingoo, and Phil can give the patch a go on the respective platforms and hopefully I will add their tested-by tags. I guess pci_sys_data.add_bus pointer is the next to go :) Lorenzo
On Thu, Oct 30, 2014 at 12:27:13PM +0000, Yijing Wang wrote: > On 2014/10/30 19:44, Lorenzo Pieralisi wrote: > > Most if not all ARM PCI host controller device drivers either ignore the > > domain field in the pci_sys_data structure or just increment it every > > time a host controller is probed, using it as a domain counter. > > > > Therefore, instead of relying on pci_sys_data to stash the domain number > > in a standard location, ARM pcibios code can be moved to the newly > > introduced generic PCI domains code, implemented in commits: > > > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > ("PCI: Add generic domain handling") > > > > In order to assign a domain number dynamically, the ARM pcibios defines > > the function, called by core PCI code: > > > > void pci_bus_assign_domain_nr(...) > > > > that relies on a DT property to define the domain number or falls back to > > a counter; its usage replaces the current domain assignment code in PCI > > host controllers present in the kernel. > > Nice clean up. > > Reviewed-by: Yijing Wang <wangyijing@huawei.com> Thank you Yijing ! Lorenzo
Hi Lorenzo, On 30 October 2014 11:45, Lorenzo wrote: > Most if not all ARM PCI host controller device drivers either ignore the > domain field in the pci_sys_data structure or just increment it every > time a host controller is probed, using it as a domain counter. > > Therefore, instead of relying on pci_sys_data to stash the domain number > in a standard location, ARM pcibios code can be moved to the newly > introduced generic PCI domains code, implemented in commits: > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > ("PCI: Add generic domain handling") > > In order to assign a domain number dynamically, the ARM pcibios defines > the function, called by core PCI code: > > void pci_bus_assign_domain_nr(...) > > that relies on a DT property to define the domain number or falls back to > a counter; its usage replaces the current domain assignment code in PCI > host controllers present in the kernel. > > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Phil Edworthy <phil.edworthy@renesas.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Jingoo Han <jg1.han@samsung.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Mohit Kumar <mohit.kumar@st.com> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> This patch fixes a current problem with R-Car devices where there is an internal PCI bridge and an external PCIe bridge on the devices. Both drivers work independently but need to be on different domains. Just needed to enable PCI_DOMAINS along with this. I've done basic testing that the internal PCI and external PCIe work at the same time. Acked-by: Phil Edworthy <phil.edworthy@renesas.com> Thanks Phil > --- > arch/arm/Kconfig | 3 +++ > arch/arm/include/asm/mach/pci.h | 3 --- > arch/arm/include/asm/pci.h | 7 ------- > arch/arm/kernel/bios32.c | 26 +++++++++++++++++++++++--- > drivers/pci/host/pci-mvebu.c | 10 ++-------- > drivers/pci/host/pcie-designware.c | 3 --- > drivers/pci/host/pcie-rcar.c | 3 --- > 7 files changed, 28 insertions(+), 27 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 89c4b5c..29544f0 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1292,6 +1292,9 @@ config PCI_DOMAINS > bool > depends on PCI > > +config PCI_DOMAINS_GENERIC > + def_bool PCI_DOMAINS > + > config PCI_NANOENGINE > bool "BSE nanoEngine PCI support" > depends on SA1100_NANOENGINE > diff --git a/arch/arm/include/asm/mach/pci.h > b/arch/arm/include/asm/mach/pci.h > index 7fc4278..f19f627 100644 > --- a/arch/arm/include/asm/mach/pci.h > +++ b/arch/arm/include/asm/mach/pci.h > @@ -44,9 +44,6 @@ struct hw_pci { > * Per-controller structure > */ > struct pci_sys_data { > -#ifdef CONFIG_PCI_DOMAINS > - int domain; > -#endif > struct list_head node; > int busnr; /* primary bus number */ > u64 mem_offset; /* bus->cpu memory mapping offset */ > diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h > index 7e95d85..585dc33 100644 > --- a/arch/arm/include/asm/pci.h > +++ b/arch/arm/include/asm/pci.h > @@ -18,13 +18,6 @@ static inline int pcibios_assign_all_busses(void) > } > > #ifdef CONFIG_PCI_DOMAINS > -static inline int pci_domain_nr(struct pci_bus *bus) > -{ > - struct pci_sys_data *root = bus->sysdata; > - > - return root->domain; > -} > - > static inline int pci_proc_domain(struct pci_bus *bus) > { > return pci_domain_nr(bus); > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index 17a26c1..d8c2b4e 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -11,6 +11,8 @@ > #include <linux/slab.h> > #include <linux/init.h> > #include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_pci.h> > > #include <asm/mach-types.h> > #include <asm/mach/map.h> > @@ -468,9 +470,6 @@ static void pcibios_init_hw(struct device *parent, struct > hw_pci *hw, > if (!sys) > panic("PCI: unable to allocate sys data!"); > > -#ifdef CONFIG_PCI_DOMAINS > - sys->domain = hw->domain; > -#endif > sys->busnr = busnr; > sys->swizzle = hw->swizzle; > sys->map_irq = hw->map_irq; > @@ -511,6 +510,27 @@ static void pcibios_init_hw(struct device *parent, struct > hw_pci *hw, > } > } > > +#ifdef CONFIG_PCI_DOMAINS_GENERIC > +static bool dt_domain_found; > + > +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 > + > void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > { > struct pci_sys_data *sys; > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index b1315e1..2b4c1f2 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -722,17 +722,11 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data > *sys) > { > struct mvebu_pcie *pcie = sys_to_pcie(sys); > int i; > - int domain = 0; > > -#ifdef CONFIG_PCI_DOMAINS > - domain = sys->domain; > -#endif > - > - snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", > - domain); > + snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", > nr); > pcie->mem.name = pcie->mem_name; > > - snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", > domain); > + snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", nr); > pcie->realio.name = pcie->io_name; > > if (request_resource(&iomem_resource, &pcie->mem)) > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie- > designware.c > index dfed00a..6790b87 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -502,9 +502,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > dw_pci.private_data = (void **)&pp; > > pci_common_init_dev(pp->dev, &dw_pci); > -#ifdef CONFIG_PCI_DOMAINS > - dw_pci.domain++; > -#endif > > return 0; > } > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 61158e0..b6b859e 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -404,9 +404,6 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie) > rcar_pci.private_data = (void **)&pcie; > > pci_common_init_dev(&pdev->dev, &rcar_pci); > -#ifdef CONFIG_PCI_DOMAINS > - rcar_pci.domain++; > -#endif > } > > static int phy_wait_for_ack(struct rcar_pcie *pcie) > -- > 2.1.2 > > -- > 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 Fri, Oct 31, 2014 at 7:43 AM, Phil Edworthy <phil.edworthy@renesas.com> wrote: > Hi Lorenzo, > > On 30 October 2014 11:45, Lorenzo wrote: >> Most if not all ARM PCI host controller device drivers either ignore the >> domain field in the pci_sys_data structure or just increment it every >> time a host controller is probed, using it as a domain counter. >> >> Therefore, instead of relying on pci_sys_data to stash the domain number >> in a standard location, ARM pcibios code can be moved to the newly >> introduced generic PCI domains code, implemented in commits: >> >> commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 >> ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") >> >> commit 670ba0c8883b576d0aec28bd7a838358a4be1 >> ("PCI: Add generic domain handling") >> >> In order to assign a domain number dynamically, the ARM pcibios defines >> the function, called by core PCI code: >> >> void pci_bus_assign_domain_nr(...) >> >> that relies on a DT property to define the domain number or falls back to >> a counter; its usage replaces the current domain assignment code in PCI >> host controllers present in the kernel. >> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Cc: Phil Edworthy <phil.edworthy@renesas.com> >> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> Cc: Jingoo Han <jg1.han@samsung.com> >> Cc: Bjorn Helgaas <bhelgaas@google.com> >> Cc: Russell King <linux@arm.linux.org.uk> >> Cc: Mohit Kumar <mohit.kumar@st.com> >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > This patch fixes a current problem with R-Car devices where there is an > internal PCI bridge and an external PCIe bridge on the devices. Both drivers > work independently but need to be on different domains. Just needed to enable > PCI_DOMAINS along with this. > I've done basic testing that the internal PCI and external PCIe work at the > same time. Hi Phil, Thanks for testing this. Can you give me some more guidance on where you'd like to see this merged? Until your comment about this fixing a current problem on R-Car, I probably would have considered this to be a cleanup and enhancement and hence material for v3.19. But if R-Car is actually broken and this fixes it, maybe this should go in for v3.18 instead. If it is currently broken, is there a point where it broke? I assume it used to work at one time. If there's a commit that broke it, it would be nice to reference that in the changelog and explain exactly what was broken and how this fixes it. Bjorn
Hi Bjorn, On 31 October 2014 16:37, Bjorn wrote: > On Fri, Oct 31, 2014 at 7:43 AM, Phil Edworthy > <phil.edworthy@renesas.com> wrote: > > Hi Lorenzo, > > > > On 30 October 2014 11:45, Lorenzo wrote: > >> Most if not all ARM PCI host controller device drivers either ignore the > >> domain field in the pci_sys_data structure or just increment it every > >> time a host controller is probed, using it as a domain counter. > >> > >> Therefore, instead of relying on pci_sys_data to stash the domain number > >> in a standard location, ARM pcibios code can be moved to the newly > >> introduced generic PCI domains code, implemented in commits: > >> > >> commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > >> ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > >> > >> commit 670ba0c8883b576d0aec28bd7a838358a4be1 > >> ("PCI: Add generic domain handling") > >> > >> In order to assign a domain number dynamically, the ARM pcibios defines > >> the function, called by core PCI code: > >> > >> void pci_bus_assign_domain_nr(...) > >> > >> that relies on a DT property to define the domain number or falls back to > >> a counter; its usage replaces the current domain assignment code in PCI > >> host controllers present in the kernel. > >> > >> Cc: Arnd Bergmann <arnd@arndb.de> > >> Cc: Phil Edworthy <phil.edworthy@renesas.com> > >> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > >> Cc: Jingoo Han <jg1.han@samsung.com> > >> Cc: Bjorn Helgaas <bhelgaas@google.com> > >> Cc: Russell King <linux@arm.linux.org.uk> > >> Cc: Mohit Kumar <mohit.kumar@st.com> > >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > This patch fixes a current problem with R-Car devices where there is an > > internal PCI bridge and an external PCIe bridge on the devices. Both drivers > > work independently but need to be on different domains. Just needed to > enable > > PCI_DOMAINS along with this. > > I've done basic testing that the internal PCI and external PCIe work at the > > same time. > > Hi Phil, > > Thanks for testing this. Can you give me some more guidance on where > you'd like to see this merged? Until your comment about this fixing a > current problem on R-Car, I probably would have considered this to be > a cleanup and enhancement and hence material for v3.19. But if R-Car > is actually broken and this fixes it, maybe this should go in for > v3.18 instead. I don’t think its urgent as most of our customers use LTSI kernels, e.g. v3.10. Renesas typically provide out-of-tree BSPs with upstream code back ported, along with other patches. Simon Horman (Renesas maintainer, cc'd) generally handles the back ports, so I'll defer to his opinion. > If it is currently broken, is there a point where it broke? I assume > it used to work at one time. If there's a commit that broke it, it > would be nice to reference that in the changelog and explain exactly > what was broken and how this fixes it. Both the internal PCI and external PCIe drivers work on their own, but have never worked at the same time. I think it was just unfortunate timing when I added the PCIe driver. At that time, the internal PCI driver didn’t have the relevant DT nodes for the board I was using so I didn't see any conflicts. Thanks Phil
On Thursday, October 30, 2014 8:45 PM, Lorenzo Pieralisi wrote: > > Most if not all ARM PCI host controller device drivers either ignore the > domain field in the pci_sys_data structure or just increment it every > time a host controller is probed, using it as a domain counter. > > Therefore, instead of relying on pci_sys_data to stash the domain number > in a standard location, ARM pcibios code can be moved to the newly > introduced generic PCI domains code, implemented in commits: > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > ("PCI: Add generic domain handling") > > In order to assign a domain number dynamically, the ARM pcibios defines > the function, called by core PCI code: > > void pci_bus_assign_domain_nr(...) > > that relies on a DT property to define the domain number or falls back to > a counter; its usage replaces the current domain assignment code in PCI > host controllers present in the kernel. > > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Phil Edworthy <phil.edworthy@renesas.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Jingoo Han <jg1.han@samsung.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Mohit Kumar <mohit.kumar@st.com> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> (+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri) Acked-by: Jingoo Han <jg1.han@samsung.com> For a long time, I have wanted the patch like this. Personally, I want to increase the build coverage and keep the same binary regardless of config options as possible; thus, I don't like #ifdef guards such as '#ifdef CONFIG_PCI_DOMAINS'. When I submitted the first patch for 'pcie-designware.c', I was not able to find a way to remove '#ifdef CONFIG_PCI_DOMAINS' from 'pcie-designware.c'. This patch looks good and I think that this patch will not make the problem. However, I didn't test this patch with Exynos platform, due to some reasons. So, I hope that other engineers test this patch on other SoC platforms related to designware PCIe. To Mohit Kumar (ST SPEAR13XX), Richard Zhu/Lucas Stach (IMX6) Kishon Vijay Abraham I (TI DRA7XX), Murali Karicheri (TI KEYSTONE) Would you test this patch and share the result? It would be very helpful. Thank you. Best regards, Jingoo Han > --- > arch/arm/Kconfig | 3 +++ > arch/arm/include/asm/mach/pci.h | 3 --- > arch/arm/include/asm/pci.h | 7 ------- > arch/arm/kernel/bios32.c | 26 +++++++++++++++++++++++--- > drivers/pci/host/pci-mvebu.c | 10 ++-------- > drivers/pci/host/pcie-designware.c | 3 --- > drivers/pci/host/pcie-rcar.c | 3 --- > 7 files changed, 28 insertions(+), 27 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 89c4b5c..29544f0 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1292,6 +1292,9 @@ config PCI_DOMAINS > bool > depends on PCI > > +config PCI_DOMAINS_GENERIC > + def_bool PCI_DOMAINS > + > config PCI_NANOENGINE > bool "BSE nanoEngine PCI support" > depends on SA1100_NANOENGINE > diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h > index 7fc4278..f19f627 100644 > --- a/arch/arm/include/asm/mach/pci.h > +++ b/arch/arm/include/asm/mach/pci.h > @@ -44,9 +44,6 @@ struct hw_pci { > * Per-controller structure > */ > struct pci_sys_data { > -#ifdef CONFIG_PCI_DOMAINS > - int domain; > -#endif > struct list_head node; > int busnr; /* primary bus number */ > u64 mem_offset; /* bus->cpu memory mapping offset */ > diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h > index 7e95d85..585dc33 100644 > --- a/arch/arm/include/asm/pci.h > +++ b/arch/arm/include/asm/pci.h > @@ -18,13 +18,6 @@ static inline int pcibios_assign_all_busses(void) > } > > #ifdef CONFIG_PCI_DOMAINS > -static inline int pci_domain_nr(struct pci_bus *bus) > -{ > - struct pci_sys_data *root = bus->sysdata; > - > - return root->domain; > -} > - > static inline int pci_proc_domain(struct pci_bus *bus) > { > return pci_domain_nr(bus); > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index 17a26c1..d8c2b4e 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -11,6 +11,8 @@ > #include <linux/slab.h> > #include <linux/init.h> > #include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_pci.h> > > #include <asm/mach-types.h> > #include <asm/mach/map.h> > @@ -468,9 +470,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > if (!sys) > panic("PCI: unable to allocate sys data!"); > > -#ifdef CONFIG_PCI_DOMAINS > - sys->domain = hw->domain; > -#endif > sys->busnr = busnr; > sys->swizzle = hw->swizzle; > sys->map_irq = hw->map_irq; > @@ -511,6 +510,27 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > } > } > > +#ifdef CONFIG_PCI_DOMAINS_GENERIC > +static bool dt_domain_found; > + > +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 > + > void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > { > struct pci_sys_data *sys; > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index b1315e1..2b4c1f2 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -722,17 +722,11 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > { > struct mvebu_pcie *pcie = sys_to_pcie(sys); > int i; > - int domain = 0; > > -#ifdef CONFIG_PCI_DOMAINS > - domain = sys->domain; > -#endif > - > - snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", > - domain); > + snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", nr); > pcie->mem.name = pcie->mem_name; > > - snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", domain); > + snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", nr); > pcie->realio.name = pcie->io_name; > > if (request_resource(&iomem_resource, &pcie->mem)) > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index dfed00a..6790b87 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -502,9 +502,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > dw_pci.private_data = (void **)&pp; > > pci_common_init_dev(pp->dev, &dw_pci); > -#ifdef CONFIG_PCI_DOMAINS > - dw_pci.domain++; > -#endif > > return 0; > } > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 61158e0..b6b859e 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -404,9 +404,6 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie) > rcar_pci.private_data = (void **)&pcie; > > pci_common_init_dev(&pdev->dev, &rcar_pci); > -#ifdef CONFIG_PCI_DOMAINS > - rcar_pci.domain++; > -#endif > } > > static int phy_wait_for_ack(struct rcar_pcie *pcie) > -- > 2.1.2
>-----Original Message----- >From: Jingoo Han [mailto:jg1.han@samsung.com] >Sent: Sunday, November 02, 2014 8:18 PM >To: 'Lorenzo Pieralisi' >Cc: linux-arm-kernel@lists.infradead.org; linux-pci@vger.kernel.org; liviu.dudau@arm.com; >'Arnd Bergmann'; 'Phil Edworthy'; 'Jason Gunthorpe'; 'Bjorn Helgaas'; 'Russell King'; 'Mohit >Kumar'; 'Richard Zhu'; 'Lucas Stach'; KISHON VIJAY ABRAHAM; Karicheri, Muralidharan; >'Jingoo Han' >Subject: Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains > >On Thursday, October 30, 2014 8:45 PM, Lorenzo Pieralisi wrote: >> >> Most if not all ARM PCI host controller device drivers either ignore >> the domain field in the pci_sys_data structure or just increment it >> every time a host controller is probed, using it as a domain counter. >> >> Therefore, instead of relying on pci_sys_data to stash the domain >> number in a standard location, ARM pcibios code can be moved to the >> newly introduced generic PCI domains code, implemented in commits: >> >> commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 >> ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") >> >> commit 670ba0c8883b576d0aec28bd7a838358a4be1 >> ("PCI: Add generic domain handling") >> >> In order to assign a domain number dynamically, the ARM pcibios >> defines the function, called by core PCI code: >> >> void pci_bus_assign_domain_nr(...) >> >> that relies on a DT property to define the domain number or falls back >> to a counter; its usage replaces the current domain assignment code in >> PCI host controllers present in the kernel. >> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Cc: Phil Edworthy <phil.edworthy@renesas.com> >> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> Cc: Jingoo Han <jg1.han@samsung.com> >> Cc: Bjorn Helgaas <bhelgaas@google.com> >> Cc: Russell King <linux@arm.linux.org.uk> >> Cc: Mohit Kumar <mohit.kumar@st.com> >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > >(+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri) > >Acked-by: Jingoo Han <jg1.han@samsung.com> > >For a long time, I have wanted the patch like this. Personally, I want to increase the build >coverage and keep the same binary regardless of config options as possible; thus, I don't >like #ifdef guards such as '#ifdef CONFIG_PCI_DOMAINS'. When I submitted the first patch >for 'pcie-designware.c', I was not able to find a way to remove '#ifdef >CONFIG_PCI_DOMAINS' from 'pcie-designware.c'. > >This patch looks good and I think that this patch will not make the problem. However, I >didn't test this patch with Exynos platform, due to some reasons. So, I hope that other >engineers test this patch on other SoC platforms related to designware PCIe. > >To Mohit Kumar (ST SPEAR13XX), Richard Zhu/Lucas Stach (IMX6) > Kishon Vijay Abraham I (TI DRA7XX), Murali Karicheri (TI KEYSTONE) > Jingoo, I will be able to try this sometime after 11/9 as I am on travel. Regards, Murali >Would you test this patch and share the result? >It would be very helpful. Thank you. > >Best regards, >Jingoo Han > >> --- >> arch/arm/Kconfig | 3 +++ >> arch/arm/include/asm/mach/pci.h | 3 --- >> arch/arm/include/asm/pci.h | 7 ------- >> arch/arm/kernel/bios32.c | 26 +++++++++++++++++++++++--- >> drivers/pci/host/pci-mvebu.c | 10 ++-------- >> drivers/pci/host/pcie-designware.c | 3 --- >> drivers/pci/host/pcie-rcar.c | 3 --- >> 7 files changed, 28 insertions(+), 27 deletions(-) >> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index >> 89c4b5c..29544f0 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -1292,6 +1292,9 @@ config PCI_DOMAINS >> bool >> depends on PCI >> >> +config PCI_DOMAINS_GENERIC >> + def_bool PCI_DOMAINS >> + >> config PCI_NANOENGINE >> bool "BSE nanoEngine PCI support" >> depends on SA1100_NANOENGINE >> diff --git a/arch/arm/include/asm/mach/pci.h >> b/arch/arm/include/asm/mach/pci.h index 7fc4278..f19f627 100644 >> --- a/arch/arm/include/asm/mach/pci.h >> +++ b/arch/arm/include/asm/mach/pci.h >> @@ -44,9 +44,6 @@ struct hw_pci { >> * Per-controller structure >> */ >> struct pci_sys_data { >> -#ifdef CONFIG_PCI_DOMAINS >> - int domain; >> -#endif >> struct list_head node; >> int busnr; /* primary bus number */ >> u64 mem_offset; /* bus->cpu memory mapping offset */ >> diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h >> index 7e95d85..585dc33 100644 >> --- a/arch/arm/include/asm/pci.h >> +++ b/arch/arm/include/asm/pci.h >> @@ -18,13 +18,6 @@ static inline int pcibios_assign_all_busses(void) >> } >> >> #ifdef CONFIG_PCI_DOMAINS >> -static inline int pci_domain_nr(struct pci_bus *bus) -{ >> - struct pci_sys_data *root = bus->sysdata; >> - >> - return root->domain; >> -} >> - >> static inline int pci_proc_domain(struct pci_bus *bus) { >> return pci_domain_nr(bus); >> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c index >> 17a26c1..d8c2b4e 100644 >> --- a/arch/arm/kernel/bios32.c >> +++ b/arch/arm/kernel/bios32.c >> @@ -11,6 +11,8 @@ >> #include <linux/slab.h> >> #include <linux/init.h> >> #include <linux/io.h> >> +#include <linux/of.h> >> +#include <linux/of_pci.h> >> >> #include <asm/mach-types.h> >> #include <asm/mach/map.h> >> @@ -468,9 +470,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci >*hw, >> if (!sys) >> panic("PCI: unable to allocate sys data!"); >> >> -#ifdef CONFIG_PCI_DOMAINS >> - sys->domain = hw->domain; >> -#endif >> sys->busnr = busnr; >> sys->swizzle = hw->swizzle; >> sys->map_irq = hw->map_irq; >> @@ -511,6 +510,27 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci >*hw, >> } >> } >> >> +#ifdef CONFIG_PCI_DOMAINS_GENERIC >> +static bool dt_domain_found; >> + >> +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 >> + >> void pci_common_init_dev(struct device *parent, struct hw_pci *hw) { >> struct pci_sys_data *sys; >> diff --git a/drivers/pci/host/pci-mvebu.c >> b/drivers/pci/host/pci-mvebu.c index b1315e1..2b4c1f2 100644 >> --- a/drivers/pci/host/pci-mvebu.c >> +++ b/drivers/pci/host/pci-mvebu.c >> @@ -722,17 +722,11 @@ static int mvebu_pcie_setup(int nr, struct >> pci_sys_data *sys) { >> struct mvebu_pcie *pcie = sys_to_pcie(sys); >> int i; >> - int domain = 0; >> >> -#ifdef CONFIG_PCI_DOMAINS >> - domain = sys->domain; >> -#endif >> - >> - snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", >> - domain); >> + snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", >> +nr); >> pcie->mem.name = pcie->mem_name; >> >> - snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", domain); >> + snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", nr); >> pcie->realio.name = pcie->io_name; >> >> if (request_resource(&iomem_resource, &pcie->mem)) diff --git >> a/drivers/pci/host/pcie-designware.c >> b/drivers/pci/host/pcie-designware.c >> index dfed00a..6790b87 100644 >> --- a/drivers/pci/host/pcie-designware.c >> +++ b/drivers/pci/host/pcie-designware.c >> @@ -502,9 +502,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) >> dw_pci.private_data = (void **)&pp; >> >> pci_common_init_dev(pp->dev, &dw_pci); -#ifdef CONFIG_PCI_DOMAINS >> - dw_pci.domain++; >> -#endif >> >> return 0; >> } >> diff --git a/drivers/pci/host/pcie-rcar.c >> b/drivers/pci/host/pcie-rcar.c index 61158e0..b6b859e 100644 >> --- a/drivers/pci/host/pcie-rcar.c >> +++ b/drivers/pci/host/pcie-rcar.c >> @@ -404,9 +404,6 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie) >> rcar_pci.private_data = (void **)&pcie; >> >> pci_common_init_dev(&pdev->dev, &rcar_pci); -#ifdef >> CONFIG_PCI_DOMAINS >> - rcar_pci.domain++; >> -#endif >> } >> >> static int phy_wait_for_ack(struct rcar_pcie *pcie) >> -- >> 2.1.2
On 2014/10/30 19:44, Lorenzo Pieralisi wrote: > Most if not all ARM PCI host controller device drivers either ignore the > domain field in the pci_sys_data structure or just increment it every > time a host controller is probed, using it as a domain counter. > > Therefore, instead of relying on pci_sys_data to stash the domain number > in a standard location, ARM pcibios code can be moved to the newly > introduced generic PCI domains code, implemented in commits: > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > ("PCI: Add generic domain handling") > > In order to assign a domain number dynamically, the ARM pcibios defines > the function, called by core PCI code: > > void pci_bus_assign_domain_nr(...) > > that relies on a DT property to define the domain number or falls back to > a counter; its usage replaces the current domain assignment code in PCI > host controllers present in the kernel. > > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Phil Edworthy <phil.edworthy@renesas.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Jingoo Han <jg1.han@samsung.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Mohit Kumar <mohit.kumar@st.com> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > --- > arch/arm/Kconfig | 3 +++ > arch/arm/include/asm/mach/pci.h | 3 --- > arch/arm/include/asm/pci.h | 7 ------- > arch/arm/kernel/bios32.c | 26 +++++++++++++++++++++++--- > drivers/pci/host/pci-mvebu.c | 10 ++-------- > drivers/pci/host/pcie-designware.c | 3 --- > drivers/pci/host/pcie-rcar.c | 3 --- > 7 files changed, 28 insertions(+), 27 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 89c4b5c..29544f0 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1292,6 +1292,9 @@ config PCI_DOMAINS > bool > depends on PCI > > +config PCI_DOMAINS_GENERIC > + def_bool PCI_DOMAINS > + > config PCI_NANOENGINE > bool "BSE nanoEngine PCI support" > depends on SA1100_NANOENGINE > diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h > index 7fc4278..f19f627 100644 > --- a/arch/arm/include/asm/mach/pci.h > +++ b/arch/arm/include/asm/mach/pci.h > @@ -44,9 +44,6 @@ struct hw_pci { > * Per-controller structure > */ domain in hw_pci also should be clean up, right ? struct hw_pci { #ifdef CONFIG_PCI_DOMAINS int domain; #endif struct pci_ops *ops; int nr_controllers; void **private_data; > struct pci_sys_data { > -#ifdef CONFIG_PCI_DOMAINS > - int domain; > -#endif > struct list_head node; > int busnr; /* primary bus number */ > u64 mem_offset; /* bus->cpu memory mapping offset */ > diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h > index 7e95d85..585dc33 100644 > --- a/arch/arm/include/asm/pci.h > +++ b/arch/arm/include/asm/pci.h > @@ -18,13 +18,6 @@ static inline int pcibios_assign_all_busses(void) > } > > #ifdef CONFIG_PCI_DOMAINS > -static inline int pci_domain_nr(struct pci_bus *bus) > -{ > - struct pci_sys_data *root = bus->sysdata; > - > - return root->domain; > -} > - > static inline int pci_proc_domain(struct pci_bus *bus) > { > return pci_domain_nr(bus); > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index 17a26c1..d8c2b4e 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -11,6 +11,8 @@ > #include <linux/slab.h> > #include <linux/init.h> > #include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_pci.h> > > #include <asm/mach-types.h> > #include <asm/mach/map.h> > @@ -468,9 +470,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > if (!sys) > panic("PCI: unable to allocate sys data!"); > > -#ifdef CONFIG_PCI_DOMAINS > - sys->domain = hw->domain; > -#endif > sys->busnr = busnr; > sys->swizzle = hw->swizzle; > sys->map_irq = hw->map_irq; > @@ -511,6 +510,27 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > } > } > > +#ifdef CONFIG_PCI_DOMAINS_GENERIC > +static bool dt_domain_found; > + > +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 > + > void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > { > struct pci_sys_data *sys; > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index b1315e1..2b4c1f2 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -722,17 +722,11 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > { > struct mvebu_pcie *pcie = sys_to_pcie(sys); > int i; > - int domain = 0; > > -#ifdef CONFIG_PCI_DOMAINS > - domain = sys->domain; > -#endif > - > - snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", > - domain); > + snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", nr); > pcie->mem.name = pcie->mem_name; > > - snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", domain); > + snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", nr); > pcie->realio.name = pcie->io_name; > > if (request_resource(&iomem_resource, &pcie->mem)) > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index dfed00a..6790b87 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -502,9 +502,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > dw_pci.private_data = (void **)&pp; > > pci_common_init_dev(pp->dev, &dw_pci); > -#ifdef CONFIG_PCI_DOMAINS > - dw_pci.domain++; > -#endif > > return 0; > } > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 61158e0..b6b859e 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -404,9 +404,6 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie) > rcar_pci.private_data = (void **)&pcie; > > pci_common_init_dev(&pdev->dev, &rcar_pci); > -#ifdef CONFIG_PCI_DOMAINS > - rcar_pci.domain++; > -#endif > } > > static int phy_wait_for_ack(struct rcar_pcie *pcie) >
On Mon, Nov 03, 2014 at 03:48:23AM +0000, Yijing Wang wrote: > On 2014/10/30 19:44, Lorenzo Pieralisi wrote: [...] > > diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h > > index 7fc4278..f19f627 100644 > > --- a/arch/arm/include/asm/mach/pci.h > > +++ b/arch/arm/include/asm/mach/pci.h > > @@ -44,9 +44,6 @@ struct hw_pci { > > * Per-controller structure > > */ > > domain in hw_pci also should be clean up, right ? > > > struct hw_pci { > #ifdef CONFIG_PCI_DOMAINS > int domain; > #endif > struct pci_ops *ops; > int nr_controllers; > void **private_data; Yes, thanks for spotting that. Lorenzo
Hi Phil, On Fri, Oct 31, 2014 at 01:43:49PM +0000, Phil Edworthy wrote: > Hi Lorenzo, > > On 30 October 2014 11:45, Lorenzo wrote: > > Most if not all ARM PCI host controller device drivers either ignore the > > domain field in the pci_sys_data structure or just increment it every > > time a host controller is probed, using it as a domain counter. > > > > Therefore, instead of relying on pci_sys_data to stash the domain number > > in a standard location, ARM pcibios code can be moved to the newly > > introduced generic PCI domains code, implemented in commits: > > > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > ("PCI: Add generic domain handling") > > > > In order to assign a domain number dynamically, the ARM pcibios defines > > the function, called by core PCI code: > > > > void pci_bus_assign_domain_nr(...) > > > > that relies on a DT property to define the domain number or falls back to > > a counter; its usage replaces the current domain assignment code in PCI > > host controllers present in the kernel. > > > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Phil Edworthy <phil.edworthy@renesas.com> > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Cc: Jingoo Han <jg1.han@samsung.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Russell King <linux@arm.linux.org.uk> > > Cc: Mohit Kumar <mohit.kumar@st.com> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > This patch fixes a current problem with R-Car devices where there is an > internal PCI bridge and an external PCIe bridge on the devices. Both drivers > work independently but need to be on different domains. Just needed to enable > PCI_DOMAINS along with this. > I've done basic testing that the internal PCI and external PCIe work at the > same time. > > Acked-by: Phil Edworthy <phil.edworthy@renesas.com> Thank you very much. Lorenzo
On Mon, Nov 03, 2014 at 01:18:12AM +0000, Jingoo Han wrote: > On Thursday, October 30, 2014 8:45 PM, Lorenzo Pieralisi wrote: > > > > Most if not all ARM PCI host controller device drivers either ignore the > > domain field in the pci_sys_data structure or just increment it every > > time a host controller is probed, using it as a domain counter. > > > > Therefore, instead of relying on pci_sys_data to stash the domain number > > in a standard location, ARM pcibios code can be moved to the newly > > introduced generic PCI domains code, implemented in commits: > > > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > ("PCI: Add generic domain handling") > > > > In order to assign a domain number dynamically, the ARM pcibios defines > > the function, called by core PCI code: > > > > void pci_bus_assign_domain_nr(...) > > > > that relies on a DT property to define the domain number or falls back to > > a counter; its usage replaces the current domain assignment code in PCI > > host controllers present in the kernel. > > > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Phil Edworthy <phil.edworthy@renesas.com> > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Cc: Jingoo Han <jg1.han@samsung.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Russell King <linux@arm.linux.org.uk> > > Cc: Mohit Kumar <mohit.kumar@st.com> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > (+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri) > > Acked-by: Jingoo Han <jg1.han@samsung.com> Thank you Jingoo. > For a long time, I have wanted the patch like this. Personally, > I want to increase the build coverage and keep the same binary > regardless of config options as possible; thus, I don't like > #ifdef guards such as '#ifdef CONFIG_PCI_DOMAINS'. When I submitted > the first patch for 'pcie-designware.c', I was not able to find > a way to remove '#ifdef CONFIG_PCI_DOMAINS' from 'pcie-designware.c'. > > This patch looks good and I think that this patch will not make > the problem. However, I didn't test this patch with Exynos platform, > due to some reasons. So, I hope that other engineers test this patch > on other SoC platforms related to designware PCIe. > > To Mohit Kumar (ST SPEAR13XX), Richard Zhu/Lucas Stach (IMX6) > Kishon Vijay Abraham I (TI DRA7XX), Murali Karicheri (TI KEYSTONE) > > Would you test this patch and share the result? > It would be very helpful. Thank you. I am happy to wait for a week or two before asking to add this code to -next, it would be definitely great to get more coverage on pcie-designware. Thanks, Lorenzo > Best regards, > Jingoo Han > > > --- > > arch/arm/Kconfig | 3 +++ > > arch/arm/include/asm/mach/pci.h | 3 --- > > arch/arm/include/asm/pci.h | 7 ------- > > arch/arm/kernel/bios32.c | 26 +++++++++++++++++++++++--- > > drivers/pci/host/pci-mvebu.c | 10 ++-------- > > drivers/pci/host/pcie-designware.c | 3 --- > > drivers/pci/host/pcie-rcar.c | 3 --- > > 7 files changed, 28 insertions(+), 27 deletions(-) > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > > index 89c4b5c..29544f0 100644 > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -1292,6 +1292,9 @@ config PCI_DOMAINS > > bool > > depends on PCI > > > > +config PCI_DOMAINS_GENERIC > > + def_bool PCI_DOMAINS > > + > > config PCI_NANOENGINE > > bool "BSE nanoEngine PCI support" > > depends on SA1100_NANOENGINE > > diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h > > index 7fc4278..f19f627 100644 > > --- a/arch/arm/include/asm/mach/pci.h > > +++ b/arch/arm/include/asm/mach/pci.h > > @@ -44,9 +44,6 @@ struct hw_pci { > > * Per-controller structure > > */ > > struct pci_sys_data { > > -#ifdef CONFIG_PCI_DOMAINS > > - int domain; > > -#endif > > struct list_head node; > > int busnr; /* primary bus number */ > > u64 mem_offset; /* bus->cpu memory mapping offset */ > > diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h > > index 7e95d85..585dc33 100644 > > --- a/arch/arm/include/asm/pci.h > > +++ b/arch/arm/include/asm/pci.h > > @@ -18,13 +18,6 @@ static inline int pcibios_assign_all_busses(void) > > } > > > > #ifdef CONFIG_PCI_DOMAINS > > -static inline int pci_domain_nr(struct pci_bus *bus) > > -{ > > - struct pci_sys_data *root = bus->sysdata; > > - > > - return root->domain; > > -} > > - > > static inline int pci_proc_domain(struct pci_bus *bus) > > { > > return pci_domain_nr(bus); > > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > > index 17a26c1..d8c2b4e 100644 > > --- a/arch/arm/kernel/bios32.c > > +++ b/arch/arm/kernel/bios32.c > > @@ -11,6 +11,8 @@ > > #include <linux/slab.h> > > #include <linux/init.h> > > #include <linux/io.h> > > +#include <linux/of.h> > > +#include <linux/of_pci.h> > > > > #include <asm/mach-types.h> > > #include <asm/mach/map.h> > > @@ -468,9 +470,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > > if (!sys) > > panic("PCI: unable to allocate sys data!"); > > > > -#ifdef CONFIG_PCI_DOMAINS > > - sys->domain = hw->domain; > > -#endif > > sys->busnr = busnr; > > sys->swizzle = hw->swizzle; > > sys->map_irq = hw->map_irq; > > @@ -511,6 +510,27 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > > } > > } > > > > +#ifdef CONFIG_PCI_DOMAINS_GENERIC > > +static bool dt_domain_found; > > + > > +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 > > + > > void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > > { > > struct pci_sys_data *sys; > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > > index b1315e1..2b4c1f2 100644 > > --- a/drivers/pci/host/pci-mvebu.c > > +++ b/drivers/pci/host/pci-mvebu.c > > @@ -722,17 +722,11 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > > { > > struct mvebu_pcie *pcie = sys_to_pcie(sys); > > int i; > > - int domain = 0; > > > > -#ifdef CONFIG_PCI_DOMAINS > > - domain = sys->domain; > > -#endif > > - > > - snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", > > - domain); > > + snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", nr); > > pcie->mem.name = pcie->mem_name; > > > > - snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", domain); > > + snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", nr); > > pcie->realio.name = pcie->io_name; > > > > if (request_resource(&iomem_resource, &pcie->mem)) > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > > index dfed00a..6790b87 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -502,9 +502,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > dw_pci.private_data = (void **)&pp; > > > > pci_common_init_dev(pp->dev, &dw_pci); > > -#ifdef CONFIG_PCI_DOMAINS > > - dw_pci.domain++; > > -#endif > > > > return 0; > > } > > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > > index 61158e0..b6b859e 100644 > > --- a/drivers/pci/host/pcie-rcar.c > > +++ b/drivers/pci/host/pcie-rcar.c > > @@ -404,9 +404,6 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie) > > rcar_pci.private_data = (void **)&pcie; > > > > pci_common_init_dev(&pdev->dev, &rcar_pci); > > -#ifdef CONFIG_PCI_DOMAINS > > - rcar_pci.domain++; > > -#endif > > } > > > > static int phy_wait_for_ack(struct rcar_pcie *pcie) > > -- > > 2.1.2 > > -- > 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 >
Am Montag, den 03.11.2014, 11:23 +0000 schrieb Lorenzo Pieralisi: > On Mon, Nov 03, 2014 at 01:18:12AM +0000, Jingoo Han wrote: > > On Thursday, October 30, 2014 8:45 PM, Lorenzo Pieralisi wrote: > > > > > > Most if not all ARM PCI host controller device drivers either ignore the > > > domain field in the pci_sys_data structure or just increment it every > > > time a host controller is probed, using it as a domain counter. > > > > > > Therefore, instead of relying on pci_sys_data to stash the domain number > > > in a standard location, ARM pcibios code can be moved to the newly > > > introduced generic PCI domains code, implemented in commits: > > > > > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > > > > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > > ("PCI: Add generic domain handling") > > > > > > In order to assign a domain number dynamically, the ARM pcibios defines > > > the function, called by core PCI code: > > > > > > void pci_bus_assign_domain_nr(...) > > > > > > that relies on a DT property to define the domain number or falls back to > > > a counter; its usage replaces the current domain assignment code in PCI > > > host controllers present in the kernel. > > > > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > Cc: Phil Edworthy <phil.edworthy@renesas.com> > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > Cc: Jingoo Han <jg1.han@samsung.com> > > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > > Cc: Russell King <linux@arm.linux.org.uk> > > > Cc: Mohit Kumar <mohit.kumar@st.com> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > (+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri) > > > > Acked-by: Jingoo Han <jg1.han@samsung.com> > > Thank you Jingoo. > > > For a long time, I have wanted the patch like this. Personally, > > I want to increase the build coverage and keep the same binary > > regardless of config options as possible; thus, I don't like > > #ifdef guards such as '#ifdef CONFIG_PCI_DOMAINS'. When I submitted > > the first patch for 'pcie-designware.c', I was not able to find > > a way to remove '#ifdef CONFIG_PCI_DOMAINS' from 'pcie-designware.c'. > > > > This patch looks good and I think that this patch will not make > > the problem. However, I didn't test this patch with Exynos platform, > > due to some reasons. So, I hope that other engineers test this patch > > on other SoC platforms related to designware PCIe. > > > > To Mohit Kumar (ST SPEAR13XX), Richard Zhu/Lucas Stach (IMX6) > > Kishon Vijay Abraham I (TI DRA7XX), Murali Karicheri (TI KEYSTONE) > > > > Would you test this patch and share the result? > > It would be very helpful. Thank you. > > I am happy to wait for a week or two before asking to add this code to -next, > it would be definitely great to get more coverage on pcie-designware. > I've just tested this on i.MX6 on top of 3.18-rc3. Given that we only use a single domain there this patch should obviously not break anything. Testing backs this claim, so: Tested-by: Lucas Stach <l.stach@pengutronix.de> I also looked through the patch briefly and it seems sane to me, so this is also: Acked-by: Lucas Stach <l.stach@pengutronix.de> I don't like the fact that a proper binding documentation is missing for the "linux,pci-domain" property, but this isn't the fault of this patchset. Regards, Lucas
On Monday, November 03, 2014 8:34 PM, Lucas Stach wrote: > Am Montag, den 03.11.2014, 11:23 +0000 schrieb Lorenzo Pieralisi: > > On Mon, Nov 03, 2014 at 01:18:12AM +0000, Jingoo Han wrote: > > > On Thursday, October 30, 2014 8:45 PM, Lorenzo Pieralisi wrote: > > > > > > > > Most if not all ARM PCI host controller device drivers either ignore the > > > > domain field in the pci_sys_data structure or just increment it every > > > > time a host controller is probed, using it as a domain counter. > > > > > > > > Therefore, instead of relying on pci_sys_data to stash the domain number > > > > in a standard location, ARM pcibios code can be moved to the newly > > > > introduced generic PCI domains code, implemented in commits: > > > > > > > > commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > > > ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > > > > > > > commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > > > ("PCI: Add generic domain handling") > > > > > > > > In order to assign a domain number dynamically, the ARM pcibios defines > > > > the function, called by core PCI code: > > > > > > > > void pci_bus_assign_domain_nr(...) > > > > > > > > that relies on a DT property to define the domain number or falls back to > > > > a counter; its usage replaces the current domain assignment code in PCI > > > > host controllers present in the kernel. > > > > > > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > > Cc: Phil Edworthy <phil.edworthy@renesas.com> > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > Cc: Jingoo Han <jg1.han@samsung.com> > > > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > > > Cc: Russell King <linux@arm.linux.org.uk> > > > > Cc: Mohit Kumar <mohit.kumar@st.com> > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > > > (+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri) > > > > > > Acked-by: Jingoo Han <jg1.han@samsung.com> > > > > Thank you Jingoo. > > > > > For a long time, I have wanted the patch like this. Personally, > > > I want to increase the build coverage and keep the same binary > > > regardless of config options as possible; thus, I don't like > > > #ifdef guards such as '#ifdef CONFIG_PCI_DOMAINS'. When I submitted > > > the first patch for 'pcie-designware.c', I was not able to find > > > a way to remove '#ifdef CONFIG_PCI_DOMAINS' from 'pcie-designware.c'. > > > > > > This patch looks good and I think that this patch will not make > > > the problem. However, I didn't test this patch with Exynos platform, > > > due to some reasons. So, I hope that other engineers test this patch > > > on other SoC platforms related to designware PCIe. > > > > > > To Mohit Kumar (ST SPEAR13XX), Richard Zhu/Lucas Stach (IMX6) > > > Kishon Vijay Abraham I (TI DRA7XX), Murali Karicheri (TI KEYSTONE) > > > > > > Would you test this patch and share the result? > > > It would be very helpful. Thank you. > > > > I am happy to wait for a week or two before asking to add this code to -next, > > it would be definitely great to get more coverage on pcie-designware. > > > I've just tested this on i.MX6 on top of 3.18-rc3. Given that we only > use a single domain there this patch should obviously not break > anything. Testing backs this claim, so: > > Tested-by: Lucas Stach <l.stach@pengutronix.de> Thank you for your testing! In fact, Exynos uses multi domain structures. > > I also looked through the patch briefly and it seems sane to me, so this > is also: > > Acked-by: Lucas Stach <l.stach@pengutronix.de> > > I don't like the fact that a proper binding documentation is missing for > the "linux,pci-domain" property, but this isn't the fault of this > patchset. I agree with your opinion. I also thought that the binding documentation for the "linux,pci-domain" property is needed. > > Regards, > Lucas > > -- > Pengutronix e.K. | Lucas Stach | > Industrial Linux Solutions | http://www.pengutronix.de/ |
On Fri, Oct 31, 2014 at 05:04:49PM +0000, Phil Edworthy wrote: > Hi Bjorn, > > On 31 October 2014 16:37, Bjorn wrote: > > On Fri, Oct 31, 2014 at 7:43 AM, Phil Edworthy > > <phil.edworthy@renesas.com> wrote: > > > Hi Lorenzo, > > > > > > On 30 October 2014 11:45, Lorenzo wrote: > > >> Most if not all ARM PCI host controller device drivers either ignore the > > >> domain field in the pci_sys_data structure or just increment it every > > >> time a host controller is probed, using it as a domain counter. > > >> > > >> Therefore, instead of relying on pci_sys_data to stash the domain number > > >> in a standard location, ARM pcibios code can be moved to the newly > > >> introduced generic PCI domains code, implemented in commits: > > >> > > >> commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > >> ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > >> > > >> commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > >> ("PCI: Add generic domain handling") > > >> > > >> In order to assign a domain number dynamically, the ARM pcibios defines > > >> the function, called by core PCI code: > > >> > > >> void pci_bus_assign_domain_nr(...) > > >> > > >> that relies on a DT property to define the domain number or falls back to > > >> a counter; its usage replaces the current domain assignment code in PCI > > >> host controllers present in the kernel. > > >> > > >> Cc: Arnd Bergmann <arnd@arndb.de> > > >> Cc: Phil Edworthy <phil.edworthy@renesas.com> > > >> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > >> Cc: Jingoo Han <jg1.han@samsung.com> > > >> Cc: Bjorn Helgaas <bhelgaas@google.com> > > >> Cc: Russell King <linux@arm.linux.org.uk> > > >> Cc: Mohit Kumar <mohit.kumar@st.com> > > >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > > > This patch fixes a current problem with R-Car devices where there is an > > > internal PCI bridge and an external PCIe bridge on the devices. Both drivers > > > work independently but need to be on different domains. Just needed to > > enable > > > PCI_DOMAINS along with this. > > > I've done basic testing that the internal PCI and external PCIe work at the > > > same time. > > > > Hi Phil, > > > > Thanks for testing this. Can you give me some more guidance on where > > you'd like to see this merged? Until your comment about this fixing a > > current problem on R-Car, I probably would have considered this to be > > a cleanup and enhancement and hence material for v3.19. But if R-Car > > is actually broken and this fixes it, maybe this should go in for > > v3.18 instead. > I don’t think its urgent as most of our customers use LTSI kernels, e.g. v3.10. > Renesas typically provide out-of-tree BSPs with upstream code back ported, along > with other patches. Simon Horman (Renesas maintainer, cc'd) generally handles > the back ports, so I'll defer to his opinion. Hi Phil, from my point of view it would be best if this went into -stable if it is a fix. However, as you point out, from a customer point of view it shouldn't be a big deal as they should get backports via our LTSI kernel regardless of if the patch goes through stable or not. > > If it is currently broken, is there a point where it broke? I assume > > it used to work at one time. If there's a commit that broke it, it > > would be nice to reference that in the changelog and explain exactly > > what was broken and how this fixes it. > Both the internal PCI and external PCIe drivers work on their own, but have > never worked at the same time. I think it was just unfortunate timing when I > added the PCIe driver. At that time, the internal PCI driver didn’t have the > relevant DT nodes for the board I was using so I didn't see any conflicts. > > Thanks > Phil
On Mon, Nov 03, 2014 at 11:26:25PM +0000, Simon Horman wrote: > On Fri, Oct 31, 2014 at 05:04:49PM +0000, Phil Edworthy wrote: > > Hi Bjorn, > > > > On 31 October 2014 16:37, Bjorn wrote: > > > On Fri, Oct 31, 2014 at 7:43 AM, Phil Edworthy > > > <phil.edworthy@renesas.com> wrote: > > > > Hi Lorenzo, > > > > > > > > On 30 October 2014 11:45, Lorenzo wrote: > > > >> Most if not all ARM PCI host controller device drivers either ignore the > > > >> domain field in the pci_sys_data structure or just increment it every > > > >> time a host controller is probed, using it as a domain counter. > > > >> > > > >> Therefore, instead of relying on pci_sys_data to stash the domain number > > > >> in a standard location, ARM pcibios code can be moved to the newly > > > >> introduced generic PCI domains code, implemented in commits: > > > >> > > > >> commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 > > > >> ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") > > > >> > > > >> commit 670ba0c8883b576d0aec28bd7a838358a4be1 > > > >> ("PCI: Add generic domain handling") > > > >> > > > >> In order to assign a domain number dynamically, the ARM pcibios defines > > > >> the function, called by core PCI code: > > > >> > > > >> void pci_bus_assign_domain_nr(...) > > > >> > > > >> that relies on a DT property to define the domain number or falls back to > > > >> a counter; its usage replaces the current domain assignment code in PCI > > > >> host controllers present in the kernel. > > > >> > > > >> Cc: Arnd Bergmann <arnd@arndb.de> > > > >> Cc: Phil Edworthy <phil.edworthy@renesas.com> > > > >> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > >> Cc: Jingoo Han <jg1.han@samsung.com> > > > >> Cc: Bjorn Helgaas <bhelgaas@google.com> > > > >> Cc: Russell King <linux@arm.linux.org.uk> > > > >> Cc: Mohit Kumar <mohit.kumar@st.com> > > > >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > > > > > This patch fixes a current problem with R-Car devices where there is an > > > > internal PCI bridge and an external PCIe bridge on the devices. Both drivers > > > > work independently but need to be on different domains. Just needed to > > > enable > > > > PCI_DOMAINS along with this. > > > > I've done basic testing that the internal PCI and external PCIe work at the > > > > same time. > > > > > > Hi Phil, > > > > > > Thanks for testing this. Can you give me some more guidance on where > > > you'd like to see this merged? Until your comment about this fixing a > > > current problem on R-Car, I probably would have considered this to be > > > a cleanup and enhancement and hence material for v3.19. But if R-Car > > > is actually broken and this fixes it, maybe this should go in for > > > v3.18 instead. > > I don’t think its urgent as most of our customers use LTSI kernels, e.g. v3.10. > > Renesas typically provide out-of-tree BSPs with upstream code back ported, along > > with other patches. Simon Horman (Renesas maintainer, cc'd) generally handles > > the back ports, so I'll defer to his opinion. > > Hi Phil, > > from my point of view it would be best if this went into -stable if it is a > fix. However, as you point out, from a customer point of view it shouldn't > be a big deal as they should get backports via our LTSI kernel regardless of > if the patch goes through stable or not. Unfortunately this patch depends on a series that only got in in v3.18-rc1, so I don't think -stable is a workable target. Best regards, Liviu > > > > If it is currently broken, is there a point where it broke? I assume > > > it used to work at one time. If there's a commit that broke it, it > > > would be nice to reference that in the changelog and explain exactly > > > what was broken and how this fixes it. > > Both the internal PCI and external PCIe drivers work on their own, but have > > never worked at the same time. I think it was just unfortunate timing when I > > added the PCIe driver. At that time, the internal PCI driver didn’t have the > > relevant DT nodes for the board I was using so I didn't see any conflicts. > > > > Thanks > > Phil >
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 89c4b5c..29544f0 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1292,6 +1292,9 @@ config PCI_DOMAINS bool depends on PCI +config PCI_DOMAINS_GENERIC + def_bool PCI_DOMAINS + config PCI_NANOENGINE bool "BSE nanoEngine PCI support" depends on SA1100_NANOENGINE diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h index 7fc4278..f19f627 100644 --- a/arch/arm/include/asm/mach/pci.h +++ b/arch/arm/include/asm/mach/pci.h @@ -44,9 +44,6 @@ struct hw_pci { * Per-controller structure */ struct pci_sys_data { -#ifdef CONFIG_PCI_DOMAINS - int domain; -#endif struct list_head node; int busnr; /* primary bus number */ u64 mem_offset; /* bus->cpu memory mapping offset */ diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h index 7e95d85..585dc33 100644 --- a/arch/arm/include/asm/pci.h +++ b/arch/arm/include/asm/pci.h @@ -18,13 +18,6 @@ static inline int pcibios_assign_all_busses(void) } #ifdef CONFIG_PCI_DOMAINS -static inline int pci_domain_nr(struct pci_bus *bus) -{ - struct pci_sys_data *root = bus->sysdata; - - return root->domain; -} - static inline int pci_proc_domain(struct pci_bus *bus) { return pci_domain_nr(bus); diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c index 17a26c1..d8c2b4e 100644 --- a/arch/arm/kernel/bios32.c +++ b/arch/arm/kernel/bios32.c @@ -11,6 +11,8 @@ #include <linux/slab.h> #include <linux/init.h> #include <linux/io.h> +#include <linux/of.h> +#include <linux/of_pci.h> #include <asm/mach-types.h> #include <asm/mach/map.h> @@ -468,9 +470,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, if (!sys) panic("PCI: unable to allocate sys data!"); -#ifdef CONFIG_PCI_DOMAINS - sys->domain = hw->domain; -#endif sys->busnr = busnr; sys->swizzle = hw->swizzle; sys->map_irq = hw->map_irq; @@ -511,6 +510,27 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, } } +#ifdef CONFIG_PCI_DOMAINS_GENERIC +static bool dt_domain_found; + +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 + void pci_common_init_dev(struct device *parent, struct hw_pci *hw) { struct pci_sys_data *sys; diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index b1315e1..2b4c1f2 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -722,17 +722,11 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) { struct mvebu_pcie *pcie = sys_to_pcie(sys); int i; - int domain = 0; -#ifdef CONFIG_PCI_DOMAINS - domain = sys->domain; -#endif - - snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", - domain); + snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", nr); pcie->mem.name = pcie->mem_name; - snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", domain); + snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", nr); pcie->realio.name = pcie->io_name; if (request_resource(&iomem_resource, &pcie->mem)) diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index dfed00a..6790b87 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -502,9 +502,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp) dw_pci.private_data = (void **)&pp; pci_common_init_dev(pp->dev, &dw_pci); -#ifdef CONFIG_PCI_DOMAINS - dw_pci.domain++; -#endif return 0; } diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c index 61158e0..b6b859e 100644 --- a/drivers/pci/host/pcie-rcar.c +++ b/drivers/pci/host/pcie-rcar.c @@ -404,9 +404,6 @@ static void rcar_pcie_enable(struct rcar_pcie *pcie) rcar_pci.private_data = (void **)&pcie; pci_common_init_dev(&pdev->dev, &rcar_pci); -#ifdef CONFIG_PCI_DOMAINS - rcar_pci.domain++; -#endif } static int phy_wait_for_ack(struct rcar_pcie *pcie)
Most if not all ARM PCI host controller device drivers either ignore the domain field in the pci_sys_data structure or just increment it every time a host controller is probed, using it as a domain counter. Therefore, instead of relying on pci_sys_data to stash the domain number in a standard location, ARM pcibios code can be moved to the newly introduced generic PCI domains code, implemented in commits: commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9 ("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()") commit 670ba0c8883b576d0aec28bd7a838358a4be1 ("PCI: Add generic domain handling") In order to assign a domain number dynamically, the ARM pcibios defines the function, called by core PCI code: void pci_bus_assign_domain_nr(...) that relies on a DT property to define the domain number or falls back to a counter; its usage replaces the current domain assignment code in PCI host controllers present in the kernel. Cc: Arnd Bergmann <arnd@arndb.de> Cc: Phil Edworthy <phil.edworthy@renesas.com> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Cc: Jingoo Han <jg1.han@samsung.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Russell King <linux@arm.linux.org.uk> Cc: Mohit Kumar <mohit.kumar@st.com> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> --- arch/arm/Kconfig | 3 +++ arch/arm/include/asm/mach/pci.h | 3 --- arch/arm/include/asm/pci.h | 7 ------- arch/arm/kernel/bios32.c | 26 +++++++++++++++++++++++--- drivers/pci/host/pci-mvebu.c | 10 ++-------- drivers/pci/host/pcie-designware.c | 3 --- drivers/pci/host/pcie-rcar.c | 3 --- 7 files changed, 28 insertions(+), 27 deletions(-)