| Message ID | 1364232811-30856-3-git-send-email-yorksun@freescale.com |
|---|---|
| State | Rejected |
| Delegated to: | Andy Fleming |
| Headers | show |
This patch causes all sorts of problems. I'm NACKing it for now. Please make sure none of these changes break 83xx before re-submitting. On Mon, Mar 25, 2013 at 12:33 PM, York Sun <yorksun@freescale.com> wrote: > From: Roy ZANG <tie-fei.zang@freescale.com> > > T4240 PCIe IP is version 3.0 and has some update comparing previous > QorIQ products. > > 1. Move Freescale specific register define > to > arch/powerpc/include/asm/fsl_pci.h > and update the register offset define for T4240. > > 2. add the status/control register define > use status/control register to judge the link status > > 3. The original code uses 'Programming Interface' field to judge if PCIE is > EP or RC mode, however, T4240 does not support this functionality. > According to PCIE specification, 'Header Type' offset 0x0e is used to > indicate header type, so for PCIE controller, the patch changes code to > use 'Header Type' field to identify if the PCIE is RC or EP mode. > > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com> > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com> > --- > arch/powerpc/include/asm/config_mpc85xx.h | 1 + > arch/powerpc/include/asm/fsl_pci.h | 35 > +++++++++++++++++++++++++++-- > drivers/pci/fsl_pci_init.c | 20 ++++++++++++----- > include/pci.h | 7 ------ > 4 files changed, 48 insertions(+), 15 deletions(-) > > [...] > index 15f583f..c0ed553 100644 > --- a/include/pci.h > +++ b/include/pci.h > @@ -426,13 +426,6 @@ > #define PCI_MAX_PCI_DEVICES 32 > #define PCI_MAX_PCI_FUNCTIONS 8 > > -#define PCI_DCR 0x54 /* PCIe Device Control Register */ > -#define PCI_DSR 0x56 /* PCIe Device Status Register */ > -#define PCI_LSR 0x5e /* PCIe Link Status Register */ > -#define PCI_LCR 0x5c /* PCIe Link Control Register */ > -#define PCI_LTSSM 0x404 /* PCIe Link Training, Status State > Machine */ > -#define PCI_LTSSM_L0 0x16 /* L0 state */ > These were being used by 83xx as well, and are now unavailable. Please devise a solution that works for all of our platforms. Andy
Andy, I think I caught this issue and asked Roy to submit another patch to fix it http://patchwork.ozlabs.org/patch/230825/ York On 05/13/2013 11:50 AM, Andy Fleming wrote: > This patch causes all sorts of problems. I'm NACKing it for now. Please > make sure none of these changes break 83xx before re-submitting. > > > On Mon, Mar 25, 2013 at 12:33 PM, York Sun <yorksun@freescale.com > <mailto:yorksun@freescale.com>> wrote: > > From: Roy ZANG <tie-fei.zang@freescale.com > <mailto:tie-fei.zang@freescale.com>> > > T4240 PCIe IP is version 3.0 and has some update comparing previous > QorIQ products. > > 1. Move Freescale specific register define > to > arch/powerpc/include/asm/fsl_pci.h > and update the register offset define for T4240. > > 2. add the status/control register define > use status/control register to judge the link status > > 3. The original code uses 'Programming Interface' field to judge if > PCIE is > EP or RC mode, however, T4240 does not support this functionality. > According to PCIE specification, 'Header Type' offset 0x0e is used to > indicate header type, so for PCIE controller, the patch changes code to > use 'Header Type' field to identify if the PCIE is RC or EP mode. > > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com > <mailto:tie-fei.zang@freescale.com>> > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com > <mailto:Minghuan.Lian@freescale.com>> > --- > arch/powerpc/include/asm/config_mpc85xx.h | 1 + > arch/powerpc/include/asm/fsl_pci.h | 35 > +++++++++++++++++++++++++++-- > drivers/pci/fsl_pci_init.c | 20 ++++++++++++----- > include/pci.h | 7 ------ > 4 files changed, 48 insertions(+), 15 deletions(-) > > > [...] > > > index 15f583f..c0ed553 100644 > --- a/include/pci.h > +++ b/include/pci.h > @@ -426,13 +426,6 @@ > #define PCI_MAX_PCI_DEVICES 32 > #define PCI_MAX_PCI_FUNCTIONS 8 > > -#define PCI_DCR 0x54 /* PCIe Device Control > Register */ > -#define PCI_DSR 0x56 /* PCIe Device Status > Register */ > -#define PCI_LSR 0x5e /* PCIe Link Status Register */ > -#define PCI_LCR 0x5c /* PCIe Link Control Register */ > -#define PCI_LTSSM 0x404 /* PCIe Link Training, Status State > Machine */ > -#define PCI_LTSSM_L0 0x16 /* L0 state */ > > > > These were being used by 83xx as well, and are now unavailable. Please > devise a solution that works for all of our platforms. > > Andy
On Mon, May 13, 2013 at 2:25 PM, York Sun <yorksun@freescale.com> wrote: > Andy, > > I think I caught this issue and asked Roy to submit another patch to fix > it http://patchwork.ozlabs.org/patch/230825/ We can't do it that way. I can't apply a patch which is known to break the build -- it breaks bisectability. Roy, please submit a new patch which works correctly for all platforms However, the linked patch looks very much like a temporary workaround. Clearly, 83xx needs some of the same definitions as 85xx, and making 2 copies isn't the solution. Somehow, they need to share the definitions. I'm not entirely clear on why 83xx has its own pci code, but it's very misleading to call the 85xx PCI code "fsl_pci", and leave out 83xx, even though it shares some of the same code. Please propose a solution which acknowledges and accommodates this shared functionality. Andy
diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h index 352e303..3740785 100644 --- a/arch/powerpc/include/asm/config_mpc85xx.h +++ b/arch/powerpc/include/asm/config_mpc85xx.h @@ -543,6 +543,7 @@ #define CONFIG_SYS_FSL_ERRATUM_A_004934 #define CONFIG_SYS_FSL_ERRATUM_A005871 #define CONFIG_SYS_CCSRBAR_DEFAULT 0xfe000000 +#define CONFIG_SYS_FSL_PCI_VER_3_X #elif defined(CONFIG_PPC_B4420) #define CONFIG_SYS_PPC64 /* 64-bit core */ diff --git a/arch/powerpc/include/asm/fsl_pci.h b/arch/powerpc/include/asm/fsl_pci.h index 49bd2bf..0c41361 100644 --- a/arch/powerpc/include/asm/fsl_pci.h +++ b/arch/powerpc/include/asm/fsl_pci.h @@ -1,5 +1,5 @@ /* - * Copyright 2007,2009-2011 Freescale Semiconductor, Inc. + * Copyright 2007,2009-2012 Freescale Semiconductor, Inc. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -27,6 +27,34 @@ #define PEX_IP_BLK_REV_2_2 0x02080202 #define PEX_IP_BLK_REV_2_3 0x02080203 +#define PEX_IP_BLK_REV_3_0 0x02080300 + +/* Freescale-specific PCI config registers */ +#define FSL_PCI_PBFR 0x44 + +#ifdef CONFIG_SYS_FSL_PCI_VER_3_X +/* Currently only the PCIe capability is used, so hardcode the offset. + * if more capabilities need to be justified, the capability link method + * should be applied here + */ +#define FSL_PCIE_CAP_ID 0x70 +#define PCI_DCR 0x78 /* PCIe Device Control Register */ +#define PCI_DSR 0x7a /* PCIe Device Status Register */ +#define PCI_LSR 0x82 /* PCIe Link Status Register */ +#define PCI_LCR 0x80 /* PCIe Link Control Register */ +#else +#define FSL_PCIE_CAP_ID 0x4c +#define PCI_DCR 0x54 /* PCIe Device Control Register */ +#define PCI_DSR 0x56 /* PCIe Device Status Register */ +#define PCI_LSR 0x5e /* PCIe Link Status Register */ +#define PCI_LCR 0x5c /* PCIe Link Control Register */ +#endif + +#define FSL_PCIE_CFG_RDY 0x4b0 +#define FSL_PROG_IF_AGENT 0x1 + +#define PCI_LTSSM 0x404 /* PCIe Link Training, Status State Machine */ +#define PCI_LTSSM_L0 0x16 /* L0 state */ int fsl_setup_hose(struct pci_controller *hose, unsigned long addr); int fsl_is_pci_agent(struct pci_controller *hose); @@ -163,7 +191,10 @@ typedef struct ccsr_pci { u32 perr_cap3; /* 0xe34 - PCIE Error Capture Register 3 */ char res23[200]; u32 pdb_stat; /* 0xf00 - PCIE Debug Status */ - char res24[252]; + char res24[16]; + u32 pex_csr0; /* 0xf14 - PEX Control/Status register 0*/ + u32 pex_csr1; /* 0xf18 - PEX Control/Status register 1*/ + char res25[228]; } ccsr_fsl_pci_t; #define PCIE_CONFIG_PC 0x00020000 #define PCIE_CONFIG_OB_CK 0x00002000 diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c index 77ac1f7..d881375 100644 --- a/drivers/pci/fsl_pci_init.c +++ b/drivers/pci/fsl_pci_init.c @@ -41,12 +41,6 @@ DECLARE_GLOBAL_DATA_PTR; #include <asm/io.h> #include <asm/fsl_pci.h> -/* Freescale-specific PCI config registers */ -#define FSL_PCI_PBFR 0x44 -#define FSL_PCIE_CAP_ID 0x4c -#define FSL_PCIE_CFG_RDY 0x4b0 -#define FSL_PROG_IF_AGENT 0x1 - #ifndef CONFIG_SYS_PCI_MEMORY_BUS #define CONFIG_SYS_PCI_MEMORY_BUS 0 #endif @@ -437,6 +431,15 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) udelay(1); #endif if (pcie_cap == PCI_CAP_ID_EXP) { + if (block_rev >= PEX_IP_BLK_REV_3_0) { +#define PEX_CSR0_LTSSM_MASK 0xFC +#define PEX_CSR0_LTSSM_SHIFT 2 + ltssm = (in_be32(&pci->pex_csr0) + & PEX_CSR0_LTSSM_MASK) >> PEX_CSR0_LTSSM_SHIFT; + enabled = (ltssm == 0x11) ? 1 : 0; + } else { + /* pci_hose_read_config_word(hose, dev, PCI_LTSSM, <ssm); */ + /* enabled = ltssm >= PCI_LTSSM_L0; */ pci_hose_read_config_word(hose, dev, PCI_LTSSM, <ssm); enabled = ltssm >= PCI_LTSSM_L0; @@ -469,6 +472,7 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) PCI_BASE_ADDRESS_0, pcicsrbar); } #endif + } #ifdef CONFIG_SYS_P4080_ERRATUM_PCIE_A003 if (enabled == 0) { @@ -577,6 +581,10 @@ int fsl_is_pci_agent(struct pci_controller *hose) u8 prog_if; pci_hose_read_config_byte(hose, dev, PCI_CLASS_PROG, &prog_if); + /* Programming Interface (PCI_CLASS_PROG) + * 0 == pci host or pcie root-complex, + * 1 == pci agent or pcie end-point + */ return (prog_if == FSL_PROG_IF_AGENT); } } diff --git a/include/pci.h b/include/pci.h index 15f583f..c0ed553 100644 --- a/include/pci.h +++ b/include/pci.h @@ -426,13 +426,6 @@ #define PCI_MAX_PCI_DEVICES 32 #define PCI_MAX_PCI_FUNCTIONS 8 -#define PCI_DCR 0x54 /* PCIe Device Control Register */ -#define PCI_DSR 0x56 /* PCIe Device Status Register */ -#define PCI_LSR 0x5e /* PCIe Link Status Register */ -#define PCI_LCR 0x5c /* PCIe Link Control Register */ -#define PCI_LTSSM 0x404 /* PCIe Link Training, Status State Machine */ -#define PCI_LTSSM_L0 0x16 /* L0 state */ - /* Include the ID list */ #include <pci_ids.h>