Re: [PATCH 3/3] PCI: dwc: Simplify platform drivers using cached capability offset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware access violation and state corruption due to prematurely cached `pci->pcie_cap` evaluating to 0 during early platform initialization.
--

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

PCI: dwc: Simplify platform drivers using cached capability offset

This commit replaces explicit dw_pcie_find_capability calls with the
cached pci->pcie_cap across several DWC-based platform drivers to
simplify the code.

> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index c21906eced618..939b9dcac7fe2 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -121,7 +121,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 = pcie->pci.pcie_cap;

Will pcie->pci.pcie_cap be correctly populated here during early
initialization?

Since some platform drivers (like Intel Gateway and SpacemiT K1) enable
clocks and deassert resets in their .init callbacks, the hardware might
be inaccessible when the DWC core initially attempts to cache the
capability offset.

If the core evaluates pcie_cap to 0 before the hardware is powered on,
would using the cached value here cause us to read and modify standard
PCI configuration header registers instead of the PCIe Capability
structure?

[ ... ]

> diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> index be20a520255b6..6f0556336f440 100644
> --- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> @@ -114,12 +114,9 @@ static void k1_pcie_disable_resources(struct k1_pcie *k1)
>  static void k1_pcie_disable_aspm_l1(struct k1_pcie *k1)
>  {
>  	struct dw_pcie *pci = &k1->pci;
> -	u8 offset;
> +	u8 offset = pci->pcie_cap + PCI_EXP_LNKCAP;
>  	u32 val;
>  
> -	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> -	offset += PCI_EXP_LNKCAP;

Could this also experience the same issue if pci->pcie_cap evaluated to 0
during early core initialization, causing offset to incorrectly point to
0 + PCI_EXP_LNKCAP (0x0c)?

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