[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d862f711-428c-4e3a-b80d-e45d14e7b781@quicinc.com>
Date: Thu, 24 Apr 2025 10:49:38 +0800
From: "Wenbin Yao (Consultant)" <quic_wenbyao@...cinc.com>
To: Niklas Cassel <cassel@...nel.org>
CC: <jingoohan1@...il.com>, <manivannan.sadhasivam@...aro.org>,
<lpieralisi@...nel.org>, <kw@...ux.com>, <robh@...nel.org>,
<bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <krishna.chundru@....qualcomm.com>,
<quic_vbadigan@...cinc.com>, <quic_mrana@...cinc.com>,
<quic_cang@...cinc.com>, <quic_qianyu@...cinc.com>
Subject: Re: [PATCH v2] PCI: dwc: Set PORT_LOGIC_LINK_WIDTH to one lane
On 4/23/2025 9:24 PM, Niklas Cassel wrote:
> 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@...cinc.com>
>> Signed-off-by: Qiang Yu <quic_qianyu@...cinc.com>
>> Signed-off-by: Wenbin Yao <quic_wenbyao@...cinc.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@...nel.org>
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.
--
With best wishes
Wenbin
Powered by blists - more mailing lists