[U-Boot] [PATCH] armv8: layerscape platform pcie link up state judgment strongly

[U-Boot] [PATCH] armv8: layerscape platform pcie link up state judgment strongly

[Bao Xiaowei]

modifiy the ls_pcie_link_up function, add the following three judging
mechanisms:

detect state: return link down status;
L0 state: return link up status;
other state: delay about 100ms retrieve Status Returns the corresponding link
status;

Signed-off-by: Bao Xiaowei <xiaowei.bao@nxp.com>
---
 drivers/pci/pcie_layerscape.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie_layerscape.c b/drivers/pci/pcie_layerscape.c
index 7565e2f..4446ac9 100644
--- a/drivers/pci/pcie_layerscape.c
+++ b/drivers/pci/pcie_layerscape.c
@@ -65,13 +65,22 @@ static int ls_pcie_ltssm(struct ls_pcie *pcie)
 
 static int ls_pcie_link_up(struct ls_pcie *pcie)
 {
-	int ltssm;
+	int ltssm, i;
 
 	ltssm = ls_pcie_ltssm(pcie);
-	if (ltssm < LTSSM_PCIE_L0)
+	if ((ltssm == 0) || (ltssm == 1))
 		return 0;
-
-	return 1;
+	else if (ltssm == LTSSM_PCIE_L0)
+		return 1;
+	else {
+		for (i = 0; i < 100; i++) {
+			udelay(1000);
+			ltssm = ls_pcie_ltssm(pcie);
+			if (ltssm == LTSSM_PCIE_L0)
+				return 1;
+		}
+		return 0;
+	}
 }
 
 static void ls_pcie_cfg0_set_busdev(struct ls_pcie *pcie, u32 busdev)
-- 
2.1.0.27.g96db324

[U-Boot] [PATCH] armv8: layerscape platform pcie link up state judgment strongly

[York Sun]

On 08/02/2017 07:58 PM, Bao Xiaowei wrote:
> modifiy the ls_pcie_link_up function, add the following three judging
> mechanisms:
> 
> detect state: return link down status;
> L0 state: return link up status;
> other state: delay about 100ms retrieve Status Returns the corresponding link
> status;

Please pay attention to your upper case and lower case in commit message.

What does the spec say about the timeout? Is 100ms the right time? If 
so, please put a comment in the source code.

> 
> Signed-off-by: Bao Xiaowei <xiaowei.bao@nxp.com>
> ---
>   drivers/pci/pcie_layerscape.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pcie_layerscape.c b/drivers/pci/pcie_layerscape.c
> index 7565e2f..4446ac9 100644
> --- a/drivers/pci/pcie_layerscape.c
> +++ b/drivers/pci/pcie_layerscape.c
> @@ -65,13 +65,22 @@ static int ls_pcie_ltssm(struct ls_pcie *pcie)
>   
>   static int ls_pcie_link_up(struct ls_pcie *pcie)
>   {
> -	int ltssm;
> +	int ltssm, i;
>   
>   	ltssm = ls_pcie_ltssm(pcie);
> -	if (ltssm < LTSSM_PCIE_L0)
> +	if ((ltssm == 0) || (ltssm == 1))
>   		return 0;
> -
> -	return 1;
> +	else if (ltssm == LTSSM_PCIE_L0)
> +		return 1;
> +	else {
> +		for (i = 0; i < 100; i++) {
> +			udelay(1000);
> +			ltssm = ls_pcie_ltssm(pcie);
> +			if (ltssm == LTSSM_PCIE_L0)
> +				return 1;
> +		}
> +		return 0;
> +	}

A comment to summary the logic would be nice.

York

[U-Boot] [PATCH] armv8: layerscape platform pcie link up state judgment strongly

[Xiaowei Bao]

Hi York,

I will pay attention to the case of the case in commit message. 

This patch is 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.

About 100ms timeout, the pcie spec does not specify the link up timeout time, and the link up state is determined by a state machine. The state machine implementation is relatively complex, refer to uboot of other platform pcie link up state to determine the realization of the mechanism, we evaluated a timeout, in detect state consider the pcie device is link down, in L0 state consider the pcie device is link up, within  100ms in other states can be restored to the L0 state considers the pcie device is link up .

Thanks

-----Original Message-----
From: York Sun 
Sent: Tuesday, August 08, 2017 6:56 AM
To: Xiaowei Bao <xiaowei.bao@nxp.com>; u-boot at lists.denx.de; Priyanka Jain <priyanka.jain@nxp.com>; Z.q. Hou <zhiqiang.hou@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>; sjg at chromium.org
Subject: Re: [PATCH] armv8: layerscape platform pcie link up state judgment strongly

On 08/02/2017 07:58 PM, Bao Xiaowei wrote:
> modifiy the ls_pcie_link_up function, add the following three judging
> mechanisms:
> 
> detect state: return link down status;
> L0 state: return link up status;
> other state: delay about 100ms retrieve Status Returns the 
> corresponding link status;

Please pay attention to your upper case and lower case in commit message.

What does the spec say about the timeout? Is 100ms the right time? If so, please put a comment in the source code.

> 
> Signed-off-by: Bao Xiaowei <xiaowei.bao@nxp.com>
> ---
>   drivers/pci/pcie_layerscape.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pcie_layerscape.c 
> b/drivers/pci/pcie_layerscape.c index 7565e2f..4446ac9 100644
> --- a/drivers/pci/pcie_layerscape.c
> +++ b/drivers/pci/pcie_layerscape.c
> @@ -65,13 +65,22 @@ static int ls_pcie_ltssm(struct ls_pcie *pcie)
>   
>   static int ls_pcie_link_up(struct ls_pcie *pcie)
>   {
> -	int ltssm;
> +	int ltssm, i;
>   
>   	ltssm = ls_pcie_ltssm(pcie);
> -	if (ltssm < LTSSM_PCIE_L0)
> +	if ((ltssm == 0) || (ltssm == 1))
>   		return 0;
> -
> -	return 1;
> +	else if (ltssm == LTSSM_PCIE_L0)
> +		return 1;
> +	else {
> +		for (i = 0; i < 100; i++) {
> +			udelay(1000);
> +			ltssm = ls_pcie_ltssm(pcie);
> +			if (ltssm == LTSSM_PCIE_L0)
> +				return 1;
> +		}
> +		return 0;
> +	}

A comment to summary the logic would be nice.

York

[U-Boot] [PATCH] armv8: layerscape platform pcie link up state judgment strongly

[York Sun]

On 08/07/2017 11:56 PM, Xiaowei Bao wrote:
> Hi York,
> 
> I will pay attention to the case of the case in commit message.
> 
> This patch is 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.
> 
> About 100ms timeout, the pcie spec does not specify the link up timeout time, and the link up state is determined by a state machine. The state machine implementation is relatively complex, refer to uboot of other platform pcie link up state to determine the realization of the mechanism, we evaluated a timeout, in detect state consider the pcie device is link down, in L0 state consider the pcie device is link up, within  100ms in other states can be restored to the L0 state considers the pcie device is link up .

Can you put this information to inline comment? It will help us when we 
read the code later.

Thanks.

York

[U-Boot] [PATCH] armv8: layerscape platform pcie link up state judgment strongly

[Xiaowei Bao]

Hi York,

I will add the inline comment in the patch, send it to you later.

thanks

-----Original Message-----
From: York Sun 
Sent: Wednesday, August 09, 2017 12:14 AM
To: Xiaowei Bao <xiaowei.bao@nxp.com>; u-boot at lists.denx.de; Priyanka Jain <priyanka.jain@nxp.com>; Z.q. Hou <zhiqiang.hou@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>; sjg at chromium.org
Subject: Re: [PATCH] armv8: layerscape platform pcie link up state judgment strongly

On 08/07/2017 11:56 PM, Xiaowei Bao wrote:
> Hi York,
> 
> I will pay attention to the case of the case in commit message.
> 
> This patch is 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.
> 
> About 100ms timeout, the pcie spec does not specify the link up timeout time, and the link up state is determined by a state machine. The state machine implementation is relatively complex, refer to uboot of other platform pcie link up state to determine the realization of the mechanism, we evaluated a timeout, in detect state consider the pcie device is link down, in L0 state consider the pcie device is link up, within  100ms in other states can be restored to the L0 state considers the pcie device is link up .

Can you put this information to inline comment? It will help us when we read the code later.

Thanks.

York

[U-Boot] [PATCH] armv8: layerscape platform pcie link up state judgment strongly

[Xiaowei Bao]

Hi York,

I have add the inline comment in the new patch and have sent to you, please review it.

Thanks

-----Original Message-----
From: Xiaowei Bao 
Sent: Friday, August 11, 2017 9:39 AM
To: York Sun <york.sun@nxp.com>; u-boot at lists.denx.de; Priyanka Jain <priyanka.jain@nxp.com>; Z.q. Hou <zhiqiang.hou@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>; sjg at chromium.org
Subject: RE: [PATCH] armv8: layerscape platform pcie link up state judgment strongly

Hi York,

I will add the inline comment in the patch, send it to you later.

thanks

-----Original Message-----
From: York Sun 
Sent: Wednesday, August 09, 2017 12:14 AM
To: Xiaowei Bao <xiaowei.bao@nxp.com>; u-boot at lists.denx.de; Priyanka Jain <priyanka.jain@nxp.com>; Z.q. Hou <zhiqiang.hou@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>; sjg at chromium.org
Subject: Re: [PATCH] armv8: layerscape platform pcie link up state judgment strongly

On 08/07/2017 11:56 PM, Xiaowei Bao wrote:
> Hi York,
> 
> I will pay attention to the case of the case in commit message.
> 
> This patch is 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.
> 
> About 100ms timeout, the pcie spec does not specify the link up timeout time, and the link up state is determined by a state machine. The state machine implementation is relatively complex, refer to uboot of other platform pcie link up state to determine the realization of the mechanism, we evaluated a timeout, in detect state consider the pcie device is link down, in L0 state consider the pcie device is link up, within  100ms in other states can be restored to the L0 state considers the pcie device is link up .

Can you put this information to inline comment? It will help us when we read the code later.

Thanks.

York