* [PATCH] Add MPC837xEMDS PCIE RC mode support
@ 2007-11-30 3:45 Li Li
2007-11-30 4:14 ` Olof Johansson
2007-11-30 7:37 ` Kumar Gala
0 siblings, 2 replies; 11+ messages in thread
From: Li Li @ 2007-11-30 3:45 UTC (permalink / raw)
To: Kumar Gala, linuxppc-dev
The PCIE controller is initiated in u-boot.
This patch is based on Leo`s mpc837xe patches.
Signed-off-by: Tony Li <tony.li@freescale.com>
---
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 +++++++++++++++++++++++++++-
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
--- a/arch/powerpc/boot/dts/mpc8377_mds.dts
+++ b/arch/powerpc/boot/dts/mpc8377_mds.dts
@@ -197,14 +197,6 @@
clock = <d#100>;
};
- serdes2:serdes@e3100 {
- compatible = "fsl,serdes";
- reg = <e3100 100>;
- vdd-1v;
- protocol = "pcie";
- clock = <d#100>;
- };
-
/* IPIC
* interrupts cell = <intr #, sense>
* sense values match linux IORESOURCE_IRQ_* defines:
@@ -279,4 +271,52 @@
compatible = "fsl,mpc8349-pci";
device_type = "pci";
};
+
+ pci2@e0009000 {
+ 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>;
+ compatible = "fsl,mpc8377-pcie";
+ device_type = "pci";
+ };
+
+ pci3@e000a000 {
+ 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 2 8
+ 0000 0 0 2 &ipic 2 8
+ 0000 0 0 3 &ipic 2 8
+ 0000 0 0 4 &ipic 2 8
+ >;
+ interrupt-parent = < &ipic >;
+ interrupts = <2 8>;
+ bus-range = <0 0>;
+ ranges = <02000000 0 C8000000 C8000000 0 10000000
+ 01000000 0 00000000 D8000000 0 00800000>;
+ clock-frequency = <0>;
+ #interrupt-cells = <1>;
+ #size-cells = <2>;
+ #address-cells = <3>;
+ reg = <e000a000 00001000
+ c0000000 08000000>;
+ compatible = "fsl,mpc8377-pcie";
+ device_type = "pci";
+ };
};
diff --git a/arch/powerpc/boot/dts/mpc8378_mds.dts b/arch/powerpc/boot/dts/mpc8378_mds.dts
index 54171f4..1503ae3 100644
--- a/arch/powerpc/boot/dts/mpc8378_mds.dts
+++ b/arch/powerpc/boot/dts/mpc8378_mds.dts
@@ -179,14 +179,6 @@
clock = <d#100>;
};
- serdes2:serdes@e3100 {
- compatible = "fsl,serdes";
- reg = <e3100 100>;
- vdd-1v;
- protocol = "pcie";
- clock = <d#100>;
- };
-
/* IPIC
* interrupts cell = <intr #, sense>
* sense values match linux IORESOURCE_IRQ_* defines:
@@ -261,4 +253,52 @@
compatible = "fsl,mpc8349-pci";
device_type = "pci";
};
+
+ pci2@e0009000 {
+ 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>;
+ compatible = "fsl,mpc8377-pcie";
+ device_type = "pci";
+ };
+
+ pci3@e000a000 {
+ 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 2 8
+ 0000 0 0 2 &ipic 2 8
+ 0000 0 0 3 &ipic 2 8
+ 0000 0 0 4 &ipic 2 8
+ >;
+ interrupt-parent = < &ipic >;
+ interrupts = <2 8>;
+ bus-range = <0 0>;
+ ranges = <02000000 0 C8000000 C8000000 0 10000000
+ 01000000 0 00000000 D8000000 0 00800000>;
+ clock-frequency = <0>;
+ #interrupt-cells = <1>;
+ #size-cells = <2>;
+ #address-cells = <3>;
+ reg = <e000a000 00001000
+ c0000000 08000000>;
+ compatible = "fsl,mpc8377-pcie";
+ device_type = "pci";
+ };
};
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
diff --git a/arch/powerpc/platforms/83xx/mpc8313_rdb.c b/arch/powerpc/platforms/83xx/mpc8313_rdb.c
index 33766b8..1a86a66 100644
--- a/arch/powerpc/platforms/83xx/mpc8313_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc8313_rdb.c
@@ -44,7 +44,7 @@ static void __init mpc8313_rdb_setup_arch(void)
#ifdef CONFIG_PCI
for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
- mpc83xx_add_bridge(np);
+ mpc83xx_add_bridge(np, PPC_83XX_PCI);
#endif
mpc831x_usb_cfg();
}
diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c b/arch/powerpc/platforms/83xx/mpc832x_mds.c
index 972fa85..8c5afaf 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
@@ -74,7 +74,7 @@ static void __init mpc832x_sys_setup_arch(void)
#ifdef CONFIG_PCI
for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
- mpc83xx_add_bridge(np);
+ mpc83xx_add_bridge(np, PPC_83XX_PCI);
#endif
#ifdef CONFIG_QUICC_ENGINE
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index fbca336..91a01bd 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -94,7 +94,7 @@ static void __init mpc832x_rdb_setup_arch(void)
#ifdef CONFIG_PCI
for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
- mpc83xx_add_bridge(np);
+ mpc83xx_add_bridge(np, PPC_83XX_PCI);
#endif
#ifdef CONFIG_QUICC_ENGINE
diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c b/arch/powerpc/platforms/83xx/mpc834x_itx.c
index aa76819..377f946 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
@@ -53,7 +53,7 @@ static void __init mpc834x_itx_setup_arch(void)
#ifdef CONFIG_PCI
for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
- mpc83xx_add_bridge(np);
+ mpc83xx_add_bridge(np, PPC_83XX_PCI);
#endif
mpc834x_usb_cfg();
diff --git a/arch/powerpc/platforms/83xx/mpc834x_mds.c b/arch/powerpc/platforms/83xx/mpc834x_mds.c
index 00aed7c..4dc17a5 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_mds.c
@@ -84,7 +84,7 @@ static void __init mpc834x_mds_setup_arch(void)
#ifdef CONFIG_PCI
for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
- mpc83xx_add_bridge(np);
+ mpc83xx_add_bridge(np, PPC_83XX_PCI);
#endif
mpc834xemds_usb_cfg();
diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c b/arch/powerpc/platforms/83xx/mpc836x_mds.c
index 0f3855c..5b13b34 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
@@ -80,7 +80,7 @@ static void __init mpc836x_mds_setup_arch(void)
#ifdef CONFIG_PCI
for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
- mpc83xx_add_bridge(np);
+ mpc83xx_add_bridge(np, PPC_83XX_PCI);
#endif
#ifdef CONFIG_QUICC_ENGINE
diff --git a/arch/powerpc/platforms/83xx/mpc837x_mds.c b/arch/powerpc/platforms/83xx/mpc837x_mds.c
index 166c111..6048f1b 100644
--- a/arch/powerpc/platforms/83xx/mpc837x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc837x_mds.c
@@ -43,7 +43,12 @@ static void __init mpc837x_mds_setup_arch(void)
#ifdef CONFIG_PCI
for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
- mpc83xx_add_bridge(np);
+ mpc83xx_add_bridge(np, PPC_83XX_PCI);
+#endif
+#ifdef CONFIG_PPC_MPC83XX_PCIE
+ for_each_compatible_node(np, "pci", "fsl,mpc8377-pcie") {
+ mpc83xx_add_bridge(np, PPC_83XX_PCIE);
+ }
#endif
}
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
+
/*
* 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 */
+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))
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ switch (len) {
+ case 2:
+ if (offset & 1)
+ return -EINVAL;
+ break;
+ case 4:
+ if (offset & 3)
+ return -EINVAL;
+ break;
+ }
+
+ 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;
+ 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);
+
+ 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))
+ 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);
+ addr = mbase + (cfg_addr & ~PAGE_MASK);
+ hose->cfg_addr = addr;
+ hose->ops = &direct_pcie_ops;
+}
+
+static void __init mpc83xx_setup_pcie(struct pci_controller *hose,
+ struct resource *reg, struct resource *cfg_space)
+{
+ void __iomem *hose_cfg_base;
+ u32 val;
+
+ hose_cfg_base = ioremap(reg->start, reg->end - reg->start + 1);
+
+ val = in_le32(hose_cfg_base + PEX_LTSSM_STAT);
+ if (val < PEX_LTSSM_STAT_L0)
+ hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+
+ setup_direct_pcie(hose, cfg_space->start,
+ cfg_space->end - cfg_space->start + 1);
+}
+#endif /* CONFIG_PPC_MPC83XX_PCIE */
+
+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
*/
/* PCI 1 */
- if ((rsrc.start & 0xfffff) == 0x8500) {
+ if ((rsrc.start & 0xfffff) == 0x8500)
setup_indirect_pci(hose, immr + 0x8300, immr + 0x8304, 0);
- }
/* PCI 2 */
- if ((rsrc.start & 0xfffff) == 0x8600) {
+ if ((rsrc.start & 0xfffff) == 0x8600)
setup_indirect_pci(hose, immr + 0x8380, immr + 0x8384, 0);
- 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");
+ return 1;
+ }
+ mpc83xx_setup_pcie(hose, &rsrc, &cfg_space);
}
+#endif
printk(KERN_INFO "Found MPC83xx PCI host bridge at 0x%016llx. "
"Firmware bus number: %d->%d\n",
@@ -86,6 +239,7 @@ int __init mpc83xx_add_bridge(struct device_node *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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 3:45 [PATCH] Add MPC837xEMDS PCIE RC mode support Li Li
@ 2007-11-30 4:14 ` Olof Johansson
2007-11-30 5:33 ` Li Li
2007-11-30 15:12 ` Scott Wood
2007-11-30 7:37 ` Kumar Gala
1 sibling, 2 replies; 11+ messages in thread
From: Olof Johansson @ 2007-11-30 4:14 UTC (permalink / raw)
To: Li Li; +Cc: linuxppc-dev, Kumar Gala
Hi,
On Fri, Nov 30, 2007 at 11:45:34AM +0800, Li Li wrote:
> + pci2@e0009000 {
Why call it pci2@ (and pci3@ below)? They are clearly identifiable with
their unit addresses anyway.
> +config PPC_MPC83XX_PCIE
> + bool "MPC837X PCI Express support"
> + depends on PCIEPORTBUS && PPC_MPC837x
> + default n
> + help
> + Enables MPC837x PCI express RC mode
Why have a separate config option for this?
For systems where you don't want PCI-e configured, it should be left
out of the device tree instead.
-Olof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 4:14 ` Olof Johansson
@ 2007-11-30 5:33 ` Li Li
2007-11-30 15:12 ` Scott Wood
1 sibling, 0 replies; 11+ messages in thread
From: Li Li @ 2007-11-30 5:33 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev, Gala Kumar, Li Tony
On Fri, 2007-11-30 at 12:14 +0800, Olof Johansson wrote:
> Hi,
>
> On Fri, Nov 30, 2007 at 11:45:34AM +0800, Li Li wrote:
>
> > + pci2@e0009000 {
>
> Why call it pci2@ (and pci3@ below)? They are clearly identifiable
> with
> their unit addresses anyway.
>
> > +config PPC_MPC83XX_PCIE
> > + bool "MPC837X PCI Express support"
> > + depends on PCIEPORTBUS && PPC_MPC837x
> > + default n
> > + help
> > + Enables MPC837x PCI express RC mode
>
> Why have a separate config option for this?
>
> For systems where you don't want PCI-e configured, it should be left
> out of the device tree instead.
It is easy to configure kernel than modify and recompile the dts.
The pcie is default enabled in dts.
>
>
>
> -Olof
>
- Tony
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 3:45 [PATCH] Add MPC837xEMDS PCIE RC mode support Li Li
2007-11-30 4:14 ` Olof Johansson
@ 2007-11-30 7:37 ` Kumar Gala
2007-11-30 8:48 ` Li Li
1 sibling, 1 reply; 11+ messages in thread
From: Kumar Gala @ 2007-11-30 7:37 UTC (permalink / raw)
To: Li Li; +Cc: linuxppc-dev
On Nov 29, 2007, at 9:45 PM, Li Li wrote:
> The PCIE controller is initiated in u-boot.
>
> This patch is based on Leo`s mpc837xe patches.
>
>
> Signed-off-by: Tony Li <tony.li@freescale.com>
> ---
> 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 ++++++++++++++++++++
> +++++++-
> 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
>
> +
> + pci2@e0009000 {
I agree w/Olof. This should be pcie@e0009000
>
> + 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?
> 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.
> 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.
>
> /*
> * 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
>
> +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))
> + 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.
>
> +
> + 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?
>
> + 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);
> +
> + 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))
> + 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.
> + 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
>
> +}
> +
> +static void __init mpc83xx_setup_pcie(struct pci_controller *hose,
> + struct resource *reg, struct resource *cfg_space)
> +{
> + void __iomem *hose_cfg_base;
> + u32 val;
> +
> + hose_cfg_base = ioremap(reg->start, reg->end - reg->start + 1);
> +
> + val = in_le32(hose_cfg_base + PEX_LTSSM_STAT);
> + if (val < PEX_LTSSM_STAT_L0)
> + hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> +
> + 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).
> +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
> */
> /* PCI 1 */
> - if ((rsrc.start & 0xfffff) == 0x8500) {
> + if ((rsrc.start & 0xfffff) == 0x8500)
> setup_indirect_pci(hose, immr + 0x8300, immr + 0x8304, 0);
> - }
> /* PCI 2 */
> - if ((rsrc.start & 0xfffff) == 0x8600) {
> + if ((rsrc.start & 0xfffff) == 0x8600)
> setup_indirect_pci(hose, immr + 0x8380, immr + 0x8384, 0);
> - 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");
> + return 1;
> + }
> + mpc83xx_setup_pcie(hose, &rsrc, &cfg_space);
> }
> +#endif
>
> printk(KERN_INFO "Found MPC83xx PCI host bridge at 0x%016llx. "
> "Firmware bus number: %d->%d\n",
> @@ -86,6 +239,7 @@ int __init mpc83xx_add_bridge(struct device_node
> *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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 7:37 ` Kumar Gala
@ 2007-11-30 8:48 ` Li Li
2007-11-30 9:05 ` Kumar Gala
0 siblings, 1 reply; 11+ messages in thread
From: Li Li @ 2007-11-30 8:48 UTC (permalink / raw)
To: Gala Kumar; +Cc: linuxppc-dev, Li Tony
On Fri, 2007-11-30 at 15:37 +0800, Gala Kumar wrote:
>
> On Nov 29, 2007, at 9:45 PM, Li Li wrote:
>
> > The PCIE controller is initiated in u-boot.
> >
> > This patch is based on Leo`s mpc837xe patches.
> >
> >
> > Signed-off-by: Tony Li <tony.li@freescale.com>
> > ---
> > 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
> ++++++++++++++++++++
> > +++++++-
> > 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
>
> >
> > +
> > + pci2@e0009000 {
>
> I agree w/Olof. This should be pcie@e0009000
> >
> > + 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.
>
>
> > 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.
> > 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.
>
> >
> > /*
> > * 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
> >
> > +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))
> > + 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.
>
> >
> > +
> > + 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.
> >
> > + 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);
> > +
> > + 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))
> > + 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.
> > + 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
>
> >
> > +}
> > +
> > +static void __init mpc83xx_setup_pcie(struct pci_controller *hose,
> > + struct resource *reg, struct resource
> *cfg_space)
> > +{
> > + void __iomem *hose_cfg_base;
> > + u32 val;
> > +
> > + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +
> 1);
> > +
> > + val = in_le32(hose_cfg_base + PEX_LTSSM_STAT);
> > + if (val < PEX_LTSSM_STAT_L0)
> > + hose->indirect_type |=
> PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> > +
> > + 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.
> > +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
> > */
> > /* PCI 1 */
> > - if ((rsrc.start & 0xfffff) == 0x8500) {
> > + if ((rsrc.start & 0xfffff) == 0x8500)
> > setup_indirect_pci(hose, immr + 0x8300, immr + 0x8304,
> 0);
> > - }
> > /* PCI 2 */
> > - if ((rsrc.start & 0xfffff) == 0x8600) {
> > + if ((rsrc.start & 0xfffff) == 0x8600)
> > setup_indirect_pci(hose, immr + 0x8380, immr + 0x8384,
> 0);
> > - 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");
> > + return 1;
> > + }
> > + mpc83xx_setup_pcie(hose, &rsrc, &cfg_space);
> > }
> > +#endif
> >
> > printk(KERN_INFO "Found MPC83xx PCI host bridge at 0x%016llx.
> "
> > "Firmware bus number: %d->%d\n",
> > @@ -86,6 +239,7 @@ int __init mpc83xx_add_bridge(struct
> device_node
> > *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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 8:48 ` Li Li
@ 2007-11-30 9:05 ` Kumar Gala
2007-11-30 9:37 ` Li Li
0 siblings, 1 reply; 11+ messages in thread
From: Kumar Gala @ 2007-11-30 9:05 UTC (permalink / raw)
To: Li Li; +Cc: linuxppc-dev, Li Tony
>>> +
>>> + pci2@e0009000 {
>>
>> I agree w/Olof. This should be pcie@e0009000
>>>
>>> + 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.
what do you mean too big? Aren't you losing access to some bus/dev/fn
than?
>>> 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.
Why do you something beyond CONFIG_PCI. if you don't want PCIe but do
want PCI the extra code for PCIe isn't going to kill you.
>>> 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.
>>
>>>
>>> /*
>>> * 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
>>>
>>> +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))
>>> + 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.
>>
>>>
>>> +
>>> + 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.
I don't follow what the number of links has to do with dev number.
>>> + 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
>>
>>>
>>> +}
>>> +
>>> +static void __init mpc83xx_setup_pcie(struct pci_controller *hose,
>>> + struct resource *reg, struct resource
>> *cfg_space)
>>> +{
>>> + void __iomem *hose_cfg_base;
>>> + u32 val;
>>> +
>>> + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +
>> 1);
>>> +
>>> + val = in_le32(hose_cfg_base + PEX_LTSSM_STAT);
>>> + if (val < PEX_LTSSM_STAT_L0)
>>> + hose->indirect_type |=
>> PPC_INDIRECT_TYPE_NO_PCIE_LINK;
>>> +
>>> + 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.
You don't have access to your own config space?
- k
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 9:05 ` Kumar Gala
@ 2007-11-30 9:37 ` Li Li
2007-11-30 14:57 ` Kumar Gala
0 siblings, 1 reply; 11+ messages in thread
From: Li Li @ 2007-11-30 9:37 UTC (permalink / raw)
To: Gala Kumar; +Cc: linuxppc-dev, Li Tony
On Fri, 2007-11-30 at 17:05 +0800, Gala Kumar wrote:
> >>> +
> >>> + pci2@e0009000 {
> >>
> >> I agree w/Olof. This should be pcie@e0009000
> >>>
> >>> + 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.
>
> what do you mean too big? Aren't you losing access to some
> bus/dev/fn
> than?
If do it standard, a 256M config space, at least 256M mem space and 16M
io space are needed for each PCIE controller.
To allocate PCIE window, the window size only can be 512M or 1G.
If we choose 1G space, two PCIE controller needs 2G space.
We do not have 2G free physical space now. Usually, we use upper 128M
configure space. So, we have to cut down the config space.
>
> >>> 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.
>
> Why do you something beyond CONFIG_PCI. if you don't want PCIe but
> do
> want PCI the extra code for PCIe isn't going to kill you.
>
Here is a little complex. The MPC837xE board needs a carrier board to
extend PCIE slot. If user does not populate carrier board onto MPC837xE
board and do PCIE scan, the system will halt.
I just want to provide a easy way to disable the PCIe other than modify
and recompile the dts.
> >>> 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.
ok.
> >>
> >>>
> >>> /*
> >>> * 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
> >>>
> >>> +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))
> >>> + 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.
> >>
> >>>
> >>> +
> >>> + 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.
>
> I don't follow what the number of links has to do with dev number.
The PCIE only have one link that mean one PCIE bus only can have one
device populate on it. The dev number of this device must be 0. If the
device number is not 0, we consider it is a error.
>
> >>> + 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
> >>
> >>>
> >>> +}
> >>> +
> >>> +static void __init mpc83xx_setup_pcie(struct pci_controller
> *hose,
> >>> + struct resource *reg, struct resource
> >> *cfg_space)
> >>> +{
> >>> + void __iomem *hose_cfg_base;
> >>> + u32 val;
> >>> +
> >>> + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +
> >> 1);
> >>> +
> >>> + val = in_le32(hose_cfg_base + PEX_LTSSM_STAT);
> >>> + if (val < PEX_LTSSM_STAT_L0)
> >>> + hose->indirect_type |=
> >> PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> >>> +
> >>> + 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.
>
> You don't have access to your own config space?
>
I can access it but can not via standard pci configure routine.
So, we use different way to access PCI and PCIE. If we want access PCI
capabilities, we must know it is PCI controller or PCIE controller
first.
- Tony
> - k
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 9:37 ` Li Li
@ 2007-11-30 14:57 ` Kumar Gala
2007-11-30 15:19 ` Scott Wood
2007-12-03 3:43 ` Li Li
0 siblings, 2 replies; 11+ messages in thread
From: Kumar Gala @ 2007-11-30 14:57 UTC (permalink / raw)
To: Li Li; +Cc: linuxppc-dev, Li Tony
On Nov 30, 2007, at 3:37 AM, Li Li wrote:
> On Fri, 2007-11-30 at 17:05 +0800, Gala Kumar wrote:
>>>>> +
>>>>> + pci2@e0009000 {
>>>>
>>>> I agree w/Olof. This should be pcie@e0009000
>>>>>
>>>>> + 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.
>>
>> what do you mean too big? Aren't you losing access to some
>> bus/dev/fn
>> than?
>
> If do it standard, a 256M config space, at least 256M mem space and
> 16M
> io space are needed for each PCIE controller.
> To allocate PCIE window, the window size only can be 512M or 1G.
> If we choose 1G space, two PCIE controller needs 2G space.
> We do not have 2G free physical space now. Usually, we use upper 128M
> configure space. So, we have to cut down the config space.
We'll cutting config space is problematic in that its a bug since you
might not be able to talk to a given device.
Is it possible to make the outbound window for cfg space 4k and change
the region of config space its looking at?
>>>>> 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.
>>
>> Why do you something beyond CONFIG_PCI. if you don't want PCIe but
>> do
>> want PCI the extra code for PCIe isn't going to kill you.
>>
>
> Here is a little complex. The MPC837xE board needs a carrier board to
> extend PCIE slot. If user does not populate carrier board onto
> MPC837xE
> board and do PCIE scan, the system will halt.
> I just want to provide a easy way to disable the PCIe other than
> modify
> and recompile the dts.
So I have to recompile the kernel for this case, that seems even more
painful to the user. Can we not detect if this board isn't there and
not hang?
>>>>> 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.
>
> ok.
>
>>>>
>>>>>
>>>>> /*
>>>>> * 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
>>>>>
>>>>> +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))
>>>>> + 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.
>>>>
>>>>>
>>>>> +
>>>>> + 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.
>>
>> I don't follow what the number of links has to do with dev number.
>
> The PCIE only have one link that mean one PCIE bus only can have one
> device populate on it. The dev number of this device must be 0. If the
> device number is not 0, we consider it is a error.
we should add a comment about that.
>>>>> + 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
>>>>
>>>>>
>>>>> +}
>>>>> +
>>>>> +static void __init mpc83xx_setup_pcie(struct pci_controller
>> *hose,
>>>>> + struct resource *reg, struct resource
>>>> *cfg_space)
>>>>> +{
>>>>> + void __iomem *hose_cfg_base;
>>>>> + u32 val;
>>>>> +
>>>>> + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +
>>>> 1);
>>>>> +
>>>>> + val = in_le32(hose_cfg_base + PEX_LTSSM_STAT);
>>>>> + if (val < PEX_LTSSM_STAT_L0)
>>>>> + hose->indirect_type |=
>>>> PPC_INDIRECT_TYPE_NO_PCIE_LINK;
>>>>> +
>>>>> + 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.
>>
>> You don't have access to your own config space?
>>
>
> I can access it but can not via standard pci configure routine.
> So, we use different way to access PCI and PCIE. If we want access PCI
> capabilities, we must know it is PCI controller or PCIE controller
> first.
Duh, your right.. we should than expand flags to also have a
PPC_83XX_PCI_IS_PRIMARY and let that get set by the caller.
- k
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 4:14 ` Olof Johansson
2007-11-30 5:33 ` Li Li
@ 2007-11-30 15:12 ` Scott Wood
1 sibling, 0 replies; 11+ messages in thread
From: Scott Wood @ 2007-11-30 15:12 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev, Kumar Gala, Li Li
On Thu, Nov 29, 2007 at 10:14:04PM -0600, Olof Johansson wrote:
> On Fri, Nov 30, 2007 at 11:45:34AM +0800, Li Li wrote:
> > + help
> > + Enables MPC837x PCI express RC mode
>
> Why have a separate config option for this?
To save code size when it isn't needed?
> For systems where you don't want PCI-e configured, it should be left
> out of the device tree instead.
The dts should represent what exists on the hardware, not what you want to
use.
-Scott
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 14:57 ` Kumar Gala
@ 2007-11-30 15:19 ` Scott Wood
2007-12-03 3:43 ` Li Li
1 sibling, 0 replies; 11+ messages in thread
From: Scott Wood @ 2007-11-30 15:19 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Li Li, Li Tony
On Fri, Nov 30, 2007 at 08:57:33AM -0600, Kumar Gala wrote:
> On Nov 30, 2007, at 3:37 AM, Li Li wrote:
> > If do it standard, a 256M config space, at least 256M mem space and
> > 16M
> > io space are needed for each PCIE controller.
> > To allocate PCIE window, the window size only can be 512M or 1G.
> > If we choose 1G space, two PCIE controller needs 2G space.
> > We do not have 2G free physical space now. Usually, we use upper 128M
> > configure space. So, we have to cut down the config space.
That decision should be made in the kernel, not the dts.
> Is it possible to make the outbound window for cfg space 4k and change
> the region of config space its looking at?
Yes, that'd be best.
> > Here is a little complex. The MPC837xE board needs a carrier board to
> > extend PCIE slot. If user does not populate carrier board onto MPC837xE
> > board and do PCIE scan, the system will halt. I just want to provide a
> > easy way to disable the PCIe other than modify and recompile the dts.
>
> So I have to recompile the kernel for this case, that seems even more
> painful to the user. Can we not detect if this board isn't there and
> not hang?
Assuming it's similar to PCI on other 83xxMDS, yes -- there's a bit in the
reset control words that can be checked.
We don't currently do it on any 83xx that I'm aware of, though. :-P
-Scott
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add MPC837xEMDS PCIE RC mode support
2007-11-30 14:57 ` Kumar Gala
2007-11-30 15:19 ` Scott Wood
@ 2007-12-03 3:43 ` Li Li
1 sibling, 0 replies; 11+ messages in thread
From: Li Li @ 2007-12-03 3:43 UTC (permalink / raw)
To: Gala Kumar, Wood Scott; +Cc: linuxppc-dev
On Fri, 2007-11-30 at 22:57 +0800, Gala Kumar wrote:
>
> On Nov 30, 2007, at 3:37 AM, Li Li wrote:
>
> > On Fri, 2007-11-30 at 17:05 +0800, Gala Kumar wrote:
> >>>>> +
> >>>>> + pci2@e0009000 {
> >>>>
> >>>> I agree w/Olof. This should be pcie@e0009000
> >>>>>
> >>>>> + 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.
> >>
> >> what do you mean too big? Aren't you losing access to some
> >> bus/dev/fn
> >> than?
> >
> > If do it standard, a 256M config space, at least 256M mem space
> and
> > 16M
> > io space are needed for each PCIE controller.
> > To allocate PCIE window, the window size only can be 512M or 1G.
> > If we choose 1G space, two PCIE controller needs 2G space.
> > We do not have 2G free physical space now. Usually, we use upper
> 128M
> > configure space. So, we have to cut down the config space.
>
> We'll cutting config space is problematic in that its a bug since
> you
> might not be able to talk to a given device.
>
> Is it possible to make the outbound window for cfg space 4k and
> change
> the region of config space its looking at?
>
I think that is possible but it means we have to reconfigure the PCIE
controller`s outbound window at every time we want do PCI config space
access in kernel. By now, all the PCIE controller is initialized on
u-boot which like PCI controller`s case.So it is a little not compatible
with the current kernel and u-boot assignment.
> >>>>> 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.
> >>
> >> Why do you something beyond CONFIG_PCI. if you don't want PCIe
> but
> >> do
> >> want PCI the extra code for PCIe isn't going to kill you.
> >>
> >
> > Here is a little complex. The MPC837xE board needs a carrier board
> to
> > extend PCIE slot. If user does not populate carrier board onto
> > MPC837xE
> > board and do PCIE scan, the system will halt.
> > I just want to provide a easy way to disable the PCIe other than
> > modify
> > and recompile the dts.
>
> So I have to recompile the kernel for this case, that seems even
> more
> painful to the user. Can we not detect if this board isn't there
> and
> not hang?
>
Yes. you are right. But I think we lack of a mean to tell the user the
exact mean of each device node and how to edit it.
The linux menuconfig is more directed.
As Scott said and AMHO, the dts describes what we have not what we want.
If dts describes what we have, we must provide many dts files to provide
various cpu feature combinations.
> >>>>> 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.
> >
> > ok.
> >
> >>>>
> >>>>>
> >>>>> /*
> >>>>> * 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
> >>>>>
> >>>>> +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))
> >>>>> + 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.
> >>>>
> >>>>>
> >>>>> +
> >>>>> + 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.
> >>
> >> I don't follow what the number of links has to do with dev number.
> >
> > The PCIE only have one link that mean one PCIE bus only can have
> one
> > device populate on it. The dev number of this device must be 0. If
> the
> > device number is not 0, we consider it is a error.
>
> we should add a comment about that.
>
yes
> >>>>> + 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
> >>>>
> >>>>>
> >>>>> +}
> >>>>> +
> >>>>> +static void __init mpc83xx_setup_pcie(struct pci_controller
> >> *hose,
> >>>>> + struct resource *reg, struct resource
> >>>> *cfg_space)
> >>>>> +{
> >>>>> + void __iomem *hose_cfg_base;
> >>>>> + u32 val;
> >>>>> +
> >>>>> + hose_cfg_base = ioremap(reg->start, reg->end - reg->start
> +
> >>>> 1);
> >>>>> +
> >>>>> + val = in_le32(hose_cfg_base + PEX_LTSSM_STAT);
> >>>>> + if (val < PEX_LTSSM_STAT_L0)
> >>>>> + hose->indirect_type |=
> >>>> PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> >>>>> +
> >>>>> + 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.
> >>
> >> You don't have access to your own config space?
> >>
> >
> > I can access it but can not via standard pci configure routine.
> > So, we use different way to access PCI and PCIE. If we want access
> PCI
> > capabilities, we must know it is PCI controller or PCIE controller
> > first.
>
> Duh, your right.. we should than expand flags to also have a
> PPC_83XX_PCI_IS_PRIMARY and let that get set by the caller.
>
yes
> - k
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2007-12-03 3:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-30 3:45 [PATCH] Add MPC837xEMDS PCIE RC mode support Li Li
2007-11-30 4:14 ` Olof Johansson
2007-11-30 5:33 ` Li Li
2007-11-30 15:12 ` Scott Wood
2007-11-30 7:37 ` Kumar Gala
2007-11-30 8:48 ` Li Li
2007-11-30 9:05 ` Kumar Gala
2007-11-30 9:37 ` Li Li
2007-11-30 14:57 ` Kumar Gala
2007-11-30 15:19 ` Scott Wood
2007-12-03 3:43 ` Li Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).