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

[Manivannan Sadhasivam]

On Sat, May 30, 2026 at 11:30:45PM +0800, Hans Zhang wrote:
> 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.
> 

Not true. As Sashiko pointed out, EP platforms like Qcom, Tegra, enable
resources like clocks only after PERST# deassert. So any DBI access before that
will cause sync abort.

- Mani

-- 
மணிவண்ணன் சதாசிவம்