Add MPC837xEMDS PCIE RC mode support
PATCH 1/1 Add MPC837xEMDS PCIE RC mode support
The PCIE controller is initiated in u-boot.
This patch is based on Leo`s mpc837xe patches.
--- 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(-)
arch/powerpc/boot/dts/mpc8377_mds.dts +48 -8
@@ -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"; + }; };
arch/powerpc/boot/dts/mpc8378_mds.dts +48 -8
66 lines not on this page · raw message
arch/powerpc/platforms/83xx/Kconfig +7 -0
@@ -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
arch/powerpc/platforms/83xx/mpc8313_rdb.c +1 -1
@@ -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 (); }
arch/powerpc/platforms/83xx/mpc832x_mds.c +1 -1
@@ -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
arch/powerpc/platforms/83xx/mpc832x_rdb.c +1 -1
@@ -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
arch/powerpc/platforms/83xx/mpc834x_itx.c +1 -1
@@ -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 ();
arch/powerpc/platforms/83xx/mpc834x_mds.c +1 -1
@@ -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 ();
arch/powerpc/platforms/83xx/mpc836x_mds.c +1 -1
@@ -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
arch/powerpc/platforms/83xx/mpc837x_mds.c +6 -1
@@ -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 }
arch/powerpc/platforms/83xx/mpc83xx.h +8 -2
@@ -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 );
arch/powerpc/platforms/83xx/pci.c +160 -6
199 lines not on this page · raw message
Signature · 1 line
1.5.2
Hi,
Review
Quoted 1 line · Li Li
(quoted diff) +1
-0+ pci2@e0009000 {
Why call it pci2@ (and pci3@ below)? They are clearly identifiable with their unit addresses anyway.
arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 6 lines
arch/powerpc/platforms/83xx/Kconfig +6
-0+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
Quoted 17 lines · Olof Johansson
Hi,
Quoted 1 line · Li Li
+ pci2@e0009000 {
Why call it pci2@ (and pci3@ below)? They are clearly identifiable with
their unit addresses anyway.arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 6 lines
arch/powerpc/platforms/83xx/Kconfig +6
-0+config PPC_MPC83XX_PCIE + bool "MPC837X PCI Express support" + depends on PCIEPORTBUS && PPC_MPC837x + default n + help + Enables MPC837x PCI express RC mode
Quoted 4 lines
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.
Quoted 5 lines
-Olof
- Tony
Quoted 5 lines · Olof Johansson
arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 2 lines · Li Li
arch/powerpc/platforms/83xx/Kconfig +2
-0+ help + Enables MPC837x PCI express RC modeWhy have a separate config option for this?
To save code size when it isn't needed?
Quoted 2 lines
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
Quoted 24 lines · Li Li
The PCIE controller is initiated in u-boot.
This patch is based on Leo`s mpc837xe patches.
Signed-off-by Tony Li <[email protected]>---
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(-)arch/powerpc/boot/dts/mpc8377_mds.dts +0
-0
arch/powerpc/boot/dts/mpc8377_mds.dts line 275
Quoted 4 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +2
-0+ + pci2@e0009000 {
I agree w/Olof. This should be pcie@e0009000
Quoted 10 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +9
-0+ 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 >;
arch/powerpc/boot/dts/mpc8377_mds.dts line 294
Quoted 10 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +10
-0+ 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?
Quoted 5 lines
arch/powerpc/platforms/83xx/Kconfig +0
-0
arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 10 lines
arch/powerpc/platforms/83xx/Kconfig +7
-0@@ -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.
arch/powerpc/platforms/83xx/mpc83xx.h line 50
Quoted 13 lines
arch/powerpc/platforms/83xx/mpc83xx.h +5
-0@@ -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 27 lines
arch/powerpc/platforms/83xx/mpc83xx.h +3
-2/* * 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 );arch/powerpc/platforms/83xx/pci.c +2
-0@@ -25,6 +25,8 @@#include <asm/prom.h> #include <sysdev/fsl_soc.h> +#include "mpc83xx.h" +#undef DEBUG#ifdef DEBUG
arch/powerpc/platforms/83xx/pci.c line 40
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +3
-1@@ -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 15 lines
arch/powerpc/platforms/83xx/pci.c +14
-0+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 ; +
arch/powerpc/platforms/83xx/pci.c line 64
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c +10
-0+switch (len) { +case 2 : +if (offset &1 ) +return -EINVAL ; +break ; +case 4 : +if (offset &3 ) +return -EINVAL ; +break ; + }
fix formatting.
arch/powerpc/platforms/83xx/pci.c line 71
Quoted 8 lines
arch/powerpc/platforms/83xx/pci.c +7
-0+ +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?
Quoted 76 lines
arch/powerpc/platforms/83xx/pci.c +75
-0+ 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,
arch/powerpc/platforms/83xx/pci.c line 155
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c +9
-0+ 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.
arch/powerpc/platforms/83xx/pci.c line 158
Quoted 3 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+ 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 10 lines
arch/powerpc/platforms/83xx/pci.c +9
-0+} + +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 );
arch/powerpc/platforms/83xx/pci.c line 177
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c +10
-0+ + 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).
Quoted 60 lines
arch/powerpc/platforms/83xx/pci.c +6
-1+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);arch/powerpc/platforms/83xx/pci.c +11
-4@@ -66,14 +212,21 @@ int __init mpc83xx_add_bridge(structdevice_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",arch/powerpc/platforms/83xx/pci.c +1
-0@@ -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 ;}
Signature · 8 lines
1.5.2 _______________________________________________ Linuxppc-dev mailing list [email protected] https://ozlabs.org/mailman/listinfo/linuxppc-dev
Quoted 56 lines · Gala Kumar
Quoted 7 lines · Li Li
The PCIE controller is initiated in u-boot.
This patch is based on Leo`s mpc837xe patches.
Signed-off-by Tony Li <[email protected]>---
Quoted 12 lines
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 10 lines
+++++++-
12 files changed, 283 insertions(+), 31 deletions(-)arch/powerpc/ renamed from arch/powerpc/boot/dts/mpc8377_mds.dts +0
-0
boot/dts/mpc8377_mds.dts
index 4402e39..1f7819e 100644+
+ pci2@e0009000 {I agree w/Olof. This should be pcie@e0009000
Quoted 10 lines
arch/powerpc/ renamed from arch/powerpc/boot/dts/mpc8377_mds.dts +9
-0+ 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 >;arch/powerpc/
Quoted 10 lines
arch/powerpc/ renamed from arch/powerpc/boot/dts/mpc8377_mds.dts +10
-0+ 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>;
Quoted 1 line
Shouldn't the reg size for the cfg space be 256M?
256M is a little too big for kernel.
Quoted 13 lines
Quoted 13 lines
arch/powerpc/ renamed from arch/powerpc/platforms/83xx/Kconfig +0
-0platforms/83xx/Kconfig
index 0c61e7a..00154c5 100644arch/powerpc/platforms/83xx/Kconfig +2
-0@@ -87,3 +87,10 @@ config PPC_MPC837x select PPC_INDIRECT_PCI select FSL_SERDES default y if MPC837x_MDS + +config PPC_MPC83XX_PCIE
Quoted 9 lines
arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 5 lines
arch/powerpc/platforms/83xx/Kconfig +5
-0+ bool "MPC837X PCI Express support" + depends on PCIEPORTBUS && PPC_MPC837x + default n + help + Enables MPC837x PCI express RC modeThis should be a hidden config that is just selected by
PPC_MPC837xinstead 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 92 lines
Quoted 6 lines
arch/powerpc/ renamed from arch/powerpc/platforms/83xx/mpc83xx.h +0
-0
platforms/83xx/mpc83xx.h
index b778cb4..2078da7 100644arch/powerpc/platforms/83xx/mpc83xx.h line 50
Quoted 8 lines
arch/powerpc/platforms/83xx/mpc83xx.h +5
-0@@ -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 28 lines
arch/powerpc/platforms/83xx/mpc83xx.h +3
-2/* * 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 );arch/powerpc/ renamed from arch/powerpc/platforms/83xx/pci.c +0
-0
platforms/83xx/pci.c
index 80425d7..0b52b2e 100644arch/powerpc/platforms/83xx/pci.c +2
-0@@ -25,6 +25,8 @@#include <asm/prom.h> #include <sysdev/fsl_soc.h> +#include "mpc83xx.h" +#undef DEBUG#ifdef DEBUG arch/powerpc/platforms/83xx/pci.c line 40
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +3
-1@@ -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_configarch/powerpc/platforms/83xx/pci.c
Quoted 13 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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))
arch/powerpc/platforms/83xx/pci.c line 64
Quoted 12 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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.
arch/powerpc/platforms/83xx/pci.c line 71
Quoted 8 lines
arch/powerpc/platforms/83xx/pci.c +7
-0+ +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 ;
Quoted 2 lines
what is the
0xf8all 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 84 lines
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c +9
-0+ 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 :arch/powerpc/platforms/83xx/pci.c
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c +10
-0+ *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 6 lines
arch/powerpc/platforms/83xx/pci.c +6
-0+ +return PCIBIOS_SUCCESSFUL ; +} + +static int direct_write_config_pcie (struct pci_bus *bus, +uint devfn,int offset,int len,u32 val)arch/powerpc/platforms/83xx/pci.c
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c +10
-0+{ +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 46 lines
arch/powerpc/platforms/83xx/pci.c +45
-0+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) +{
Quoted 9 lines
arch/powerpc/platforms/83xx/pci.c line 155
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+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_sizeis going to be 256M. and we will not have enough kernel virtual address space to deal with it.
I agree.
Quoted 24 lines
arch/powerpc/platforms/83xx/pci.c line 158
Quoted 3 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+ 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_pciearch/powerpc/platforms/83xx/pci.c
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+} + +static void __init mpc83xx_setup_pcie (struct pci_controller *hose, + struct resource *reg, struct resource
*cfg_space)arch/powerpc/platforms/83xx/pci.c
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +5
-0+{ +void __iomem *hose_cfg_base; +u32 val; + + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +1);
arch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+ + 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 13 lines
arch/powerpc/platforms/83xx/pci.c line 177
Quoted 6 lines
arch/powerpc/platforms/83xx/pci.c +6
-0+ +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_pcidoes for primary, its passed into_add_bridge. Also, can we not do whatfsl_pcidoes 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 67 lines
Quoted 16 lines
arch/powerpc/platforms/83xx/pci.c +6
-1+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);arch/powerpc/platforms/83xx/pci.c int __init mpc83xx_add_bridge(struct · line 213
Quoted 2 lines
arch/powerpc/platforms/83xx/pci.c +0
-0@@ -66,14 +212,21 @@ int __init mpc83xx_add_bridge(struct device_node *dev) * the other at 0x8600, we consider the 0x8500 the primarycontroller
arch/powerpc/platforms/83xx/pci.c int __init mpc83xx_add_bridge(struct device_node *dev)
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +1
-1*/ /* PCI 1 */ -if ((rsrc.start &0xfffff ) ==0x8500 ) { +if ((rsrc.start &0xfffff ) ==0x8500 ) setup_indirect_pci(hose, immr + 0x8300, immr + 0x8304,0);
arch/powerpc/platforms/83xx/pci.c int __init mpc83xx_add_bridge(struct device_node *dev)
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +1
-2- }/* PCI 2 */ -if ((rsrc.start &0xfffff ) ==0x8600 ) { +if ((rsrc.start &0xfffff ) ==0x8600 ) setup_indirect_pci(hose, immr + 0x8380, immr + 0x8384,0);
arch/powerpc/platforms/83xx/pci.c int __init mpc83xx_add_bridge(struct device_node *dev)
Quoted 6 lines
arch/powerpc/platforms/83xx/pci.c +5
-1- 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");
arch/powerpc/platforms/83xx/pci.c int __init mpc83xx_add_bridge(struct device_node *dev)
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+ return 1; + } +mpc83xx_setup_pcie (hose, &rsrc, &cfg_space); } +#endif printk(KERN_INFO "Found MPC83xx PCI host bridge at 0x%016llx."
arch/powerpc/platforms/83xx/pci.c int __init mpc83xx_add_bridge(struct
Quoted 2 lines
"Firmware bus number: %d->%d\n",arch/powerpc/platforms/83xx/pci.c +0
-0@@ -86,6 +239,7 @@ int __init mpc83xx_add_bridge(struct
device_nodeQuoted 17 lines
arch/powerpc/platforms/83xx/pci.c +1
-0*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 ;}
Signature · 8 lines
1.5.2 _______________________________________________ Linuxppc-dev mailing list [email protected] https://ozlabs.org/mailman/listinfo/linuxppc-dev
Quoted 25 lines
Quoted 25 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ + pci2@e0009000 {I agree w/Olof. This should be pcie@e0009000
Quoted 20 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +19
-0+ 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>;
Quoted 3 lines
Quoted 1 line
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?
Quoted 21 lines
Quoted 11 lines
Quoted 11 lines
arch/powerpc/ renamed from arch/powerpc/platforms/83xx/Kconfig +0
-0platforms/83xx/Kconfig
index 0c61e7a..00154c5 100644arch/powerpc/platforms/83xx/Kconfig +2
-0@@ -87,3 +87,10 @@ config PPC_MPC837x select PPC_INDIRECT_PCI select FSL_SERDES default y if MPC837x_MDS + +config PPC_MPC83XX_PCIEQuoted 9 lines
arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 5 lines
arch/powerpc/platforms/83xx/Kconfig +5
-0+ bool "MPC837X PCI Express support" + depends on PCIEPORTBUS && PPC_MPC837x + default n + help + Enables MPC837x PCI express RC modeThis should be a hidden config that is just selected by
PPC_MPC837xinstead of a choice. Also drop the depends on.
Quoted 3 lines
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.
Quoted 95 lines
Quoted 84 lines
Quoted 6 lines
arch/powerpc/ renamed from arch/powerpc/platforms/83xx/mpc83xx.h +0
-0
platforms/83xx/mpc83xx.h
index b778cb4..2078da7 100644arch/powerpc/platforms/83xx/mpc83xx.h line 50
Quoted 8 lines
arch/powerpc/platforms/83xx/mpc83xx.h +5
-0@@ -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 28 lines
arch/powerpc/platforms/83xx/mpc83xx.h +3
-2/* * 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 );arch/powerpc/ renamed from arch/powerpc/platforms/83xx/pci.c +0
-0
platforms/83xx/pci.c
index 80425d7..0b52b2e 100644arch/powerpc/platforms/83xx/pci.c +2
-0@@ -25,6 +25,8 @@#include <asm/prom.h> #include <sysdev/fsl_soc.h> +#include "mpc83xx.h" +#undef DEBUG#ifdef DEBUG arch/powerpc/platforms/83xx/pci.c line 40
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +3
-1@@ -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_configarch/powerpc/platforms/83xx/pci.c
Quoted 13 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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))
arch/powerpc/platforms/83xx/pci.c line 64
Quoted 12 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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 1 line
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c line 71
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +7
-0+ +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
0xf8all about?
Quoted 2 lines
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.
Quoted 38 lines
Quoted 24 lines
arch/powerpc/platforms/83xx/pci.c line 158
Quoted 3 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+ 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_pciearch/powerpc/platforms/83xx/pci.c
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+} + +static void __init mpc83xx_setup_pcie (struct pci_controller *hose, + struct resource *reg, struct resource
*cfg_space)arch/powerpc/platforms/83xx/pci.c
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +5
-0+{ +void __iomem *hose_cfg_base; +u32 val; + + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +1);
arch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+ + 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 13 lines
arch/powerpc/platforms/83xx/pci.c line 177
Quoted 6 lines
arch/powerpc/platforms/83xx/pci.c +6
-0+ +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_pcidoes for primary, its passed into_add_bridge. Also, can we not do whatfsl_pcidoes and use PCI capabilities to determine we are a PCIe controller (and drop passing it in via flags).
Quoted 3 lines
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
Quoted 25 lines · Gala Kumar
Quoted 25 lines
Quoted 25 lines
Quoted 2 lines
+
+ pci2@e0009000 {I agree w/Olof. This should be pcie@e0009000
Quoted 20 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +19
-0+ 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>;
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 23 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
arch/powerpc/ renamed from arch/powerpc/platforms/83xx/Kconfig +0
-0platforms/83xx/Kconfig
index 0c61e7a..00154c5 100644arch/powerpc/platforms/83xx/Kconfig +1
-0@@ -87,3 +87,10 @@ config PPC_MPC837x select PPC_INDIRECT_PCI select FSL_SERDES default y if MPC837x_MDS +Quoted 8 lines
Quoted 8 lines
arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 6 lines
arch/powerpc/platforms/83xx/Kconfig +6
-0+config PPC_MPC83XX_PCIE + bool "MPC837X PCI Express support" + depends on PCIEPORTBUS && PPC_MPC837x + default n + help + Enables MPC837x PCI express RC modeThis should be a hidden config that is just selected by
PPC_MPC837xQuoted 3 lines
Quoted 2 lines
instead of a choice. Also drop the depends on.
Quoted 8 lines
Quoted 3 lines
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.
Quoted 17 lines
Quoted 1 line
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0
-0b/arch/powerpc/
Quoted 15 lines
Quoted 15 lines
arch/powerpc/platforms/83xx/mpc83xx.h line 50
Quoted 13 lines
platforms/83xx/mpc83xx.h
index b778cb4..2078da7 100644arch/powerpc/platforms/83xx/mpc83xx.h +5
-0@@ -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.
Quoted 77 lines
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/mpc83xx.h
Quoted 11 lines
arch/powerpc/platforms/83xx/mpc83xx.h +3
-2/* * 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, intflags);
Quoted 65 lines
Quoted 65 lines
Quoted 18 lines
arch/powerpc/platforms/83xx/mpc83xx.h +0
-0extern void mpc83xx_restart (char *cmd);extern long mpc83xx_time_init (void );extern int mpc834x_usb_cfg (void );arch/powerpc/ renamed from arch/powerpc/platforms/83xx/pci.c +0
-0
platforms/83xx/pci.c
index 80425d7..0b52b2e 100644arch/powerpc/platforms/83xx/pci.c +2
-0@@ -25,6 +25,8 @@#include <asm/prom.h> #include <sysdev/fsl_soc.h> +#include "mpc83xx.h" +#undef DEBUG#ifdef DEBUG arch/powerpc/platforms/83xx/pci.c line 40
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +3
-1@@ -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_configarch/powerpc/platforms/83xx/pci.c
Quoted 13 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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))
arch/powerpc/platforms/83xx/pci.c line 64
Quoted 12 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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 8 lines
arch/powerpc/platforms/83xx/pci.c +7
-0+ +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 ;
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
what is the
0xf8all about?The pcie only have one link, so the dev number only can be 0, as
well
Quoted 1 line
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.
Quoted 38 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+ addr = mbase + (cfg_addr & ~PAGE_MASK ); + hose->cfg_addr = addr; + hose->ops = &direct_pcie_ops;Quoted 7 lines
Quoted 7 lines
what's the point of this function, why not just fold it into
mpc83xx_setup_pciearch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+} + +static void __init mpc83xx_setup_pcie(struct pci_controller*hose,
Quoted 13 lines
Quoted 13 lines
arch/powerpc/platforms/83xx/pci.c line 78
Quoted 1 line
arch/powerpc/platforms/83xx/pci.c +1
-0+ struct resource *reg, struct resource
*cfg_space)arch/powerpc/platforms/83xx/pci.c
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +5
-0+{ +void __iomem *hose_cfg_base; +u32 val; + + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +1);
arch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+ + 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 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/pci.c line 177
Quoted 6 lines
arch/powerpc/platforms/83xx/pci.c +6
-0+ +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_pcidoes for primary, its passed into_add_bridge. Also, can we not do whatfsl_pcidoes and use PCI capabilities to determine we are a PCIe controller (and droppassing
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
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
Quoted 2 lines
- k
Quoted 34 lines · Li Li
Quoted 25 lines · Gala Kumar
Quoted 25 lines
Quoted 25 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ + pci2@e0009000 {I agree w/Olof. This should be pcie@e0009000
Quoted 20 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +19
-0+ 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>;Quoted 7 lines
Quoted 3 lines
Quoted 1 line
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?
Quoted 6 lines
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?
Quoted 31 lines
Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
arch/powerpc/ renamed from arch/powerpc/platforms/83xx/Kconfig +0
-0platforms/83xx/Kconfig
index 0c61e7a..00154c5 100644arch/powerpc/platforms/83xx/Kconfig +1
-0@@ -87,3 +87,10 @@ config PPC_MPC837x select PPC_INDIRECT_PCI select FSL_SERDES default y if MPC837x_MDS +Quoted 8 lines
Quoted 8 lines
arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 6 lines
arch/powerpc/platforms/83xx/Kconfig +6
-0+config PPC_MPC83XX_PCIE + bool "MPC837X PCI Express support" + depends on PCIEPORTBUS && PPC_MPC837x + default n + help + Enables MPC837x PCI express RC modeThis should be a hidden config that is just selected by
PPC_MPC837xQuoted 3 lines
Quoted 2 lines
instead of a choice. Also drop the depends on.
Quoted 8 lines
Quoted 3 lines
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.
Quoted 5 lines
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?
Quoted 101 lines
Quoted 17 lines
Quoted 1 line
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0
-0b/arch/powerpc/
Quoted 15 lines
Quoted 15 lines
arch/powerpc/platforms/83xx/mpc83xx.h line 50
Quoted 13 lines
platforms/83xx/mpc83xx.h
index b778cb4..2078da7 100644arch/powerpc/platforms/83xx/mpc83xx.h +5
-0@@ -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.
Quoted 81 lines
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/mpc83xx.h
Quoted 11 lines
arch/powerpc/platforms/83xx/mpc83xx.h +3
-2/* * 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, intflags);
Quoted 69 lines
Quoted 57 lines
Quoted 18 lines
arch/powerpc/platforms/83xx/mpc83xx.h +0
-0extern void mpc83xx_restart (char *cmd);extern long mpc83xx_time_init (void );extern int mpc834x_usb_cfg (void );arch/powerpc/ renamed from arch/powerpc/platforms/83xx/pci.c +0
-0
platforms/83xx/pci.c
index 80425d7..0b52b2e 100644arch/powerpc/platforms/83xx/pci.c +2
-0@@ -25,6 +25,8 @@#include <asm/prom.h> #include <sysdev/fsl_soc.h> +#include "mpc83xx.h" +#undef DEBUG#ifdef DEBUG arch/powerpc/platforms/83xx/pci.c line 40
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +3
-1@@ -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_configarch/powerpc/platforms/83xx/pci.c
Quoted 13 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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))
arch/powerpc/platforms/83xx/pci.c line 64
Quoted 12 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+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 1 line
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c line 71
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +7
-0+ +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
0xf8all about?The pcie only have one link, so the dev number only can be 0, as
Quoted 8 lines
Quoted 4 lines
well
Quoted 1 line
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.
Quoted 44 lines
Quoted 44 lines
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/pci.c line 158
Quoted 3 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+ 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_pciearch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+} + +static void __init mpc83xx_setup_pcie(struct pci_controller*hose,
Quoted 13 lines
Quoted 13 lines
Quoted 1 line
+ struct resource *reg, struct resource
*cfg_space)arch/powerpc/platforms/83xx/pci.c
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +5
-0+{ +void __iomem *hose_cfg_base; +u32 val; + + hose_cfg_base = ioremap(reg->start, reg->end - reg->start +1);
arch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+ + 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 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/pci.c line 177
Quoted 6 lines
arch/powerpc/platforms/83xx/pci.c +6
-0+ +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_pcidoes for primary, its passed into_add_bridge. Also, can we not do whatfsl_pcidoes and use PCI capabilities to determine we are a PCIe controller (and droppassing
Quoted 6 lines
Quoted 2 lines
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 7 lines
Quoted 2 lines
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
Quoted 8 lines · Kumar Gala
Quoted 7 lines · Li Li
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.
Quoted 2 lines
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.
Quoted 8 lines
Quoted 4 lines
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
Quoted 48 lines · Gala Kumar
Quoted 6 lines · Li Li
Quoted 5 lines · Gala Kumar
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
+
+ pci2@e0009000 {I agree w/Olof. This should be pcie@e0009000
Quoted 1 line
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +5
-0+ 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 8Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/powerpc/boot/dts/mpc8377_mds.dts
Quoted 8 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +8
-0+ 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 000800000>;
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
arch/powerpc/boot/dts/mpc8377_mds.dts line 294
Quoted 6 lines
arch/powerpc/boot/dts/mpc8377_mds.dts +6
-0+ 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?
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
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
Quoted 5 lines
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 upper128M
Quoted 1 line
configure space. So, we have to cut down the config space.
Quoted 8 lines
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 controllers 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 controllers case.So it is a little not compatible with the current kernel and u-boot assignment.
Quoted 43 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0
-0b/arch/powerpc/
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Quoted 5 lines
platforms/83xx/Kconfig
index 0c61e7a..00154c5 100644arch/powerpc/platforms/83xx/Kconfig config PPC_MPC837x · line 96
Quoted 10 lines
arch/powerpc/platforms/83xx/Kconfig +7
-0@@ -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 modeThis should be a hidden config that is just selected by
Quoted 5 lines
Quoted 5 lines
PPC_MPC837xQuoted 4 lines
Quoted 2 lines
instead of a choice. Also drop the depends on.
In the dts file, the PCIE is default enabled. So, we should
provide
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
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 PCIebut
Quoted 5 lines
Quoted 3 lines
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
Quoted 6 lines
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.
Quoted 6 lines
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.
Quoted 108 lines
Quoted 18 lines
Quoted 17 lines
Quoted 1 line
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0
-0b/arch/powerpc/
Quoted 15 lines
Quoted 15 lines
arch/powerpc/platforms/83xx/mpc83xx.h line 50
Quoted 13 lines
platforms/83xx/mpc83xx.h
index b778cb4..2078da7 100644arch/powerpc/platforms/83xx/mpc83xx.h +5
-0@@ -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 44 lines
ok.
Quoted 42 lines
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/mpc83xx.h
Quoted 11 lines
arch/powerpc/platforms/83xx/mpc83xx.h +3
-2/* * 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, intflags);
Quoted 30 lines
Quoted 30 lines
Quoted 18 lines
arch/powerpc/platforms/83xx/mpc83xx.h +0
-0extern void mpc83xx_restart (char *cmd);extern long mpc83xx_time_init (void );extern int mpc834x_usb_cfg (void );arch/powerpc/ renamed from arch/powerpc/platforms/83xx/pci.c +0
-0
platforms/83xx/pci.c
index 80425d7..0b52b2e 100644arch/powerpc/platforms/83xx/pci.c +2
-0@@ -25,6 +25,8 @@#include <asm/prom.h> #include <sysdev/fsl_soc.h> +#include "mpc83xx.h" +#undef DEBUG#ifdef DEBUG arch/powerpc/platforms/83xx/pci.c line 40
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c +3
-1@@ -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_configQuoted 3 lines
arch/powerpc/platforms/83xx/pci.c +2
-0+static int direct_read_config_pcie (struct pci_bus *bus, + uint devfn, int offset, int len, u32*val)
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
arch/powerpc/platforms/83xx/pci.c
Quoted 10 lines
arch/powerpc/platforms/83xx/pci.c +10
-0+{ +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 12 lines
arch/powerpc/platforms/83xx/pci.c +12
-0+return PCIBIOS_DEVICE_NOT_FOUND ; + +switch (len) { +case 2 : +if (offset &1 ) +return -EINVAL ; +break ; +case 4 : +if (offset &3 ) +return -EINVAL ; +break ; + }Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
fix formatting.
Quoted 3 lines
arch/powerpc/platforms/83xx/pci.c +2
-0+ + pr_debug("_read_cfg_pcie: bus=%d devfn=%x off=%x len=%x\n",
Quoted 15 lines
Quoted 14 lines
Quoted 10 lines
Quoted 8 lines
arch/powerpc/platforms/83xx/pci.c line 71
Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +5
-0+ bus->number, devfn, offset, len); + +if (bus->number == hose->first_busno) { +if (devfn &0xf8 ) +return PCIBIOS_DEVICE_NOT_FOUND ;what is the
0xf8all about?The pcie only have one link, so the dev number only can be 0, as
well
Quoted 1 line
the function number can be 0 ~ 7.
I don't follow what the number of links has to do with dev number.
Quoted 8 lines
Quoted 1 line
The PCIE only have one link that mean one PCIE bus only can have
one
Quoted 1 line
device populate on it. The dev number of this device must be 0. If
the
Quoted 1 line
device number is not 0, we consider it is a error.
we should add a comment about that.
yes
Quoted 48 lines
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/pci.c line 158
Quoted 3 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+ 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_pciearch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +3
-0+} + +static void __init mpc83xx_setup_pcie(struct pci_controller*hose,
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
arch/powerpc/platforms/83xx/pci.c line 78
Quoted 1 line
arch/powerpc/platforms/83xx/pci.c +1
-0+ struct resource *reg, struct resource
*cfg_space)Quoted 5 lines
arch/powerpc/platforms/83xx/pci.c +5
-0+{ +void __iomem *hose_cfg_base; +u32 val; + + hose_cfg_base = ioremap(reg->start, reg->end - reg->start+
Quoted 28 lines
Quoted 27 lines
Quoted 6 lines
Quoted 6 lines
1);
arch/powerpc/platforms/83xx/pci.c
Quoted 4 lines
arch/powerpc/platforms/83xx/pci.c +4
-0+ + 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 11 lines
Quoted 11 lines
arch/powerpc/platforms/83xx/pci.c line 177
Quoted 6 lines
arch/powerpc/platforms/83xx/pci.c +6
-0+ +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_pcidoes for primary, its passed into_add_bridge. Also, can we not do whatfsl_pcidoes and use PCI capabilities to determine we are a PCIe controller (and droppassing
Quoted 6 lines
Quoted 2 lines
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?
Quoted 9 lines
Quoted 2 lines
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
Quoted 2 lines
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_PRIMARYand let that get set by the caller.
yes
Quoted 2 lines
- k