* [PATCH] PCI: mediatek: increase link training timeout @ 2023-03-26 12:12 Oskari Lemmela 2023-05-01 20:36 ` Bjorn Helgaas 0 siblings, 1 reply; 3+ messages in thread From: Oskari Lemmela @ 2023-03-26 12:12 UTC (permalink / raw) To: Matthias Brugger Cc: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, linux-pci, Oskari Lemmela Some PCI devices fail to complete link training in 100ms. Increase link training timeout by 20ms to 120ms. Signed-off-by: Oskari Lemmela <oskari@lemmela.net> --- 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 */ 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] PCI: mediatek: increase link training timeout 2023-03-26 12:12 [PATCH] PCI: mediatek: increase link training timeout Oskari Lemmela @ 2023-05-01 20:36 ` Bjorn Helgaas 2023-05-02 17:43 ` Oskari Lemmelä 0 siblings, 1 reply; 3+ messages in thread From: Bjorn Helgaas @ 2023-05-01 20:36 UTC (permalink / raw) To: Oskari Lemmela Cc: Matthias Brugger, Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, linux-pci 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@lemmela.net> 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? Is the 100ms (or 120ms) based on some requirement from the spec? If so, it would be good to include the reference. > --- > 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. 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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PCI: mediatek: increase link training timeout 2023-05-01 20:36 ` Bjorn Helgaas @ 2023-05-02 17:43 ` Oskari Lemmelä 0 siblings, 0 replies; 3+ messages in thread From: Oskari Lemmelä @ 2023-05-02 17:43 UTC (permalink / raw) To: Bjorn Helgaas Cc: Matthias Brugger, Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, linux-pci 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@lemmela.net> > 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 >> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-02 17:51 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-26 12:12 [PATCH] PCI: mediatek: increase link training timeout Oskari Lemmela 2023-05-01 20:36 ` Bjorn Helgaas 2023-05-02 17:43 ` Oskari Lemmelä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).