Re: [PATCH v2] PCI: dwc: Set PORT_LOGIC_LINK_WIDTH to one lane

[Niklas Cassel <cassel@kernel.org>]

On Tue, Apr 22, 2025 at 06:36:23PM +0800, Wenbin Yao wrote:
> As per DWC PCIe registers description 4.30a, section 1.13.43, NUM_OF_LANES
> named as PORT_LOGIC_LINK_WIDTH in PCIe DWC driver, is referred to as the
> "Predetermined Number of Lanes" in section 4.2.6.2.1 of the PCI Express
> Base 3.0 Specification, revision 1.0. This section explains the conditions
> need be satisfied for entering Polling.Configuration:
> 
> "Next state is Polling.Configuration after at least 1024 TS1 Ordered Sets
> were transmitted, and all Lanes that detected a Receiver during Detect
> receive eight consecutive training sequences.
> 
> Otherwise, after a 24 ms timeout the next state is:
> Polling.Configuration if
> (i) Any Lane, which detected a Receiver during Detect, received eight
> consecutive training sequences and a minimum of 1024 TS1 Ordered Sets are
> transmitted after receiving one TS1 or TS2 Ordered Set.
> And
> (ii) At least a predetermined set of Lanes that detected a Receiver during
> Detect have detected an exit from Electrical Idle at least once since
> entering Polling.Active.
> 
> Note: This may prevent one or more bad Receivers or Transmitters from
> holding up a valid Link from being configured, and allow for additional
> training in Polling.Configuration. The exact set of predetermined Lanes is
> implementation specific.
> 
> Note: Any Lane that receives eight consecutive TS1 or TS2 Ordered Sets
> should have detected an exit from Electrical Idle at least once since
> entering Polling.Active."
> 
> In a PCIe link that supports multiple lanes, if PORT_LOGIC_LINK_WIDTH is
> set to lane width hardware supports, all lanes that detect a receiver
> during the Detect phase must receive eight consecutive training sequences.
> Otherwise, the LTSSM cannot enter Polling.Configuration and link training
> will fail.
> 
> Therefore, always set PORT_LOGIC_LINK_WIDTH to 1, regardless of the number
> of lanes the port actually supports, to make linking up more robust. This
> setting will not affect the intended link width if all lanes are
> functional. Additionally, the link can still be established with at least
> one lane if other lanes are faulty.
> 
> Co-developed-by: Qiang Yu <quic_qianyu@quicinc.com>
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
> ---
> Changes in v2:
> - Reword commit message.
> - Link to v1: https://lore.kernel.org/all/1524e971-8433-1e2d-b39e-65bad0d6c6ce@quicinc.com/
> 
>  drivers/pci/controller/dwc/pcie-designware.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 97d76d3dc..be348b341 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -797,22 +797,19 @@ static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
>  	/* Set link width speed control register */
>  	lwsc = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>  	lwsc &= ~PORT_LOGIC_LINK_WIDTH_MASK;
> +	lwsc |= PORT_LOGIC_LINK_WIDTH_1_LANES;
>  	switch (num_lanes) {
>  	case 1:
>  		plc |= PORT_LINK_MODE_1_LANES;
> -		lwsc |= PORT_LOGIC_LINK_WIDTH_1_LANES;
>  		break;
>  	case 2:
>  		plc |= PORT_LINK_MODE_2_LANES;
> -		lwsc |= PORT_LOGIC_LINK_WIDTH_2_LANES;
>  		break;
>  	case 4:
>  		plc |= PORT_LINK_MODE_4_LANES;
> -		lwsc |= PORT_LOGIC_LINK_WIDTH_4_LANES;
>  		break;
>  	case 8:
>  		plc |= PORT_LINK_MODE_8_LANES;
> -		lwsc |= PORT_LOGIC_LINK_WIDTH_8_LANES;
>  		break;
>  	default:
>  		dev_err(pci->dev, "num-lanes %u: invalid value\n", num_lanes);
> -- 
> 2.34.1
> 

I still see the link to my EP (which also have this patch) using all
four lanes according to lspci, so:

Tested-by: Niklas Cassel <cassel@kernel.org>