linux-kernel - Re: [PATCH 2/2] PCI: dwc: Do not return failure from dw_pcie_wait_for

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <40e3197b-1670-4b63-a973-98012bcc623a@rock-chips.com>
Date: Thu, 20 Nov 2025 09:13:24 +0800
From: Shawn Lin <shawn.lin@...k-chips.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>,
 Jingoo Han <jingoohan1@...il.com>, Manivannan Sadhasivam <mani@...nel.org>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Krzysztof Wilczyński <kwilczynski@...nel.org>,
 Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>
Cc: shawn.lin@...k-chips.com, linux-pci@...r.kernel.org,
 linux-kernel@...r.kernel.org, vincent.guittot@...aro.org,
 zhangsenchuan@...incomputing.com
Subject: Re: [PATCH 2/2] PCI: dwc: Do not return failure from
 dw_pcie_wait_for_link() if link is in Detect/Poll state

在 2025/11/20 星期四 2:10, Manivannan Sadhasivam 写道:
> dw_pcie_wait_for_link() API waits for the link to be up and returns failure
> if the link is not up within the 1 second interval. But if there was no
> device connected to the bus, then the link up failure would be expected.
> In that case, the callers might want to skip the failure in a hope that the
> link will be up later when a device gets connected.
> 
> One of the callers, dw_pcie_host_init() is currently skipping the failure
> irrespective of the link state, in an assumption that the link may come up
> later. But this assumption is wrong, since LTSSM states other than Detect
> and Poll during link training phase are considered to be fatal and the link
> needs to be retrained.
> 
> So to avoid callers making wrong assumptions, skip returning failure from
> dw_pcie_wait_for_link() if the link is in Detect or Poll state after
> timeout and also check the return value of the API in dw_pcie_host_init().
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c | 8 +++++---
>   drivers/pci/controller/dwc/pcie-designware.c      | 8 ++++++++
>   2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 8fe3454f3b13..8c4845fd24aa 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -671,9 +671,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	 * If there is no Link Up IRQ, we should not bypass the delay
>   	 * because that would require users to manually rescan for devices.
>   	 */
> -	if (!pp->use_linkup_irq)
> -		/* Ignore errors, the link may come up later */
> -		dw_pcie_wait_for_link(pci);
> +	if (!pp->use_linkup_irq) {
> +		ret = dw_pcie_wait_for_link(pci);
> +		if (ret)
> +			goto err_stop_link;
> +	}
>   
>   	ret = pci_host_probe(bridge);
>   	if (ret)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c644216995f6..fe13c6b10ccb 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -651,6 +651,14 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>   	}
>   
>   	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
> +		/*
> +		 * If the link is in Detect or Poll state, it indicates that no
> +		 * device is connected. So return success to allow the device to
> +		 * show up later.
> +		 */
> +		if (dw_pcie_get_ltssm(pci) <= DW_PCIE_LTSSM_DETECT_WAIT)
> +			return 0;

I'm afraid this might not be true. If there is no devices connected or
the device connected without power supplied, it means there is no
far-end pull-up termination resistor from TX view of RC. TX pulse 
detection signal from the RC side will not undergo voltage division, and
its LTSSM state machine will only toggle between
DW_PCIE_LTSSM_DETECT_QUIET and DW_PCIE_LTSSM_DETECT_ACT.


> +
>   		dev_info(pci->dev, "Phy link never came up\n");
>   		return -ETIMEDOUT;
>   	}
>