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

["Wenbin Yao (Consultant)" <quic_wenbyao@quicinc.com>]

On 12/12/2024 4:19 PM, Wenbin Yao (Consultant) wrote:
> PORT_LOGIC_LINK_WIDTH field of the PCIE_LINK_WIDTH_SPEED_CONTROL register
> indicates the number of lanes to check for exit from Electrical Idle in
> Polling.Active and L2.Idle. It is used to limit the effective link 
> width to
> ignore broken or unused lanes that detect a receiver to prevent one or 
> more
> bad Receivers or Transmitters from holding up a valid Link from being
> configured.
>
> In a PCIe link that support muiltiple lanes, setting 
> PORT_LOGIC_LINK_WIDTH
> to 1 will not affect the link width that is actually intended to be used.
> But setting it to a value other than 1 will lead to link training fail if
> one or more lanes are broken.
>
> Hence, always set PORT_LOGIC_LINK_WIDTH to 1 no matter how many lanes the
> port actually supports to make linking up more robust. Link can still be
> established with one lane at least if other lanes are broken.
>
> Signed-off-by: Wenbin Yao <quic_wenbyao@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 6d6cbc8b5b2c..d40afe74ddd1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -752,22 +752,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);

Hello, do you have any futher comments?

-- 
With best wishes
Wenbin