On 1.5.2023 23.36, Bjorn Helgaas wrote: > On Sun, Mar 26, 2023 at 03:12:17PM +0300, Oskari Lemmela wrote: >> Some PCI devices fail to complete link training in 100ms. >> Increase link training timeout by 20ms to 120ms. >> >> Signed-off-by: Oskari Lemmela <oskari@xxxxxxxxxxx> > Doesn't look like this went anywhere, probably because we really don't > have enough information about why and where this is needed. > > Does this mean some *endpoints* don't train fast enough? Or is this > something to do with MediaTek host controllers? > > If some endpoints don't train correctly, maybe it's a defect that > we should have a quirk for so we can wait longer for all host > controllers, not just MediaTek. Or maybe it's a signal integrity > problem or something on systems using MediaTek? I can't rule out the possibility that the PCIe signal path in the BananaPi R64 board might be sub-optimal, and training just takes a bit longer. It might even be that LTSSM timeout is hit once in L0 or L1 state. The same endpoints work without problems on the MT7621 board with the timeout value of 100ms. One difference is that the MT7621 only supports PCIe gen1 speeds, which are faster to train. > Is the 100ms (or 120ms) based on some requirement from the spec? > If so, it would be good to include the reference. I looked into the PCIe spec a bit more and there is no clear timeout value for link training. State-specific timeout values are defined in the LTSSM definition. The timeout values for these states are Detect (12ms), Polling (24ms), Configuration (24ms), L0 (24ms), L1 (32ms), L2 (24ms) and Recovery (24ms). The total timeout value of the necessary state transitions is 140ms (Detect->Polling-> Configuration->L0->L1->L2). If you switch once from L1 state to recovery state and from there L0->L1->L2, the maximum timeout will be a total of 220ms. >> --- >> drivers/pci/controller/pcie-mediatek.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c >> index ae5ad05ddc1d..67b8cf0dc9f7 100644 >> --- a/drivers/pci/controller/pcie-mediatek.c >> +++ b/drivers/pci/controller/pcie-mediatek.c >> @@ -720,10 +720,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port) >> if (soc->need_fix_device_id) >> writew(soc->device_id, port->base + PCIE_CONF_DEVICE_ID); >> >> - /* 100ms timeout value should be enough for Gen1/2 training */ >> + /* 120ms timeout value should be enough for Gen1/2 training */ > There are a lot of 100ms-ish delays in PCIe. It would be nice to have > a #define for this so we can connect this back to something in the > spec and potentially share it across host controller drivers. It might be a good idea to set the timeout to at least 140ms or even 220ms. Oskari > Bjorn > >> err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val, >> !!(val & PCIE_PORT_LINKUP_V2), 20, >> - 100 * USEC_PER_MSEC); >> + 120 * USEC_PER_MSEC); >> if (err) >> return -ETIMEDOUT; >> >> @@ -785,10 +785,10 @@ static int mtk_pcie_startup_port(struct mtk_pcie_port *port) >> val &= ~PCIE_PORT_PERST(port->slot); >> writel(val, pcie->base + PCIE_SYS_CFG); >> >> - /* 100ms timeout value should be enough for Gen1/2 training */ >> + /* 120ms timeout value should be enough for Gen1/2 training */ >> err = readl_poll_timeout(port->base + PCIE_LINK_STATUS, val, >> !!(val & PCIE_PORT_LINKUP), 20, >> - 100 * USEC_PER_MSEC); >> + 120 * USEC_PER_MSEC); >> if (err) >> return -ETIMEDOUT; >> >> -- >> 2.34.1 >>
