On Fri, 2007-11-30 at 15:37 +0800, Gala Kumar wrote:

On Nov 29, 2007, at 9:45 PM, Li Li wrote:

quoted

The PCIE controller is initiated in u-boot. 

This patch is based on Leo`s mpc837xe patches. 


Signed-off-by: Tony Li <redacted> 
--- 
arch/powerpc/boot/dts/mpc8377_mds.dts     |   56 ++++++++-- 
arch/powerpc/boot/dts/mpc8378_mds.dts     |   56 ++++++++-- 
arch/powerpc/platforms/83xx/Kconfig       |    7 ++ 
arch/powerpc/platforms/83xx/mpc8313_rdb.c |    2 +- 
arch/powerpc/platforms/83xx/mpc832x_mds.c |    2 +- 
arch/powerpc/platforms/83xx/mpc832x_rdb.c |    2 +- 
arch/powerpc/platforms/83xx/mpc834x_itx.c |    2 +- 
arch/powerpc/platforms/83xx/mpc834x_mds.c |    2 +- 
arch/powerpc/platforms/83xx/mpc836x_mds.c |    2 +- 
arch/powerpc/platforms/83xx/mpc837x_mds.c |    7 +- 
arch/powerpc/platforms/83xx/mpc83xx.h     |   10 ++- 
arch/powerpc/platforms/83xx/pci.c         |  166

++++++++++++++++++++ 

quoted

+++++++- 
12 files changed, 283 insertions(+), 31 deletions(-) 

diff --git a/arch/powerpc/boot/dts/mpc8377_mds.dts b/arch/powerpc/ 

boot/dts/mpc8377_mds.dts 
index 4402e39..1f7819e 100644

quoted

+ 
+     pci2@e0009000 {

I agree w/Olof.  This should be pcie@e0009000 

quoted

+             interrupt-map-mask = <f800 0 0 7>; 
+             msi-available-ranges = <43 4 51 52 56 57 58 59>; 
+             interrupt-map = < 
+                     0000 0 0 1 &ipic 1 8 
+                     0000 0 0 2 &ipic 1 8 
+                     0000 0 0 3 &ipic 1 8 
+                     0000 0 0 4 &ipic 1 8 
+             >; 
+             interrupt-parent = < &ipic >; 
+             interrupts = <1 8>; 
+             bus-range = <0 0>; 
+             ranges = <02000000 0 A8000000 A8000000 0 10000000 
+                       01000000 0 00000000 B8000000 0 00800000>; 
+             clock-frequency = <0>; 
+             #interrupt-cells = <1>; 
+             #size-cells = <2>; 
+             #address-cells = <3>; 
+             reg = <e0009000 00001000 
+                    a0000000 08000000>;

Shouldn't the reg size for the cfg space be 256M?

256M is a little too big for kernel.

quoted

diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/ 

platforms/83xx/Kconfig 
index 0c61e7a..00154c5 100644 

--- a/arch/powerpc/platforms/83xx/Kconfig 
+++ b/arch/powerpc/platforms/83xx/Kconfig 

@@ -87,3 +87,10 @@ config PPC_MPC837x 
      select PPC_INDIRECT_PCI 
      select FSL_SERDES 
      default y if MPC837x_MDS 
+ 
+config PPC_MPC83XX_PCIE 
+     bool "MPC837X PCI Express support" 
+     depends on PCIEPORTBUS && PPC_MPC837x 
+     default n 
+     help 
+       Enables MPC837x PCI express RC mode

This should be a hidden config that is just selected by PPC_MPC837x  
instead of a choice.  Also drop the depends on.

In the dts file, the PCIE is default enabled. So, we should provide
another way to disable the PCIE.
Modify and recompile the dts is a little unkind to user.

quoted

diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h b/arch/powerpc/ 

platforms/83xx/mpc83xx.h 
index b778cb4..2078da7 100644 

--- a/arch/powerpc/platforms/83xx/mpc83xx.h 
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h 

@@ -43,12 +43,18 @@ 

#define PORTSCX_PTS_UTMI           0x00000000 
#define PORTSCX_PTS_ULPI           0x80000000 

+/* PCIE Registers */ 
+#define PEX_LTSSM_STAT               0x404 
+#define PEX_LTSSM_STAT_L0    0x16 
+#define PEX_GCLK_RATIO               0x440 
+

just move these into the .c file.

quoted

/* 
 * Declaration for the various functions exported by the 
 * mpc83xx_* files. Mostly for use by mpc83xx_setup 
 */ 
- 
-extern int mpc83xx_add_bridge(struct device_node *dev); 
+#define PPC_83XX_PCI 0x1 
+#define PPC_83XX_PCIE        0x2 
+extern int mpc83xx_add_bridge(struct device_node *dev, int flags); 
extern void mpc83xx_restart(char *cmd); 
extern long mpc83xx_time_init(void); 
extern int mpc834x_usb_cfg(void); 

diff --git a/arch/powerpc/platforms/83xx/pci.c b/arch/powerpc/ 

platforms/83xx/pci.c 
index 80425d7..0b52b2e 100644 

--- a/arch/powerpc/platforms/83xx/pci.c 
+++ b/arch/powerpc/platforms/83xx/pci.c 

@@ -25,6 +25,8 @@ 

#include <asm/prom.h> 
#include <sysdev/fsl_soc.h> 

+#include "mpc83xx.h" 
+ 
#undef DEBUG 

#ifdef DEBUG 

@@ -33,13 +35,157 @@ 

#define DBG(x...) 
#endif 

-int __init mpc83xx_add_bridge(struct device_node *dev) 
+#if defined(CONFIG_PPC_MPC83XX_PCIE) 
+ 
+/* PCIE config space Read/Write routines */

should really be called something like mpc83xx_pciex_read_config 

quoted

+static int direct_read_config_pcie(struct pci_bus *bus, 
+                     uint devfn, int offset, int len, u32 *val) 
+{ 
+     struct pci_controller *hose = bus->sysdata; 
+     void __iomem *cfg_addr; 
+     u32 bus_no; 
+ 
+     if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) 
+             return PCIBIOS_DEVICE_NOT_FOUND; 
+ 
+     if (ppc_md.pci_exclude_device) 
+             if (ppc_md.pci_exclude_device(hose, bus->number,

devfn)) 

quoted

+                     return PCIBIOS_DEVICE_NOT_FOUND; 
+ 
+     switch (len) { 
+     case 2: 
+             if (offset & 1) 
+                     return -EINVAL; 
+             break; 
+     case 4: 
+     if (offset & 3) 
+             return -EINVAL; 
+             break; 
+     }

fix formatting.

quoted

+ 
+     pr_debug("_read_cfg_pcie: bus=%d devfn=%x off=%x len=%x\n", 
+             bus->number, devfn, offset, len); 
+ 
+     if (bus->number == hose->first_busno) { 
+             if (devfn & 0xf8) 
+                     return PCIBIOS_DEVICE_NOT_FOUND;

what is the 0xf8 all about?

The pcie only have one link, so the dev number only can be 0, as well
the function number can be 0 ~ 7.

quoted

+             bus_no = hose->self_busno; 
+     } else 
+             bus_no = bus->number; 
+ 
+     cfg_addr = (void __iomem *)((ulong) hose->cfg_addr + 
+             ((bus_no << 20) | (devfn << 12) | (offset & 0xfff))); 
+ 
+     switch (len) { 
+     case 1: 
+             *val = in_8(cfg_addr); 
+             break; 
+     case 2: 
+             *val = in_le16(cfg_addr); 
+             break; 
+     default: 
+             *val = in_le32(cfg_addr); 
+             break; 
+     } 
+     pr_debug("_read_cfg_pcie: val=%x cfg_addr=%p\n", *val,

cfg_addr); 

quoted

+ 
+     return PCIBIOS_SUCCESSFUL; 
+} 
+ 
+static int direct_write_config_pcie(struct pci_bus *bus, 
+                     uint devfn, int offset, int len, u32 val) 
+{ 
+     struct pci_controller *hose = bus->sysdata; 
+     void __iomem *cfg_addr; 
+     u32 bus_no; 
+ 
+     if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) 
+             return PCIBIOS_DEVICE_NOT_FOUND; 
+ 
+     if (ppc_md.pci_exclude_device) 
+             if (ppc_md.pci_exclude_device(hose, bus->number,

devfn)) 

quoted

+                     return PCIBIOS_DEVICE_NOT_FOUND; 
+ 
+     switch (len) { 
+     case 2: 
+             if (offset & 1) 
+                     return -EINVAL; 
+             break; 
+     case 4: 
+             if (offset & 3) 
+                     return -EINVAL; 
+             break; 
+     } 
+ 
+     if (bus->number == hose->first_busno) { 
+             if (devfn & 0xf8) 
+                     return PCIBIOS_DEVICE_NOT_FOUND; 
+             bus_no = hose->self_busno; 
+     } else 
+             bus_no = bus->number; 
+ 
+     cfg_addr = (void __iomem *)((ulong) hose->cfg_addr + 
+             ((bus_no << 20) | (devfn << 12) | (offset & 0xfff))); 
+ 
+     switch (len) { 
+     case 1: 
+             out_8(cfg_addr, val); 
+             break; 
+     case 2: 
+             out_le16(cfg_addr, val); 
+             break; 
+     default: 
+             out_le32(cfg_addr, val); 
+             break; 
+     } 
+ 
+     return PCIBIOS_SUCCESSFUL; 
+} 
+ 
+static struct pci_ops direct_pcie_ops = { 
+     direct_read_config_pcie, 
+     direct_write_config_pcie 
+}; 
+ 
+void __init setup_direct_pcie(struct pci_controller *hose, u32  
cfg_addr, u32 cfg_size) 
+{ 
+     ulong base = cfg_addr & PAGE_MASK; 
+     void __iomem *mbase, *addr; 
+ 
+     mbase = ioremap(base, cfg_size);

this ioremap is going to be problematic in that cfg_size is going to  
be 256M. and we will not have enough kernel virtual address space to  
deal with it.

I agree.

quoted

+     addr = mbase + (cfg_addr & ~PAGE_MASK); 
+     hose->cfg_addr = addr; 
+     hose->ops = &direct_pcie_ops;

what's the point of this function, why not just fold it into  
mpc83xx_setup_pcie

quoted

+} 
+ 
+static void __init mpc83xx_setup_pcie(struct pci_controller *hose, 
+                     struct resource *reg, struct resource

*cfg_space) 

quoted

+{ 
+     void __iomem *hose_cfg_base; 
+     u32 val; 
+ 
+     hose_cfg_base = ioremap(reg->start, reg->end - reg->start +

1); 

quoted

+ 
+     val = in_le32(hose_cfg_base + PEX_LTSSM_STAT); 
+     if (val < PEX_LTSSM_STAT_L0) 
+             hose->indirect_type |=

PPC_INDIRECT_TYPE_NO_PCIE_LINK; 

quoted

+ 
+     setup_direct_pcie(hose, cfg_space->start, 
+                     cfg_space->end - cfg_space->start + 1); 
+} 
+#endif /* CONFIG_PPC_MPC83XX_PCIE */ 
+

We should do the same think fsl_pci does for primary, its passed
into  
_add_bridge.  Also, can we not do what fsl_pci does and use PCI  
capabilities to determine we are a PCIe controller (and drop passing  
it in via flags).

The mpc837x PCIE controller is a little different from mpc85xx.
It can not be access via PCI configure read/write. It only can be
accessed via memory-mapped read/write from core.

quoted

+int __init mpc83xx_add_bridge(struct device_node *dev, int flags) 
{ 
      int len; 
      struct pci_controller *hose; 
      struct resource rsrc; 
+#if defined(CONFIG_PPC_MPC83XX_PCIE) 
+     struct resource cfg_space; 
+#endif 
      const int *bus_range; 
-     int primary = 1, has_address = 0; 
+     static int primary = 1; 
+     int has_address = 0; 
      phys_addr_t immr = get_immrbase(); 

      DBG("Adding PCI host bridge %s\n", dev->full_name); 

@@ -66,14 +212,21 @@ int __init mpc83xx_add_bridge(struct  

device_node *dev) 
       * the other at 0x8600, we consider the 0x8500 the primary

controller 

quoted

       */ 
      /* PCI 1 */ 
-     if ((rsrc.start & 0xfffff) == 0x8500) { 
+     if ((rsrc.start & 0xfffff) == 0x8500) 
              setup_indirect_pci(hose, immr + 0x8300, immr + 0x8304,

0); 

quoted

-     } 
      /* PCI 2 */ 
-     if ((rsrc.start & 0xfffff) == 0x8600) { 
+     if ((rsrc.start & 0xfffff) == 0x8600) 
              setup_indirect_pci(hose, immr + 0x8380, immr + 0x8384,

0); 

quoted

-             primary = 0; 
+ 
+#if defined(CONFIG_PPC_MPC83XX_PCIE) 
+     if (flags & PPC_83XX_PCIE) { 
+             if (of_address_to_resource(dev, 1, &cfg_space)) { 
+                     printk("PCIE RC losts configure space. Skip it

\n"); 

quoted

+                     return 1; 
+             } 
+             mpc83xx_setup_pcie(hose, &rsrc, &cfg_space); 
      } 
+#endif 

      printk(KERN_INFO "Found MPC83xx PCI host bridge at 0x%016llx.

" 

quoted

             "Firmware bus number: %d->%d\n", 

@@ -86,6 +239,7 @@ int __init mpc83xx_add_bridge(struct

device_node  

quoted

*dev) 
      /* Interpret the "ranges" property */ 
      /* This also maps the I/O region and sets isa_io/mem_base */ 
      pci_process_bridge_OF_ranges(hose, dev, primary); 
+     primary = 0; 

      return 0; 
} 
-- 
1.5.2 



_______________________________________________ 
Linuxppc-dev mailing list 
Linuxppc-dev@ozlabs.org 
https://ozlabs.org/mailman/listinfo/linuxppc-dev