Re: [PATCH v5 01/16] PCI: dwc: Add pcie_cap field and caching helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI: dwc: unconfirmed offset returned in dw_pcie_get_pcie_cap()
--

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

PCI: dwc: Add pcie_cap field and caching helper

This commit adds a pcie_cap field to struct dw_pcie and introduces a
dw_pcie_get_pcie_cap() helper that lazily looks up and caches the PCI
Express Capability offset on first use once confirmed via a DBI read-back.

> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 6d788aa000117..dede6b3966585 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
[ ... ]
> @@ -832,6 +834,42 @@ 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
> + *
[ ... ]
> + * Return: the offset of the PCIe Capability, or 0 if it could not be
> + * found or has not been confirmed yet.
> + */
> +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);
> +		if (cap && dw_pcie_readb_dbi(pci, cap) == PCI_CAP_ID_EXP)
> +			WRITE_ONCE(pci->pcie_cap, cap);
> +	}
> +
> +	return cap;
> +}

[Severity: Medium]
In drivers/pci/controller/dwc/pcie-designware.h, the kernel-doc for
dw_pcie_get_pcie_cap() states that it returns 0 if the capability could not
be found or has not been confirmed yet.

However, when pci->pcie_cap is 0 and transient DBI data causes
dw_pcie_find_capability(pci, PCI_CAP_ID_EXP) to return a non-zero offset cap
while the confirmation check dw_pcie_readb_dbi(pci, cap) == PCI_CAP_ID_EXP
evaluates to false, pci->pcie_cap is left as 0, but the unconfirmed non-zero
cap is still returned to the caller.

Callers that guard DBI writes on a non-zero return (such as
rockchip_pcie_enable_l0s()) or cache a non-zero return in driver state (such
as tegra_pcie_dw_host_init() setting pcie->pcie_cap_base when
!pcie->pcie_cap_base) will then consume or permanently store the unconfirmed
offset.

Should dw_pcie_get_pcie_cap() return 0 when the confirmation read-back fails,
or should the kernel-doc comment be updated to match the implementation?

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic