Re: [PATCH] PCI: mediatek: increase link training timeout

[Bjorn Helgaas <helgaas@kernel.org>]

On Sun, Mar 26, 2023 at 03:12:17PM +0300, Oskari Lemmela wrote:
> Some PCI devices fail to complete link training in 100ms.
> Increase link training timeout by 20ms to 120ms.
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>

Doesn't look like this went anywhere, probably because we really don't
have enough information about why and where this is needed.

Does this mean some *endpoints* don't train fast enough?  Or is this
something to do with MediaTek host controllers?

If some endpoints don't train correctly, maybe it's a defect that
we should have a quirk for so we can wait longer for all host
controllers, not just MediaTek.  Or maybe it's a signal integrity
problem or something on systems using MediaTek?

Is the 100ms (or 120ms) based on some requirement from the spec?
If so, it would be good to include the reference.

> ---
>  drivers/pci/controller/pcie-mediatek.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index ae5ad05ddc1d..67b8cf0dc9f7 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -720,10 +720,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  	if (soc->need_fix_device_id)
>  		writew(soc->device_id, port->base + PCIE_CONF_DEVICE_ID);
>  
> -	/* 100ms timeout value should be enough for Gen1/2 training */
> +	/* 120ms timeout value should be enough for Gen1/2 training */

There are a lot of 100ms-ish delays in PCIe.  It would be nice to have
a #define for this so we can connect this back to something in the
spec and potentially share it across host controller drivers.

Bjorn

>  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
>  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> -				 100 * USEC_PER_MSEC);
> +				 120 * USEC_PER_MSEC);
>  	if (err)
>  		return -ETIMEDOUT;
>  
> @@ -785,10 +785,10 @@ static int mtk_pcie_startup_port(struct mtk_pcie_port *port)
>  	val &= ~PCIE_PORT_PERST(port->slot);
>  	writel(val, pcie->base + PCIE_SYS_CFG);
>  
> -	/* 100ms timeout value should be enough for Gen1/2 training */
> +	/* 120ms timeout value should be enough for Gen1/2 training */
>  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS, val,
>  				 !!(val & PCIE_PORT_LINKUP), 20,
> -				 100 * USEC_PER_MSEC);
> +				 120 * USEC_PER_MSEC);
>  	if (err)
>  		return -ETIMEDOUT;
>  
> -- 
> 2.34.1
>