arm: pcibios: remove pci_sys_data domain
Subsystems (4)
cover arm: pcibios: remove pci_sys_data domain
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(-)
Signature · 1 line
2.1.2
RFC PATCH 1/2 arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain
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.
--- arch/arm/mach-cns3xxx/pcie.c | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-)
arch/arm/mach-cns3xxx/pcie.c +17 -23
@@ -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 ();
Signature · 1 line
2.1.2
RFC PATCH 1/2 arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain
2014-10-30 12:44 GMT+01:00 Lorenzo Pieralisi <[email protected]>:
[...]
Quoted 16 lines
arch/arm/mach-cns3xxx/pcie.c +8
-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 ;
arch/arm/mach-cns3xxx/pcie.c void __init cns3xxx_pcie_init_late(void) · line 333
Quoted 7 lines
arch/arm/mach-cns3xxx/pcie.c +2
-1@@ -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
RFC PATCH 1/2 arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain
Quoted 12 lines · On Saturday 01 November 2014 13:32:08 Micha? Miros?aw wrote:
arch/arm/mach-cns3xxx/pcie.c void __init cns3xxx_pcie_init_late(void) · line 333
Quoted 8 lines
arch/arm/mach-cns3xxx/pcie.c +2
-1@@ -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
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
In reply to Lorenzo PieralisiMost 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.
--- 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(-)
arch/arm/Kconfig +3 -0
@@ -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
arch/arm/include/asm/mach/pci.h +0 -3
@@ -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 */
arch/arm/include/asm/pci.h +0 -7
@@ -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);
arch/arm/kernel/bios32.c +23 -3
@@ -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;
drivers/pci/host/pci-mvebu.c +2 -8
@@ -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))
drivers/pci/host/pcie-designware.c +0 -3
@@ -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 ; }
drivers/pci/host/pcie-rcar.c +0 -3
@@ -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)
Signature · 1 line
2.1.2
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 23 lines · 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_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 9 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>
Very nice!
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 34 lines · Arnd Bergmann
Quoted 23 lines · 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_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 9 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>
Quoted 3 lines
Acked-by Arnd Bergmann <[email protected]>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
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
In reply to Lorenzo PieralisiQuoted 14 lines · Lorenzo Pieralisi
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_get_pci_domain_nr()")commit
670ba0c8883b576d0aec28bd7a838358a4be1
("PCI: Add generic domain handling")
Quoted 8 lines
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.
Quoted 169 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>--- 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(-)arch/arm/Kconfig +3
-0@@ -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_NANOENGINEarch/arm/include/asm/mach/pci.h +0
-3@@ -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 */ arch/arm/include/asm/pci.h +0
-7@@ -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);arch/arm/kernel/bios32.c +23
-3@@ -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;drivers/pci/host/pci-mvebu.c +2
-8@@ -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))drivers/pci/host/pcie-designware.c +0
-3@@ -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 ; }drivers/pci/host/pcie-rcar.c +0
-3@@ -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)
Signature · 2 lines
Thanks! Yijing
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 20 lines · Yijing Wang
Quoted 19 lines · Lorenzo Pieralisi
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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(...)
Quoted 7 lines
Quoted 3 lines
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 <[email protected]>
Thank you Yijing !
Lorenzo
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
In reply to Lorenzo PieralisiHi Lorenzo,
Quoted 23 lines · Lorenzo
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>
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.
Thanks
Phil
Quoted 174 lines
--- 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(-)arch/arm/Kconfig +3
-0@@ -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_NANOENGINEarch/arm/include/asm/mach/pci.h +0
-3@@ -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 */ arch/arm/include/asm/pci.h +0
-7@@ -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);arch/arm/kernel/bios32.c +2
-3@@ -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;arch/arm/kernel/bios32.c +21
-0sys->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){
arch/arm/kernel/bios32.c +0
-0struct pci_sys_data *sys;drivers/pci/host/pci-mvebu.c +2
-8@@ -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);drivers/pci/host/pci-mvebu.c +0
-0pcie->realio.name = pcie->io_name;if (request_resource (&iomem_resource, &pcie->mem))drivers/pci/host/pcie- renamed from drivers/pci/host/pcie-designware.c +0
-0
designware.c
index dfed00a..6790b87 100644drivers/pci/host/pcie-designware.c +0
-3@@ -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 ; }drivers/pci/host/pcie-rcar.c +0
-4@@ -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 at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas
maintainerRFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 35 lines · Phil Edworthy
Hi Lorenzo,
Quoted 23 lines · Lorenzo
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>
Quoted 6 lines
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_DOMAINSalong 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
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Hi Bjorn,
Quoted 45 lines · Bjorn
Quoted 35 lines · Phil Edworthy
Hi Lorenzo,
Quoted 23 lines · Lorenzo
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>Quoted 3 lines
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
Quoted 3 lines
PCI_DOMAINSalong with this.
I've done basic testing that the internal PCI and external PCIe work at the same time.
Quoted 8 lines
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.
Quoted 4 lines
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
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 56 lines · Phil Edworthy
Hi Bjorn,
Quoted 45 lines · Bjorn
Quoted 35 lines · Phil Edworthy
Hi Lorenzo,
Quoted 23 lines · Lorenzo
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>Quoted 3 lines
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
Quoted 3 lines
PCI_DOMAINSalong with this.
I've done basic testing that the internal PCI and external PCIe work at the same time.Quoted 8 lines
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.
Quoted 4 lines
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.
Quoted 11 lines
Quoted 4 lines
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
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 62 lines · Simon Horman
Quoted 56 lines · Phil Edworthy
Hi Bjorn,
Quoted 45 lines · Bjorn
Quoted 35 lines · Phil Edworthy
Hi Lorenzo,
Quoted 23 lines · Lorenzo
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>Quoted 3 lines
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
Quoted 3 lines
PCI_DOMAINSalong with this.
I've done basic testing that the internal PCI and external PCIe work at the same time.Quoted 8 lines
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.
Quoted 4 lines
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.
Quoted 6 lines
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
Quoted 13 lines
Quoted 12 lines
Quoted 5 lines
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
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
?\_(?)_/?RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
In reply to Phil EdworthyHi Phil,
Quoted 35 lines · Phil Edworthy
Hi Lorenzo,
Quoted 23 lines · Lorenzo
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>
Quoted 8 lines
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_DOMAINSalong with this.
I've done basic testing that the internal PCI and external PCIe work at the same time.Acked-by Phil Edworthy <[email protected]>
Thank you very much.
Lorenzo
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
In reply to Lorenzo PieralisiQuoted 24 lines · Lorenzo Pieralisi
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>
(+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri)
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
Quoted 161 lines
--- 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(-)arch/arm/Kconfig +3
-0@@ -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_NANOENGINEarch/arm/include/asm/mach/pci.h +0
-3@@ -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 */ arch/arm/include/asm/pci.h +0
-7@@ -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);arch/arm/kernel/bios32.c +23
-3@@ -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;drivers/pci/host/pci-mvebu.c +2
-8@@ -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))drivers/pci/host/pcie-designware.c +0
-3@@ -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 ; }drivers/pci/host/pcie-rcar.c +0
-4@@ -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
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 54 lines
-----Original Message-----
From: Jingoo Han [mailto:jg1.han at samsung.com]
Sent: Sunday, November 02, 2014 8:18 PM
To: 'Lorenzo Pieralisi'
Cc: linux-arm-kernel at lists.infradead.org; linux-pci at vger.kernel.org; liviu.dudau at 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 domainsQuoted 24 lines · Lorenzo Pieralisi
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>(+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri)
Acked-by Jingoo Han <[email protected]>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 '#ifdefCONFIG_PCI_DOMAINS' from 'pcie-designware.c'.
Quoted 7 lines
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
Quoted 167 lines
Would you test this patch and share the result?
It would be very helpful. Thank you.Best regards,
Jingoo Hanarch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci
Quoted 68 lines
--- 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(-)arch/arm/Kconfig index renamed from arch/arm/Kconfig +0
-089c4b5c..29544f0 100644
arch/arm/Kconfig +3
-0@@ -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(quoted diff) +0
-0
b/arch/arm/include/asm/mach/pci.hindex 7fc4278..f19f627 100644arch/arm/include/asm/mach/pci.h +0
-3@@ -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 */ arch/arm/include/asm/pci.h +0
-6@@ -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);arch/arm/kernel/bios32.c index renamed from arch/arm/kernel/bios32.c +0
-017a26c1..d8c2b4e 100644
arch/arm/kernel/bios32.c +2
-0@@ -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,
arch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci
Quoted 10 lines
arch/arm/kernel/bios32.c +0
-3if (!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,
Quoted 81 lines
arch/arm/kernel/bios32.c +21
-0} } +#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;(quoted diff) +0
-0
b/drivers/pci/host/pci-mvebu.cindex b1315e1..2b4c1f2 100644drivers/pci/host/pci-mvebu.c +3
-8@@ -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.cb/drivers/pci/host/pcie-designware.c
index dfed00a..6790b87 100644drivers/pci/host/pcie-designware.c +0
-2@@ -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; }(quoted diff) +0
-0
b/drivers/pci/host/pcie-rcar.cindex 61158e0..b6b859e 100644drivers/pci/host/pcie-rcar.c +0
-0@@ -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_DOMAINSdrivers/pci/host/pcie-rcar.c +0
-3- rcar_pci.domain++; -#endif }static int phy_wait_for_ack (struct rcar_pcie *pcie) --2.1.2
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
In reply to Jingoo HanQuoted 34 lines · Jingoo Han
Quoted 24 lines · Lorenzo Pieralisi
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>
Quoted 3 lines
(+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri)
Acked-by Jingoo Han <[email protected]>
Thank you Jingoo.
Quoted 12 lines
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 '#ifdefCONFIG_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.
Quoted 5 lines
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
Quoted 170 lines
Best regards,
Jingoo HanQuoted 160 lines
--- 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(-)arch/arm/Kconfig +3
-0@@ -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_NANOENGINEarch/arm/include/asm/mach/pci.h +0
-3@@ -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 */ arch/arm/include/asm/pci.h +0
-7@@ -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);arch/arm/kernel/bios32.c +23
-3@@ -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;drivers/pci/host/pci-mvebu.c +2
-8@@ -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))drivers/pci/host/pcie-designware.c +0
-3@@ -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 ; }drivers/pci/host/pcie-rcar.c +0
-4@@ -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) --Quoted 1 line
2.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Am Montag, den 03.11.2014, 11:23 +0000 schrieb Lorenzo Pieralisi:
Quoted 53 lines · Jingoo Han
Quoted 34 lines · Lorenzo Pieralisi
Quoted 24 lines
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>Quoted 3 lines
(+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri)
Acked-by Jingoo Han <[email protected]>Thank you Jingoo.
Quoted 12 lines
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 '#ifdefCONFIG_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.
Quoted 9 lines
Quoted 5 lines
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:
I also looked through the patch briefly and it seems sane to me, so this is also:
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
Signature · 2 lines
Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ |
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 60 lines · Lucas Stach
Am Montag, den 03.11.2014, 11:23 +0000 schrieb Lorenzo Pieralisi:
Quoted 59 lines · Jingoo Han
Quoted 34 lines · Lorenzo Pieralisi
Quoted 24 lines
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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.
Quoted 8 lines
Cc Arnd Bergmann <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>Quoted 3 lines
(+CC Richard Zhu, Lucas Stach, Kishon Vijay Abraham I, Murali Karicheri)
Acked-by Jingoo Han <[email protected]>Thank you Jingoo.
Quoted 17 lines
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 '#ifdefCONFIG_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.
Quoted 8 lines
Quoted 3 lines
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 <[email protected]>
Thank you for your testing!
In fact, Exynos uses multi domain structures.
Quoted 9 lines
I also looked through the patch briefly and it seems sane to me, so this is also:
Acked-by Lucas Stach <[email protected]>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.
Quoted 7 lines
Regards,
Lucas--
Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ |
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
In reply to Lorenzo PieralisiQuoted 60 lines · Lorenzo Pieralisi
Most if not all ARM PCI host controller device drivers either ignore the domain field in the
pci_sys_datastructure or just increment it every time a host controller is probed, using it as a domain counter.Therefore, instead of relying on
pci_sys_datato 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: Addpci_get_new_domain_nr()andof_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 <[email protected]>Cc Phil Edworthy <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Jingoo Han <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Mohit Kumar <[email protected]>Signed-off-by Lorenzo Pieralisi <[email protected]>--- 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(-)arch/arm/Kconfig +3
-0@@ -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_NANOENGINEarch/arm/include/asm/mach/pci.h +0
-0
arch/arm/include/asm/mach/pci.h struct hw_pci { · line 45
Quoted 2 lines
arch/arm/include/asm/mach/pci.h +0
-0@@ -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;
Quoted 129 lines
arch/arm/include/asm/mach/pci.h +0
-3struct 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 */ arch/arm/include/asm/pci.h +0
-7@@ -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);arch/arm/kernel/bios32.c +23
-3@@ -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;drivers/pci/host/pci-mvebu.c +2
-8@@ -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))drivers/pci/host/pcie-designware.c +0
-3@@ -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 ; }drivers/pci/host/pcie-rcar.c +0
-3@@ -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)
Signature · 2 lines
Thanks! Yijing
RFC PATCH 2/2 arm: pcibios: move to generic PCI domains
Quoted 1 line · Yijing Wang
On 2014/10/30 19:44, Lorenzo Pieralisi wrote:
[...]
Quoted 11 lines
arch/arm/include/asm/mach/pci.h struct hw_pci { · line 45
Quoted 7 lines
arch/arm/include/asm/mach/pci.h +0
-0@@ -44,9 +44,6 @@ struct hw_pci {* Per-controller structure */ domain in
hw_pcialso should be clean up, right ?
Quoted 7 lines
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
cover arm: pcibios: remove pci_sys_data domain
In reply to Lorenzo PieralisiQuoted 4 lines · Lorenzo Pieralisi
Code in
drivers/pci/pci-mvebu.chas 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.
Review
Quoted 3 lines
(quoted diff) +1
-2- 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
cover arm: pcibios: remove pci_sys_data domain
[Dropped Anton, his email address bounces]
Quoted 7 lines · Jason Gunthorpe
Quoted 4 lines · Lorenzo Pieralisi
Code in
drivers/pci/pci-mvebu.chas 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.
Quoted 7 lines
The intent of the string was to have the domain number so that resources in
/proc/iomemcan 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.
Quoted 7 lines
Review
Quoted 3 lines
(quoted diff) +1
-2- 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
cover arm: pcibios: remove pci_sys_data domain
Quoted 21 lines · Lorenzo Pieralisi
Quoted 7 lines · Jason Gunthorpe
Quoted 4 lines · Lorenzo Pieralisi
Code in
drivers/pci/pci-mvebu.chas 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.Quoted 7 lines
The intent of the string was to have the domain number so that resources in
/proc/iomemcan 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.
Quoted 6 lines
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 :|
Quoted 3 lines
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
cover arm: pcibios: remove pci_sys_data domain
Quoted 24 lines · Jason Gunthorpe
Quoted 23 lines · Lorenzo Pieralisi
Quoted 7 lines · Jason Gunthorpe
Quoted 4 lines · Lorenzo Pieralisi
Code in
drivers/pci/pci-mvebu.chas 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.Quoted 7 lines
The intent of the string was to have the domain number so that resources in
/proc/iomemcan 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:
Quoted 7 lines
Quoted 4 lines
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
Quoted 12 lines
But it also isn't really a big deal in the grand scheme of things. Maybe it can be fixed later :|
Quoted 3 lines
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/iomemJason
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
?\_(?)_/?cover arm: pcibios: remove pci_sys_data domain
Quoted 5 lines · Liviu Dudau
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
cover arm: pcibios: remove pci_sys_data domain
Quoted 11 lines · Jason Gunthorpe
Quoted 5 lines · Liviu Dudau
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_pcicallback 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
cover arm: pcibios: remove pci_sys_data domain
Quoted 4 lines · Lorenzo Pieralisi
It is done through core code in
pci_create_root_bus(), that in turn callspci_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
cover arm: pcibios: remove pci_sys_data domain
In reply to Jason GunthorpeQuoted 9 lines · On Thursday 30 October 2014 11:45:41 Jason Gunthorpe wrote:
Quoted 5 lines · Liviu Dudau
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.
Quoted 1 line
Which
hw_pcicallback 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
cover arm: pcibios: remove pci_sys_data domain
Quoted 5 lines · Arnd Bergmann
Quoted 1 line
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
cover arm: pcibios: remove pci_sys_data domain
Quoted 14 lines · On Thursday 30 October 2014 13:35:38 Jason Gunthorpe wrote:
Quoted 5 lines · Arnd Bergmann
Quoted 1 line
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_probeexpects to create a new domain with a unique bus number set for thatplatform_device. AFAIK everything is uniq'd to the structmcebu_pcie, so there is nothing precluding the driver from being instantiated twice.
Quoted 4 lines
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.
Quoted 4 lines
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.
Quoted 3 lines
I thought older PCI platforms tended to call
pci_common_initfor 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:
drivers/pci/host/pci-mvebu.c +1 -1
@@ -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