| Message ID | 20250321101710.371480-2-18255117159@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce generic capability search functions | expand |
Hi Hans,
kernel test robot noticed the following build warnings:
[auto build test WARNING on a1cffe8cc8aef85f1b07c4464f0998b9785b795a]
url: https://github.com/intel-lab-lkp/linux/commits/Hans-Zhang/PCI-Introduce-generic-capability-search-functions/20250321-182140
base: a1cffe8cc8aef85f1b07c4464f0998b9785b795a
patch link: https://lore.kernel.org/r/20250321101710.371480-2-18255117159%40163.com
patch subject: [v4 1/4] PCI: Introduce generic capability search functions
config: arc-randconfig-002-20250321 (https://download.01.org/0day-ci/archive/20250321/202503212059.oxvxSlCc-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250321/202503212059.oxvxSlCc-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503212059.oxvxSlCc-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/nvme/host/nvme.h:11,
from drivers/nvme/host/fc.c:13:
>> include/linux/pci.h:2024:12: warning: 'pci_generic_find_ext_capability' defined but not used [-Wunused-function]
2024 | static u16 pci_generic_find_ext_capability(void *priv,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/pci.h:2021:11: warning: 'pci_generic_find_capability' defined but not used [-Wunused-function]
2021 | static u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/pci_generic_find_ext_capability +2024 include/linux/pci.h
1999
2000 static inline void pci_set_master(struct pci_dev *dev) { }
2001 static inline void pci_clear_master(struct pci_dev *dev) { }
2002 static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; }
2003 static inline void pci_disable_device(struct pci_dev *dev) { }
2004 static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; }
2005 static inline int pci_assign_resource(struct pci_dev *dev, int i)
2006 { return -EBUSY; }
2007 static inline int __must_check __pci_register_driver(struct pci_driver *drv,
2008 struct module *owner,
2009 const char *mod_name)
2010 { return 0; }
2011 static inline int pci_register_driver(struct pci_driver *drv)
2012 { return 0; }
2013 static inline void pci_unregister_driver(struct pci_driver *drv) { }
2014 static inline u8 pci_find_capability(struct pci_dev *dev, int cap)
2015 { return 0; }
2016 static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post, int cap)
2017 { return 0; }
2018 static inline u16 pci_find_ext_capability(struct pci_dev *dev, int cap)
2019 { return 0; }
2020 typedef u32 (*pci_generic_read_cfg)(void *priv, int where, int size);
> 2021 static u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
2022 u8 cap)
2023 { return 0; }
> 2024 static u16 pci_generic_find_ext_capability(void *priv,
2025 pci_generic_read_cfg read_cfg,
2026 u8 cap)
2027 { return 0; }
2028 static inline u64 pci_get_dsn(struct pci_dev *dev)
2029 { return 0; }
2030
On Fri, Mar 21, 2025 at 06:17:07PM +0800, Hans Zhang wrote: > Existing controller drivers (e.g., DWC, custom out-of-tree drivers) > duplicate logic for scanning PCI capability lists. This creates > maintenance burdens and risks inconsistencies. > > To resolve this: > > 1. Add pci_generic_find_capability() and pci_generic_find_ext_capability() > in drivers/pci/pci.c, accepting controller-specific read functions > and device data as parameters. > I'd reword pci_generic* as pci_host_bridge* to reflect the fact that these APIs are meant for host bridges. > 2. Refactor dwc_pcie_find_capability() and similar functions to utilize > these new generic interfaces. > This is not part of this patch. So should be dropped. > 3. Update out-of-tree drivers to leverage the common implementation, > eliminating code duplication. > This also. > This approach: > - Centralizes critical PCI capability scanning logic > - Allows flexible adaptation to varied hardware access methods > - Reduces future maintenance overhead > - Aligns with kernel code reuse best practices > > Tested with DWC PCIe controller and CDNS PCIe drivers. > This tested info is also not required since the DWC and CDNS changes are not part of this patch. - Mani
On 2025/3/21 21:00, Manivannan Sadhasivam wrote: > On Fri, Mar 21, 2025 at 06:17:07PM +0800, Hans Zhang wrote: >> Existing controller drivers (e.g., DWC, custom out-of-tree drivers) >> duplicate logic for scanning PCI capability lists. This creates >> maintenance burdens and risks inconsistencies. >> >> To resolve this: >> >> 1. Add pci_generic_find_capability() and pci_generic_find_ext_capability() >> in drivers/pci/pci.c, accepting controller-specific read functions >> and device data as parameters. >> > > I'd reword pci_generic* as pci_host_bridge* to reflect the fact that these APIs > are meant for host bridges. > Hi Mani, Thanks your for reply. Will change. >> 2. Refactor dwc_pcie_find_capability() and similar functions to utilize >> these new generic interfaces. >> > > This is not part of this patch. So should be dropped. Will change. >> 3. Update out-of-tree drivers to leverage the common implementation, >> eliminating code duplication. >> > > This also. Will change. >> This approach: >> - Centralizes critical PCI capability scanning logic >> - Allows flexible adaptation to varied hardware access methods >> - Reduces future maintenance overhead >> - Aligns with kernel code reuse best practices >> >> Tested with DWC PCIe controller and CDNS PCIe drivers. >> > > This tested info is also not required since the DWC and CDNS changes are not > part of this patch. Will change. Best regards, Hans
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 869d204a70a3..3c381827a3a0 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -612,6 +612,89 @@ u16 pci_find_ext_capability(struct pci_dev *dev, int cap) } EXPORT_SYMBOL_GPL(pci_find_ext_capability); +/* + * These interfaces resemble the pci_find_*capability() interfaces, but these + * are for configuring host controllers, which are bridges *to* PCI devices but + * are not PCI devices themselves. + */ +static u8 __pci_generic_find_next_cap(void *priv, pci_generic_read_cfg read_cfg, + u8 cap_ptr, u8 cap) +{ + u8 cap_id, next_cap_ptr; + u16 reg; + + if (!cap_ptr) + return 0; + + reg = read_cfg(priv, cap_ptr, 2); + cap_id = (reg & 0x00ff); + + if (cap_id > PCI_CAP_ID_MAX) + return 0; + + if (cap_id == cap) + return cap_ptr; + + next_cap_ptr = (reg & 0xff00) >> 8; + return __pci_generic_find_next_cap(priv, read_cfg, next_cap_ptr, cap); +} + +u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg, + u8 cap) +{ + u8 next_cap_ptr; + u16 reg; + + reg = read_cfg(priv, PCI_CAPABILITY_LIST, 2); + next_cap_ptr = (reg & 0x00ff); + + return __pci_generic_find_next_cap(priv, read_cfg, next_cap_ptr, cap); +} +EXPORT_SYMBOL_GPL(pci_generic_find_capability); + +static u16 pci_generic_find_next_ext_capability(void *priv, + pci_generic_read_cfg read_cfg, + u16 start, u8 cap) +{ + u32 header; + int ttl; + int pos = PCI_CFG_SPACE_SIZE; + + /* minimum 8 bytes per capability */ + ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8; + + if (start) + pos = start; + + header = read_cfg(priv, pos, 4); + /* + * If we have no capabilities, this is indicated by cap ID, + * cap version and next pointer all being 0. + */ + if (header == 0) + return 0; + + while (ttl-- > 0) { + if (PCI_EXT_CAP_ID(header) == cap && pos != start) + return pos; + + pos = PCI_EXT_CAP_NEXT(header); + if (pos < PCI_CFG_SPACE_SIZE) + break; + + header = read_cfg(priv, pos, 4); + } + + return 0; +} + +u16 pci_generic_find_ext_capability(void *priv, pci_generic_read_cfg read_cfg, + u8 cap) +{ + return pci_generic_find_next_ext_capability(priv, read_cfg, 0, cap); +} +EXPORT_SYMBOL_GPL(pci_generic_find_ext_capability); + /** * pci_get_dsn - Read and return the 8-byte Device Serial Number * @dev: PCI device to query diff --git a/include/linux/pci.h b/include/linux/pci.h index 47b31ad724fa..800da2cb884d 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1205,6 +1205,11 @@ u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap); u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap); u16 pci_find_ext_capability(struct pci_dev *dev, int cap); u16 pci_find_next_ext_capability(struct pci_dev *dev, u16 pos, int cap); +typedef u32 (*pci_generic_read_cfg)(void *priv, int where, int size); +u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg, + u8 cap); +u16 pci_generic_find_ext_capability(void *priv, pci_generic_read_cfg read_cfg, + u8 cap); struct pci_bus *pci_find_next_bus(const struct pci_bus *from); u16 pci_find_vsec_capability(struct pci_dev *dev, u16 vendor, int cap); u16 pci_find_dvsec_capability(struct pci_dev *dev, u16 vendor, u16 dvsec); @@ -2012,7 +2017,14 @@ static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post, int cap) { return 0; } static inline u16 pci_find_ext_capability(struct pci_dev *dev, int cap) { return 0; } - +typedef u32 (*pci_generic_read_cfg)(void *priv, int where, int size); +static u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg, + u8 cap) +{ return 0; } +static u16 pci_generic_find_ext_capability(void *priv, + pci_generic_read_cfg read_cfg, + u8 cap) +{ return 0; } static inline u64 pci_get_dsn(struct pci_dev *dev) { return 0; }
Existing controller drivers (e.g., DWC, custom out-of-tree drivers) duplicate logic for scanning PCI capability lists. This creates maintenance burdens and risks inconsistencies. To resolve this: 1. Add pci_generic_find_capability() and pci_generic_find_ext_capability() in drivers/pci/pci.c, accepting controller-specific read functions and device data as parameters. 2. Refactor dwc_pcie_find_capability() and similar functions to utilize these new generic interfaces. 3. Update out-of-tree drivers to leverage the common implementation, eliminating code duplication. This approach: - Centralizes critical PCI capability scanning logic - Allows flexible adaptation to varied hardware access methods - Reduces future maintenance overhead - Aligns with kernel code reuse best practices Tested with DWC PCIe controller and CDNS PCIe drivers. Signed-off-by: Hans Zhang <18255117159@163.com> --- Changes since v3: https://lore.kernel.org/linux-pci/20250321040358.360755-2-18255117159@163.com/ - Resolved compilation error. --- drivers/pci/pci.c | 83 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 14 +++++++- 2 files changed, 96 insertions(+), 1 deletion(-)