[PATCH v2 00/16] PCI: dwc: Cache PCIe capability offset and simplify drivers

[PATCH v2 00/16] PCI: dwc: Cache PCIe capability offset and simplify drivers

[Hans Zhang]

Hi,

The DWC PCIe core and its many platform drivers repeatedly call
dw_pcie_find_capability(pci, PCI_CAP_ID_EXP) to obtain the offset of the
PCI Express Capability structure. This is wasteful and makes the code
verbose. Some drivers even search for the offset in suspend/resume paths.

Add a cached pcie_cap field in struct dw_pcie and a helper
dw_pcie_get_pcie_cap() to initialize it once at the point when the
hardware is ready. Then replace all explicit capability searches with
the cached value across the entire dwc subtree.

**Safety analysis: DBI access timing**
The PCIe Capability offset is read from DBI configuration space. DBI
registers are only accessible after the controller's clocks, resets, and
power are enabled. The following call graph demonstrates that all
dw_pcie_find_capability() replacements occur only after hardware is ready:

- For Root Complex mode:
  dw_pcie_host_init()
    ...
    if (pp->ops->init)
      pp->ops->init   [enables clocks/resets]
    dw_pcie_get_pcie_cap()   [caches offset]
    ...
    dw_pcie_link_up
      pci->ops->link_up
    ...
    dw_pcie_start_link
      pci->ops->start_link
    ...
    pci_host_probe
    if (pp->ops->post_init)
      pp->ops->post_init
    ...

- For Endpoint mode:
  dw_pcie_ep_init()
    ep->ops->pre_init()    [enables clocks/resets]
    dw_pcie_get_pcie_cap()   [caches offset]
    dw_pcie_ep_init_non_sticky_registers() etc.

- Some platform drivers (e.g., layerscape-ep, tegra194) need the offset
  before calling dw_pcie_ep_init() / dw_pcie_host_init(). They already
  enable hardware themselves and explicitly call dw_pcie_get_pcie_cap()
  before the core caches it. This is safe and ensures the cached value is
  available for all later users.

Thus, no DBI access occurs before hardware is operational.

Changes in v2:
- Move dw_pcie_get_pcie_cap() from the beginning of dw_pcie_host_init()
  and dw_pcie_ep_init() to after the hardware initialization callbacks
  (host_init/pre_init). This can prevent the reading of DBI when the clock
  is turned off or when the hardware is not ready.
- Convert all platform drivers to use the cached offset via
  dw_pcie_get_pcie_cap() or directly pci->pcie_cap where safe.
- Fix type mismatches (u16 -> u8).
- Properly split per-driver modifications.

v1:
https://patchwork.kernel.org/project/linux-pci/cover/20260509135152.2241235-1-18255117159@163.com/

---
**Why splitting into per-driver patches**  
To facilitate review and potential bisection, each platform driver is
modified in its own patch. The core changes are in patches 1-2, and
follow-up patches convert individual glue drivers. If maintainers prefer
to squash them, please let me know and I will merge in v3.

This splitting patch of each controller was carried out solely for
the convenience of code review.
---

Hans Zhang (16):
  PCI: dwc: Add pcie_cap field and helper in designware header
  PCI: dwc: Use cached PCIe capability offset in core
  PCI: dwc: imx6: Use cached PCIe capability offset
  PCI: dwc: layerscape-ep: Use cached PCIe capability offset
  PCI: dwc: meson: Use cached PCIe capability offset
  PCI: dwc: rockchip: Use cached PCIe capability offset
  PCI: dwc: eswin: Use cached PCIe capability offset
  PCI: dwc: fu740: Use cached PCIe capability offset
  PCI: dwc: intel-gw: Use cached PCIe capability offset
  PCI: dwc: qcom-ep: Use cached PCIe capability offset
  PCI: dwc: qcom: Use cached PCIe capability offset
  PCI: dwc: sophgo: Use cached PCIe capability offset
  PCI: dwc: spacemit-k1: Use cached PCIe capability offset
  PCI: dwc: spear13xx: Use cached PCIe capability offset
  PCI: dwc: tegra194: Use cached PCIe capability offset
  PCI: dwc: ultrarisc: Use cached PCIe capability offset

 drivers/pci/controller/dwc/pci-imx6.c         |  6 +++---
 .../pci/controller/dwc/pci-layerscape-ep.c    |  9 +++-----
 drivers/pci/controller/dwc/pci-meson.c        |  4 ++--
 .../pci/controller/dwc/pcie-designware-ep.c   |  4 +++-
 .../pci/controller/dwc/pcie-designware-host.c |  2 ++
 drivers/pci/controller/dwc/pcie-designware.c  | 16 ++++++--------
 drivers/pci/controller/dwc/pcie-designware.h  | 17 +++++++++++++++
 drivers/pci/controller/dwc/pcie-dw-rockchip.c |  2 +-
 drivers/pci/controller/dwc/pcie-eswin.c       |  3 +--
 drivers/pci/controller/dwc/pcie-fu740.c       |  2 +-
 drivers/pci/controller/dwc/pcie-intel-gw.c    |  2 +-
 drivers/pci/controller/dwc/pcie-qcom-ep.c     | 11 ++++------
 drivers/pci/controller/dwc/pcie-qcom.c        | 21 ++++++++-----------
 drivers/pci/controller/dwc/pcie-sophgo.c      |  5 +++--
 drivers/pci/controller/dwc/pcie-spacemit-k1.c |  2 +-
 drivers/pci/controller/dwc/pcie-spear13xx.c   |  2 +-
 drivers/pci/controller/dwc/pcie-tegra194.c    |  6 ++----
 drivers/pci/controller/dwc/pcie-ultrarisc.c   |  2 +-
 18 files changed, 61 insertions(+), 55 deletions(-)


base-commit: 3a97877d13e1a29c50ab15ed0a0aba87b75061dd
-- 
2.34.1

[PATCH v2 01/16] PCI: dwc: Add pcie_cap field and helper in designware header

[Hans Zhang]

Add a pcie_cap field to struct dw_pcie to store the offset of the
PCI Express Capability structure. Provide a helper dw_pcie_get_pcie_cap()
which performs the capability search on first call and caches the result.

This is a preparatory step for replacing repetitive capability searches
in both core and platform drivers.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pcie-designware.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 7dac37521b8e..561b434fe335 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -596,6 +596,8 @@ struct dw_pcie {
 	 * use_parent_dt_ranges to true to avoid this warning.
 	 */
 	bool			use_parent_dt_ranges;
+
+	u8			pcie_cap;	/* PCIe capability offset */
 };
 
 #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
@@ -834,6 +836,21 @@ 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 cached PCIe Capability offset
+ * @pci: DWC instance
+ *
+ * Finds and caches the offset of PCI_CAP_ID_EXP on first call.
+ * Returns 0 if the capability is not present.
+ */
+static inline u8 dw_pcie_get_pcie_cap(struct dw_pcie *pci)
+{
+	if (!pci->pcie_cap)
+		pci->pcie_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+
+	return pci->pcie_cap;
+}
+
 static inline int dw_pcie_start_link(struct dw_pcie *pci)
 {
 	if (pci->ops && pci->ops->start_link)
-- 
2.34.1

[PATCH v2 02/16] PCI: dwc: Use cached PCIe capability offset in core

[Hans Zhang]

Modify the DWC core functions to use the cached pcie_cap offset instead
of calling dw_pcie_find_capability() each time.

In the DWC core, dw_pcie_find_capability() is called at several locations:
- dw_pcie_ep_init_non_sticky_registers()
- dw_pcie_wait_for_link()
- dw_pcie_link_set_max_speed()
- dw_pcie_link_get_max_link_width()
- dw_pcie_link_set_max_link_width()

The cached offset is initialized after hardware is ready:
- In host mode: dw_pcie_host_init() calls pp->ops->host_init() (enables
  clocks/resets), then dw_pcie_get_pcie_cap() caches the offset.
- In endpoint mode: dw_pcie_ep_init() calls ep->ops->pre_init() (enables
  hardware), then dw_pcie_get_pcie_cap() caches the offset.

dw_pcie_ep_init_non_sticky_registers() is called after pre_init, so it can
use dw_pcie_get_pcie_cap() safely. The other functions run after probe,
so pci->pcie_cap is already valid and can be used directly.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c  |  4 +++-
 .../pci/controller/dwc/pcie-designware-host.c    |  2 ++
 drivers/pci/controller/dwc/pcie-designware.c     | 16 ++++++----------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index a396b58d6746..11b4fa6d129c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -1246,7 +1246,7 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
 	 * to all other functions as well.
 	 */
 	if (funcs > 1) {
-		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+		offset = dw_pcie_get_pcie_cap(pci);
 		func0_lnkcap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 		func0_lnkcap = FIELD_GET(PCI_EXP_LNKCAP_MLW |
 					 PCI_EXP_LNKCAP_SLS, func0_lnkcap);
@@ -1524,6 +1524,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 	if (ep->ops->pre_init)
 		ep->ops->pre_init(ep);
 
+	dw_pcie_get_pcie_cap(pci);
+
 	ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
 			       ep->page_size);
 	if (ret < 0) {
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index cffb34f6f3a9..b8175138e47a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -593,6 +593,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 			goto err_free_ecam;
 	}
 
+	dw_pcie_get_pcie_cap(pci);
+
 	if (pci_msi_enabled()) {
 		pp->use_imsi_rx = !(pp->ops->msi_init ||
 				     of_property_present(np, "msi-parent") ||
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 37d2715e38eb..810e920c1d8b 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -765,7 +765,7 @@ const char *dw_pcie_ltssm_status_string(enum dw_pcie_ltssm ltssm)
  */
 int dw_pcie_wait_for_link(struct dw_pcie *pci)
 {
-	u32 offset, val, ltssm;
+	u32 val, ltssm;
 	int retries;
 
 	/* Check if the link is up or not */
@@ -811,8 +811,7 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
 	if (pci->max_link_speed > 2)
 		msleep(PCIE_RESET_CONFIG_WAIT_MS);
 
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
+	val = dw_pcie_readw_dbi(pci, pci->pcie_cap + PCI_EXP_LNKSTA);
 
 	dev_info(pci->dev, "PCIe Gen.%u x%u link up\n",
 		 FIELD_GET(PCI_EXP_LNKSTA_CLS, val),
@@ -848,7 +847,7 @@ EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
 static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
 {
 	u32 cap, ctrl2, link_speed;
-	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 offset = pci->pcie_cap;
 
 	cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 
@@ -894,8 +893,7 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
 
 int dw_pcie_link_get_max_link_width(struct dw_pcie *pci)
 {
-	u8 cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	u32 lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
+	u32 lnkcap = dw_pcie_readl_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP);
 
 	return FIELD_GET(PCI_EXP_LNKCAP_MLW, lnkcap);
 }
@@ -903,7 +901,6 @@ int dw_pcie_link_get_max_link_width(struct dw_pcie *pci)
 static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
 {
 	u32 lnkcap, lwsc, plc;
-	u8 cap;
 
 	if (!num_lanes)
 		return;
@@ -940,10 +937,9 @@ static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
 	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, plc);
 	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, lwsc);
 
-	cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
+	lnkcap = dw_pcie_readl_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP);
 	FIELD_MODIFY(PCI_EXP_LNKCAP_MLW, &lnkcap, num_lanes);
-	dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, lnkcap);
+	dw_pcie_writel_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP, lnkcap);
 }
 
 void dw_pcie_iatu_detect(struct dw_pcie *pci)
-- 
2.34.1

Re: [PATCH v2 02/16] PCI: dwc: Use cached PCIe capability offset in core

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Direct usage of cached `pci->pcie_cap` leads to incorrect offset calculation and DBI corruption in endpoint drivers with deferred hardware enablement.
--

commit 02cd0d194f8ffdc28d0564602c5f35da3c6e8582
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: Use cached PCIe capability offset in core

This patch modifies the DWC core functions to use the cached pcie_cap offset
instead of calling dw_pcie_find_capability() each time. The cached offset is
initialized after hardware is supposedly ready in host and endpoint modes.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index a396b58d67462..11b4fa6d129c7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c

[ ... ]

> @@ -1524,6 +1524,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  	if (ep->ops->pre_init)
>  		ep->ops->pre_init(ep);
>  
> +	dw_pcie_get_pcie_cap(pci);
> +

[Severity: High]
Does this code incorrectly attempt to read the capability offset before the
hardware is powered on in endpoint drivers with deferred enablement, like
qcom-pcie-ep?

In these drivers, hardware resources like clocks and PHY are not enabled
during initial probe, but are deferred until the host de-asserts PERST#.
Because the DBI space is unclocked and inaccessible at this time, reading it
can either trigger a synchronous external abort or return 0xFFFFFFFF.

This would cause the capability search to fail, leaving pci->pcie_cap as 0.

>  	ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
>  			       ep->page_size);
>  	if (ret < 0) {

[ ... ]

> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 37d2715e38eb4..810e920c1d8b5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c

[ ... ]

> @@ -848,7 +847,7 @@ EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
>  static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
>  {
>  	u32 cap, ctrl2, link_speed;
> -	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u8 offset = pci->pcie_cap;
>  
>  	cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);

[Severity: High]
If pci->pcie_cap is 0 due to the earlier failed capability search in
dw_pcie_ep_init(), will this read and write to offset 0 (the standard PCI
configuration space header) instead of the actual PCIe capability offset?

Since dw_pcie_setup() calls this function after the link is initialized,
operating on offset 0 could corrupt the Vendor ID and Device ID, completely
breaking link initialization.

[ ... ]

> @@ -940,10 +937,9 @@ static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
>  	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, plc);
>  	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, lwsc);
>  
> -	cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> -	lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
> +	lnkcap = dw_pcie_readl_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP);
>  	FIELD_MODIFY(PCI_EXP_LNKCAP_MLW, &lnkcap, num_lanes);
> -	dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, lnkcap);
> +	dw_pcie_writel_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP, lnkcap);
>  }

[Severity: High]
Could this similarly corrupt the standard PCI configuration space header at
offset 0 if pci->pcie_cap was not properly initialized?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=2

[PATCH v2 03/16] PCI: dwc: imx6: Use cached PCIe capability offset

[Hans Zhang]

Thus, the cached offset is available when these functions are called, and
DBI access occurs only after hardware is enabled.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pci-imx6, dw_pcie_find_capability() is called in the following
call chain:

  static const struct dw_pcie_ops dw_pcie_ops = {
    .start_link = imx_pcie_start_link,
  };
  imx_pcie_start_link()
    -> dw_pcie_find_capability()
    -> imx_pcie_ltssm_enable()
      -> dw_pcie_find_capability()

Replace these calls with the cached pci->pcie_cap. The call chain after
modification becomes:

  dw_pcie_host_init()
    -> pp->ops->init() [imx6_pcie_host_init]
    -> dw_pcie_get_pcie_cap()  (caches offset)
    -> pci->ops->start_link = imx_pcie_start_link
      -> uses pci->pcie_cap
      -> imx_pcie_ltssm_enable()
        -> uses pci->pcie_cap
---
 drivers/pci/controller/dwc/pci-imx6.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 773ab65b2afa..de06ff429eff 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -933,10 +933,10 @@ static void imx_pcie_ltssm_enable(struct device *dev)
 {
 	struct imx_pcie *imx_pcie = dev_get_drvdata(dev);
 	const struct imx_pcie_drvdata *drvdata = imx_pcie->drvdata;
-	u8 offset = dw_pcie_find_capability(imx_pcie->pci, PCI_CAP_ID_EXP);
+	struct dw_pcie *pci = imx_pcie->pci;
 	u32 tmp;
 
-	tmp = dw_pcie_readl_dbi(imx_pcie->pci, offset + PCI_EXP_LNKCAP);
+	tmp = dw_pcie_readl_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP);
 	phy_set_speed(imx_pcie->phy, FIELD_GET(PCI_EXP_LNKCAP_SLS, tmp));
 	if (drvdata->ltssm_mask)
 		regmap_update_bits(imx_pcie->iomuxc_gpr, drvdata->ltssm_off, drvdata->ltssm_mask,
@@ -962,7 +962,7 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
 {
 	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
 	struct device *dev = pci->dev;
-	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 offset = pci->pcie_cap;
 	u32 tmp;
 	int ret;
 
-- 
2.34.1

[PATCH v2 04/16] PCI: dwc: layerscape-ep: Use cached PCIe capability offset

[Hans Zhang]

Replace these with dw_pcie_get_pcie_cap(). The hardware is already enabled
by the driver before ls_pcie_ep_probe() and before the interrupt handler
runs. dw_pcie_get_pcie_cap() will cache the offset on first call, and
subsequent calls (including inside dw_pcie_ep_init) will use the cached
value without re-searching.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pci-layerscape-ep, dw_pcie_find_capability() is called in:

  ls_pcie_ep_probe()
    -> offset = dw_pcie_find_capability()
    -> pcie->lnkcap = dw_pcie_readl_dbi(offset + PCI_EXP_LNKCAP)
    -> dw_pcie_ep_init()
    -> ls_pcie_ep_interrupt_init()
        -> devm_request_irq(..., ls_pcie_ep_event_handler)
          -> ls_pcie_ep_event_handler()
            -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pci-layerscape-ep.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 8936975ff104..b2d0b51df3c5 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -74,7 +74,6 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
 	struct ls_pcie_ep *pcie = dev_id;
 	struct dw_pcie *pci = pcie->pci;
 	u32 val, cfg;
-	u8 offset;
 
 	val = ls_pcie_pf_lut_readl(pcie, PEX_PF0_PME_MES_DR);
 	ls_pcie_pf_lut_writel(pcie, PEX_PF0_PME_MES_DR, val);
@@ -83,9 +82,6 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	if (val & PEX_PF0_PME_MES_DR_LUD) {
-
-		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-
 		/*
 		 * The values of the Maximum Link Width and Supported Link
 		 * Speed from the Link Capabilities Register will be lost
@@ -93,7 +89,8 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
 		 * that configured by the Reset Configuration Word (RCW).
 		 */
 		dw_pcie_dbi_ro_wr_en(pci);
-		dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, pcie->lnkcap);
+		dw_pcie_writel_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP,
+				   pcie->lnkcap);
 		dw_pcie_dbi_ro_wr_dis(pci);
 
 		cfg = ls_pcie_pf_lut_readl(pcie, PEX_PF0_CONFIG);
@@ -266,7 +263,7 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pcie);
 
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	offset = dw_pcie_get_pcie_cap(pci);
 	pcie->lnkcap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 
 	ret = dw_pcie_ep_init(&pci->ep);
-- 
2.34.1

Re: [PATCH v2 04/16] PCI: dwc: layerscape-ep: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Sleeping in atomic context due to mutex_lock() inside hardirq handler
- [High] Race condition in DBI register access leading to silent PCIe configuration failures
--

commit 68e2e619ed44719a7c9ec1273ed94120c179ed09
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: layerscape-ep: Use cached PCIe capability offset
    
Replace these with dw_pcie_get_pcie_cap(). The hardware is already enabled
by the driver before ls_pcie_ep_probe() and before the interrupt handler
runs. dw_pcie_get_pcie_cap() will cache the offset on first call, and
subsequent calls (including inside dw_pcie_ep_init) will use the cached
value without re-searching.

> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 8936975ff104e..b2d0b51df3c53 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -83,9 +82,6 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
>  		return IRQ_NONE;
>  
>  	if (val & PEX_PF0_PME_MES_DR_LUD) {
> -
> -		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> -
>  		/*
>  		 * The values of the Maximum Link Width and Supported Link
>  		 * Speed from the Link Capabilities Register will be lost
> @@ -93,7 +89,8 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
>  		 * that configured by the Reset Configuration Word (RCW).
>  		 */
>  		dw_pcie_dbi_ro_wr_en(pci);
> -		dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, pcie->lnkcap);
> +		dw_pcie_writel_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP,
> +				   pcie->lnkcap);
>  		dw_pcie_dbi_ro_wr_dis(pci);

[Severity: High]
This isn't a bug introduced by this patch, but does using
dw_pcie_dbi_ro_wr_en() and dw_pcie_dbi_ro_wr_dis() in hardirq context
here create a race condition with process-context operations?

Since dw_pcie_dbi_ro_wr_dis() unconditionally clears PCIE_DBI_RO_WR_EN:

static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
{
	...
	val &= ~PCIE_DBI_RO_WR_EN;
	dw_pcie_writel_dbi(pci, reg, val);
}

If this hardirq interrupts a process-context operation (like
dw_pcie_ep_set_bar_programmable()) right after it enabled RO_WR_EN but
before it writes to a DBI register, will this unconditionally clear the
flag, causing the interrupted thread's subsequent writes to be silently
ignored by the hardware?

>  
>  		cfg = ls_pcie_pf_lut_readl(pcie, PEX_PF0_CONFIG);

[Severity: High]
This is a pre-existing issue, but does the code in this same hardirq
handler sleep in atomic context?

Shortly after this point in ls_pcie_ep_event_handler(), the code calls:

	dw_pcie_ep_linkup(&pci->ep);

Which invokes pci_epc_linkup() in the EPC core, doing:

void pci_epc_linkup(struct pci_epc *epc)
{
	mutex_lock(&epc->list_lock);
	...
}

Will attempting to acquire this sleepable mutex from hardirq context
trigger a BUG for scheduling while atomic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=4

[PATCH v2 05/16] PCI: dwc: meson: Use cached PCIe capability offset

[Hans Zhang]

dw_pcie_host_init() calls pp->ops->init (meson_pcie_host_init) before
caching the offset. Therefore, inside .init we must call
dw_pcie_get_pcie_cap() to obtain the offset (the helper will perform the
DBI read and cache the result). This is safe because the hardware is
already enabled by the driver's own initialization.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pci-meson, the call chain is:

  static const struct dw_pcie_host_ops meson_pcie_host_ops = {
    .init = meson_pcie_host_init,
  };
  meson_pcie_host_init()
    -> meson_set_max_payload()
      -> dw_pcie_find_capability()
    -> meson_set_max_rd_req_size()
      -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pci-meson.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 0694084f612b..81767d010c84 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -276,7 +276,7 @@ static void meson_set_max_payload(struct meson_pcie *mp, int size)
 {
 	struct dw_pcie *pci = &mp->pci;
 	u32 val;
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 offset = dw_pcie_get_pcie_cap(pci);
 	int max_payload_size = meson_size_to_payload(mp, size);
 
 	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
@@ -292,7 +292,7 @@ static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
 {
 	struct dw_pcie *pci = &mp->pci;
 	u32 val;
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 offset = dw_pcie_get_pcie_cap(pci);
 	int max_rd_req_size = meson_size_to_payload(mp, size);
 
 	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
-- 
2.34.1

[PATCH v2 06/16] PCI: dwc: rockchip: Use cached PCIe capability offset

[Hans Zhang]

dw_pcie_host_init() calls .init before caching the offset. So inside
.init we call dw_pcie_get_pcie_cap() to trigger caching. The helper will
perform the DBI read (hardware is already enabled) and cache the result.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-dw-rockchip, the call chain is:

  static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
    .init = rockchip_pcie_host_init,
  };
  rockchip_pcie_host_init()
    -> rockchip_pcie_enable_l0s()
      -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 731d93663cca..be8b6187913d 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -369,7 +369,7 @@ static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
 	u32 cap, lnkcap;
 
 	/* Enable L0S capability for all SoCs */
-	cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	cap = dw_pcie_get_pcie_cap(pci);
 	if (cap) {
 		lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
 		lnkcap |= PCI_EXP_LNKCAP_ASPM_L0S;
-- 
2.34.1

Re: [PATCH v2 06/16] PCI: dwc: rockchip: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The IRQ domain and chained IRQ handler created in RC mode initialization are never cleaned up if host initialization fails.
- [High] In EP mode, the devres-managed 'sys' interrupt can fire while clocks are disabled during the probe error path.
--

commit 2570eaad60265910abfe106597d119f992b11a1b
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: rockchip: Use cached PCIe capability offset

This commit modifies the initialization sequence to trigger caching of the
PCIe capability offset. It replaces a direct capability search with the
cached helper function dw_pcie_get_pcie_cap().

> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 731d93663ccae..be8b6187913df 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c

[ ... ]

> @@ -369,7 +369,7 @@ static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
>  	u32 cap, lnkcap;
>  
>  	/* Enable L0S capability for all SoCs */
> -	cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	cap = dw_pcie_get_pcie_cap(pci);
>  	if (cap) {
>  		lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
>  		lnkcap |= PCI_EXP_LNKCAP_ASPM_L0S;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the RC mode
initialization in rockchip_pcie_host_init():

    irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler, rockchip);

The IRQ domain and chained IRQ handler are created here, but there does not
appear to be a corresponding teardown path if host initialization fails later.
If a subsequent step in dw_pcie_host_init() fails, it relies on a .deinit
callback to clean up.

Since rockchip_pcie_host_ops lacks a .deinit callback, does this leak the IRQ
domain and chained handler? When the probe function returns the error and
disables the APB clocks, could a subsequent interrupt on the legacy line
invoke rockchip_pcie_intx_handler() while clocks are disabled, causing a
synchronous external abort?

[Severity: High]
This is a pre-existing issue, but I noticed a potential race on the error path
in EP mode in rockchip_pcie_configure_ep():

    ret = devm_request_threaded_irq(dev, irq, NULL, rockchip_pcie_ep_sys_irq_thread, IRQF_ONESHOT, "pcie-sys-ep", rockchip);

We request the 'sys' interrupt using a devres-managed function. If a later
initialization step like dw_pcie_ep_init_registers() fails, the probe
function jumps to the deinit_clk label and manually disables APB clocks using
clk_bulk_disable_unprepare().

Because devres resources are only freed after the probe function completely
returns, the IRQ remains active while the clocks are turned off. If the
interrupt fires or is pending during this window, will the IRQ thread attempt
to read the PCIE_CLIENT_INTR_STATUS_MISC register without clock access and
trigger a kernel crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=6

Re: [PATCH v2 06/16] PCI: dwc: rockchip: Use cached PCIe capability offset

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]

Hi,

On Sat, May 30, 2026 at 11:30:51PM +0800, Hans Zhang wrote:
> dw_pcie_host_init() calls .init before caching the offset. So inside
> .init we call dw_pcie_get_pcie_cap() to trigger caching. The helper will
> perform the DBI read (hardware is already enabled) and cache the result.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Greetings,

-- Sebastian

> In pcie-dw-rockchip, the call chain is:
> 
>   static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
>     .init = rockchip_pcie_host_init,
>   };
>   rockchip_pcie_host_init()
>     -> rockchip_pcie_enable_l0s()
>       -> dw_pcie_find_capability()
> ---
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 731d93663cca..be8b6187913d 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -369,7 +369,7 @@ static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
>  	u32 cap, lnkcap;
>  
>  	/* Enable L0S capability for all SoCs */
> -	cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	cap = dw_pcie_get_pcie_cap(pci);
>  	if (cap) {
>  		lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
>  		lnkcap |= PCI_EXP_LNKCAP_ASPM_L0S;
> -- 
> 2.34.1
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 07/16] PCI: dwc: eswin: Use cached PCIe capability offset

[Hans Zhang]

This function is called after dw_pcie_host_init() has already cached the
offset (dw_pcie_link_up() is called after .init and after caching).
Therefore, we can directly use pci->pcie_cap.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-eswin, the call chain is:

  static const struct dw_pcie_ops dw_pcie_ops = {
    .link_up = eswin_pcie_link_up,
  };
  eswin_pcie_link_up()
    -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-eswin.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
index ce8d64f8a395..6a37fd5a8384 100644
--- a/drivers/pci/controller/dwc/pcie-eswin.c
+++ b/drivers/pci/controller/dwc/pcie-eswin.c
@@ -84,8 +84,7 @@ static int eswin_pcie_start_link(struct dw_pcie *pci)
 
 static bool eswin_pcie_link_up(struct dw_pcie *pci)
 {
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	u16 val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
+	u16 val = dw_pcie_readw_dbi(pci, pci->pcie_cap + PCI_EXP_LNKSTA);
 
 	return val & PCI_EXP_LNKSTA_DLLLA;
 }
-- 
2.34.1

[PATCH v2 08/16] PCI: dwc: fu740: Use cached PCIe capability offset

[Hans Zhang]

This callback is invoked after dw_pcie_host_init() has cached the offset,
so we can use pci->pcie_cap directly.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-fu740, the call chain is:

  static const struct dw_pcie_ops dw_pcie_ops = {
    .start_link = fu740_pcie_start_link,
  };
  fu740_pcie_start_link()
    -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
index d0a34f680397..72c0a6316fff 100644
--- a/drivers/pci/controller/dwc/pcie-fu740.c
+++ b/drivers/pci/controller/dwc/pcie-fu740.c
@@ -178,7 +178,7 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
 {
 	struct device *dev = pci->dev;
 	struct fu740_pcie *afp = dev_get_drvdata(dev);
-	u8 cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 cap_exp = pci->pcie_cap;
 	int ret;
 	u32 orig, tmp;
 
-- 
2.34.1

[PATCH v2 09/16] PCI: dwc: intel-gw: Use cached PCIe capability offset

[Hans Zhang]

dw_pcie_host_init() calls .init before caching the offset, so inside
.init we must call dw_pcie_get_pcie_cap() to obtain the offset (hardware
is already enabled). The helper will cache the result for later use.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-intel-gw, the call chain is:

  static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
    .init = intel_pcie_rc_init,
  };
  intel_pcie_rc_init()
    -> intel_pcie_host_setup()
      -> intel_pcie_link_setup()
        -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-intel-gw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 2674cd376f49..568b2634d08d 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -120,7 +120,7 @@ static void intel_pcie_ltssm_disable(struct intel_pcie *pcie)
 static void intel_pcie_link_setup(struct intel_pcie *pcie)
 {
 	u32 val;
-	u8 offset = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP);
+	u8 offset = dw_pcie_get_pcie_cap(&pcie->pci);
 
 	val = pcie_rc_cfg_rd(pcie, offset + PCI_EXP_LNKCTL);
 
-- 
2.34.1

Re: [PATCH v2 09/16] PCI: dwc: intel-gw: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The `intel_pcie_resume_noirq` function resets the DWC PCIe core but fails to restore the Root Complex configuration (ATU, BARs, MSI).
- [High] Reading and writing back 32-bit values to 16-bit configuration registers (`PCI_EXP_LNKCTL` and `PCI_COMMAND`) unintentionally clears Write-1-to-Clear (W1C) bits in their adjacent registers (`PCI_EXP_LNKSTA` and `PCI_STATUS`).
--

commit a2b3beed30ec9a660c2061cc97db74ca27bb57d7
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: intel-gw: Use cached PCIe capability offset

dw_pcie_host_init() calls .init before caching the offset, so inside
.init we must call dw_pcie_get_pcie_cap() to obtain the offset (hardware
is already enabled). The helper will cache the result for later use.

> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 2674cd376f495..568b2634d08da 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -120,7 +120,7 @@ static void intel_pcie_ltssm_disable(struct intel_pcie *pcie)
>  static void intel_pcie_link_setup(struct intel_pcie *pcie)
>  {
>  	u32 val;
> -	u8 offset = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP);
> +	u8 offset = dw_pcie_get_pcie_cap(&pcie->pci);
>  
>  	val = pcie_rc_cfg_rd(pcie, offset + PCI_EXP_LNKCTL);

[Severity: High]
This is a pre-existing issue, but does this 32-bit read-modify-write clear
Write-1-to-Clear (W1C) bits in the adjacent PCI_EXP_LNKSTA register?

Modifying PCI_EXP_LNKCTL via a 32-bit read and write back will include
the upper 16 bits containing PCI_EXP_LNKSTA. Writing the read value back
to these W1C bits could clear any set error or status flags, such as the
Link Bandwidth Management Status or Detected Parity Error.

Would the same issue occur in pcie_update_bits() when it modifies
PCI_COMMAND, potentially clearing W1C bits in PCI_STATUS?

>  
>  	val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
>  	pcie_rc_cfg_wr(pcie, offset + PCI_EXP_LNKCTL, val);
>  }

There is also another pre-existing issue in this file regarding the resume
flow. When intel_pcie_resume_noirq() runs, it calls intel_pcie_host_setup():

drivers/pci/controller/dwc/pcie-intel-gw.c:intel_pcie_host_setup() {
    ...
    intel_pcie_core_rst_assert(pcie);
    intel_pcie_device_rst_assert(pcie);
    intel_pcie_core_rst_deassert(pcie);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does the core reset in
intel_pcie_host_setup() wipe the internal DWC controller state, including
ATU mappings?

By overriding the generic resume flow and bypassing dw_pcie_setup_rc(), the
driver appears to leave the ATU windows unprogrammed. Will outbound memory
accesses from the CPU to PCIe devices and inbound DMA/MSI transactions fail
upon resume without the ATU being restored?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=9

[PATCH v2 10/16] PCI: dwc: qcom-ep: Use cached PCIe capability offset

[Hans Zhang]

dw_pcie_ep_init() calls ep->ops->pre_init() (enables hardware) and then
dw_pcie_get_pcie_cap() to cache the offset. Therefore, the IRQ handlers
run after the cache is populated, so they can safely use pci->pcie_cap
directly.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-qcom-ep, dw_pcie_find_capability() appears in:

  qcom_pcie_ep_probe()
    -> dw_pcie_ep_init()
    -> qcom_pcie_ep_enable_irq_resources()
      -> qcom_pcie_ep_global_irq_thread()
	-> qcom_pcie_ep_icc_update()
	  -> dw_pcie_find_capability()
	-> qcom_pcie_ep_perst_irq_thread()
	  -> qcom_pcie_perst_deassert()
	    -> offset = dw_pcie_find_capability()  (called twice)
---
 drivers/pci/controller/dwc/pcie-qcom-ep.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 56184e6ca6e6..0471f96d3c78 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -307,15 +307,13 @@ static void qcom_pcie_dw_write_dbi2(struct dw_pcie *pci, void __iomem *base,
 static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep)
 {
 	struct dw_pcie *pci = &pcie_ep->pci;
-	u32 offset, status;
-	int speed, width;
-	int ret;
+	int speed, width, ret;
+	u32 status;
 
 	if (!pcie_ep->icc_mem)
 		return;
 
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+	status = readw(pci->dbi_base + pci->pcie_cap + PCI_EXP_LNKSTA);
 
 	speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
 	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
@@ -492,13 +490,12 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
 	dw_pcie_dbi_ro_wr_en(pci);
 
 	/* Set the L0s Exit Latency to 2us-4us = 0x6 */
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	offset = pci->pcie_cap;
 	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 	FIELD_MODIFY(PCI_EXP_LNKCAP_L0SEL, &val, 0x6);
 	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, val);
 
 	/* Set the L1 Exit Latency to be 32us-64 us = 0x6 */
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 	FIELD_MODIFY(PCI_EXP_LNKCAP_L1EL, &val, 0x6);
 	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, val);
-- 
2.34.1

Re: [PATCH v2 10/16] PCI: dwc: qcom-ep: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch replaces `dw_pcie_find_capability()` with a cached `pci->pcie_cap`, but hardware clocks are not enabled during `dw_pcie_ep_init()` on `qcom-ep`, causing an unclocked MMIO access or caching an invalid 0 offset that later corrupts standard PCI configuration space.

Pre-existing issues:
- [High] Concurrent execution of global and PERST threaded IRQs causes fatal unclocked MMIO accesses and data races.
--

commit 4d4786c3c255dd8f971ef41e85f15f1fa9ead9d4
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: qcom-ep: Use cached PCIe capability offset

dw_pcie_ep_init() calls ep->ops->pre_init() (enables hardware) and then
dw_pcie_get_pcie_cap() to cache the offset. Therefore, the IRQ handlers
run after the cache is populated, so they can safely use pci->pcie_cap
directly.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 56184e6ca6e69..0471f96d3c781 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -307,15 +307,13 @@ static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep)
>  {
>  	struct dw_pcie *pci = &pcie_ep->pci;
> -	u32 offset, status;
> -	int speed, width;
> -	int ret;
> +	int speed, width, ret;
> +	u32 status;
>  
>  	if (!pcie_ep->icc_mem)
>  		return;
>  
> -	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> -	status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
> +	status = readw(pci->dbi_base + pci->pcie_cap + PCI_EXP_LNKSTA);

[Severity: High]
Does this caching work safely on qcom-ep? The commit message states that
dw_pcie_ep_init() calls pre_init() to enable hardware clocks. However, for
qcom-ep, clocks are only enabled later via qcom_pcie_enable_resources() in
the PERST deassert IRQ path.

This means clocks are disabled during dw_pcie_ep_init(), which will either
cause a synchronous external abort when reading the capability, or result in
pci->pcie_cap being populated with a cached offset of 0.

If pci->pcie_cap is 0, won't this readw() fetch from pci->dbi_base + 0x12,
which is part of BAR0, instead of the link status register? This could fetch
incorrect values and break interconnect bandwidth scaling.

[ ... ]

> @@ -492,13 +490,12 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
>  	dw_pcie_dbi_ro_wr_en(pci);
>  
>  	/* Set the L0s Exit Latency to 2us-4us = 0x6 */
> -	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	offset = pci->pcie_cap;
>  	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
>  	FIELD_MODIFY(PCI_EXP_LNKCAP_L0SEL, &val, 0x6);
>  	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, val);

[Severity: High]
Similarly, if pci->pcie_cap is cached as 0 because clocks were disabled
during initialization, won't this overwrite the base PCI configuration header?

Using an offset of 0 means offset + PCI_EXP_LNKCAP evaluates to 0x0c, which
would erroneously modify the cache line size, latency timer, header type,
and BIST registers instead of the link capabilities register.

[Severity: High]
This is a pre-existing issue, but is there a race condition between the
PERST IRQ thread and the global IRQ thread that can cause a panic?

The qcom_pcie_ep_perst_irq_thread() and qcom_pcie_ep_global_irq_thread()
execute concurrently without mutual exclusion. If the host asserts PERST
while the global IRQ thread is processing an event, the PERST thread will
disable clocks:

qcom_pcie_ep_perst_irq_thread()
    qcom_pcie_perst_assert()
        qcom_pcie_disable_resources()

If the global IRQ thread is simultaneously performing MMIO access:

qcom_pcie_ep_global_irq_thread()
    status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);

Could this MMIO access while clocks are asynchronously disabled cause an
immediate synchronous external abort?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=10

[PATCH v2 11/16] PCI: dwc: qcom: Use cached PCIe capability offset

[Hans Zhang]

dw_pcie_host_init() caches the offset after .init, so .post_init callbacks
and later functions (.link_up, icc_opp_update) can use pci->pcie_cap
directly. For .init itself, we must call dw_pcie_get_pcie_cap() inside
qcom_pcie_host_init() to obtain the offset (hardware is already enabled).

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-qcom, dw_pcie_find_capability() appears in multiple call chains:

  static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
    .init = qcom_pcie_host_init,
  };
  qcom_pcie_host_init()
    -> qcom_pcie_clear_aspm_l0s()
      -> dw_pcie_find_capability()

  static const struct qcom_pcie_ops ops_* = {
    .post_init = qcom_pcie_post_init_*,
  };
  qcom_pcie_post_init_*()
    -> qcom_pcie_set_slot_nccs()  (for many versions)
      -> dw_pcie_find_capability()
    -> For 2_3_3 and 2_9_0: also calls dw_pcie_find_capability() directly

  static const struct dw_pcie_ops dw_pcie_ops = {
    .link_up = qcom_pcie_link_up,
  };
  qcom_pcie_link_up()
    -> dw_pcie_find_capability()

  qcom_pcie_probe()
    -> dw_pcie_host_init()
    -> qcom_pcie_icc_opp_update()
      -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-qcom.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 11fc60489892..80783353d539 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -341,13 +341,13 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
 static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
 {
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
-	u16 offset;
+	u8 offset;
 	u32 val;
 
 	if (!pcie->cfg->no_l0s)
 		return;
 
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	offset = dw_pcie_get_pcie_cap(pci);
 
 	dw_pcie_dbi_ro_wr_en(pci);
 
@@ -360,7 +360,6 @@ static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
 
 static void qcom_pcie_set_slot_nccs(struct dw_pcie *pci)
 {
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	u32 val;
 
 	dw_pcie_dbi_ro_wr_en(pci);
@@ -370,9 +369,9 @@ static void qcom_pcie_set_slot_nccs(struct dw_pcie *pci)
 	 * notifications for the Hot-Plug commands. So set the NCCS field to
 	 * avoid waiting for the completions.
 	 */
-	val = readl(pci->dbi_base + offset + PCI_EXP_SLTCAP);
+	val = readl(pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);
 	val |= PCI_EXP_SLTCAP_NCCS;
-	writel(val, pci->dbi_base + offset + PCI_EXP_SLTCAP);
+	writel(val, pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);
 
 	dw_pcie_dbi_ro_wr_dis(pci);
 }
@@ -935,7 +934,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
 static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie)
 {
 	struct dw_pcie *pci = pcie->pci;
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 offset = pci->pcie_cap;
 	u32 val;
 
 	/* Force PHY out of lowest power state */
@@ -1257,7 +1256,7 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
 static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
 {
 	struct dw_pcie *pci = pcie->pci;
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 offset = pci->pcie_cap;
 	u32 val;
 	int i;
 
@@ -1303,8 +1302,7 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
 
 static bool qcom_pcie_link_up(struct dw_pcie *pci)
 {
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	u16 val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+	u16 val = readw(pci->dbi_base + pci->pcie_cap + PCI_EXP_LNKSTA);
 
 	return val & PCI_EXP_LNKSTA_DLLLA;
 }
@@ -1663,15 +1661,14 @@ static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
 
 static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
 {
-	u32 offset, status, width, speed;
+	u32 status, width, speed;
 	struct dw_pcie *pci = pcie->pci;
 	struct dev_pm_opp_key key = {};
 	unsigned long freq_kbps;
 	struct dev_pm_opp *opp;
 	int ret, freq_mbps;
 
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+	status = readw(pci->dbi_base + pci->pcie_cap + PCI_EXP_LNKSTA);
 
 	/* Only update constraints if link is up. */
 	if (!(status & PCI_EXP_LNKSTA_DLLLA))
-- 
2.34.1

Re: [PATCH v2 11/16] PCI: dwc: qcom: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized cached PCIe capability offset causes memory corruption in PCI configuration space.
--

commit 1d7762e21cba69c340ccc430485c442e06488028
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: qcom: Use cached PCIe capability offset

This patch replaces calls to dw_pcie_find_capability() with the cached
pcie_cap offset in the designware PCIe driver for Qualcomm. It aims to
optimize access to PCIe capabilities by reusing the offset.

>     For .init itself, we must call dw_pcie_get_pcie_cap() inside
>     qcom_pcie_host_init() to obtain the offset (hardware is already enabled).

[Severity: High]
Does the patch actually make this call? It appears this was left out of the
code changes.

Without this call, pci->pcie_cap remains uninitialized (0) when the
.post_init callbacks run.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 11fc604898926..80783353d5398 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c

[ ... ]

> @@ -370,9 +369,9 @@ static void qcom_pcie_set_slot_nccs(struct dw_pcie *pci)
>  	 * notifications for the Hot-Plug commands. So set the NCCS field to
>  	 * avoid waiting for the completions.
>  	 */
> -	val = readl(pci->dbi_base + offset + PCI_EXP_SLTCAP);
> +	val = readl(pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);
>  	val |= PCI_EXP_SLTCAP_NCCS;
> -	writel(val, pci->dbi_base + offset + PCI_EXP_SLTCAP);
> +	writel(val, pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);

[Severity: High]
Will pci->pcie_cap be 0 here?

Since dw_pcie_get_pcie_cap() is not called prior to this, pci->pcie_cap
evaluates to 0. This causes the code to write to offset 0 + PCI_EXP_SLTCAP
(0x14, which is PCI_BASE_ADDRESS_1) instead of the actual capability,
corrupting the Root Port hardware configuration.

[ ... ]

> @@ -935,7 +934,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
>  static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie)
>  {
>  	struct dw_pcie *pci = pcie->pci;
> -	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u8 offset = pci->pcie_cap;

[Severity: High]
Does this also suffer from the uninitialized pci->pcie_cap?

Because offset becomes 0, subsequent writes using this offset (like writing
PCIE_CAP_SLOT_VAL to PCI_EXP_SLTCAP) will also corrupt BAR1 instead of updating
the PCIe capability register.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=11

[PATCH v2 12/16] PCI: dwc: sophgo: Use cached PCIe capability offset

[Hans Zhang]

dw_pcie_host_init() calls .init before caching, so we must call
dw_pcie_get_pcie_cap() inside .init. The hardware is already enabled by
the driver's own initialization before this point. The helper will cache
the offset and avoid redundant searches.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-sophgo, the call chain is:

  static const struct dw_pcie_host_ops sophgo_pcie_host_ops = {
    .init = sophgo_pcie_host_init,
  };
  sophgo_pcie_host_init()
    -> sophgo_pcie_disable_l0s_l1()
      -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-sophgo.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-sophgo.c b/drivers/pci/controller/dwc/pcie-sophgo.c
index 044088898819..39703d2b7b5f 100644
--- a/drivers/pci/controller/dwc/pcie-sophgo.c
+++ b/drivers/pci/controller/dwc/pcie-sophgo.c
@@ -164,9 +164,10 @@ static void sophgo_pcie_msi_enable(struct dw_pcie_rp *pp)
 static void sophgo_pcie_disable_l0s_l1(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-	u32 offset, val;
+	u8 offset;
+	u32 val;
 
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	offset = dw_pcie_get_pcie_cap(pci);
 
 	dw_pcie_dbi_ro_wr_en(pci);
 
-- 
2.34.1

Re: [PATCH v2 12/16] PCI: dwc: sophgo: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `.deinit` callback causes IRQ domain leak and leaves a dangling chained IRQ handler if `dw_pcie_host_init()` fails.
--

commit bec8b7e2c9e17ca16137f5895fed067563f850fa
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: sophgo: Use cached PCIe capability offset

dw_pcie_host_init() calls .init before caching, so we must call
dw_pcie_get_pcie_cap() inside .init. The hardware is already enabled by
the driver's own initialization before this point. The helper will cache
the offset and avoid redundant searches.

> diff --git a/drivers/pci/controller/dwc/pcie-sophgo.c b/drivers/pci/controller/dwc/pcie-sophgo.c
> index 044088898819e..39703d2b7b5f2 100644
> --- a/drivers/pci/controller/dwc/pcie-sophgo.c
> +++ b/drivers/pci/controller/dwc/pcie-sophgo.c
> @@ -164,9 +164,10 @@ static void sophgo_pcie_msi_enable(struct dw_pcie_rp *pp)
>  static void sophgo_pcie_disable_l0s_l1(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -	u32 offset, val;
> +	u8 offset;
> +	u32 val;
>  
> -	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	offset = dw_pcie_get_pcie_cap(pci);
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
>  

[Severity: High]
This is a pre-existing issue, but while reviewing this init sequence,
I noticed a potential resource leak and dangling pointer on the error path.

If dw_pcie_host_init() fails (e.g. due to a link training timeout), it invokes
the .deinit callback to tear down host-specific resources. However, the driver
is missing a .deinit callback:

drivers/pci/controller/dwc/pcie-sophgo.c:
static const struct dw_pcie_host_ops sophgo_pcie_host_ops = {
	.init = sophgo_pcie_host_init,
};

In sophgo_pcie_host_init(), an IRQ domain and a chained handler are set up:

drivers/pci/controller/dwc/pcie-sophgo.c:sophgo_pcie_host_init() {
    ...
	irq = sophgo_pcie_init_irq_domain(pp);
	if (irq < 0)
		return irq;

	irq_set_chained_handler_and_data(irq, sophgo_pcie_intx_handler, pp);
    ...
}

Because there is no .deinit callback, the IRQ domain and chained handler are
never removed when dw_pcie_host_init() fails.

When sophgo_pcie_probe() returns an error, devres will free the struct
sophgo_pcie (which contains pp).

If the parent interrupt fires later, could the dangling chained handler
(sophgo_pcie_intx_handler) execute and dereference the freed pp pointer,
leading to a use-after-free panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=12

[PATCH v2 13/16] PCI: dwc: spacemit-k1: Use cached PCIe capability offset

[Hans Zhang]

Because .init runs before core caching, we call dw_pcie_get_pcie_cap()
inside k1_pcie_disable_aspm_l1() to get the capability base, then add
PCI_EXP_LNKCAP. Hardware is already enabled at this point.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-spacemit-k1, the call chain is:

  static const struct dw_pcie_host_ops k1_pcie_host_ops = {
    .init = k1_pcie_init,
  };
  k1_pcie_init()
    -> k1_pcie_disable_aspm_l1()
      -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-spacemit-k1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
index be20a520255b..65114060311e 100644
--- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
+++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
@@ -117,7 +117,7 @@ static void k1_pcie_disable_aspm_l1(struct k1_pcie *k1)
 	u8 offset;
 	u32 val;
 
-	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	offset = dw_pcie_get_pcie_cap(pci);
 	offset += PCI_EXP_LNKCAP;
 
 	dw_pcie_dbi_ro_wr_en(pci);
-- 
2.34.1

[PATCH v2 14/16] PCI: dwc: spear13xx: Use cached PCIe capability offset

[Hans Zhang]

Inside .init we must call dw_pcie_get_pcie_cap() to obtain the offset,
because the core has not yet cached it. The hardware is already enabled
by the driver's own initialization before this point.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-spear13xx, the call chain is:

  static const struct dw_pcie_host_ops spear13xx_pcie_host_ops = {
    .init = spear13xx_pcie_host_init,
  };
  spear13xx_pcie_host_init()
    -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-spear13xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-spear13xx.c b/drivers/pci/controller/dwc/pcie-spear13xx.c
index 01794a9d3ad2..6e4c11b497d4 100644
--- a/drivers/pci/controller/dwc/pcie-spear13xx.c
+++ b/drivers/pci/controller/dwc/pcie-spear13xx.c
@@ -122,7 +122,7 @@ static int spear13xx_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pci);
-	u32 exp_cap_off = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u8 exp_cap_off = dw_pcie_get_pcie_cap(pci);
 	u32 val;
 
 	spear13xx_pcie->app_base = pci->dbi_base + 0x2000;
-- 
2.34.1

Re: [PATCH v2 14/16] PCI: dwc: spear13xx: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Uninitialized variable usage leading to PCI register corruption.

Pre-existing issues:
- [High] Race condition in shared IRQ registration causing NULL pointer dereference.
--

commit f26cde06f20af920dcaa766788767c35c06c971e
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: spear13xx: Use cached PCIe capability offset

Inside .init we must call dw_pcie_get_pcie_cap() to obtain the offset,
because the core has not yet cached it. The hardware is already enabled
by the driver's own initialization before this point.

> diff --git a/drivers/pci/controller/dwc/pcie-spear13xx.c b/drivers/pci/controller/dwc/pcie-spear13xx.c
> index 01794a9d3ad29..6e4c11b497d45 100644
> --- a/drivers/pci/controller/dwc/pcie-spear13xx.c
> +++ b/drivers/pci/controller/dwc/pcie-spear13xx.c
> @@ -122,7 +122,7 @@ static int spear13xx_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pci);
> -	u32 exp_cap_off = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u8 exp_cap_off = dw_pcie_get_pcie_cap(pci);
>  	u32 val;
>  
>  	spear13xx_pcie->app_base = pci->dbi_base + 0x2000;

[Severity: High]
This is a pre-existing issue, but is it possible for a shared interrupt
to fire before app_base is initialized here?

In spear13xx_add_pcie_port(), devm_request_irq() requests the shared IRQ
before dw_pcie_host_init() is called. If the shared IRQ fires early,
spear13xx_pcie_irq_handler() will execute and unconditionally read from
the uninitialized app_base:

drivers/pci/controller/dwc/pcie-spear13xx.c:spear13xx_pcie_irq_handler() {
    ...
    struct pcie_app_reg __iomem *app_reg = spear13xx_pcie->app_base;
    ...
    status = readl(&app_reg->int_sts);
    ...
}

Because app_base is still NULL, would this result in a read from physical
address 0x18 and lead to a kernel crash?


Additionally, an issue was found in another commit in this series:

commit 1d7762e21cba69c340ccc430485c442e06488028
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: qcom: Use cached PCIe capability offset

dw_pcie_host_init() caches the offset after .init, so .post_init callbacks
and later functions (.link_up, icc_opp_update) can use pci->pcie_cap
directly. For .init itself, we must call dw_pcie_get_pcie_cap() inside
qcom_pcie_host_init() to obtain the offset (hardware is already enabled).

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 11fc604898926..80783353d5398 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[ ... ]
> @@ -360,7 +360,6 @@ static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
>  
>  static void qcom_pcie_set_slot_nccs(struct dw_pcie *pci)
>  {
> -	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>  	u32 val;
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
> @@ -370,9 +369,9 @@ static void qcom_pcie_set_slot_nccs(struct dw_pcie *pci)
>  	 * notifications for the Hot-Plug commands. So set the NCCS field to
>  	 * avoid waiting for the completions.
>  	 */
> -	val = readl(pci->dbi_base + offset + PCI_EXP_SLTCAP);
> +	val = readl(pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);
>  	val |= PCI_EXP_SLTCAP_NCCS;
> -	writel(val, pci->dbi_base + offset + PCI_EXP_SLTCAP);
> +	writel(val, pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);

[Severity: High]
Since qcom_pcie_set_slot_nccs() is called during qcom_pcie_host_init()
via the pcie->cfg->ops->post_init(pcie) callback, is pci->pcie_cap
guaranteed to be initialized here?

The DWC core in dw_pcie_host_init() appears to cache pci->pcie_cap only
after the pp->ops->init callback returns. If pci->pcie_cap is still 0
when this executes, won't this write to offset 0x14 (PCI_BASE_ADDRESS_1)
and corrupt the configuration space instead of configuring the intended
slot capability?

[ ... ]
> @@ -935,7 +934,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
>  static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie)
>  {
>  	struct dw_pcie *pci = pcie->pci;
> -	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u8 offset = pci->pcie_cap;
>  	u32 val;
>  
>  	/* Force PHY out of lowest power state */

[Severity: High]
Similarly, will pci->pcie_cap be 0 when qcom_pcie_post_init_2_3_3() is
called from the post_init callback during qcom_pcie_host_init()?

If so, this would result in a corrupted offset here as well.

[ ... ]
> @@ -1257,7 +1256,7 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
>  static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>  {
>  	struct dw_pcie *pci = pcie->pci;
> -	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u8 offset = pci->pcie_cap;
>  	u32 val;
>  	int i;

[Severity: High]
Does this suffer from the same uninitialized pci->pcie_cap issue as
qcom_pcie_post_init_2_3_3() when called from the post_init callback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=14

[PATCH v2 15/16] PCI: dwc: tegra194: Use cached PCIe capability offset

[Hans Zhang]

In host mode: tegra_pcie_dw_host_init() runs inside .start_link, which is
called after dw_pcie_host_init() has cached the offset. However, the
driver's own initialization already enables hardware, so calling
dw_pcie_get_pcie_cap() is safe and will return the cached value.

In endpoint mode: pex_ep_event_pex_rst_deassert() runs before
dw_pcie_ep_init(), but the driver has already enabled hardware.
dw_pcie_get_pcie_cap() will perform the DBI read and cache the offset.

Thus, the private pcie_cap_base is always valid and can still be used for
register accesses.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-tegra194, dw_pcie_find_capability() appears in:

  static const struct dw_pcie_ops tegra_dw_pcie_ops = {
    .start_link = tegra_pcie_dw_start_link,
  };
  tegra_pcie_dw_start_link()
    -> tegra_pcie_dw_host_init()
      -> dw_pcie_find_capability()

  tegra_pcie_dw_probe()
    case DW_PCIE_EP_TYPE:
      tegra_pcie_config_ep()
        -> pex_ep_event_pex_rst_deassert()
	  -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 795cef5a915d..bf482bc66a92 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -905,8 +905,7 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp)
 	pp->bridge->ops = &tegra_pci_ops;
 
 	if (!pcie->pcie_cap_base)
-		pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
-							      PCI_CAP_ID_EXP);
+		pcie->pcie_cap_base = dw_pcie_get_pcie_cap(pci);
 
 	val = dw_pcie_readl_dbi(pci, PCI_IO_BASE);
 	val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8);
@@ -1889,8 +1888,7 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
 		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
 	}
 
-	pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
-						      PCI_CAP_ID_EXP);
+	pcie->pcie_cap_base = dw_pcie_get_pcie_cap(pci);
 
 	/* Clear Slot Clock Configuration bit if SRNS configuration */
 	if (pcie->enable_srns) {
-- 
2.34.1

Re: [PATCH v2 15/16] PCI: dwc: tegra194: Use cached PCIe capability offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling `dw_pcie_get_pcie_cap()` during `dw_pcie_ep_init()` causes a synchronous external abort in Tegra194 Endpoint mode because the hardware is unclocked.
--

commit 240e0a20036fb631179ee7e94b3f17f4d5523ad8
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: tegra194: Use cached PCIe capability offset

This patch updates the Tegra194 PCIe host and endpoint initialization paths to
use dw_pcie_get_pcie_cap() to retrieve and cache the capability offset. The
commit message explains that this is safe because the driver has already
enabled the hardware before these functions are called.

> In endpoint mode: pex_ep_event_pex_rst_deassert() runs before
> dw_pcie_ep_init(), but the driver has already enabled hardware.
> dw_pcie_get_pcie_cap() will perform the DBI read and cache the offset.

[Severity: High]
Is this sequence accurate for the endpoint mode?

Looking at tegra_pcie_config_ep() in pcie-tegra194.c, dw_pcie_ep_init() is
invoked synchronously during device probe:

tegra_pcie_config_ep()
    dw_pcie_ep_init()

This happens before the endpoint hardware clocks and power domains are
enabled, because clock enablement is deferred until the PERST# interrupt
triggers pex_ep_event_pex_rst_deassert().

Since a prior commit in this series modified dw_pcie_ep_init() in
pcie-designware-ep.c to unconditionally cache the PCIe capability offset by
calling dw_pcie_get_pcie_cap(), this triggers an early DBI read.

Will this MMIO read on the unclocked PCIe controller trigger a synchronous
external abort on Tegra platforms and cause a kernel panic during boot?

> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 795cef5a915dc..bf482bc66a929 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
[ ... ]
> @@ -1889,8 +1888,7 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
>  		dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
>  	}
>  
> -	pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
> -						      PCI_CAP_ID_EXP);
> +	pcie->pcie_cap_base = dw_pcie_get_pcie_cap(pci);
>  
>  	/* Clear Slot Clock Configuration bit if SRNS configuration */
>  	if (pcie->enable_srns) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=15

[PATCH v2 16/16] PCI: dwc: ultrarisc: Use cached PCIe capability offset

[Hans Zhang]

Inside .init we call dw_pcie_get_pcie_cap() to obtain the offset, because
the core has not yet cached it. Hardware is already enabled by the driver
before this point.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
In pcie-ultrarisc, the call chain is:

  static const struct dw_pcie_host_ops ultrarisc_pcie_host_ops = {
    .init = ultrarisc_pcie_host_init,
  };
  ultrarisc_pcie_host_init()
    -> dw_pcie_find_capability()
---
 drivers/pci/controller/dwc/pcie-ultrarisc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-ultrarisc.c b/drivers/pci/controller/dwc/pcie-ultrarisc.c
index 6ee661ceff67..3f6e3ab0aa69 100644
--- a/drivers/pci/controller/dwc/pcie-ultrarisc.c
+++ b/drivers/pci/controller/dwc/pcie-ultrarisc.c
@@ -49,7 +49,7 @@ static int ultrarisc_pcie_host_init(struct dw_pcie_rp *pp)
 	FIELD_MODIFY(PORT_FLT_SF_MASK, &val, PORT_FLT_SF_VAL_64);
 	dw_pcie_writel_dbi(pci, PCIE_TIMER_CTRL_MAX_FUNC_NUM, val);
 
-	cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	cap_exp = dw_pcie_get_pcie_cap(pci);
 	val = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCTL2);
 	FIELD_MODIFY(PCI_EXP_LNKCTL2_TLS, &val, PCI_EXP_LNKCTL2_TLS_16_0GT);
 	dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCTL2, val);
-- 
2.34.1