[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 at nxp.com>; u-boot at lists.denx.de; Priyanka Jain <priyanka.jain at nxp.com>; Z.q. Hou <zhiqiang.hou at nxp.com>; M.h. Lian <minghuan.lian at 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 at 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