On Mon, Mar 24, 2025 at 12:48:48AM +0800, Hans Zhang wrote: > Existing controller drivers (e.g., DWC, custom out-of-tree drivers) Oh, you should not mention 'out-of-tree drivers' as this patch is not anyway intented to benefit them. We certainly do not care about out of tree drivers. - Mani > duplicate logic for scanning PCI capability lists. This creates > maintenance burdens and risks inconsistencies. > > To resolve this: > > Add pci_host_bridge_find_*capability() in pci-host-helpers.c, accepting > controller-specific read functions and device data as parameters. > > 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 > > Signed-off-by: Hans Zhang <18255117159@xxxxxxx> > --- > Changes since v5: > https://lore.kernel.org/linux-pci/20250321163803.391056-2-18255117159@xxxxxxx > > - If you put the helpers in drivers/pci/pci.c, they unnecessarily enlarge > the kernel's .text section even if it's known already at compile time > that they're never going to be used (e.g. on x86). > > - Move the API for find capabilitys to a new file called > pci-host-helpers.c. > > Changes since v4: > https://lore.kernel.org/linux-pci/20250321101710.371480-2-18255117159@xxxxxxx > > - Resolved [v4 1/4] compilation warning. > - The patch commit message were modified. > --- > drivers/pci/controller/Kconfig | 17 ++++ > drivers/pci/controller/Makefile | 1 + > drivers/pci/controller/pci-host-helpers.c | 98 +++++++++++++++++++++++ > drivers/pci/pci.h | 7 ++ > 4 files changed, 123 insertions(+) > create mode 100644 drivers/pci/controller/pci-host-helpers.c > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > index 9800b7681054..0020a892a55b 100644 > --- a/drivers/pci/controller/Kconfig > +++ b/drivers/pci/controller/Kconfig > @@ -132,6 +132,23 @@ config PCI_HOST_GENERIC > Say Y here if you want to support a simple generic PCI host > controller, such as the one emulated by kvmtool. > > +config PCI_HOST_HELPERS > + bool > + prompt "PCI Host Controller Helper Functions" if EXPERT > + help > + This provides common infrastructure for PCI host controller drivers to > + handle PCI capability scanning and other shared operations. The helper > + functions eliminate code duplication across controller drivers. > + > + These functions are used by PCI controller drivers that need to scan > + PCI capabilities using controller-specific access methods (e.g. when > + the controller is behind a non-standard configuration space). > + > + If you are using any PCI host controller drivers that require these > + helpers (such as DesignWare, Cadence, etc), this will be > + automatically selected. Say N unless you are developing a custom PCI > + host controller driver. > + > config PCIE_HISI_ERR > depends on ACPI_APEI_GHES && (ARM64 || COMPILE_TEST) > bool "HiSilicon HIP PCIe controller error handling driver" > diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile > index 038ccbd9e3ba..e80091eb7597 100644 > --- a/drivers/pci/controller/Makefile > +++ b/drivers/pci/controller/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_PCIE_RCAR_HOST) += pcie-rcar.o pcie-rcar-host.o > obj-$(CONFIG_PCIE_RCAR_EP) += pcie-rcar.o pcie-rcar-ep.o > obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o > obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o > +obj-$(CONFIG_PCI_HOST_HELPERS) += pci-host-helpers.o > obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o > obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o > obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o > diff --git a/drivers/pci/controller/pci-host-helpers.c b/drivers/pci/controller/pci-host-helpers.c > new file mode 100644 > index 000000000000..cd261a281c60 > --- /dev/null > +++ b/drivers/pci/controller/pci-host-helpers.c > @@ -0,0 +1,98 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PCI Host Controller Helper Functions > + * > + * Copyright (C) 2025 Hans Zhang > + * > + * Author: Hans Zhang <18255117159@xxxxxxx> > + */ > + > +#include <linux/pci.h> > + > +#include "../pci.h" > + > +/* > + * 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_host_bridge_find_next_cap(void *priv, > + pci_host_bridge_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_host_bridge_find_next_cap(priv, read_cfg, next_cap_ptr, > + cap); > +} > + > +u8 pci_host_bridge_find_capability(void *priv, > + pci_host_bridge_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_host_bridge_find_next_cap(priv, read_cfg, next_cap_ptr, > + cap); > +} > +EXPORT_SYMBOL_GPL(pci_host_bridge_find_capability); > + > +static u16 pci_host_bridge_find_next_ext_capability( > + void *priv, pci_host_bridge_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_host_bridge_find_ext_capability(void *priv, > + pci_host_bridge_read_cfg read_cfg, > + u8 cap) > +{ > + return pci_host_bridge_find_next_ext_capability(priv, read_cfg, 0, cap); > +} > +EXPORT_SYMBOL_GPL(pci_host_bridge_find_ext_capability); > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index 01e51db8d285..8d1c919cbfef 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -1034,4 +1034,11 @@ void pcim_release_region(struct pci_dev *pdev, int bar); > (PCI_CONF1_ADDRESS(bus, dev, func, reg) | \ > PCI_CONF1_EXT_REG(reg)) > > +typedef u32 (*pci_host_bridge_read_cfg)(void *priv, int where, int size); > +u8 pci_host_bridge_find_capability(void *priv, > + pci_host_bridge_read_cfg read_cfg, u8 cap); > +u16 pci_host_bridge_find_ext_capability(void *priv, > + pci_host_bridge_read_cfg read_cfg, > + u8 cap); > + > #endif /* DRIVERS_PCI_H */ > -- > 2.25.1 > -- மணிவண்ணன் சதாசிவம்