Re: [PATCH 4/4] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS

[Damien Le Moal <dlemoal@kernel.org>]

On 6/11/25 19:51, Niklas Cassel wrote:
> There is no reason for the delay, in each loop iteration, while polling for
> link up (LINK_WAIT_SLEEP_MS), to be so long as 90 ms.
> 
> PCIe r6.0, sec 6.6.1, still require us to wait for up to 1.0 s for the link
> to come up, thus the number of retries (LINK_WAIT_MAX_RETRIES) is increased
> to keep the total timeout to 1.0 s.
> 
> PCIe r6.0, sec 6.6.1, also mandates that there is a 100 ms delay, after the
> link has been established, before performing configuration requests (this
> delay already exists in dw_pcie_wait_for_link() and is unchanged).
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c |  6 +++++-
>  drivers/pci/controller/dwc/pcie-designware.h | 11 ++++++++---
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index dbb21a9c93d7..8ef1e42b7168 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -701,7 +701,11 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>  	u32 offset, val;
>  	int retries;
>  
> -	/* Check if the link is up or not */
> +	/*
> +	 * Check if the link is up or not. As per PCIe r6.0, sec 6.6.1, software
> +	 * must allow at least 1.0 s following exit from a Conventional Reset of
> +	 * a device, before determining that the device is broken.
> +	 */
>  	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
>  		if (dw_pcie_link_up(pci))
>  			break;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ce9e18554e42..52daf9525bae 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -62,9 +62,14 @@
>  #define dw_pcie_cap_set(_pci, _cap) \
>  	set_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
>  
> -/* Parameters for the waiting for link up routine */
> -#define LINK_WAIT_MAX_RETRIES		10
> -#define LINK_WAIT_SLEEP_MS		90
> +/*
> + * Parameters for the waiting for link up routine. As per PCIe r6.0, sec 6.6.1,

s/for the/for

and maybe reword this to something like:

* Parameters for waiting for a link to be established.

> + * software must allow at least 1.0 s following exit from a Conventional Reset
> + * of a device, before determining that the device is broken.
> + * Therefore LINK_WAIT_MAX_RETRIES * LINK_WAIT_SLEEP_MS should equal 1.0 s.
> + */
> +#define LINK_WAIT_MAX_RETRIES		100
> +#define LINK_WAIT_SLEEP_MS		10
>  
>  /* Parameters for the waiting for iATU enabled routine */
>  #define LINK_WAIT_MAX_IATU_RETRIES	5

Other than that, looks good to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research