[PATCH 3/8] pci: pcie_dw_rockchip: Speed up link probe

[Kever Yang]

Hi Jonas,

On 2023/4/23 02:19, Jonas Karlman wrote:
> Use a similar pattern and delay values as the linux mainline driver to
> speed up failing when nothing is connected.
>
> Reduce fail speed from around 5+ seconds down to around one second on a
> Radxa ROCK 3 Model A, where pcie2x1 is probed before pcie3x2 M2 slot.
>
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
>   drivers/pci/pcie_dw_rockchip.c | 68 ++++++++++++++++++----------------
>   1 file changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index a5b900f95981..fd3da47272b3 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -61,9 +61,6 @@ struct rk_pcie {
>   #define PCIE_CLIENT_DBG_TRANSITION_DATA	0xffff0000
>   #define PCIE_CLIENT_DBF_EN		0xffff0003
>   
> -/* Parameters for the waiting for #perst signal */
> -#define MACRO_US			1000
> -
>   static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
>   {
>   	if ((uintptr_t)addr & (size - 1)) {
> @@ -242,43 +239,46 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
>   	/* DW pre link configurations */
>   	rk_pcie_configure(priv, cap_speed);
>   
> -	/* Rest the device */
> -	if (dm_gpio_is_valid(&priv->rst_gpio)) {
> -		dm_gpio_set_value(&priv->rst_gpio, 0);
> -		/*
> -		 * Minimal is 100ms from spec but we see
> -		 * some wired devices need much more, such as 600ms.
> -		 * Add a enough delay to cover all cases.
> -		 */
> -		udelay(MACRO_US * 1000);
This delay has been changed.
> -		dm_gpio_set_value(&priv->rst_gpio, 1);
> -	}
> -
>   	rk_pcie_disable_ltssm(priv);
>   	rk_pcie_link_status_clear(priv);
>   	rk_pcie_enable_debug(priv);
>   
> +	/* Reset the device */
> +	if (dm_gpio_is_valid(&priv->rst_gpio))
> +		dm_gpio_set_value(&priv->rst_gpio, 0);
> +
>   	/* Enable LTSSM */
>   	rk_pcie_enable_ltssm(priv);
>   
> -	for (retries = 0; retries < 5; retries++) {
> -		if (is_link_up(priv)) {
> -			dev_info(priv->dw.dev, "PCIe Link up, LTSSM is 0x%x\n",
> -				 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
> -			rk_pcie_debug_dump(priv);
> -			return 0;
> -		}
> -
> -		dev_info(priv->dw.dev, "PCIe Linking... LTSSM is 0x%x\n",
> -			 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
> -		rk_pcie_debug_dump(priv);
> -		udelay(MACRO_US * 1000);
> +	/*
> +	 * PCIe requires the refclk to be stable for 100ms prior to releasing
> +	 * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI
> +	 * Express Card Electromechanical Specification, 1.1. However, we don't
> +	 * know if the refclk is coming from RC's PHY or external OSC. If it's
> +	 * from RC, so enabling LTSSM is the just right place to release #PERST.
> +	 */
> +	mdelay(100);

There is a common about this delay, the spec is 100ms, but some device 
on the public market

may not work with this delay, it's better to keep previous delay value.


Thanks,

- Kever

> +	if (dm_gpio_is_valid(&priv->rst_gpio))
> +		dm_gpio_set_value(&priv->rst_gpio, 1);
> +
> +	/* Check if the link is up or not */
> +	for (retries = 0; retries < 10; retries++) {
> +		if (is_link_up(priv))
> +			break;
> +
> +		mdelay(100);
> +	}
> +
> +	if (retries >= 10) {
> +		dev_err(priv->dw.dev, "PCIe-%d Link Fail\n",
> +			dev_seq(priv->dw.dev));
> +		return -EIO;
>   	}
>   
> -	dev_err(priv->dw.dev, "PCIe-%d Link Fail\n", dev_seq(priv->dw.dev));
> -	/* Link maybe in Gen switch recovery but we need to wait more 1s */
> -	udelay(MACRO_US * 1000);
> -	return -EIO;
> +	dev_info(priv->dw.dev, "PCIe Link up, LTSSM is 0x%x\n",
> +		 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
> +	rk_pcie_debug_dump(priv);
> +	return 0;
>   }
>   
>   static int rockchip_pcie_init_port(struct udevice *dev)
> @@ -287,6 +287,12 @@ static int rockchip_pcie_init_port(struct udevice *dev)
>   	u32 val;
>   	struct rk_pcie *priv = dev_get_priv(dev);
>   
> +	ret = reset_assert_bulk(&priv->rsts);
> +	if (ret) {
> +		dev_err(dev, "failed to assert resets (ret=%d)\n", ret);
> +		return ret;
> +	}
> +
>   	/* Set power and maybe external ref clk input */
>   	ret = regulator_set_enable_if_allowed(priv->vpcie3v3, true);
>   	if (ret && ret != -ENOSYS) {