[PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
The pci_process_bridge_OF_ranges function, used to parse the "ranges" property of a PCI host device, is found in both Microblaze and PowerPC architectures. These implementations are nearly identical. This patch moves this common code to a common place. Signed-off-by: Andrew Murray Signed-off-by: Liviu Dudau Reviewed-by: Rob Herring Tested-by: Thomas Petazzoni --- arch/microblaze/include/asm/pci-bridge.h |5 +- arch/microblaze/pci/pci-common.c | 192 arch/powerpc/include/asm/pci-bridge.h|5 +- arch/powerpc/kernel/pci-common.c | 192 drivers/of/of_pci.c | 200 ++ include/linux/of_pci.h |4 + 6 files changed, 206 insertions(+), 392 deletions(-) diff --git a/arch/microblaze/include/asm/pci-bridge.h b/arch/microblaze/include/asm/pci-bridge.h index cb5d397..5783cd6 100644 --- a/arch/microblaze/include/asm/pci-bridge.h +++ b/arch/microblaze/include/asm/pci-bridge.h @@ -10,6 +10,7 @@ #include #include #include +#include struct device_node; @@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller *hose, extern struct pci_controller *pci_find_hose_for_OF_device( struct device_node *node); -/* Fill up host controller resources from the OF node */ -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose, - struct device_node *dev, int primary); - /* Allocate & free a PCI host bridge structure */ extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev); extern void pcibios_free_controller(struct pci_controller *phb); diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c index 9ea521e..2735ad9 100644 --- a/arch/microblaze/pci/pci-common.c +++ b/arch/microblaze/pci/pci-common.c @@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, int bar, *end = rsrc->end - offset; } -/** - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree - * @hose: newly allocated pci_controller to be setup - * @dev: device node of the host bridge - * @primary: set if primary bus (32 bits only, soon to be deprecated) - * - * This function will parse the "ranges" property of a PCI host bridge device - * node and setup the resource mapping of a pci controller based on its - * content. - * - * Life would be boring if it wasn't for a few issues that we have to deal - * with here: - * - * - We can only cope with one IO space range and up to 3 Memory space - * ranges. However, some machines (thanks Apple !) tend to split their - * space into lots of small contiguous ranges. So we have to coalesce. - * - * - We can only cope with all memory ranges having the same offset - * between CPU addresses and PCI addresses. Unfortunately, some bridges - * are setup for a large 1:1 mapping along with a small "window" which - * maps PCI address 0 to some arbitrary high address of the CPU space in - * order to give access to the ISA memory hole. - * The way out of here that I've chosen for now is to always set the - * offset based on the first resource found, then override it if we - * have a different offset and the previous was set by an ISA hole. - * - * - Some busses have IO space not starting at 0, which causes trouble with - * the way we do our IO resource renumbering. The code somewhat deals with - * it for 64 bits but I would expect problems on 32 bits. - * - * - Some 32 bits platforms such as 4xx can have physical space larger than - * 32 bits so we need to use 64 bits values for the parsing - */ -void pci_process_bridge_OF_ranges(struct pci_controller *hose, - struct device_node *dev, int primary) -{ - const u32 *ranges; - int rlen; - int pna = of_n_addr_cells(dev); - int np = pna + 5; - int memno = 0, isa_hole = -1; - u32 pci_space; - unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size; - unsigned long long isa_mb = 0; - struct resource *res; - - pr_info("PCI host bridge %s %s ranges:\n", - dev->full_name, primary ? "(primary)" : ""); - - /* Get ranges property */ - ranges = of_get_property(dev, "ranges", &rlen); - if (ranges == NULL) - return; - - /* Parse it */ - pr_debug("Parsing ranges property...\n"); - while ((rlen -= np * 4) >= 0) { - /* Read next ranges element */ - pci_space = ranges[0]; - pci_addr = of_read_number(ranges + 1, 2); - cpu_addr = of_translate_address(dev, ranges + 3); - size = of_read_number(ranges + pna + 3, 2); - - pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ", - pci_space, pci_addr); - pr_debug("cpu_addr:0x%016llx size:0x
Re: [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
Michal, Ben, Would you have some time to look at this patch and give your comments and/or ACK ? Since it touches the PowerPC and Microblaze core code, we need your agreement to merge this code, and quite a bit of code pending for 3.10 depends on this patch. Rob, alternatively, could we imagine doing a different version of the 'of/pci: Provide support for parsing PCI DT ranges property' that introduces the new API only, leaving the PowerPC and Microblaze rework as follow-up efforts, so that all the PCIe drivers that depend on this patch can get in for 3.10 ? I'd find it pretty sad if the Marvell PCIe driver that has been worked on since 4+ months does not get into 3.10 just because this patch cannot be merged. Thanks! Thomas On Thu, 11 Apr 2013 16:26:07 +0100, Andrew Murray wrote: > The pci_process_bridge_OF_ranges function, used to parse the "ranges" > property of a PCI host device, is found in both Microblaze and PowerPC > architectures. These implementations are nearly identical. This patch > moves this common code to a common place. > > Signed-off-by: Andrew Murray > Signed-off-by: Liviu Dudau > Reviewed-by: Rob Herring > Tested-by: Thomas Petazzoni > --- > arch/microblaze/include/asm/pci-bridge.h |5 +- > arch/microblaze/pci/pci-common.c | 192 > arch/powerpc/include/asm/pci-bridge.h|5 +- > arch/powerpc/kernel/pci-common.c | 192 > drivers/of/of_pci.c | 200 > ++ > include/linux/of_pci.h |4 + > 6 files changed, 206 insertions(+), 392 deletions(-) > > diff --git a/arch/microblaze/include/asm/pci-bridge.h > b/arch/microblaze/include/asm/pci-bridge.h > index cb5d397..5783cd6 100644 > --- a/arch/microblaze/include/asm/pci-bridge.h > +++ b/arch/microblaze/include/asm/pci-bridge.h > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > > struct device_node; > > @@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller > *hose, > extern struct pci_controller *pci_find_hose_for_OF_device( > struct device_node *node); > > -/* Fill up host controller resources from the OF node */ > -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose, > - struct device_node *dev, int primary); > - > /* Allocate & free a PCI host bridge structure */ > extern struct pci_controller *pcibios_alloc_controller(struct device_node > *dev); > extern void pcibios_free_controller(struct pci_controller *phb); > diff --git a/arch/microblaze/pci/pci-common.c > b/arch/microblaze/pci/pci-common.c > index 9ea521e..2735ad9 100644 > --- a/arch/microblaze/pci/pci-common.c > +++ b/arch/microblaze/pci/pci-common.c > @@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, > int bar, > *end = rsrc->end - offset; > } > > -/** > - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree > - * @hose: newly allocated pci_controller to be setup > - * @dev: device node of the host bridge > - * @primary: set if primary bus (32 bits only, soon to be deprecated) > - * > - * This function will parse the "ranges" property of a PCI host bridge device > - * node and setup the resource mapping of a pci controller based on its > - * content. > - * > - * Life would be boring if it wasn't for a few issues that we have to deal > - * with here: > - * > - * - We can only cope with one IO space range and up to 3 Memory space > - * ranges. However, some machines (thanks Apple !) tend to split their > - * space into lots of small contiguous ranges. So we have to coalesce. > - * > - * - We can only cope with all memory ranges having the same offset > - * between CPU addresses and PCI addresses. Unfortunately, some bridges > - * are setup for a large 1:1 mapping along with a small "window" which > - * maps PCI address 0 to some arbitrary high address of the CPU space in > - * order to give access to the ISA memory hole. > - * The way out of here that I've chosen for now is to always set the > - * offset based on the first resource found, then override it if we > - * have a different offset and the previous was set by an ISA hole. > - * > - * - Some busses have IO space not starting at 0, which causes trouble with > - * the way we do our IO resource renumbering. The code somewhat deals > with > - * it for 64 bits but I would expect problems on 32 bits. > - * > - * - Some 32 bits platforms such as 4xx can have physical space larger than > - * 32 bits so we need to use 64 bits values for the parsing > - */ > -void pci_process_bridge_OF_ranges(struct pci_controller *hose, > - struct device_node *dev, int primary) > -{ > - const u32 *ranges; > - int rlen; > - int pna = of_n_addr_cells(dev); > - int np = pna + 5; > - int memno = 0, isa_hole = -1; > -
Re: [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
On Mon, 2013-04-15 at 14:57 +0200, Thomas Petazzoni wrote: > Michal, Ben, > > Would you have some time to look at this patch and give your comments > and/or ACK ? Since it touches the PowerPC and Microblaze core code, we > need your agreement to merge this code, and quite a bit of code pending > for 3.10 depends on this patch. I'm currently still on vacation. I will be able to look at this after I'm back in about a week. > Rob, alternatively, could we imagine doing a different version of the > 'of/pci: Provide support for parsing PCI DT ranges property' that > introduces the new API only, leaving the PowerPC and Microblaze rework > as follow-up efforts, so that all the PCIe drivers that depend on this > patch can get in for 3.10 ? I'd find it pretty sad if the Marvell PCIe > driver that has been worked on since 4+ months does not get into 3.10 > just because this patch cannot be merged. Cheers, Ben. > Thanks! > > Thomas > > On Thu, 11 Apr 2013 16:26:07 +0100, Andrew Murray wrote: > > The pci_process_bridge_OF_ranges function, used to parse the "ranges" > > property of a PCI host device, is found in both Microblaze and PowerPC > > architectures. These implementations are nearly identical. This patch > > moves this common code to a common place. > > > > Signed-off-by: Andrew Murray > > Signed-off-by: Liviu Dudau > > Reviewed-by: Rob Herring > > Tested-by: Thomas Petazzoni > > --- > > arch/microblaze/include/asm/pci-bridge.h |5 +- > > arch/microblaze/pci/pci-common.c | 192 > > > > arch/powerpc/include/asm/pci-bridge.h|5 +- > > arch/powerpc/kernel/pci-common.c | 192 > > > > drivers/of/of_pci.c | 200 > > ++ > > include/linux/of_pci.h |4 + > > 6 files changed, 206 insertions(+), 392 deletions(-) > > > > diff --git a/arch/microblaze/include/asm/pci-bridge.h > > b/arch/microblaze/include/asm/pci-bridge.h > > index cb5d397..5783cd6 100644 > > --- a/arch/microblaze/include/asm/pci-bridge.h > > +++ b/arch/microblaze/include/asm/pci-bridge.h > > @@ -10,6 +10,7 @@ > > #include > > #include > > #include > > +#include > > > > struct device_node; > > > > @@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller > > *hose, > > extern struct pci_controller *pci_find_hose_for_OF_device( > > struct device_node *node); > > > > -/* Fill up host controller resources from the OF node */ > > -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose, > > - struct device_node *dev, int primary); > > - > > /* Allocate & free a PCI host bridge structure */ > > extern struct pci_controller *pcibios_alloc_controller(struct device_node > > *dev); > > extern void pcibios_free_controller(struct pci_controller *phb); > > diff --git a/arch/microblaze/pci/pci-common.c > > b/arch/microblaze/pci/pci-common.c > > index 9ea521e..2735ad9 100644 > > --- a/arch/microblaze/pci/pci-common.c > > +++ b/arch/microblaze/pci/pci-common.c > > @@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, > > int bar, > > *end = rsrc->end - offset; > > } > > > > -/** > > - * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device > > tree > > - * @hose: newly allocated pci_controller to be setup > > - * @dev: device node of the host bridge > > - * @primary: set if primary bus (32 bits only, soon to be deprecated) > > - * > > - * This function will parse the "ranges" property of a PCI host bridge > > device > > - * node and setup the resource mapping of a pci controller based on its > > - * content. > > - * > > - * Life would be boring if it wasn't for a few issues that we have to deal > > - * with here: > > - * > > - * - We can only cope with one IO space range and up to 3 Memory space > > - * ranges. However, some machines (thanks Apple !) tend to split their > > - * space into lots of small contiguous ranges. So we have to coalesce. > > - * > > - * - We can only cope with all memory ranges having the same offset > > - * between CPU addresses and PCI addresses. Unfortunately, some bridges > > - * are setup for a large 1:1 mapping along with a small "window" which > > - * maps PCI address 0 to some arbitrary high address of the CPU space > > in > > - * order to give access to the ISA memory hole. > > - * The way out of here that I've chosen for now is to always set the > > - * offset based on the first resource found, then override it if we > > - * have a different offset and the previous was set by an ISA hole. > > - * > > - * - Some busses have IO space not starting at 0, which causes trouble > > with > > - * the way we do our IO resource renumbering. The code somewhat deals > > with > > - * it for 64 bits but I would expect problems on 32 bits. > > - * > > - * - Some 32 bits platforms such as 4xx can
Re: [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
Hi Thomas and Andrew, First of all I would recommend you to add your tree to 0-day testing system where you can easily catch up compilation failures for microblaze and others. Getting this compilation failure: drivers/of/of_pci.c: In function 'pci_process_bridge_OF_ranges': drivers/of/of_pci.c:88:22: error: storage size of 'range' isn't known drivers/of/of_pci.c:89:29: error: storage size of 'parser' isn't known drivers/of/of_pci.c:96:2: error: implicit declaration of function 'of_pci_range_parser' [-Werror=implicit-function-declaration] drivers/of/of_pci.c:100:2: error: implicit declaration of function 'for_each_of_pci_range' [-Werror=implicit-function-declaration] drivers/of/of_pci.c:100:41: error: expected ';' before '{' token drivers/of/of_pci.c:90:6: warning: unused variable 'res_type' [-Wunused-variable] drivers/of/of_pci.c:89:29: warning: unused variable 'parser' [-Wunused-variable] drivers/of/of_pci.c:88:22: warning: unused variable 'range' [-Wunused-variable] drivers/of/of_pci.c:87:19: warning: unused variable 'res' [-Wunused-variable] drivers/of/of_pci.c:86:21: warning: unused variable 'isa_mb' [-Wunused-variable] drivers/of/of_pci.c:85:17: warning: unused variable 'isa_hole' [-Wunused-variable] drivers/of/of_pci.c:85:6: warning: unused variable 'memno' [-Wunused-variable] which is caused missing linux/of_addresss.h in of_pci.c. PowerPC probably won't have this problem because you have this header in asm/prom.h but based on the same comment it is better to add it directly to the files. That's why please add this header to this patch. diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c index 3e428a1..f30887e 100644 --- a/drivers/of/of_pci.c +++ b/drivers/of/of_pci.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include When you fix it: Acked-by: Michal Simek Thanks, Michal On 04/15/2013 02:57 PM, Thomas Petazzoni wrote: > Michal, Ben, > > Would you have some time to look at this patch and give your comments > and/or ACK ? Since it touches the PowerPC and Microblaze core code, we > need your agreement to merge this code, and quite a bit of code pending > for 3.10 depends on this patch. > > Rob, alternatively, could we imagine doing a different version of the > 'of/pci: Provide support for parsing PCI DT ranges property' that > introduces the new API only, leaving the PowerPC and Microblaze rework > as follow-up efforts, so that all the PCIe drivers that depend on this > patch can get in for 3.10 ? I'd find it pretty sad if the Marvell PCIe > driver that has been worked on since 4+ months does not get into 3.10 > just because this patch cannot be merged. > > Thanks! > > Thomas > > On Thu, 11 Apr 2013 16:26:07 +0100, Andrew Murray wrote: >> The pci_process_bridge_OF_ranges function, used to parse the "ranges" >> property of a PCI host device, is found in both Microblaze and PowerPC >> architectures. These implementations are nearly identical. This patch >> moves this common code to a common place. >> >> Signed-off-by: Andrew Murray >> Signed-off-by: Liviu Dudau >> Reviewed-by: Rob Herring >> Tested-by: Thomas Petazzoni >> --- >> arch/microblaze/include/asm/pci-bridge.h |5 +- >> arch/microblaze/pci/pci-common.c | 192 >> arch/powerpc/include/asm/pci-bridge.h|5 +- >> arch/powerpc/kernel/pci-common.c | 192 >> drivers/of/of_pci.c | 200 >> ++ >> include/linux/of_pci.h |4 + >> 6 files changed, 206 insertions(+), 392 deletions(-) >> >> diff --git a/arch/microblaze/include/asm/pci-bridge.h >> b/arch/microblaze/include/asm/pci-bridge.h >> index cb5d397..5783cd6 100644 >> --- a/arch/microblaze/include/asm/pci-bridge.h >> +++ b/arch/microblaze/include/asm/pci-bridge.h >> @@ -10,6 +10,7 @@ >> #include >> #include >> #include >> +#include >> >> struct device_node; >> >> @@ -132,10 +133,6 @@ extern void setup_indirect_pci(struct pci_controller >> *hose, >> extern struct pci_controller *pci_find_hose_for_OF_device( >> struct device_node *node); >> >> -/* Fill up host controller resources from the OF node */ >> -extern void pci_process_bridge_OF_ranges(struct pci_controller *hose, >> -struct device_node *dev, int primary); >> - >> /* Allocate & free a PCI host bridge structure */ >> extern struct pci_controller *pcibios_alloc_controller(struct device_node >> *dev); >> extern void pcibios_free_controller(struct pci_controller *phb); >> diff --git a/arch/microblaze/pci/pci-common.c >> b/arch/microblaze/pci/pci-common.c >> index 9ea521e..2735ad9 100644 >> --- a/arch/microblaze/pci/pci-common.c >> +++ b/arch/microblaze/pci/pci-common.c >> @@ -622,198 +622,6 @@ void pci_resource_to_user(const struct pci_dev *dev, >> int bar, >> *end = rsrc->end - offset; >> } >> >> -/** >> - * pci_process_bridge_OF_ranges - Parse PCI bridge resour
Re: [PATCH v6 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
On Mon, Apr 15, 2013 at 05:50:55PM +0100, Michal Simek wrote: > That's why please add this header to this patch. > > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c > index 3e428a1..f30887e 100644 > --- a/drivers/of/of_pci.c > +++ b/drivers/of/of_pci.c > @@ -1,6 +1,7 @@ > #include > #include > #include > +#include > #include > #include > > > When you fix it: > Acked-by: Michal Simek I've added this fix to the latest patchset (v7) and also your Ack - thanks for your feedback (and the robot...). Andrew Murray ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss