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 <[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(-)
> >
> > diff --git a/arch/powerpc/boot/dts/mpc8377_mds.dts b/arch/powerpc/
> > boot/dts/mpc8377_mds.dts
> > index 4402e39..1f7819e 100644
>
> >
> > +
> > + [EMAIL PROTECTED] {
>
> I agree w/Olof. This should be [EMAIL PROTECTED]
> >
> > + 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
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
