Re: [PATCH v5 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC

2013-04-11 Thread Andrew Murray 
On Wed, Apr 10, 2013 at 02:13:54PM +0100, Rob Herring wrote:
 Adding Ben H and Michal...
 
 On 04/10/2013 02:29 AM, 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 andrew.mur...@arm.com
  Signed-off-by: Liviu Dudau liviu.du...@arm.com
 
 One comment below. Otherwise,
 
 Reviewed-by: Rob Herring rob.herr...@calxeda.com
 
 You need also need acks from Ben and Michal.
 
 [...]
 
  +   /* Act based on address space type */
  +   res = NULL;
  +   switch ((pci_space  24)  0x3) {
  +   case 1: /* PCI IO space */
  +   pr_info(  IO 0x%016llx..0x%016llx - 0x%016llx\n,
  +  cpu_addr, cpu_addr + size - 1, pci_addr);
  +
  +   /* We support only one IO range */
  +   if (hose-pci_io_size) {
  +   pr_info( \\-- Skipped (too many) !\n);
  +   continue;
  +   }
  +#if defined(CONFIG_PPC32) || defined(CONFIG_MICROBLAZE)
 
 How about if (!IS_ENABLED(CONFIG_64BIT)) instead.

OK I'll add in my next re-spin. Would #ifndef CONFIG_64BIT suffice?

 
  +   /* On 32 bits, limit I/O space to 16MB */
  +   if (size  0x0100)
  +   size = 0x0100;
  +
  +   /* 32 bits needs to map IOs here */
  +   hose-io_base_virt = ioremap(cpu_addr, size);
  +
  +   /* Expect trouble if pci_addr is not 0 */
  +   if (primary)
  +   isa_io_base =
  +   (unsigned long)hose-io_base_virt;
  +#endif /* CONFIG_PPC32 || CONFIG_MICROBLAZE */
  +   /* pci_io_size and io_base_phys always represent IO
  +* space starting at 0 so we factor in pci_addr
  +*/
  +   hose-pci_io_size = pci_addr + size;
  +   hose-io_base_phys = cpu_addr - pci_addr;
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC

2013-04-11 Thread Linus Walleij 
On Wed, Apr 10, 2013 at 11:30 PM, Thomas Petazzoni
thomas.petazz...@free-electrons.com wrote:

 Ben, Michal, could you review/test this patch from Andrew Murray? I
 need it as a dependency of [PATCH v5 2/3] of/pci: Provide support for
 parsing PCI DT ranges property, which itself is used by the Marvell
 PCIe driver I'm hoping to get merged in 3.10.

Second this, I also have a patch series that depend on these patches.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC

2013-04-10 Thread Andrew Murray 
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 andrew.mur...@arm.com
Signed-off-by: Liviu Dudau liviu.du...@arm.com
---
 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   |3 +
 6 files changed, 205 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 linux/pci.h
 #include linux/list.h
 #include linux/ioport.h
+#include linux/of_pci.h
 
 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);
-

Re: [PATCH v5 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC

2013-04-10 Thread Rob Herring 
Adding Ben H and Michal...

On 04/10/2013 02:29 AM, 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 andrew.mur...@arm.com
 Signed-off-by: Liviu Dudau liviu.du...@arm.com

One comment below. Otherwise,

Reviewed-by: Rob Herring rob.herr...@calxeda.com

You need also need acks from Ben and Michal.

[...]

 + /* Act based on address space type */
 + res = NULL;
 + switch ((pci_space  24)  0x3) {
 + case 1: /* PCI IO space */
 + pr_info(  IO 0x%016llx..0x%016llx - 0x%016llx\n,
 +cpu_addr, cpu_addr + size - 1, pci_addr);
 +
 + /* We support only one IO range */
 + if (hose-pci_io_size) {
 + pr_info( \\-- Skipped (too many) !\n);
 + continue;
 + }
 +#if defined(CONFIG_PPC32) || defined(CONFIG_MICROBLAZE)

How about if (!IS_ENABLED(CONFIG_64BIT)) instead.

 + /* On 32 bits, limit I/O space to 16MB */
 + if (size  0x0100)
 + size = 0x0100;
 +
 + /* 32 bits needs to map IOs here */
 + hose-io_base_virt = ioremap(cpu_addr, size);
 +
 + /* Expect trouble if pci_addr is not 0 */
 + if (primary)
 + isa_io_base =
 + (unsigned long)hose-io_base_virt;
 +#endif /* CONFIG_PPC32 || CONFIG_MICROBLAZE */
 + /* pci_io_size and io_base_phys always represent IO
 +  * space starting at 0 so we factor in pci_addr
 +  */
 + hose-pci_io_size = pci_addr + size;
 + hose-io_base_phys = cpu_addr - pci_addr;

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC

2013-04-10 Thread Thomas Petazzoni 
Ben, Michal,

On Wed, 10 Apr 2013 08:13:54 -0500, Rob Herring wrote:
 Adding Ben H and Michal...
 
 On 04/10/2013 02:29 AM, 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 andrew.mur...@arm.com
  Signed-off-by: Liviu Dudau liviu.du...@arm.com
 
 One comment below. Otherwise,
 
 Reviewed-by: Rob Herring rob.herr...@calxeda.com
 
 You need also need acks from Ben and Michal.

Ben, Michal, could you review/test this patch from Andrew Murray? I
need it as a dependency of [PATCH v5 2/3] of/pci: Provide support for
parsing PCI DT ranges property, which itself is used by the Marvell
PCIe driver I'm hoping to get merged in 3.10.

Thanks a lot for your feedback,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC

2013-04-10 Thread Thomas Petazzoni 
Dear Andrew Murray,

On Wed, 10 Apr 2013 08:29:26 +0100, Andrew Murray wrote:

 diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
 index bb115de..6852481 100644
 --- a/include/linux/of_pci.h
 +++ b/include/linux/of_pci.h
 @@ -11,4 +11,7 @@ struct device_node;
  struct device_node *of_pci_find_child_device(struct device_node *parent,
unsigned int devfn);
  
 +void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 + struct device_node *dev, int primary);
 +
  #endif

In this file, 'struct pci_controller' is not defined anywhere, and not
in any header file that is included. So I get a warning at compile time
when linux/of_pci.h is included, but nothing has defined 'struct
pci_controller' beforehand. So I think this file should carry a change
like:

+struct pci_controller;

In my version of the patch I added it, see:

diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index bb115de..e56182f 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -4,6 +4,7 @@
 #include linux/pci.h
 
 struct pci_dev;
+struct pci_controller;
 struct of_irq;
 int of_irq_map_pci(const struct pci_dev *pdev, struct of_irq *out_irq);
 
@@ -11,4 +12,7 @@ struct device_node;
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 unsigned int devfn);
 
+void pci_process_bridge_OF_ranges(struct pci_controller *hose,
+   struct device_node *dev, int primary);
+
 #endif

But otherwise, for PATCH 1/3 and 2/3,

Tested-by: Thomas Petazzoni thomas.petazz...@free-electrons.com

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html