Patchwork [U-Boot,v2,batch,2,03/23] powerpc/pcie: add PCIe version 3.x support

login
register
mail settings
Submitter York Sun
Date March 25, 2013, 5:33 p.m.
Message ID <1364232811-30856-3-git-send-email-yorksun@freescale.com>
Download mbox | patch
Permalink /patch/230807/
State Rejected
Delegated to: Andy Fleming
Headers show

Comments

York Sun - March 25, 2013, 5:33 p.m.
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(-)
Andy Fleming - May 13, 2013, 6:50 p.m.
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
York Sun - May 13, 2013, 7:25 p.m.
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
Andy Fleming - May 13, 2013, 10:49 p.m.
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

Patch

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, &ltssm); */
+		/* enabled = ltssm >= PCI_LTSSM_L0; */
 		pci_hose_read_config_word(hose, dev, PCI_LTSSM, &ltssm);
 		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>