Re: [PATCH] PCI: stm32: Fix LTSSM EP race with start link.
From: Christian Bruel
Date: Tue Nov 18 2025 - 13:34:40 EST
On 11/17/25 21:00, Bjorn Helgaas wrote:
On Mon, Nov 17, 2025 at 01:04:47PM +0100, Christian Bruel wrote:
On 11/14/25 19:59, Bjorn Helgaas wrote:
On Fri, Nov 14, 2025 at 08:45:52AM +0100, Christian Bruel wrote:
If the host has deasserted PERST# and started link training before the link
is started on EP side, enabling LTSSM before the endpoint registers are
initialized in the perst_irq handler results in probing incorrect values.
Thus, wait for the PERST# level-triggered interrupt to start link training
at the end of initialization and cleanup the stm32_pcie_[start stop]_link
functions.
I've seen this kind of thing in other drivers, and I wondered whether
it was safe because the host asserts and deasserts PERST#
asynchronously, independent of anything the endpoint is doing.
I assume it's possible that the host deasserts PERST# before this
driver has the stm32_pcie_ep_perst_irq_thread() thread set up. If
that happens and the driver doesn't see the PERST# interrupt, does
everything still work correctly?
yes it does. the PERST# interrupt is level-triggered and, if already
de-asserted, fires only when enabled (it is NOAUTOEN) with start_link.
At that point, the host can enumerate by performing a manual rescan or
rebind the PCIe driver, restarting the entire probe sequence.
Tested the pcie_epf_test driver with various power-up sequences: full
power-up the host or device first, and stop or standby PM suspend/resume.
Help me think through this. I guess the interesting case is when the
host boots first and enumerates devices before the stm32 endpoint is
ready, right?
I suppose the endpoint LTSSM is initially disabled, so the link is
down, and the host enumeration didn't find anything?
yes. When the device is not started (start_link) dw_pcie_wait_for_link() fails and pci_host_probe() only register the root port:
/*
* Note: Skip the link up delay only when a Link Up IRQ is present.
* If there is no Link Up IRQ, we should not bypass the delay
* because that would require users to manually rescan for devices.
*/
if (!pp->use_linkup_irq)
/* Ignore errors, the link may come up later */
dw_pcie_wait_for_link(pci);
ret = pci_host_probe(bridge);
Where does the link come up? I see the pci_epc_start_store() that
eventually leads to stm32_pcie_start_link(), which enables perst_irq.
The link appears when explicitly requested by writing '1' into the bound endpoint device driver sysfs 'start' once the device is configured.
see https://www.kernel.org/doc/html/latest/PCI/endpoint/pci-test-howto.html
But how pci_epc_start_store() is iterated from the configfs_write_iter() mechanism is beyond my knowledge...
Since you requested perst_irq with IRQF_TRIGGER_HIGH, and PERST# is
deasserted, does that trigger stm32_pcie_ep_perst_irq_thread() and
call stm32_pcie_perst_deassert() to enable the LTSSM?
perst_gpio is active low. So requesting the perst_irq with IRQF_TRIGGER_HIGH triggers when deasserted.
This parts is quite confusing (for me) because gpiod_get_value() correctly returns 0 when the gpio is de-asserted, when the irq API does not know about active low so we must use TRIGGER_HIGH
When the link comes up, if the Downstream Port supports hotplug and
pciehp is enabled, it might notice the link-up event and treat this as
a hot-add?
yes, I just tried with a host pc with hot-plug enabled. lspci found the stm32 EP, as you anticipated
Otherwise the user would have to manually rescan to notice the
endpoint?
indeed, this is how I proceed when the host does not support hot-plug,
thank you,
Christian
Signed-off-by: Christian Bruel <christian.bruel@xxxxxxxxxxx>
---
drivers/pci/controller/dwc/pcie-stm32-ep.c | 38 ++++++------------------------
1 file changed, 7 insertions(+), 31 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-stm32-ep.c b/drivers/pci/controller/dwc/pcie-stm32-ep.c
index 3400c7cd2d88a279c49ef36a99fc7537c381c384..d0654bb43759bb8d0f0d7badbf7bdae839241fcf 100644
--- a/drivers/pci/controller/dwc/pcie-stm32-ep.c
+++ b/drivers/pci/controller/dwc/pcie-stm32-ep.c
@@ -37,36 +37,9 @@ static void stm32_pcie_ep_init(struct dw_pcie_ep *ep)
dw_pcie_ep_reset_bar(pci, bar);
}
-static int stm32_pcie_enable_link(struct dw_pcie *pci)
-{
- struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
-
- regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
- STM32MP25_PCIECR_LTSSM_EN,
- STM32MP25_PCIECR_LTSSM_EN);
-
- return dw_pcie_wait_for_link(pci);
-}
-
-static void stm32_pcie_disable_link(struct dw_pcie *pci)
-{
- struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
-
- regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR, STM32MP25_PCIECR_LTSSM_EN, 0);
-}
-
static int stm32_pcie_start_link(struct dw_pcie *pci)
{
struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
- int ret;
-
- dev_dbg(pci->dev, "Enable link\n");
-
- ret = stm32_pcie_enable_link(pci);
- if (ret) {
- dev_err(pci->dev, "PCIe cannot establish link: %d\n", ret);
- return ret;
- }
enable_irq(stm32_pcie->perst_irq);
@@ -77,11 +50,7 @@ static void stm32_pcie_stop_link(struct dw_pcie *pci)
{
struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
- dev_dbg(pci->dev, "Disable link\n");
-
disable_irq(stm32_pcie->perst_irq);
-
- stm32_pcie_disable_link(pci);
}
static int stm32_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
@@ -152,6 +121,8 @@ static void stm32_pcie_perst_assert(struct dw_pcie *pci)
dev_dbg(dev, "PERST asserted by host\n");
+ regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR, STM32MP25_PCIECR_LTSSM_EN, 0);
+
pci_epc_deinit_notify(ep->epc);
stm32_pcie_disable_resources(stm32_pcie);
@@ -192,6 +163,11 @@ static void stm32_pcie_perst_deassert(struct dw_pcie *pci)
pci_epc_init_notify(ep->epc);
+ /* Enable link training */
+ regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
+ STM32MP25_PCIECR_LTSSM_EN,
+ STM32MP25_PCIECR_LTSSM_EN);
+
return;
err_disable_resources:
---
base-commit: 31115ecec74fe5c679a149d7037009f26b3aa8a9
change-id: 20251113-perst_ep-0b57b9679cf9
Best regards,
--
Christian Bruel <christian.bruel@xxxxxxxxxxx>