On 10/27/26 21:12, Shawn Lin wrote:
Hi Hans,
在 2026/10/27 星期二 20:28, Hans Zhang 写道:
The DWC PCIe core and its platform drivers repeatedly call
dw_pcie_find_capability(pci, PCI_CAP_ID_EXP) to obtain the offset of the
PCI Express Capability structure. Besides being verbose, that performs a
DBI read every time, including in paths that run repeatedly such as link
up polling, suspend/resume and interrupt handlers.
Add a pcie_cap field to struct dw_pcie and a helper
dw_pcie_get_pcie_cap() that performs the lookup on first use and caches
the result for the subsequent calls.
The lookup is deliberately done lazily, at the first place that needs the
offset, instead of at a fixed point during controller initialization.
This guarantees that the DBI read happens exactly when the open coded
dw_pcie_find_capability() call it replaces used to happen, so no DBI
access can be made before the controller clocks, resets and power are
With the controller inaccessible (clocks/power gated), what a DBI
read returns is platform dependent, most integrations just hang the
accessing CPU, like Rockchip's SoCs, others might return zero or
garbage. Zero self-heals as the commit message describes, but a garbage
non-zero lookup would now be cached indefinitely, so every later caller
would use a bogus offset?
Perhaps the "TX Interface Progression Detection Features" of dwc IP
could help return a dedicated value? I don't know, so I need to check
it internally. I will get back to you later.
Hi,
Thanks for the review.
You are right that the cache must not turn a bogus value into a permanent
one. Some details and one change in v5:
- dw_pcie_get_pcie_cap() is only called at the sites that previously
called dw_pcie_find_capability() directly, so this series does not add
or move a DBI access. It does not try to fix the "DBI accessed while
the controller is inaccessible" problem; that behavior is platform
specific and pre-existing.
- dw_pcie_find_capability() already rejects offsets below the standard
header, unaligned offsets, and entries whose capability ID is not
PCI_CAP_ID_EXP, so a bogus value has to look like a valid capability
entry to be returned at all.
- To make sure that a transient bogus read cannot stick anyway, v5 only
caches the offset after the capability ID has been read back at that
offset. If the read-back does not confirm PCI_CAP_ID_EXP, the offset is
still used for the current access (same as the open coded search
before), but it is not cached, and the next call searches again - the
same self-healing behavior as the zero/not-found case.
If the DWC IP has a reliable way to tell whether the core is accessible
(the "TX Interface Progression Detection" feature you mentioned), that
would be even better than validating the value. I would be glad to use it
if you can share the details; it probably deserves a separate patch so
that all DBI accesses can be guarded, not only this lookup.
Best regards,
Hans
enabled. This matters for endpoint controllers such as qcom-ep and
tegra194, where DBI is only accessible after PERST# is deasserted, and it
also means platform drivers do not have to call the helper from a
specific callback at the right time.
The cached value is accessed with READ_ONCE()/WRITE_ONCE(), because the
first lookup may be triggered from different contexts, for example the
qcom-ep global IRQ and PERST# IRQ handlers. A concurrent lookup stores
the same value, so this race is benign.
A return value of 0 means that the capability is not present. This
cannot happen on real DWC hardware, where the PCI Express Capability is
mandatory, so a negative result is not cached separately: it only causes
the search to be repeated, which does not affect correctness.
Signed-off-by: Hans Zhang <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.h | 34 ++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/
pci/controller/dwc/pcie-designware.h
index 0735ae940924..fa31e8b763bc 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -592,6 +592,8 @@ struct dw_pcie {
* use_parent_dt_ranges to true to avoid this warning.
*/
bool use_parent_dt_ranges;
+
+ u8 pcie_cap; /* Offset of PCI_CAP_ID_EXP */
};
#define to_dw_pcie_from_pp(port) container_of((port), struct
dw_pcie, pp)
@@ -830,6 +832,38 @@ static inline void dw_pcie_dbi_ro_wr_dis(struct
dw_pcie *pci)
dw_pcie_writel_dbi(pci, reg, val);
}
+/**
+ * dw_pcie_get_pcie_cap() - Return the PCIe Capability offset
+ * @pci: DWC instance
+ *
+ * The offset of the PCI Express Capability structure is read once from
+ * the DBI configuration space on the first call and cached in
+ * @pci->pcie_cap for the subsequent ones.
+ *
+ * The lookup is done lazily at the first place that actually needs the
+ * offset, so the DBI read happens exactly when it used to happen before
+ * caching was introduced, i.e. never before the controller's clocks,
+ * resets and power are enabled.
+ *
+ * DWC PCIe controllers always implement the PCI Express Capability,
so a
+ * return value of 0 means that the hardware is not usable. Callers that
+ * check the returned offset keep working, and the (never expected)
+ * lookup is simply repeated on the next call.
+ *
+ * Return: the cached offset, or 0 if the capability is not present.
+ */
+static inline u8 dw_pcie_get_pcie_cap(struct dw_pcie *pci)
+{
+ u8 cap = READ_ONCE(pci->pcie_cap);
+
+ if (!cap) {
+ cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+ WRITE_ONCE(pci->pcie_cap, cap);
+ }
+
+ return cap;
+}
+
static inline int dw_pcie_start_link(struct dw_pcie *pci)
{
if (pci->ops && pci->ops->start_link)
