[RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Lorenzo Pieralisi]

This patchset is a first RFC stab at removing the dependency on pci_sys_data
domain field on ARM platforms and by replacing it with generic code that
stashes the domain value in the pci_bus control structure, introduced in

commit 41e5c0f81d3e676d671d96a0a1fafb27abfbd9
("of/pci: Add pci_get_new_domain_nr() and of_get_pci_domain_nr()")

commit 670ba0c8883b576d0aec28bd7a838358a4be1
("PCI: Add generic domain handling")

All the drivers converted (apart from PCIe designware, tested on iMX6SL)
were only compile tested for lack of HW, so along some comments, testing
and verifying that patchset does not break any existing platform are
very appreciated.

Code in drivers/pci/pci-mvebu.c has been changed to add a domain
number to PCI resources by using the nr value coming from the setup
pcibios32 callback, which may not be correct and should be considered
a temporary solution waiting for review comments.

The patchset removes entirely the pci_sys_data.domain field, since its
usage is removed at the same time in the respective host controllers
and ARM is made to select the configuration option
CONFIG_PCI_DOMAINS_GENERIC by default, which compiles a domain_nr in the
pci_bus structure so that the pci_domain_nr() look-up can rely on it.

Lorenzo Pieralisi (2):
  arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain
  arm: pcibios: move to generic PCI domains

 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 ++++++++++++++++++++++---
 arch/arm/mach-cns3xxx/pcie.c       | 40 ++++++++++++++++----------------------
 drivers/pci/host/pci-mvebu.c       | 10 ++--------
 drivers/pci/host/pcie-designware.c |  3 ---
 drivers/pci/host/pcie-rcar.c       |  3 ---
 8 files changed, 45 insertions(+), 50 deletions(-)

-- 
2.1.2

[RFC PATCH 1/2] arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain

[Lorenzo Pieralisi]

On cns3xxx platforms the PCI controller probing code relies on an
artificial dependency on the domain number to look-up the internal
data structures.

This patch reworks the host controller control data structure and
adds a domain equivalent field named port in it so that the dependency on
pci_sys_data domain field can be eventually removed.

Cc: Krzysztof Halasa <khalasa@piap.pl>
Cc: Anton Vorontsov <avorontsov@mvista.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[lp: added commit log, removed pci_sys_data domain references]
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/mach-cns3xxx/pcie.c | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
index 45d6bd0..f6bf9f6 100644
--- a/arch/arm/mach-cns3xxx/pcie.c
+++ b/arch/arm/mach-cns3xxx/pcie.c
@@ -30,18 +30,15 @@ struct cns3xxx_pcie {
 	unsigned int irqs[2];
 	struct resource res_io;
 	struct resource res_mem;
-	struct hw_pci hw_pci;
-
+	int port;
 	bool linked;
 };
 
-static struct cns3xxx_pcie cns3xxx_pcie[]; /* forward decl. */
-
 static struct cns3xxx_pcie *sysdata_to_cnspci(void *sysdata)
 {
 	struct pci_sys_data *root = sysdata;
 
-	return &cns3xxx_pcie[root->domain];
+	return root->private_data;
 }
 
 static struct cns3xxx_pcie *pdev_to_cnspci(const struct pci_dev *dev)
@@ -192,13 +189,7 @@ static struct cns3xxx_pcie cns3xxx_pcie[] = {
 			.flags = IORESOURCE_MEM,
 		},
 		.irqs = { IRQ_CNS3XXX_PCIE0_RC, IRQ_CNS3XXX_PCIE0_DEVICE, },
-		.hw_pci = {
-			.domain = 0,
-			.nr_controllers = 1,
-			.ops = &cns3xxx_pcie_ops,
-			.setup = cns3xxx_pci_setup,
-			.map_irq = cns3xxx_pcie_map_irq,
-		},
+		.port = 0,
 	},
 	[1] = {
 		.host_regs = (void __iomem *)CNS3XXX_PCIE1_HOST_BASE_VIRT,
@@ -217,19 +208,13 @@ static struct cns3xxx_pcie cns3xxx_pcie[] = {
 			.flags = IORESOURCE_MEM,
 		},
 		.irqs = { IRQ_CNS3XXX_PCIE1_RC, IRQ_CNS3XXX_PCIE1_DEVICE, },
-		.hw_pci = {
-			.domain = 1,
-			.nr_controllers = 1,
-			.ops = &cns3xxx_pcie_ops,
-			.setup = cns3xxx_pci_setup,
-			.map_irq = cns3xxx_pcie_map_irq,
-		},
+		.port = 1,
 	},
 };
 
 static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
 {
-	int port = cnspci->hw_pci.domain;
+	int port = cnspci->port;
 	u32 reg;
 	unsigned long time;
 
@@ -260,9 +245,9 @@ static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
 
 static void __init cns3xxx_pcie_hw_init(struct cns3xxx_pcie *cnspci)
 {
-	int port = cnspci->hw_pci.domain;
+	int port = cnspci->port;
 	struct pci_sys_data sd = {
-		.domain = port,
+		.private_data = cnspci,
 	};
 	struct pci_bus bus = {
 		.number = 0,
@@ -323,6 +308,14 @@ static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
 void __init cns3xxx_pcie_init_late(void)
 {
 	int i;
+	void *private_data;
+	struct hw_pci hw_pci = {
+	       .nr_controllers = 1,
+	       .ops = &cns3xxx_pcie_ops,
+	       .setup = cns3xxx_pci_setup,
+	       .map_irq = cns3xxx_pcie_map_irq,
+	       .private_data = &private_data,
+	};
 
 	pcibios_min_io = 0;
 	pcibios_min_mem = 0;
@@ -335,7 +328,8 @@ void __init cns3xxx_pcie_init_late(void)
 		cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
 		cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
 		cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
-		pci_common_init(&cns3xxx_pcie[i].hw_pci);
+		private_data = &cns3xxx_pcie[i];
+		pci_common_init(&hw_pci);
 	}
 
 	pci_assign_unassigned_resources();
-- 
2.1.2

[RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Lorenzo Pieralisi]

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
  */
 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

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Arnd Bergmann]

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!

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Yijing Wang]

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)
> 


-- 
Thanks!
Yijing

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Lorenzo Pieralisi]

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

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Lorenzo Pieralisi]

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

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Jason Gunthorpe]

On Thu, Oct 30, 2014 at 11:44:46AM +0000, Lorenzo Pieralisi wrote:

> Code in drivers/pci/pci-mvebu.c has been changed to add a domain
> number to PCI resources by using the nr value coming from the setup
> pcibios32 callback, which may not be correct and should be considered
> a temporary solution waiting for review comments.

The intent of the string was to have the domain number so that
resources in /proc/iomem can be correlated with lspci.

This would be a 'best practice' - all PCI drivers need to request
resource, and the resource should be relatable back to the PCI
domain... So it would be best if the domain number was available at
this point in a driver's flow.

> -       snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x",
> -                domain);
> +       snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", nr);

I'm not sure what 'nr' is in this context, if it is not the domain
number then I'd just drop the 0x04x entirely rather than include
some nonsense number...

Jason

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Lorenzo Pieralisi]

[Dropped Anton, his email address bounces]

On Thu, Oct 30, 2014 at 04:25:52PM +0000, Jason Gunthorpe wrote:
> On Thu, Oct 30, 2014 at 11:44:46AM +0000, Lorenzo Pieralisi wrote:
> 
> > Code in drivers/pci/pci-mvebu.c has been changed to add a domain
> > number to PCI resources by using the nr value coming from the setup
> > pcibios32 callback, which may not be correct and should be considered
> > a temporary solution waiting for review comments.
> 
> The intent of the string was to have the domain number so that
> resources in /proc/iomem can be correlated with lspci.
> 
> This would be a 'best practice' - all PCI drivers need to request
> resource, and the resource should be relatable back to the PCI
> domain... So it would be best if the domain number was available at
> this point in a driver's flow.

It is not available with the new approach and the generic PCI domains since
the set-up hook is called before creating the pci_bus. That's why
I mentioned that in the cover letter, and that's good it caught your
attention.

On a side note, when the resources are parsed from DT ranges, ie in:

of_pci_get_host_bridge_resources()

the resources won't contain the domain number you are looking for here, for
the records, so we'd better find an agreement sooner rather than later.

> > -       snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x",
> > -                domain);
> > +       snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", nr);
> 
> I'm not sure what 'nr' is in this context, if it is not the domain
> number then I'd just drop the 0x04x entirely rather than include
> some nonsense number...

nr is the index (0 to nr_controllers) in struct hw_pci of the controller
being set-up. For this driver it is always 0, and BTW, sys->domain was 0 too
and I do not see any code that can change its value to anything other
than 0, at least in the mainline kernel.

So basically I could go as far as sticking 0000 to the string given the
current code. I did not drop the 0x04x entirely since I do not want to break
userspace, I was tempted though, let me know if I am allowed to do that.

Thanks,
Lorenzo

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Jason Gunthorpe]

On Thu, Oct 30, 2014 at 04:52:46PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Oct 30, 2014 at 04:25:52PM +0000, Jason Gunthorpe wrote:
> > On Thu, Oct 30, 2014 at 11:44:46AM +0000, Lorenzo Pieralisi wrote:
> > 
> > > Code in drivers/pci/pci-mvebu.c has been changed to add a domain
> > > number to PCI resources by using the nr value coming from the setup
> > > pcibios32 callback, which may not be correct and should be considered
> > > a temporary solution waiting for review comments.
> > 
> > The intent of the string was to have the domain number so that
> > resources in /proc/iomem can be correlated with lspci.
> > 
> > This would be a 'best practice' - all PCI drivers need to request
> > resource, and the resource should be relatable back to the PCI
> > domain... So it would be best if the domain number was available at
> > this point in a driver's flow.
> 
> It is not available with the new approach and the generic PCI domains since
> the set-up hook is called before creating the pci_bus. That's why
> I mentioned that in the cover letter, and that's good it caught your
> attention.
> 
> On a side note, when the resources are parsed from DT ranges, ie in:
> 
> of_pci_get_host_bridge_resources()
> 
> the resources won't contain the domain number you are looking for here, for
> the records, so we'd better find an agreement sooner rather than later.

Well, it is very unfortunate that things are globally not sequenced to
allow proper resource names :(

But it also isn't really a big deal in the grand scheme of
things. Maybe it can be fixed later :|

> So basically I could go as far as sticking 0000 to the string given the
> current code. I did not drop the 0x04x entirely since I do not want to break
> userspace, I was tempted though, let me know if I am allowed to do that.

I'd just drop it entirely then, nobody is going to parse /proc/iomem

Jason

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Liviu Dudau]

On Thu, Oct 30, 2014 at 05:03:05PM +0000, Jason Gunthorpe wrote:
> On Thu, Oct 30, 2014 at 04:52:46PM +0000, Lorenzo Pieralisi wrote:
> > On Thu, Oct 30, 2014 at 04:25:52PM +0000, Jason Gunthorpe wrote:
> > > On Thu, Oct 30, 2014 at 11:44:46AM +0000, Lorenzo Pieralisi wrote:
> > > 
> > > > Code in drivers/pci/pci-mvebu.c has been changed to add a domain
> > > > number to PCI resources by using the nr value coming from the setup
> > > > pcibios32 callback, which may not be correct and should be considered
> > > > a temporary solution waiting for review comments.
> > > 
> > > The intent of the string was to have the domain number so that
> > > resources in /proc/iomem can be correlated with lspci.
> > > 
> > > This would be a 'best practice' - all PCI drivers need to request
> > > resource, and the resource should be relatable back to the PCI
> > > domain... So it would be best if the domain number was available at
> > > this point in a driver's flow.
> > 
> > It is not available with the new approach and the generic PCI domains since
> > the set-up hook is called before creating the pci_bus. That's why
> > I mentioned that in the cover letter, and that's good it caught your
> > attention.
> > 
> > On a side note, when the resources are parsed from DT ranges, ie in:
> > 
> > of_pci_get_host_bridge_resources()
> > 
> > the resources won't contain the domain number you are looking for here, for
> > the records, so we'd better find an agreement sooner rather than later.
> 
> Well, it is very unfortunate that things are globally not sequenced to
> allow proper resource names :(

But of_pci_get_host_bridge_resources() does not request the resources, it only
creates them out of the DT ranges. If/when the driver decides the list of
resources is correct and it can work with it can also request them and use
whatever domain number allocation strategy it wants (auto-incrementing or DT
based). So I don't think the global sequencing is broken here.

Best regards,
Liviu

> 
> But it also isn't really a big deal in the grand scheme of
> things. Maybe it can be fixed later :|
> 
> > So basically I could go as far as sticking 0000 to the string given the
> > current code. I did not drop the 0x04x entirely since I do not want to break
> > userspace, I was tempted though, let me know if I am allowed to do that.
> 
> I'd just drop it entirely then, nobody is going to parse /proc/iomem
> 
> Jason
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Jason Gunthorpe]

On Thu, Oct 30, 2014 at 05:39:15PM +0000, Liviu Dudau wrote:

> But of_pci_get_host_bridge_resources() does not request the resources, it only
> creates them out of the DT ranges. If/when the driver decides the list of
> resources is correct and it can work with it can also request them and use
> whatever domain number allocation strategy it wants (auto-incrementing or DT
> based). So I don't think the global sequencing is broken here.

So how does mvebu now allocate a unique domain number per mvebu_pcie?

Which hw_pci callback should do that?

Jason

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Lorenzo Pieralisi]

On Thu, Oct 30, 2014 at 05:45:41PM +0000, Jason Gunthorpe wrote:
> On Thu, Oct 30, 2014 at 05:39:15PM +0000, Liviu Dudau wrote:
> 
> > But of_pci_get_host_bridge_resources() does not request the resources, it only
> > creates them out of the DT ranges. If/when the driver decides the list of
> > resources is correct and it can work with it can also request them and use
> > whatever domain number allocation strategy it wants (auto-incrementing or DT
> > based). So I don't think the global sequencing is broken here.
> 
> So how does mvebu now allocate a unique domain number per mvebu_pcie?
> 
> Which hw_pci callback should do that?

It is done through core code in pci_create_root_bus(), that in turn
calls pci_bus_assign_domain_nr() which is implemented now in pcibios
for arm, it is all in patch 2. What Liviu is saying is correct, it all
depends on *when* the resource naming should be applied. For code
relying on pcibios the setup hw_pci hook() is likely to be the place where
resources are requested and the domain is not set-up yet there with
the generic PCI domains approach, hence I can't attach a domain number
to the resource names in question.

Lorenzo

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Jason Gunthorpe]

On Thu, Oct 30, 2014 at 06:09:41PM +0000, Lorenzo Pieralisi wrote:

> It is done through core code in pci_create_root_bus(), that in turn
> calls pci_bus_assign_domain_nr() which is implemented now in pcibios
> for arm, it is all in patch 2. What Liviu is saying is correct, it
> all

Does request_resources have to be called before pci_create_root_bus
for everything to work right? I didn't trace too deeply, but
pci_create_root_bus is doing all sorts of things with the resource
list.. 

We already know that missing the request_resource causes some subtle
misbehavior in the PCI core...

Also:

+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;

There isn't any way to return an error from pci_bus_assign_domain_nr,
so I'd think it must always assign something to bus->domain_nr?
Or does higher level code bail out of there are duplicate domains?

Should this error case call pci_get_new_domain_nr() anyhow?

Jason

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Arnd Bergmann]

On Thursday 30 October 2014 11:45:41 Jason Gunthorpe wrote:
> On Thu, Oct 30, 2014 at 05:39:15PM +0000, Liviu Dudau wrote:
> 
> > But of_pci_get_host_bridge_resources() does not request the resources, it only
> > creates them out of the DT ranges. If/when the driver decides the list of
> > resources is correct and it can work with it can also request them and use
> > whatever domain number allocation strategy it wants (auto-incrementing or DT
> > based). So I don't think the global sequencing is broken here.
> 
> So how does mvebu now allocate a unique domain number per mvebu_pcie?

I believe the answer to that is that the mvebu PCIe driver currently only
supports one domain, and it will have the unique number '0', which is the
default.

> Which hw_pci callback should do that?

I think the purpose Lorenzo's patches (more to come) is to obsolete all
of the hw_pci callbacks for modern drivers.

	Arnd

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Jason Gunthorpe]

On Thu, Oct 30, 2014 at 08:21:40PM +0100, Arnd Bergmann wrote:

> > So how does mvebu now allocate a unique domain number per mvebu_pcie?
> 
> I believe the answer to that is that the mvebu PCIe driver currently only
> supports one domain, and it will have the unique number '0', which is the
> default.

It is like most of the the other new drivers, each mvebu_pcie_probe
expects to create a new domain with a unique bus number set for that
platform_device. AFAIK everything is uniq'd to the struct mcebu_pcie,
so there is nothing precluding the driver from being instantiated
twice.

Indeed, the way mvebu hardware works you could actually create a DT
that assigned some ports to one domain and some other ports to a
different domain, using two platform_devices. All that was missing
from the driver was to increment the domain number.

I think Lorenzo's patches improve this, at least it appears that
unique domain numbers are now being assigned, I'm not sure - I'm a
little confused how we can safely blindly apply the new domain logic
without the driver opt'ing in....

I thought older PCI platforms tended to call pci_common_init for each
physical PCI bus, and we don't want them to suddenly have non-zero
domain numbers??

Jason

Re: [RFC PATCH 0/2] arm: pcibios: remove pci_sys_data domain

[Arnd Bergmann]

On Thursday 30 October 2014 13:35:38 Jason Gunthorpe wrote:
> On Thu, Oct 30, 2014 at 08:21:40PM +0100, Arnd Bergmann wrote:
> 
> > > So how does mvebu now allocate a unique domain number per mvebu_pcie?
> > 
> > I believe the answer to that is that the mvebu PCIe driver currently only
> > supports one domain, and it will have the unique number '0', which is the
> > default.
> 
> It is like most of the the other new drivers, each mvebu_pcie_probe
> expects to create a new domain with a unique bus number set for that
> platform_device. AFAIK everything is uniq'd to the struct mcebu_pcie,
> so there is nothing precluding the driver from being instantiated
> twice.
> 
> Indeed, the way mvebu hardware works you could actually create a DT
> that assigned some ports to one domain and some other ports to a
> different domain, using two platform_devices. All that was missing
> from the driver was to increment the domain number.

Right, makes sense.

> I think Lorenzo's patches improve this, at least it appears that
> unique domain numbers are now being assigned, I'm not sure - I'm a
> little confused how we can safely blindly apply the new domain logic
> without the driver opt'ing in....

I think it will just work: the host driver should not care about
the particular domain number, it just needs to be unique, which
the core now ensures.

> I thought older PCI platforms tended to call pci_common_init for each
> physical PCI bus, and we don't want them to suddenly have non-zero
> domain numbers??

Only cns3xxx calls pci_common_init with fixed domain numbers, and
Lorenzo's patch 1/2 changes it. All other drivers fall into one
of three categories:

a) there is only ever one host bridge
b) There are multiple host bridges, but they are all in the same
   domain, and the driver calls pci_common_init() once to initialize
   them all, and pci_common_init calls back into the driver with
   the hw_pci->setup() function passing the instance number.
c) The driver calls pci_common_init_dev() for each new host bridge
   and assigns a new domain every time but never has more than one
   host bridge per domain.

a) and b) are not impacted by this patch, while c) becomes simpler.

BTW, pci-mvebu is currently the only driver using c) with pci_common_init
rather than pci_common_init_dev. We should probably apply this trivial
patch to make it more consistent:

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index b1315e197ffb..efe4bd370b37 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -825,7 +825,7 @@ static void mvebu_pcie_enable(struct mvebu_pcie *pcie)
 	hw.align_resource = mvebu_pcie_align_resource;
 	hw.add_bus        = mvebu_pcie_add_bus;
 
-	pci_common_init(&hw);
+	pci_common_init_dev(&pcie->pdev.dev, &hw);
 }
 
 /*


	Arnd

RE: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Phil Edworthy]

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

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Bjorn Helgaas]

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

RE: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Phil Edworthy]

SGkgQmpvcm4sDQoNCk9uIDMxIE9jdG9iZXIgMjAxNCAxNjozNywgQmpvcm4gd3JvdGU6DQo+IE9u
IEZyaSwgT2N0IDMxLCAyMDE0IGF0IDc6NDMgQU0sIFBoaWwgRWR3b3J0aHkNCj4gPHBoaWwuZWR3
b3J0aHlAcmVuZXNhcy5jb20+IHdyb3RlOg0KPiA+IEhpIExvcmVuem8sDQo+ID4NCj4gPiBPbiAz
MCBPY3RvYmVyIDIwMTQgMTE6NDUsIExvcmVuem8gd3JvdGU6DQo+ID4+IE1vc3QgaWYgbm90IGFs
bCBBUk0gUENJIGhvc3QgY29udHJvbGxlciBkZXZpY2UgZHJpdmVycyBlaXRoZXIgaWdub3JlIHRo
ZQ0KPiA+PiBkb21haW4gZmllbGQgaW4gdGhlIHBjaV9zeXNfZGF0YSBzdHJ1Y3R1cmUgb3IganVz
dCBpbmNyZW1lbnQgaXQgZXZlcnkNCj4gPj4gdGltZSBhIGhvc3QgY29udHJvbGxlciBpcyBwcm9i
ZWQsIHVzaW5nIGl0IGFzIGEgZG9tYWluIGNvdW50ZXIuDQo+ID4+DQo+ID4+IFRoZXJlZm9yZSwg
aW5zdGVhZCBvZiByZWx5aW5nIG9uIHBjaV9zeXNfZGF0YSB0byBzdGFzaCB0aGUgZG9tYWluIG51
bWJlcg0KPiA+PiBpbiBhIHN0YW5kYXJkIGxvY2F0aW9uLCBBUk0gcGNpYmlvcyBjb2RlIGNhbiBi
ZSBtb3ZlZCB0byB0aGUgbmV3bHkNCj4gPj4gaW50cm9kdWNlZCBnZW5lcmljIFBDSSBkb21haW5z
IGNvZGUsIGltcGxlbWVudGVkIGluIGNvbW1pdHM6DQo+ID4+DQo+ID4+IGNvbW1pdCA0MWU1YzBm
ODFkM2U2NzZkNjcxZDk2YTBhMWZhZmIyN2FiZmJkOQ0KPiA+PiAoIm9mL3BjaTogQWRkIHBjaV9n
ZXRfbmV3X2RvbWFpbl9ucigpIGFuZCBvZl9nZXRfcGNpX2RvbWFpbl9ucigpIikNCj4gPj4NCj4g
Pj4gY29tbWl0IDY3MGJhMGM4ODgzYjU3NmQwYWVjMjhiZDdhODM4MzU4YTRiZTENCj4gPj4gKCJQ
Q0k6IEFkZCBnZW5lcmljIGRvbWFpbiBoYW5kbGluZyIpDQo+ID4+DQo+ID4+IEluIG9yZGVyIHRv
IGFzc2lnbiBhIGRvbWFpbiBudW1iZXIgZHluYW1pY2FsbHksIHRoZSBBUk0gcGNpYmlvcyBkZWZp
bmVzDQo+ID4+IHRoZSBmdW5jdGlvbiwgY2FsbGVkIGJ5IGNvcmUgUENJIGNvZGU6DQo+ID4+DQo+
ID4+IHZvaWQgcGNpX2J1c19hc3NpZ25fZG9tYWluX25yKC4uLikNCj4gPj4NCj4gPj4gdGhhdCBy
ZWxpZXMgb24gYSBEVCBwcm9wZXJ0eSB0byBkZWZpbmUgdGhlIGRvbWFpbiBudW1iZXIgb3IgZmFs
bHMgYmFjayB0bw0KPiA+PiBhIGNvdW50ZXI7IGl0cyB1c2FnZSByZXBsYWNlcyB0aGUgY3VycmVu
dCBkb21haW4gYXNzaWdubWVudCBjb2RlIGluIFBDSQ0KPiA+PiBob3N0IGNvbnRyb2xsZXJzIHBy
ZXNlbnQgaW4gdGhlIGtlcm5lbC4NCj4gPj4NCj4gPj4gQ2M6IEFybmQgQmVyZ21hbm4gPGFybmRA
YXJuZGIuZGU+DQo+ID4+IENjOiBQaGlsIEVkd29ydGh5IDxwaGlsLmVkd29ydGh5QHJlbmVzYXMu
Y29tPg0KPiA+PiBDYzogSmFzb24gR3VudGhvcnBlIDxqZ3VudGhvcnBlQG9ic2lkaWFucmVzZWFy
Y2guY29tPg0KPiA+PiBDYzogSmluZ29vIEhhbiA8amcxLmhhbkBzYW1zdW5nLmNvbT4NCj4gPj4g
Q2M6IEJqb3JuIEhlbGdhYXMgPGJoZWxnYWFzQGdvb2dsZS5jb20+DQo+ID4+IENjOiBSdXNzZWxs
IEtpbmcgPGxpbnV4QGFybS5saW51eC5vcmcudWs+DQo+ID4+IENjOiBNb2hpdCBLdW1hciA8bW9o
aXQua3VtYXJAc3QuY29tPg0KPiA+PiBTaWduZWQtb2ZmLWJ5OiBMb3JlbnpvIFBpZXJhbGlzaSA8
bG9yZW56by5waWVyYWxpc2lAYXJtLmNvbT4NCj4gPg0KPiA+IFRoaXMgcGF0Y2ggZml4ZXMgYSBj
dXJyZW50IHByb2JsZW0gd2l0aCBSLUNhciBkZXZpY2VzIHdoZXJlIHRoZXJlIGlzIGFuDQo+ID4g
aW50ZXJuYWwgUENJIGJyaWRnZSBhbmQgYW4gZXh0ZXJuYWwgUENJZSBicmlkZ2Ugb24gdGhlIGRl
dmljZXMuIEJvdGggZHJpdmVycw0KPiA+IHdvcmsgaW5kZXBlbmRlbnRseSBidXQgbmVlZCB0byBi
ZSBvbiBkaWZmZXJlbnQgZG9tYWlucy4gSnVzdCBuZWVkZWQgdG8NCj4gZW5hYmxlDQo+ID4gUENJ
X0RPTUFJTlMgYWxvbmcgd2l0aCB0aGlzLg0KPiA+IEkndmUgZG9uZSBiYXNpYyB0ZXN0aW5nIHRo
YXQgdGhlIGludGVybmFsIFBDSSBhbmQgZXh0ZXJuYWwgUENJZSB3b3JrIGF0IHRoZQ0KPiA+IHNh
bWUgdGltZS4NCj4gDQo+IEhpIFBoaWwsDQo+IA0KPiBUaGFua3MgZm9yIHRlc3RpbmcgdGhpcy4g
IENhbiB5b3UgZ2l2ZSBtZSBzb21lIG1vcmUgZ3VpZGFuY2Ugb24gd2hlcmUNCj4geW91J2QgbGlr
ZSB0byBzZWUgdGhpcyBtZXJnZWQ/ICBVbnRpbCB5b3VyIGNvbW1lbnQgYWJvdXQgdGhpcyBmaXhp
bmcgYQ0KPiBjdXJyZW50IHByb2JsZW0gb24gUi1DYXIsIEkgcHJvYmFibHkgd291bGQgaGF2ZSBj
b25zaWRlcmVkIHRoaXMgdG8gYmUNCj4gYSBjbGVhbnVwIGFuZCBlbmhhbmNlbWVudCBhbmQgaGVu
Y2UgbWF0ZXJpYWwgZm9yIHYzLjE5LiAgQnV0IGlmIFItQ2FyDQo+IGlzIGFjdHVhbGx5IGJyb2tl
biBhbmQgdGhpcyBmaXhlcyBpdCwgbWF5YmUgdGhpcyBzaG91bGQgZ28gaW4gZm9yDQo+IHYzLjE4
IGluc3RlYWQuDQpJIGRvbuKAmXQgdGhpbmsgaXRzIHVyZ2VudCBhcyBtb3N0IG9mIG91ciBjdXN0
b21lcnMgdXNlIExUU0kga2VybmVscywgZS5nLiB2My4xMC4NClJlbmVzYXMgdHlwaWNhbGx5IHBy
b3ZpZGUgb3V0LW9mLXRyZWUgQlNQcyB3aXRoIHVwc3RyZWFtIGNvZGUgYmFjayBwb3J0ZWQsIGFs
b25nDQp3aXRoIG90aGVyIHBhdGNoZXMuIFNpbW9uIEhvcm1hbiAoUmVuZXNhcyBtYWludGFpbmVy
LCBjYydkKSBnZW5lcmFsbHkgaGFuZGxlcw0KdGhlIGJhY2sgcG9ydHMsIHNvIEknbGwgZGVmZXIg
dG8gaGlzIG9waW5pb24uDQoNCj4gSWYgaXQgaXMgY3VycmVudGx5IGJyb2tlbiwgaXMgdGhlcmUg
YSBwb2ludCB3aGVyZSBpdCBicm9rZT8gIEkgYXNzdW1lDQo+IGl0IHVzZWQgdG8gd29yayBhdCBv
bmUgdGltZS4gIElmIHRoZXJlJ3MgYSBjb21taXQgdGhhdCBicm9rZSBpdCwgaXQNCj4gd291bGQg
YmUgbmljZSB0byByZWZlcmVuY2UgdGhhdCBpbiB0aGUgY2hhbmdlbG9nIGFuZCBleHBsYWluIGV4
YWN0bHkNCj4gd2hhdCB3YXMgYnJva2VuIGFuZCBob3cgdGhpcyBmaXhlcyBpdC4NCkJvdGggdGhl
IGludGVybmFsIFBDSSBhbmQgZXh0ZXJuYWwgUENJZSBkcml2ZXJzIHdvcmsgb24gdGhlaXIgb3du
LCBidXQgaGF2ZQ0KbmV2ZXIgd29ya2VkIGF0IHRoZSBzYW1lIHRpbWUuIEkgdGhpbmsgaXQgd2Fz
IGp1c3QgdW5mb3J0dW5hdGUgdGltaW5nIHdoZW4gSQ0KYWRkZWQgdGhlIFBDSWUgZHJpdmVyLiBB
dCB0aGF0IHRpbWUsIHRoZSBpbnRlcm5hbCBQQ0kgZHJpdmVyIGRpZG7igJl0IGhhdmUgdGhlDQpy
ZWxldmFudCBEVCBub2RlcyBmb3IgdGhlIGJvYXJkIEkgd2FzIHVzaW5nIHNvIEkgZGlkbid0IHNl
ZSBhbnkgY29uZmxpY3RzLg0KDQpUaGFua3MNClBoaWwNCg==

Re: [RFC PATCH 1/2] arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain

[Michał Mirosław]

2014-10-30 12:44 GMT+01:00 Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>:
[...]
> @@ -323,6 +308,14 @@ static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
>  void __init cns3xxx_pcie_init_late(void)
>  {
>         int i;
> +       void *private_data;
> +       struct hw_pci hw_pci = {
> +              .nr_controllers = 1,
> +              .ops = &cns3xxx_pcie_ops,
> +              .setup = cns3xxx_pci_setup,
> +              .map_irq = cns3xxx_pcie_map_irq,
> +              .private_data = &private_data,
> +       };
>
>         pcibios_min_io = 0;
>         pcibios_min_mem = 0;
> @@ -335,7 +328,8 @@ void __init cns3xxx_pcie_init_late(void)
>                 cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
>                 cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
>                 cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
> -               pci_common_init(&cns3xxx_pcie[i].hw_pci);
> +               private_data = &cns3xxx_pcie[i];
> +               pci_common_init(&hw_pci);
>         }

This looks weird. hw_pci.private_data is a pointer to a (temporary)
pointer to private data?

Best Regards,
Michał Mirosław

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Jingoo Han]

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

RE: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Karicheri, Muralidharan]

>-----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

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Yijing Wang]

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)
> 


-- 
Thanks!
Yijing

Re: [RFC PATCH 1/2] arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain

[Arnd Bergmann]

On Saturday 01 November 2014 13:32:08 Michał Mirosław wrote:
> > @@ -335,7 +328,8 @@ void __init cns3xxx_pcie_init_late(void)
> >                 cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
> >                 cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
> >                 cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
> > -               pci_common_init(&cns3xxx_pcie[i].hw_pci);
> > +               private_data = &cns3xxx_pcie[i];
> > +               pci_common_init(&hw_pci);
> >         }
> 
> This looks weird. hw_pci.private_data is a pointer to a (temporary)
> pointer to private data?
> 

hw_pci is a local data structure that is only used to pass arguments
to pci_common_init. hw_pci->private_data is a pointer to an array of
pointers to the private data in each host bridge that is created in
the new domain during the call to pci_common_init. Like most other
callers, cns3xxx creates only one host bridge per domain, so it's
an array that contains only one entry.

	Arnd

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Lorenzo Pieralisi]

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

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Lorenzo Pieralisi]

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

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[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.

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
> 

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Lucas Stach]

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

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Jingoo Han]

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/  |

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Simon Horman]

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

Re: [RFC PATCH 2/2] arm: pcibios: move to generic PCI domains

[Liviu Dudau]

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
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯