subject:"\[U\-Boot\] \[PATCH\] Powerpc\: Make pcie link state judge more specific"

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

2017-10-10 Thread Mingkai Hu



> -Original Message-
> From: York Sun
> Sent: Tuesday, October 03, 2017 11:32 PM
> To: Xiaowei Bao ; Mingkai Hu
> 
> Cc: u-boot@lists.denx.de
> Subject: Re: [PATCH] Powerpc: Make pcie link state judge more specific
> 
> On 09/24/2017 08:44 PM, Bao Xiaowei wrote:
> > For some special reset times for longer pcie devices, in this case,
> > the pcie device may on polling compliance state, the RC considers the
> > pcie device is link up, but the pcie device is not link up, only the
> > L0 state is link up state. So add the link up status judgement mechanisms.
> >
> > Signed-off-by: Bao Xiaowei 
> > ---
> > v2:
> >  - Detailed function module
> >  - Adjust the code structure
> 
> Mingkai,
> 
> Please ack if you agree with this change.
> 

York,

I had discussion with xiaowei and also posted the comments on the patch.

Thanks,
Mingkai
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

2017-10-10 Thread Mingkai Hu



> -Original Message-
> From: Bao Xiaowei [mailto:xiaowei@nxp.com]
> Sent: Monday, September 25, 2017 11:27 AM
> To: M.h. Lian ; Z.q. Hou ;
> Mingkai Hu ; York Sun ;
> hamish.mar...@alliedtelesis.co.nz; w...@denx.de;
> tony.obr...@alliedtelesis.co.nz; u-boot@lists.denx.de
> Cc: Xiaowei Bao 
> Subject: [PATCH] Powerpc: Make pcie link state judge more specific
> 
> For some special reset times for longer pcie devices, in this case, the pcie
> device may on polling compliance state, the RC considers the pcie device is
> link up, but the pcie device is not link up, only the L0 state is link up 
> state. So
> add the link up status judgement mechanisms.
> 
> Signed-off-by: Bao Xiaowei 
> ---
> v2:
>  - Detailed function module
>  - Adjust the code structure
> 

I suggest to split this patch to two patches, one is for format change, another 
one is for LTSSM change.

>  arch/powerpc/include/asm/fsl_pci.h |   3 +
>  drivers/pci/fsl_pci_init.c | 151 
> -
>  2 files changed, 86 insertions(+), 68 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/fsl_pci.h
> b/arch/powerpc/include/asm/fsl_pci.h
> index cad341e..9dfbf19 100644
> --- a/arch/powerpc/include/asm/fsl_pci.h
> +++ b/arch/powerpc/include/asm/fsl_pci.h
> @@ -24,6 +24,9 @@
> 
>  #define PCI_LTSSM0x404   /* PCIe Link Training, Status State Machine */
>  #define  PCI_LTSSM_L00x16/* L0 state */

It's better to submit a patch to fix the leading space.

> +#define PCIE_GEN3_LTSSM_L0 0x11/* L0 state */

Follow the same name rule as PCI_LTSSM_L0? Like PCI_LTSSM_L0_PEX_REV3?

> +#define LTSSM_PCIE_DETECT_QUIET0x00 /* Detect state */
> +#define LTSSM_PCIE_DETECT_ACTIVE   0x01 /* Detect state */
> 
>  int fsl_setup_hose(struct pci_controller *hose, unsigned long addr);  int
> fsl_is_pci_agent(struct pci_controller *hose); diff --git
> a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c index 
> af20cf0..5d697bc
> 100644
> --- a/drivers/pci/fsl_pci_init.c
> +++ b/drivers/pci/fsl_pci_init.c
> @@ -39,6 +39,8 @@ DECLARE_GLOBAL_DATA_PTR;  #if
> defined(CONFIG_SYS_PCI_64BIT)
> && !defined(CONFIG_SYS_PCI64_MEMORY_BUS)
>  #define CONFIG_SYS_PCI64_MEMORY_BUS (64ull*1024*1024*1024)  #endif
> +#define PEX_CSR0_LTSSM_MASK  0xFC
> +#define PEX_CSR0_LTSSM_SHIFT 2
> 
>  /* Setup one inbound ATMU window.
>   *
> @@ -290,6 +292,81 @@ static void fsl_pcie_boot_master_release_slave(int
> port)  }  #endif
> 
> +static int is_pcie_gen3(struct fsl_pci_info *pci_info) {
> + u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
> + volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
> + u32 block_rev;
> +
> + block_rev = in_be32(>block_rev1);
> + if (block_rev >= PEX_IP_BLK_REV_3_0)
> + return 1;
> + else
> + return 0;
> +}
> +

As discussed, it's not related to gen3. It's related to PEX block version. 
Please use
The PCIe controller version here.

> +static int get_ltssm_val(struct pci_controller *hose,
> +  struct fsl_pci_info *pci_info)
> +{
> + u16 ltssm = 0;
> + pci_dev_t dev = PCI_BDF(hose->first_busno, 0, 0);
> + u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
> + volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
> +
> + if (is_pcie_gen3(pci_info))
> + ltssm = (in_be32(>pex_csr0)
> + & PEX_CSR0_LTSSM_MASK) >>
> PEX_CSR0_LTSSM_SHIFT;
> + else
> + pci_hose_read_config_word(hose, dev, PCI_LTSSM, );
> +
> + return ltssm;
> +}
> +
> +static int pci_link_up(struct pci_controller *hose,
> + struct fsl_pci_info *pci_info)
> +{
> + int enabled = 0;
> + u16 ltssm;
> + int i, pcie_ltssm_l0;
> +
> + if (is_pcie_gen3(pci_info))
> + pcie_ltssm_l0 = PCIE_GEN3_LTSSM_L0;
> + else
> + pcie_ltssm_l0 = PCI_LTSSM_L0;
> +
> + ltssm = get_ltssm_val(hose, pci_info);
> + if (ltssm == LTSSM_PCIE_DETECT_QUIET ||
> + ltssm == LTSSM_PCIE_DETECT_ACTIVE)
> + enabled = 0;
> + else if (ltssm == PCIE_GEN3_LTSSM_L0)
> + enabled = 1;
> + else {
> + for (i = 0; i < 100 && ltssm != pcie_ltssm_l0; i++) {
> + ltssm = get_ltssm_val(hose, pci_info);
> + udelay(1000);
> + }
> + enabled = (ltssm == pcie_ltssm_l0) ? 1 : 0;
> + }
> + return enabled;
> +}
> +
> +#if defined(CONFIG_FSL_PCIE_RESET) || \
> + defined(CONFIG_SYS_P4080_ERRATUM_PCIE_A003)
> +static void do_pcie_reset(struct fsl_pci_info *pci_info) {
> + u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
> + volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
> +
> + /* assert PCIe reset */
> +

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

2017-10-09 Thread Xiaowei Bao

Hi York,

I will revise the commit message later.

Best regards
xiaowei

-Original Message-
From: York Sun 
Sent: Monday, October 09, 2017 11:44 PM
To: Xiaowei Bao ; M.h. Lian ; Z.q. 
Hou ; Mingkai Hu ; 
u-boot@lists.denx.de
Subject: Re: [PATCH] Powerpc: Make pcie link state judge more specific

On 09/24/2017 08:44 PM, Bao Xiaowei wrote:
> For some special reset times for longer pcie devices, in this case, 
> the pcie device may on polling compliance state, the RC considers the 
> pcie device is link up, but the pcie device is not link up, only the 
> L0 state is link up state. So add the link up status judgement mechanisms.
> 
> Signed-off-by: Bao Xiaowei 
> ---
> v2:
>  - Detailed function module
>  - Adjust the code structure
> 

Xiaowei,

Can you revise the commit message? I don't quite understand it. It is similar 
but not the same as the change you made for Layerscape, isn't it?

While you respin this patch, please consider my comments earlier.

York
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

2017-10-09 Thread York Sun

On 09/24/2017 08:44 PM, Bao Xiaowei wrote:
> For some special reset times for longer pcie devices, in this case, the
> pcie device may on polling compliance state, the RC considers the pcie
> device is link up, but the pcie device is not link up, only the L0 state
> is link up state. So add the link up status judgement mechanisms.
> 
> Signed-off-by: Bao Xiaowei 
> ---
> v2:
>  - Detailed function module
>  - Adjust the code structure
> 

Xiaowei,

Can you revise the commit message? I don't quite understand it. It is
similar but not the same as the change you made for Layerscape, isn't it?

While you respin this patch, please consider my comments earlier.

York
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

2017-10-03 Thread York Sun

On 09/24/2017 08:44 PM, Bao Xiaowei wrote:
> For some special reset times for longer pcie devices, in this case, the
> pcie device may on polling compliance state, the RC considers the pcie
> device is link up, but the pcie device is not link up, only the L0 state
> is link up state. So add the link up status judgement mechanisms.
> 
> Signed-off-by: Bao Xiaowei 
> ---
> v2:
>  - Detailed function module
>  - Adjust the code structure

Mingkai,

Please ack if you agree with this change.

Xiaowei,

Please see some inline comments.

> 
>  arch/powerpc/include/asm/fsl_pci.h |   3 +
>  drivers/pci/fsl_pci_init.c | 151 
> -
>  2 files changed, 86 insertions(+), 68 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/fsl_pci.h 
> b/arch/powerpc/include/asm/fsl_pci.h
> index cad341e..9dfbf19 100644
> --- a/arch/powerpc/include/asm/fsl_pci.h
> +++ b/arch/powerpc/include/asm/fsl_pci.h
> @@ -24,6 +24,9 @@
>  
>  #define PCI_LTSSM0x404   /* PCIe Link Training, Status State Machine */
>  #define  PCI_LTSSM_L00x16/* L0 state */
> +#define PCIE_GEN3_LTSSM_L0 0x11/* L0 state */
> +#define LTSSM_PCIE_DETECT_QUIET0x00 /* Detect state */
> +#define LTSSM_PCIE_DETECT_ACTIVE   0x01 /* Detect state */
>  
>  int fsl_setup_hose(struct pci_controller *hose, unsigned long addr);
>  int fsl_is_pci_agent(struct pci_controller *hose);
> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
> index af20cf0..5d697bc 100644
> --- a/drivers/pci/fsl_pci_init.c
> +++ b/drivers/pci/fsl_pci_init.c
> @@ -39,6 +39,8 @@ DECLARE_GLOBAL_DATA_PTR;
>  #if defined(CONFIG_SYS_PCI_64BIT) && !defined(CONFIG_SYS_PCI64_MEMORY_BUS)
>  #define CONFIG_SYS_PCI64_MEMORY_BUS (64ull*1024*1024*1024)
>  #endif
> +#define PEX_CSR0_LTSSM_MASK  0xFC
> +#define PEX_CSR0_LTSSM_SHIFT 2
>  
>  /* Setup one inbound ATMU window.
>   *
> @@ -290,6 +292,81 @@ static void fsl_pcie_boot_master_release_slave(int port)
>  }
>  #endif
>  
> +static int is_pcie_gen3(struct fsl_pci_info *pci_info)
> +{
> + u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
> + volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
> + u32 block_rev;
> +
> + block_rev = in_be32(>block_rev1);
> + if (block_rev >= PEX_IP_BLK_REV_3_0)
> + return 1;
> + else

You don't need the "else" here.

> + return 0;
> +}
> +
> +static int get_ltssm_val(struct pci_controller *hose,
> +  struct fsl_pci_info *pci_info)
> +{
> + u16 ltssm = 0;
> + pci_dev_t dev = PCI_BDF(hose->first_busno, 0, 0);
> + u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
> + volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
> +
> + if (is_pcie_gen3(pci_info))
> + ltssm = (in_be32(>pex_csr0)
> + & PEX_CSR0_LTSSM_MASK) >> PEX_CSR0_LTSSM_SHIFT;
> + else
> + pci_hose_read_config_word(hose, dev, PCI_LTSSM, );
> +
> + return ltssm;
> +}
> +
> +static int pci_link_up(struct pci_controller *hose,
> + struct fsl_pci_info *pci_info)
> +{
> + int enabled = 0;
> + u16 ltssm;
> + int i, pcie_ltssm_l0;
> +
> + if (is_pcie_gen3(pci_info))
> + pcie_ltssm_l0 = PCIE_GEN3_LTSSM_L0;
> + else
> + pcie_ltssm_l0 = PCI_LTSSM_L0;
> +
> + ltssm = get_ltssm_val(hose, pci_info);
> + if (ltssm == LTSSM_PCIE_DETECT_QUIET ||
> + ltssm == LTSSM_PCIE_DETECT_ACTIVE)
> + enabled = 0;
> + else if (ltssm == PCIE_GEN3_LTSSM_L0)
> + enabled = 1;
> + else {
> + for (i = 0; i < 100 && ltssm != pcie_ltssm_l0; i++) {
> + ltssm = get_ltssm_val(hose, pci_info);
> + udelay(1000);
> + }
> + enabled = (ltssm == pcie_ltssm_l0) ? 1 : 0;
> + }
> + return enabled;

Add blank line above "return".

> +}

You don't have to send an update unless you have something else to fix.
I can make cosmetic fix when I merge it.

York
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

2017-09-24 Thread Bao Xiaowei

For some special reset times for longer pcie devices, in this case, the
pcie device may on polling compliance state, the RC considers the pcie
device is link up, but the pcie device is not link up, only the L0 state
is link up state. So add the link up status judgement mechanisms.

Signed-off-by: Bao Xiaowei 
---
v2:
 - Detailed function module
 - Adjust the code structure

 arch/powerpc/include/asm/fsl_pci.h |   3 +
 drivers/pci/fsl_pci_init.c | 151 -
 2 files changed, 86 insertions(+), 68 deletions(-)

diff --git a/arch/powerpc/include/asm/fsl_pci.h 
b/arch/powerpc/include/asm/fsl_pci.h
index cad341e..9dfbf19 100644
--- a/arch/powerpc/include/asm/fsl_pci.h
+++ b/arch/powerpc/include/asm/fsl_pci.h
@@ -24,6 +24,9 @@
 
 #define PCI_LTSSM  0x404   /* PCIe Link Training, Status State Machine */
 #define  PCI_LTSSM_L0  0x16/* L0 state */
+#define PCIE_GEN3_LTSSM_L0 0x11/* L0 state */
+#define LTSSM_PCIE_DETECT_QUIET0x00 /* Detect state */
+#define LTSSM_PCIE_DETECT_ACTIVE   0x01 /* Detect state */
 
 int fsl_setup_hose(struct pci_controller *hose, unsigned long addr);
 int fsl_is_pci_agent(struct pci_controller *hose);
diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
index af20cf0..5d697bc 100644
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -39,6 +39,8 @@ DECLARE_GLOBAL_DATA_PTR;
 #if defined(CONFIG_SYS_PCI_64BIT) && !defined(CONFIG_SYS_PCI64_MEMORY_BUS)
 #define CONFIG_SYS_PCI64_MEMORY_BUS (64ull*1024*1024*1024)
 #endif
+#define PEX_CSR0_LTSSM_MASK0xFC
+#define PEX_CSR0_LTSSM_SHIFT   2
 
 /* Setup one inbound ATMU window.
  *
@@ -290,6 +292,81 @@ static void fsl_pcie_boot_master_release_slave(int port)
 }
 #endif
 
+static int is_pcie_gen3(struct fsl_pci_info *pci_info)
+{
+   u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
+   volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
+   u32 block_rev;
+
+   block_rev = in_be32(>block_rev1);
+   if (block_rev >= PEX_IP_BLK_REV_3_0)
+   return 1;
+   else
+   return 0;
+}
+
+static int get_ltssm_val(struct pci_controller *hose,
+struct fsl_pci_info *pci_info)
+{
+   u16 ltssm = 0;
+   pci_dev_t dev = PCI_BDF(hose->first_busno, 0, 0);
+   u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
+   volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
+
+   if (is_pcie_gen3(pci_info))
+   ltssm = (in_be32(>pex_csr0)
+   & PEX_CSR0_LTSSM_MASK) >> PEX_CSR0_LTSSM_SHIFT;
+   else
+   pci_hose_read_config_word(hose, dev, PCI_LTSSM, );
+
+   return ltssm;
+}
+
+static int pci_link_up(struct pci_controller *hose,
+   struct fsl_pci_info *pci_info)
+{
+   int enabled = 0;
+   u16 ltssm;
+   int i, pcie_ltssm_l0;
+
+   if (is_pcie_gen3(pci_info))
+   pcie_ltssm_l0 = PCIE_GEN3_LTSSM_L0;
+   else
+   pcie_ltssm_l0 = PCI_LTSSM_L0;
+
+   ltssm = get_ltssm_val(hose, pci_info);
+   if (ltssm == LTSSM_PCIE_DETECT_QUIET ||
+   ltssm == LTSSM_PCIE_DETECT_ACTIVE)
+   enabled = 0;
+   else if (ltssm == PCIE_GEN3_LTSSM_L0)
+   enabled = 1;
+   else {
+   for (i = 0; i < 100 && ltssm != pcie_ltssm_l0; i++) {
+   ltssm = get_ltssm_val(hose, pci_info);
+   udelay(1000);
+   }
+   enabled = (ltssm == pcie_ltssm_l0) ? 1 : 0;
+   }
+   return enabled;
+}
+
+#if defined(CONFIG_FSL_PCIE_RESET) || \
+   defined(CONFIG_SYS_P4080_ERRATUM_PCIE_A003)
+static void do_pcie_reset(struct fsl_pci_info *pci_info)
+{
+   u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
+   volatile ccsr_fsl_pci_t *pci = (ccsr_fsl_pci_t *)cfg_addr;
+
+   /* assert PCIe reset */
+   setbits_be32(>pdb_stat, 0x0800);
+   (void) in_be32(>pdb_stat);
+   udelay(1000);
+   /* clear PCIe reset */
+   clrbits_be32(>pdb_stat, 0x0800);
+   asm("sync;isync");
+}
+#endif
+
 void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info)
 {
u32 cfg_addr = (u32)&((ccsr_fsl_pci_t *)pci_info->regs)->cfg_addr;
@@ -298,7 +375,6 @@ void fsl_pci_init(struct pci_controller *hose, struct 
fsl_pci_info *pci_info)
u32 temp32;
u32 block_rev;
int enabled, r, inbound = 0;
-   u16 ltssm;
u8 temp8, pcie_cap;
int pcie_cap_pos;
int pci_dcr;
@@ -438,63 +514,12 @@ 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_MASK0xFC
-#define PEX_CSR0_LTSSM_SHIFT   2
-   ltssm = (in_be32(>pex_csr0)
-

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

Re: [U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

[U-Boot] [PATCH] Powerpc: Make pcie link state judge more specific

6 matches

Site Navigation

Mail list logo

Footer information