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

[Hans Zhang <18255117159@163.com>]

On 10/28/26 11:15, sashiko-bot@kernel.org wrote:
> 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?
> 

Hi,

We have already replied to Bjorn before. According to the specification, 
the value of pcie_cap is not equal to 0.


Best regards,
Hans




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