[PATCH v12 0/6] Refactor capability search into common macros

[PATCH v12 0/6] Refactor capability search into common macros

[Hans Zhang]

Dear Maintainers,

This patch series addresses long-standing code duplication in PCI
capability discovery logic across the PCI core and controller drivers.
The existing implementation ties capability search to fully initialized
PCI device structures, limiting its usability during early controller
initialization phases where device/bus structures may not yet be
available.

The primary goal is to decouple capability discovery from PCI device
dependencies by introducing a unified framework using config space
accessor-based macros. This enables:

1. Early Capability Discovery: Host controllers (e.g., Cadence, DWC)
can now perform capability searches during pre-initialization stages
using their native config accessors.

2. Code Consolidation: Common logic for standard and extended capability
searches is refactored into shared macros (`PCI_FIND_NEXT_CAP_TTL` and
`PCI_FIND_NEXT_EXT_CAPABILITY`), eliminating redundant implementations.

3. Safety and Maintainability: TTL checks are centralized within the
macros to prevent infinite loops, while hardcoded offsets in drivers
are replaced with dynamic discovery, reducing fragility.

Key improvements include:  
- Driver Conversions: DesignWare and Cadence drivers are migrated to
  use the new macros, removing device-specific assumptions and ensuring
  consistent error handling.

- Enhanced Readability: Magic numbers are replaced with symbolic
  constants, and config space accessors are standardized for clarity.

- Backward Compatibility: Existing PCI core behavior remains unchanged.

---
Changes since v11:
- Resolved some compilation warning.
- Add some include.
- Add the *** BLURB HERE *** description(Corrected by Mani and Krzysztof).

Changes since v10:
- The patch [v10 2/6] remove #include <uapi/linux/pci_regs.h> and add macro definition comments.
- The patch [v10 3/6] remove #include <uapi/linux/pci_regs.h> and commit message were modified.
- The other patches have not been modified.

Changes since v9:
- Resolved [v9 4/6] compilation error.
  The latest 6.15 rc1 merge __dw_pcie_find_vsec_capability, which uses 
  dw_pcie_find_next_ext_capability.
- The other patches have not been modified.

Changes since v8:
- Split patch.
- The patch commit message were modified.
- Other patches(4/6, 5/6, 6/6) are unchanged.

Changes since v7:
- Patch 2/5 and 3/5 compilation error resolved.
- Other patches are unchanged.

Changes since v6:
- Refactor capability search into common macros.
- Delete pci-host-helpers.c and MAINTAINERS.

Changes since v5:
- 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.
- Add new patch for MAINTAINERS.

Changes since v4:
- Resolved [v4 1/4] compilation warning.
- The patch subject and commit message were modified.

Changes since v3:
- Resolved [v3 1/4] compilation error.
- Other patches are not modified.

Changes since v2:
- Add and split into a series of patches.
---

Hans Zhang (6):
  PCI: Introduce generic bus config read helper function
  PCI: Clean up __pci_find_next_cap_ttl() readability
  PCI: Refactor capability search into common macros
  PCI: dwc: Use common PCI host bridge APIs for finding the capabilities
  PCI: cadence: Use common PCI host bridge APIs for finding the
    capabilities
  PCI: cadence: Use cdns_pcie_find_*capability to avoid hardcode.

 drivers/pci/access.c                          | 17 ++++
 .../pci/controller/cadence/pcie-cadence-ep.c  | 40 +++++----
 drivers/pci/controller/cadence/pcie-cadence.c | 28 ++++++
 drivers/pci/controller/cadence/pcie-cadence.h | 18 ++--
 drivers/pci/controller/dwc/pcie-designware.c  | 81 +++--------------
 drivers/pci/pci.c                             | 68 ++------------
 drivers/pci/pci.h                             | 88 +++++++++++++++++++
 include/uapi/linux/pci_regs.h                 |  2 +
 8 files changed, 194 insertions(+), 148 deletions(-)


base-commit: 82f2b0b97b36ee3fcddf0f0780a9a0825d52fec3
-- 
2.25.1

[PATCH v12 1/6] PCI: Introduce generic bus config read helper function

[Hans Zhang]

The primary PCI config space accessors are tied to the size of the read
(byte/word/dword). Upcoming refactoring of PCI capability discovery logic
requires passing a config accessor function that must be able to perform
read with different sizes.

Add any size config space read accessor pci_bus_read_config() to allow
giving it as the config space accessor to the upcoming PCI capability
discovery macro.

Reconstructs the PCI function discovery logic to prepare for unified
configuration of access modes. No function changes are intended.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v9 ~ v11:
- None

Changes since v8:
- The new split is patch 1/6.
- The patch commit message were modified.
---
 drivers/pci/access.c | 17 +++++++++++++++++
 drivers/pci/pci.h    |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index b123da16b63b..603332658ab3 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -85,6 +85,23 @@ EXPORT_SYMBOL(pci_bus_write_config_byte);
 EXPORT_SYMBOL(pci_bus_write_config_word);
 EXPORT_SYMBOL(pci_bus_write_config_dword);
 
+int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
+			u32 *val)
+{
+	struct pci_bus *bus = priv;
+	int ret;
+
+	if (size == 1)
+		ret = pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
+	else if (size == 2)
+		ret = pci_bus_read_config_word(bus, devfn, where, (u16 *)val);
+	else
+		ret = pci_bus_read_config_dword(bus, devfn, where, val);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_bus_read_config);
+
 int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
 			    int where, int size, u32 *val)
 {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index b81e99cd4b62..5e1477d6e254 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -88,6 +88,8 @@ extern bool pci_early_dump;
 bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
 bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
 bool pcie_cap_has_rtctl(const struct pci_dev *dev);
+int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
+			u32 *val);
 
 /* Functions internal to the PCI core code */
 
-- 
2.25.1

[PATCH v12 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability

[Hans Zhang]

Refactor the __pci_find_next_cap_ttl() to improve code clarity:
- Replace magic number 0x40 with PCI_STD_HEADER_SIZEOF.
- Use ALIGN_DOWN() for position alignment instead of manual bitmask.
- Extract PCI capability fields via FIELD_GET() with standardized masks.
- Add necessary headers (linux/align.h).

The changes are purely non-functional cleanups, ensuring behavior remains
identical to the original implementation.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v11:
- None

Changes since v10:
- Remove #include <uapi/linux/pci_regs.h> and add macro definition comments.

Changes since v9:
- None

Changes since v8:
- Split into patch 1/6, patch 2/6.
- The
---
 drivers/pci/pci.c             | 9 +++++----
 include/uapi/linux/pci_regs.h | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e77d5b53c0ce..27d2adb18a30 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/align.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
 #include <linux/dmi.h>
@@ -432,17 +433,17 @@ static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
 	pci_bus_read_config_byte(bus, devfn, pos, &pos);
 
 	while ((*ttl)--) {
-		if (pos < 0x40)
+		if (pos < PCI_STD_HEADER_SIZEOF)
 			break;
-		pos &= ~3;
+		pos = ALIGN_DOWN(pos, 4);
 		pci_bus_read_config_word(bus, devfn, pos, &ent);
 
-		id = ent & 0xff;
+		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
 		if (id == 0xff)
 			break;
 		if (id == cap)
 			return pos;
-		pos = (ent >> 8);
+		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
 	}
 	return 0;
 }
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index ba326710f9c8..35051f9ac16a 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -206,6 +206,8 @@
 /* 0x48-0x7f reserved */
 
 /* Capability lists */
+#define PCI_CAP_ID_MASK		0x00ff	/* Capability ID mask */
+#define PCI_CAP_LIST_NEXT_MASK	0xff00	/* Next Capability Pointer mask */
 
 #define PCI_CAP_LIST_ID		0	/* Capability ID */
 #define  PCI_CAP_ID_PM		0x01	/* Power Management */
-- 
2.25.1

[PATCH v12 3/6] PCI: Refactor capability search into common macros

[Hans Zhang]

The PCI Capability search functionality is duplicated across the PCI core
and several controller drivers. The core's current implementation requires
fully initialized PCI device and bus structures, which prevents controller
drivers from using it during early initialization phases before these
structures are available.

Move the Capability search logic into a header-based macro that accepts a
config space accessor function as an argument. This enables controller
drivers to perform Capability discovery using their early access
mechanisms prior to full device initialization while sharing the
Capability search code.

Convert the existing PCI core Capability search implementation to use this
new macro. Controller drivers can later use the same macros with their
early access mechanisms while maintaining the existing protection against
infinite loops through preserved TTL checks.

The ttl parameter was originally an additional safeguard to prevent
infinite loops in corrupted config space.  However, the
PCI_FIND_NEXT_CAP_TTL macro already enforces a TTL limit internally.
Removing redundant ttl handling simplifies the interface while maintaining
the safety guarantee. This aligns with the macro's design intent of
encapsulating TTL management.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v11:
- Add #include <linux/bitfield.h>, solve the compilation warnings caused by the subsequent patch calls.

Changes since v10:
- Remove #include <uapi/linux/pci_regs.h>.
- The patch commit message were modified.

Changes since v9:
- None

Changes since v8:
- The patch commit message were modified.
---
 drivers/pci/pci.c | 69 +++++--------------------------------
 drivers/pci/pci.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 60 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 27d2adb18a30..271d922abdcc 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -9,7 +9,6 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/align.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
 #include <linux/dmi.h>
@@ -425,35 +424,16 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 }
 
 static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
-				  u8 pos, int cap, int *ttl)
+				  u8 pos, int cap)
 {
-	u8 id;
-	u16 ent;
-
-	pci_bus_read_config_byte(bus, devfn, pos, &pos);
-
-	while ((*ttl)--) {
-		if (pos < PCI_STD_HEADER_SIZEOF)
-			break;
-		pos = ALIGN_DOWN(pos, 4);
-		pci_bus_read_config_word(bus, devfn, pos, &ent);
-
-		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
-		if (id == 0xff)
-			break;
-		if (id == cap)
-			return pos;
-		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
-	}
-	return 0;
+	return PCI_FIND_NEXT_CAP_TTL(pci_bus_read_config, pos, cap, bus,
+				     devfn);
 }
 
 static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
 			      u8 pos, int cap)
 {
-	int ttl = PCI_FIND_CAP_TTL;
-
-	return __pci_find_next_cap_ttl(bus, devfn, pos, cap, &ttl);
+	return __pci_find_next_cap_ttl(bus, devfn, pos, cap);
 }
 
 u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap)
@@ -554,42 +534,11 @@ EXPORT_SYMBOL(pci_bus_find_capability);
  */
 u16 pci_find_next_ext_capability(struct pci_dev *dev, u16 start, int cap)
 {
-	u32 header;
-	int ttl;
-	u16 pos = PCI_CFG_SPACE_SIZE;
-
-	/* minimum 8 bytes per capability */
-	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
-
 	if (dev->cfg_size <= PCI_CFG_SPACE_SIZE)
 		return 0;
 
-	if (start)
-		pos = start;
-
-	if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
-		return 0;
-
-	/*
-	 * 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;
-
-		if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
-			break;
-	}
-
-	return 0;
+	return PCI_FIND_NEXT_EXT_CAPABILITY(pci_bus_read_config, start, cap,
+					    dev->bus, dev->devfn);
 }
 EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
 
@@ -649,7 +598,7 @@ EXPORT_SYMBOL_GPL(pci_get_dsn);
 
 static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
 {
-	int rc, ttl = PCI_FIND_CAP_TTL;
+	int rc;
 	u8 cap, mask;
 
 	if (ht_cap == HT_CAPTYPE_SLAVE || ht_cap == HT_CAPTYPE_HOST)
@@ -658,7 +607,7 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
 		mask = HT_5BIT_CAP_MASK;
 
 	pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn, pos,
-				      PCI_CAP_ID_HT, &ttl);
+				      PCI_CAP_ID_HT);
 	while (pos) {
 		rc = pci_read_config_byte(dev, pos + 3, &cap);
 		if (rc != PCIBIOS_SUCCESSFUL)
@@ -669,7 +618,7 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
 
 		pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn,
 					      pos + PCI_CAP_LIST_NEXT,
-					      PCI_CAP_ID_HT, &ttl);
+					      PCI_CAP_ID_HT);
 	}
 
 	return 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 5e1477d6e254..f9cf45026e6e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -2,6 +2,8 @@
 #ifndef DRIVERS_PCI_H
 #define DRIVERS_PCI_H
 
+#include <linux/align.h>
+#include <linux/bitfield.h>
 #include <linux/pci.h>
 
 struct pcie_tlp_log;
@@ -91,6 +93,90 @@ bool pcie_cap_has_rtctl(const struct pci_dev *dev);
 int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
 			u32 *val);
 
+/* Standard Capability finder */
+/**
+ * PCI_FIND_NEXT_CAP_TTL - Find a PCI standard capability
+ * @read_cfg: Function pointer for reading PCI config space
+ * @start: Starting position to begin search
+ * @cap: Capability ID to find
+ * @args: Arguments to pass to read_cfg function
+ *
+ * Iterates through the capability list in PCI config space to find
+ * the specified capability. Implements TTL (time-to-live) protection
+ * against infinite loops.
+ *
+ * Returns: Position of the capability if found, 0 otherwise.
+ */
+#define PCI_FIND_NEXT_CAP_TTL(read_cfg, start, cap, args...)		\
+({									\
+	int __ttl = PCI_FIND_CAP_TTL;					\
+	u8 __id, __found_pos = 0;					\
+	u8 __pos = (start);						\
+	u16 __ent;							\
+									\
+	read_cfg(args, __pos, 1, (u32 *)&__pos);			\
+									\
+	while (__ttl--) {						\
+		if (__pos < PCI_STD_HEADER_SIZEOF)			\
+			break;						\
+									\
+		__pos = ALIGN_DOWN(__pos, 4);				\
+		read_cfg(args, __pos, 2, (u32 *)&__ent);		\
+									\
+		__id = FIELD_GET(PCI_CAP_ID_MASK, __ent);		\
+		if (__id == 0xff)					\
+			break;						\
+									\
+		if (__id == (cap)) {					\
+			__found_pos = __pos;				\
+			break;						\
+		}							\
+									\
+		__pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, __ent);	\
+	}								\
+	__found_pos;							\
+})
+
+/* Extended Capability finder */
+/**
+ * PCI_FIND_NEXT_EXT_CAPABILITY - Find a PCI extended capability
+ * @read_cfg: Function pointer for reading PCI config space
+ * @start: Starting position to begin search (0 for initial search)
+ * @cap: Extended capability ID to find
+ * @args: Arguments to pass to read_cfg function
+ *
+ * Searches the extended capability space in PCI config registers
+ * for the specified capability. Implements TTL protection against
+ * infinite loops using a calculated maximum search count.
+ *
+ * Returns: Position of the capability if found, 0 otherwise.
+ */
+#define PCI_FIND_NEXT_EXT_CAPABILITY(read_cfg, start, cap, args...)		\
+({										\
+	u16 __pos = (start) ?: PCI_CFG_SPACE_SIZE;				\
+	u16 __found_pos = 0;							\
+	int __ttl, __ret;							\
+	u32 __header;								\
+										\
+	__ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;		\
+	while (__ttl-- > 0 && __pos >= PCI_CFG_SPACE_SIZE) {			\
+		__ret = read_cfg(args, __pos, 4, &__header);			\
+		if (__ret != PCIBIOS_SUCCESSFUL)				\
+			break;							\
+										\
+		if (__header == 0)						\
+			break;							\
+										\
+		if (PCI_EXT_CAP_ID(__header) == (cap) && __pos != start) {	\
+			__found_pos = __pos;					\
+			break;							\
+		}								\
+										\
+		__pos = PCI_EXT_CAP_NEXT(__header);				\
+	}									\
+	__found_pos;								\
+})
+
 /* Functions internal to the PCI core code */
 
 #ifdef CONFIG_DMI
-- 
2.25.1

[PATCH v12 4/6] PCI: dwc: Use common PCI host bridge APIs for finding the capabilities

[Hans Zhang]

Use the PCI core is now exposing generic macros for the host bridges to
search for the PCIe capabilities, make use of them in the DWC driver.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v11:
- Resolve compilation errors. s/dw_pcie_read_dbi/dw_pcie_read*_dbi

Changes since v10:
- None

Changes since v9:
- Resolved [v9 4/6] compilation error.
  The latest 6.15 rc1 merge __dw_pcie_find_vsec_capability, which uses 
  dw_pcie_find_next_ext_capability.

Changes since v8:
- None

Changes since v7:
- Resolve compilation errors.

Changes since v6:
https://lore.kernel.org/linux-pci/20250323164852.430546-3-18255117159@163.com/

- The patch commit message were modified.

Changes since v5:
https://lore.kernel.org/linux-pci/20250321163803.391056-3-18255117159@163.com/

- Kconfig add "select PCI_HOST_HELPERS"
---
 drivers/pci/controller/dwc/pcie-designware.c | 81 ++++----------------
 1 file changed, 14 insertions(+), 67 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 97d76d3dc066..7939411a24eb 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -205,83 +205,30 @@ void dw_pcie_version_detect(struct dw_pcie *pci)
 		pci->type = ver;
 }
 
-/*
- * 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 __dw_pcie_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
-				  u8 cap)
+static int dw_pcie_read_cfg(void *priv, int where, int size, u32 *val)
 {
-	u8 cap_id, next_cap_ptr;
-	u16 reg;
-
-	if (!cap_ptr)
-		return 0;
+	struct dw_pcie *pci = priv;
 
-	reg = dw_pcie_readw_dbi(pci, cap_ptr);
-	cap_id = (reg & 0x00ff);
-
-	if (cap_id > PCI_CAP_ID_MAX)
-		return 0;
-
-	if (cap_id == cap)
-		return cap_ptr;
+	if (size == 4)
+		*val = dw_pcie_readl_dbi(pci, where);
+	else if (size == 2)
+		*val = dw_pcie_readw_dbi(pci, where);
+	else if (size == 1)
+		*val = dw_pcie_readb_dbi(pci, where);
 
-	next_cap_ptr = (reg & 0xff00) >> 8;
-	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
+	return PCIBIOS_SUCCESSFUL;
 }
 
 u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap)
 {
-	u8 next_cap_ptr;
-	u16 reg;
-
-	reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
-	next_cap_ptr = (reg & 0x00ff);
-
-	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
+	return PCI_FIND_NEXT_CAP_TTL(dw_pcie_read_cfg, PCI_CAPABILITY_LIST, cap,
+				     pci);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_find_capability);
 
-static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, 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 = dw_pcie_readl_dbi(pci, pos);
-	/*
-	 * 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 = dw_pcie_readl_dbi(pci, pos);
-	}
-
-	return 0;
-}
-
 u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)
 {
-	return dw_pcie_find_next_ext_capability(pci, 0, cap);
+	return PCI_FIND_NEXT_EXT_CAPABILITY(dw_pcie_read_cfg, 0, cap, pci);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);
 
@@ -294,8 +241,8 @@ static u16 __dw_pcie_find_vsec_capability(struct dw_pcie *pci, u16 vendor_id,
 	if (vendor_id != dw_pcie_readw_dbi(pci, PCI_VENDOR_ID))
 		return 0;
 
-	while ((vsec = dw_pcie_find_next_ext_capability(pci, vsec,
-						       PCI_EXT_CAP_ID_VNDR))) {
+	while ((vsec = PCI_FIND_NEXT_EXT_CAPABILITY(
+			dw_pcie_read_cfg, vsec, PCI_EXT_CAP_ID_VNDR, pci))) {
 		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
 		if (PCI_VNDR_HEADER_ID(header) == vsec_id)
 			return vsec;
-- 
2.25.1

[PATCH v12 5/6] PCI: cadence: Use common PCI host bridge APIs for finding the capabilities

[Hans Zhang]

Use the PCI core is now exposing generic macros for the host bridges to
search for the PCIe capabilities, make use of them in the CDNS driver.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v11:
- Modify the return value of cdns_pcie_readw from u32 to u16. 
- Modify the return value of cdns_pcie_readb from u32 to u8. 

Changes since v8 ~ v10:
- None

Changes since v7:
- Resolve compilation errors.

Changes since v6:
https://lore.kernel.org/linux-pci/20250323164852.430546-4-18255117159@163.com/

- The patch commit message were modified.

Changes since v5:
https://lore.kernel.org/linux-pci/20250321163803.391056-4-18255117159@163.com

- Kconfig add "select PCI_HOST_HELPERS"
---
 drivers/pci/controller/cadence/pcie-cadence.c | 28 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h | 13 +++++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
index 204e045aed8c..ca4a1a809fcb 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.c
+++ b/drivers/pci/controller/cadence/pcie-cadence.c
@@ -7,6 +7,34 @@
 #include <linux/of.h>
 
 #include "pcie-cadence.h"
+#include "../../pci.h"
+
+static int cdns_pcie_read_cfg(void *priv, int where, int size, u32 *val)
+{
+	struct cdns_pcie *pcie = priv;
+
+	if (size == 4)
+		*val = cdns_pcie_readl(pcie, where);
+	else if (size == 2)
+		*val = cdns_pcie_readw(pcie, where);
+	else if (size == 1)
+		*val = cdns_pcie_readb(pcie, where);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+u8 cdns_pcie_find_capability(struct cdns_pcie *pcie, u8 cap)
+{
+	return PCI_FIND_NEXT_CAP_TTL(cdns_pcie_read_cfg, PCI_CAPABILITY_LIST,
+				     cap, pcie);
+}
+EXPORT_SYMBOL_GPL(cdns_pcie_find_capability);
+
+u16 cdns_pcie_find_ext_capability(struct cdns_pcie *pcie, u8 cap)
+{
+	return PCI_FIND_NEXT_EXT_CAPABILITY(cdns_pcie_read_cfg, 0, cap, pcie);
+}
+EXPORT_SYMBOL_GPL(cdns_pcie_find_ext_capability);
 
 void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie)
 {
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 39ee9945c903..0a4a8bfd3174 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -398,6 +398,16 @@ static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
 	return readl(pcie->reg_base + reg);
 }
 
+static inline u16 cdns_pcie_readw(struct cdns_pcie *pcie, u32 reg)
+{
+	return readw(pcie->reg_base + reg);
+}
+
+static inline u8 cdns_pcie_readb(struct cdns_pcie *pcie, u32 reg)
+{
+	return readb(pcie->reg_base + reg);
+}
+
 static inline u32 cdns_pcie_read_sz(void __iomem *addr, int size)
 {
 	void __iomem *aligned_addr = PTR_ALIGN_DOWN(addr, 0x4);
@@ -557,6 +567,9 @@ static inline int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
 }
 #endif
 
+u8 cdns_pcie_find_capability(struct cdns_pcie *pcie, u8 cap);
+u16 cdns_pcie_find_ext_capability(struct cdns_pcie *pcie, u8 cap);
+
 void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie);
 
 void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 busnr, u8 fn,
-- 
2.25.1

[PATCH v12 6/6] PCI: cadence: Use cdns_pcie_find_*capability to avoid hardcode.

[Hans Zhang]

The PCIe capability/extended capability offsets are not guaranteed to be
the same across all SoCs integrating the Cadence PCIe IP. Hence, use the
cdns_pcie_find_{ext}_capability() APIs for finding them.

This avoids hardcoding the offsets in the driver.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v8 ~ v11:
- None

Changes since v7:
- Resolve compilation errors.

Changes since v6:
https://lore.kernel.org/linux-pci/20250323164852.430546-4-18255117159@163.com/

- The patch commit message were modified.

Changes since v5:
https://lore.kernel.org/linux-pci/20250321163803.391056-4-18255117159@163.com

- Kconfig add "select PCI_HOST_HELPERS"
---
 .../pci/controller/cadence/pcie-cadence-ep.c  | 40 +++++++++++--------
 drivers/pci/controller/cadence/pcie-cadence.h |  5 ---
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 599ec4b1223e..5c4b2151d181 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -19,12 +19,13 @@
 
 static u8 cdns_pcie_get_fn_from_vfn(struct cdns_pcie *pcie, u8 fn, u8 vfn)
 {
-	u32 cap = CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET;
 	u32 first_vf_offset, stride;
+	u16 cap;
 
 	if (vfn == 0)
 		return fn;
 
+	cap = cdns_pcie_find_ext_capability(pcie, PCI_EXT_CAP_ID_SRIOV);
 	first_vf_offset = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_SRIOV_VF_OFFSET);
 	stride = cdns_pcie_ep_fn_readw(pcie, fn, cap +  PCI_SRIOV_VF_STRIDE);
 	fn = fn + first_vf_offset + ((vfn - 1) * stride);
@@ -36,10 +37,11 @@ static int cdns_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
 				     struct pci_epf_header *hdr)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
-	u32 cap = CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET;
 	struct cdns_pcie *pcie = &ep->pcie;
 	u32 reg;
+	u16 cap;
 
+	cap = cdns_pcie_find_ext_capability(pcie, PCI_EXT_CAP_ID_SRIOV);
 	if (vfn > 1) {
 		dev_err(&epc->dev, "Only Virtual Function #1 has deviceID\n");
 		return -EINVAL;
@@ -224,9 +226,10 @@ static int cdns_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn, u8 mmc)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
-	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
 	u16 flags;
+	u8 cap;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
 	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
 
 	/*
@@ -246,9 +249,10 @@ static int cdns_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
-	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
 	u16 flags, mme;
+	u8 cap;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
 	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
 
 	/* Validate that the MSI feature is actually enabled. */
@@ -269,9 +273,10 @@ static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
-	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
 	u32 val, reg;
+	u8 cap;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
 	func_no = cdns_pcie_get_fn_from_vfn(pcie, func_no, vfunc_no);
 
 	reg = cap + PCI_MSIX_FLAGS;
@@ -290,9 +295,10 @@ static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
-	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
 	u32 val, reg;
+	u8 cap;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
 	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
 
 	reg = cap + PCI_MSIX_FLAGS;
@@ -378,11 +384,11 @@ static int cdns_pcie_ep_send_msi_irq(struct cdns_pcie_ep *ep, u8 fn, u8 vfn,
 				     u8 interrupt_num)
 {
 	struct cdns_pcie *pcie = &ep->pcie;
-	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
 	u16 flags, mme, data, data_mask;
-	u8 msi_count;
 	u64 pci_addr, pci_addr_mask = 0xff;
+	u8 msi_count, cap;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
 	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
 
 	/* Check whether the MSI feature has been enabled by the PCI host. */
@@ -430,14 +436,14 @@ static int cdns_pcie_ep_map_msi_irq(struct pci_epc *epc, u8 fn, u8 vfn,
 				    u32 *msi_addr_offset)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
-	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
 	struct cdns_pcie *pcie = &ep->pcie;
 	u64 pci_addr, pci_addr_mask = 0xff;
 	u16 flags, mme, data, data_mask;
-	u8 msi_count;
+	u8 msi_count, cap;
 	int ret;
 	int i;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
 	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
 
 	/* Check whether the MSI feature has been enabled by the PCI host. */
@@ -480,16 +486,16 @@ static int cdns_pcie_ep_map_msi_irq(struct pci_epc *epc, u8 fn, u8 vfn,
 static int cdns_pcie_ep_send_msix_irq(struct cdns_pcie_ep *ep, u8 fn, u8 vfn,
 				      u16 interrupt_num)
 {
-	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
 	u32 tbl_offset, msg_data, reg;
 	struct cdns_pcie *pcie = &ep->pcie;
 	struct pci_epf_msix_tbl *msix_tbl;
 	struct cdns_pcie_epf *epf;
 	u64 pci_addr_mask = 0xff;
 	u64 msg_addr;
+	u8 bir, cap;
 	u16 flags;
-	u8 bir;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
 	epf = &ep->epf[fn];
 	if (vfn > 0)
 		epf = &epf->epf[vfn - 1];
@@ -563,7 +569,9 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
 	int max_epfs = sizeof(epc->function_num_map) * 8;
 	int ret, epf, last_fn;
 	u32 reg, value;
+	u8 cap;
 
+	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_EXP);
 	/*
 	 * BIT(0) is hardwired to 1, hence function 0 is always enabled
 	 * and can't be disabled anyway.
@@ -587,12 +595,10 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
 				continue;
 
 			value = cdns_pcie_ep_fn_readl(pcie, epf,
-					CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET +
-					PCI_EXP_DEVCAP);
+						      cap + PCI_EXP_DEVCAP);
 			value &= ~PCI_EXP_DEVCAP_FLR;
-			cdns_pcie_ep_fn_writel(pcie, epf,
-					CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET +
-					PCI_EXP_DEVCAP, value);
+			cdns_pcie_ep_fn_writel(pcie, epf, cap + PCI_EXP_DEVCAP,
+					       value);
 		}
 	}
 
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 0a4a8bfd3174..e7c108f6e0b2 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -125,11 +125,6 @@
  */
 #define CDNS_PCIE_EP_FUNC_BASE(fn)	(((fn) << 12) & GENMASK(19, 12))
 
-#define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET	0x90
-#define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET	0xb0
-#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET	0xc0
-#define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET	0x200
-
 /*
  * Endpoint PF Registers
  */
-- 
2.25.1

Re: [PATCH v12 0/6] Refactor capability search into common macros

[Hans Zhang]

Dear Ilpo,

May I ask if you have time to review the patches of this series? If 
there are any problems, I can continue to improve. If you think there is 
no problem, could you review it and add review tags?


Best regards,
Hans

On 2025/5/15 00:12, Hans Zhang wrote:
> Dear Maintainers,
> 
> This patch series addresses long-standing code duplication in PCI
> capability discovery logic across the PCI core and controller drivers.
> The existing implementation ties capability search to fully initialized
> PCI device structures, limiting its usability during early controller
> initialization phases where device/bus structures may not yet be
> available.
> 
> The primary goal is to decouple capability discovery from PCI device
> dependencies by introducing a unified framework using config space
> accessor-based macros. This enables:
> 
> 1. Early Capability Discovery: Host controllers (e.g., Cadence, DWC)
> can now perform capability searches during pre-initialization stages
> using their native config accessors.
> 
> 2. Code Consolidation: Common logic for standard and extended capability
> searches is refactored into shared macros (`PCI_FIND_NEXT_CAP_TTL` and
> `PCI_FIND_NEXT_EXT_CAPABILITY`), eliminating redundant implementations.
> 
> 3. Safety and Maintainability: TTL checks are centralized within the
> macros to prevent infinite loops, while hardcoded offsets in drivers
> are replaced with dynamic discovery, reducing fragility.
> 
> Key improvements include:
> - Driver Conversions: DesignWare and Cadence drivers are migrated to
>    use the new macros, removing device-specific assumptions and ensuring
>    consistent error handling.
> 
> - Enhanced Readability: Magic numbers are replaced with symbolic
>    constants, and config space accessors are standardized for clarity.
> 
> - Backward Compatibility: Existing PCI core behavior remains unchanged.
> 
> ---
> Changes since v11:
> - Resolved some compilation warning.
> - Add some include.
> - Add the *** BLURB HERE *** description(Corrected by Mani and Krzysztof).
> 
> Changes since v10:
> - The patch [v10 2/6] remove #include <uapi/linux/pci_regs.h> and add macro definition comments.
> - The patch [v10 3/6] remove #include <uapi/linux/pci_regs.h> and commit message were modified.
> - The other patches have not been modified.
> 
> Changes since v9:
> - Resolved [v9 4/6] compilation error.
>    The latest 6.15 rc1 merge __dw_pcie_find_vsec_capability, which uses
>    dw_pcie_find_next_ext_capability.
> - The other patches have not been modified.
> 
> Changes since v8:
> - Split patch.
> - The patch commit message were modified.
> - Other patches(4/6, 5/6, 6/6) are unchanged.
> 
> Changes since v7:
> - Patch 2/5 and 3/5 compilation error resolved.
> - Other patches are unchanged.
> 
> Changes since v6:
> - Refactor capability search into common macros.
> - Delete pci-host-helpers.c and MAINTAINERS.
> 
> Changes since v5:
> - 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.
> - Add new patch for MAINTAINERS.
> 
> Changes since v4:
> - Resolved [v4 1/4] compilation warning.
> - The patch subject and commit message were modified.
> 
> Changes since v3:
> - Resolved [v3 1/4] compilation error.
> - Other patches are not modified.
> 
> Changes since v2:
> - Add and split into a series of patches.
> ---
> 
> Hans Zhang (6):
>    PCI: Introduce generic bus config read helper function
>    PCI: Clean up __pci_find_next_cap_ttl() readability
>    PCI: Refactor capability search into common macros
>    PCI: dwc: Use common PCI host bridge APIs for finding the capabilities
>    PCI: cadence: Use common PCI host bridge APIs for finding the
>      capabilities
>    PCI: cadence: Use cdns_pcie_find_*capability to avoid hardcode.
> 
>   drivers/pci/access.c                          | 17 ++++
>   .../pci/controller/cadence/pcie-cadence-ep.c  | 40 +++++----
>   drivers/pci/controller/cadence/pcie-cadence.c | 28 ++++++
>   drivers/pci/controller/cadence/pcie-cadence.h | 18 ++--
>   drivers/pci/controller/dwc/pcie-designware.c  | 81 +++--------------
>   drivers/pci/pci.c                             | 68 ++------------
>   drivers/pci/pci.h                             | 88 +++++++++++++++++++
>   include/uapi/linux/pci_regs.h                 |  2 +
>   8 files changed, 194 insertions(+), 148 deletions(-)
> 
> 
> base-commit: 82f2b0b97b36ee3fcddf0f0780a9a0825d52fec3

Re: [PATCH v12 1/6] PCI: Introduce generic bus config read helper function

[Ilpo Järvinen]

On Thu, 15 May 2025, Hans Zhang wrote:

> The primary PCI config space accessors are tied to the size of the read
> (byte/word/dword). Upcoming refactoring of PCI capability discovery logic
> requires passing a config accessor function that must be able to perform
> read with different sizes.
> 
> Add any size config space read accessor pci_bus_read_config() to allow
> giving it as the config space accessor to the upcoming PCI capability
> discovery macro.
> 
> Reconstructs the PCI function discovery logic to prepare for unified
> configuration of access modes. No function changes are intended.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> Changes since v9 ~ v11:
> - None
> 
> Changes since v8:
> - The new split is patch 1/6.
> - The patch commit message were modified.
> ---
>  drivers/pci/access.c | 17 +++++++++++++++++
>  drivers/pci/pci.h    |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index b123da16b63b..603332658ab3 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -85,6 +85,23 @@ EXPORT_SYMBOL(pci_bus_write_config_byte);
>  EXPORT_SYMBOL(pci_bus_write_config_word);
>  EXPORT_SYMBOL(pci_bus_write_config_dword);
>  
> +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
> +			u32 *val)
> +{
> +	struct pci_bus *bus = priv;
> +	int ret;
> +
> +	if (size == 1)
> +		ret = pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
> +	else if (size == 2)
> +		ret = pci_bus_read_config_word(bus, devfn, where, (u16 *)val);
> +	else
> +		ret = pci_bus_read_config_dword(bus, devfn, where, val);

Perhaps this should check also size == 4 and return 
PCIBIOS_BAD_REGISTER_NUMBER in case size is wrong.

> +
> +	return ret;


> +}
> +EXPORT_SYMBOL_GPL(pci_bus_read_config);

Does this even need to be exported? Isn't the capability search always 
built in?

>  int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
>  			    int where, int size, u32 *val)
>  {
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index b81e99cd4b62..5e1477d6e254 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -88,6 +88,8 @@ extern bool pci_early_dump;
>  bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
>  bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
>  bool pcie_cap_has_rtctl(const struct pci_dev *dev);
> +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
> +			u32 *val);
>  
>  /* Functions internal to the PCI core code */
>  
> 

-- 
 i.

Re: [PATCH v12 1/6] PCI: Introduce generic bus config read helper function

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2878 bytes --]

On Tue, 3 Jun 2025, Ilpo Järvinen wrote:

> On Thu, 15 May 2025, Hans Zhang wrote:
> 
> > The primary PCI config space accessors are tied to the size of the read
> > (byte/word/dword). Upcoming refactoring of PCI capability discovery logic
> > requires passing a config accessor function that must be able to perform
> > read with different sizes.
> > 
> > Add any size config space read accessor pci_bus_read_config() to allow
> > giving it as the config space accessor to the upcoming PCI capability
> > discovery macro.
> > 
> > Reconstructs the PCI function discovery logic to prepare for unified
> > configuration of access modes. No function changes are intended.
> > 
> > Signed-off-by: Hans Zhang <18255117159@163.com>
> > ---
> > Changes since v9 ~ v11:
> > - None
> > 
> > Changes since v8:
> > - The new split is patch 1/6.
> > - The patch commit message were modified.
> > ---
> >  drivers/pci/access.c | 17 +++++++++++++++++
> >  drivers/pci/pci.h    |  2 ++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> > index b123da16b63b..603332658ab3 100644
> > --- a/drivers/pci/access.c
> > +++ b/drivers/pci/access.c
> > @@ -85,6 +85,23 @@ EXPORT_SYMBOL(pci_bus_write_config_byte);
> >  EXPORT_SYMBOL(pci_bus_write_config_word);
> >  EXPORT_SYMBOL(pci_bus_write_config_dword);
> >  
> > +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
> > +			u32 *val)
> > +{
> > +	struct pci_bus *bus = priv;
> > +	int ret;
> > +
> > +	if (size == 1)
> > +		ret = pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
> > +	else if (size == 2)
> > +		ret = pci_bus_read_config_word(bus, devfn, where, (u16 *)val);
> > +	else
> > +		ret = pci_bus_read_config_dword(bus, devfn, where, val);
> 
> Perhaps this should check also size == 4 and return 
> PCIBIOS_BAD_REGISTER_NUMBER in case size is wrong.
> 
> > +
> > +	return ret;

I'd also forgo ret variable and return directly.

> > +}
> > +EXPORT_SYMBOL_GPL(pci_bus_read_config);
> 
> Does this even need to be exported? Isn't the capability search always 
> built in?
> 
> >  int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
> >  			    int where, int size, u32 *val)
> >  {
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index b81e99cd4b62..5e1477d6e254 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -88,6 +88,8 @@ extern bool pci_early_dump;
> >  bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
> >  bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
> >  bool pcie_cap_has_rtctl(const struct pci_dev *dev);
> > +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
> > +			u32 *val);
> >  
> >  /* Functions internal to the PCI core code */
> >  
> > 
> 
> 

-- 
 i.

Re: [PATCH v12 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability

[Ilpo Järvinen]

On Thu, 15 May 2025, Hans Zhang wrote:

> Refactor the __pci_find_next_cap_ttl() to improve code clarity:
> - Replace magic number 0x40 with PCI_STD_HEADER_SIZEOF.
> - Use ALIGN_DOWN() for position alignment instead of manual bitmask.
> - Extract PCI capability fields via FIELD_GET() with standardized masks.
> - Add necessary headers (linux/align.h).
> 
> The changes are purely non-functional cleanups, ensuring behavior remains
> identical to the original implementation.

If you want a simpler wording for this, this is often used:

No functional changes intended.

> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> Changes since v11:
> - None
> 
> Changes since v10:
> - Remove #include <uapi/linux/pci_regs.h> and add macro definition comments.
> 
> Changes since v9:
> - None
> 
> Changes since v8:
> - Split into patch 1/6, patch 2/6.
> - The
> ---
>  drivers/pci/pci.c             | 9 +++++----
>  include/uapi/linux/pci_regs.h | 2 ++
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e77d5b53c0ce..27d2adb18a30 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/acpi.h>
> +#include <linux/align.h>
>  #include <linux/kernel.h>
>  #include <linux/delay.h>
>  #include <linux/dmi.h>
> @@ -432,17 +433,17 @@ static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>  	pci_bus_read_config_byte(bus, devfn, pos, &pos);
>  
>  	while ((*ttl)--) {
> -		if (pos < 0x40)
> +		if (pos < PCI_STD_HEADER_SIZEOF)
>  			break;
> -		pos &= ~3;
> +		pos = ALIGN_DOWN(pos, 4);
>  		pci_bus_read_config_word(bus, devfn, pos, &ent);
>  
> -		id = ent & 0xff;
> +		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
>  		if (id == 0xff)
>  			break;
>  		if (id == cap)
>  			return pos;
> -		pos = (ent >> 8);
> +		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
>  	}
>  	return 0;
>  }
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index ba326710f9c8..35051f9ac16a 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -206,6 +206,8 @@
>  /* 0x48-0x7f reserved */
>  
>  /* Capability lists */
> +#define PCI_CAP_ID_MASK		0x00ff	/* Capability ID mask */
> +#define PCI_CAP_LIST_NEXT_MASK	0xff00	/* Next Capability Pointer mask */
>  
>  #define PCI_CAP_LIST_ID		0	/* Capability ID */

I'd add those here with the extra space before name and add empty line in 
between them and the capability id list.

>  #define  PCI_CAP_ID_PM		0x01	/* Power Management */
> 

-- 
 i.

Re: [PATCH v12 3/6] PCI: Refactor capability search into common macros

[Ilpo Järvinen]

On Thu, 15 May 2025, Hans Zhang wrote:

> The PCI Capability search functionality is duplicated across the PCI core
> and several controller drivers. The core's current implementation requires
> fully initialized PCI device and bus structures, which prevents controller
> drivers from using it during early initialization phases before these
> structures are available.
> 
> Move the Capability search logic into a header-based macro that accepts a
> config space accessor function as an argument. This enables controller
> drivers to perform Capability discovery using their early access
> mechanisms prior to full device initialization while sharing the
> Capability search code.
> 
> Convert the existing PCI core Capability search implementation to use this
> new macro. Controller drivers can later use the same macros with their
> early access mechanisms while maintaining the existing protection against
> infinite loops through preserved TTL checks.
> 
> The ttl parameter was originally an additional safeguard to prevent
> infinite loops in corrupted config space.  However, the
> PCI_FIND_NEXT_CAP_TTL macro already enforces a TTL limit internally.

PCI_FIND_NEXT_CAP_TTL()

> Removing redundant ttl handling simplifies the interface while maintaining
> the safety guarantee. This aligns with the macro's design intent of
> encapsulating TTL management.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> Changes since v11:
> - Add #include <linux/bitfield.h>, solve the compilation warnings caused by the subsequent patch calls.
> 
> Changes since v10:
> - Remove #include <uapi/linux/pci_regs.h>.
> - The patch commit message were modified.
> 
> Changes since v9:
> - None
> 
> Changes since v8:
> - The patch commit message were modified.
> ---
>  drivers/pci/pci.c | 69 +++++--------------------------------
>  drivers/pci/pci.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 27d2adb18a30..271d922abdcc 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -9,7 +9,6 @@
>   */
>  
>  #include <linux/acpi.h>
> -#include <linux/align.h>
>  #include <linux/kernel.h>
>  #include <linux/delay.h>
>  #include <linux/dmi.h>
> @@ -425,35 +424,16 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>  }
>  
>  static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
> -				  u8 pos, int cap, int *ttl)
> +				  u8 pos, int cap)
>  {
> -	u8 id;
> -	u16 ent;
> -
> -	pci_bus_read_config_byte(bus, devfn, pos, &pos);
> -
> -	while ((*ttl)--) {
> -		if (pos < PCI_STD_HEADER_SIZEOF)
> -			break;
> -		pos = ALIGN_DOWN(pos, 4);
> -		pci_bus_read_config_word(bus, devfn, pos, &ent);
> -
> -		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
> -		if (id == 0xff)
> -			break;
> -		if (id == cap)
> -			return pos;
> -		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
> -	}
> -	return 0;
> +	return PCI_FIND_NEXT_CAP_TTL(pci_bus_read_config, pos, cap, bus,
> +				     devfn);
>  }
>  
>  static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
>  			      u8 pos, int cap)
>  {
> -	int ttl = PCI_FIND_CAP_TTL;
> -
> -	return __pci_find_next_cap_ttl(bus, devfn, pos, cap, &ttl);
> +	return __pci_find_next_cap_ttl(bus, devfn, pos, cap);
>  }

Please just get rid of the ttl variant, use PCI_FIND_NEXT_CAP_TTL() 
directly here, and adjust the other callers of the ttl variable to call 
this one instead.

>  
>  u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap)
> @@ -554,42 +534,11 @@ EXPORT_SYMBOL(pci_bus_find_capability);
>   */
>  u16 pci_find_next_ext_capability(struct pci_dev *dev, u16 start, int cap)
>  {
> -	u32 header;
> -	int ttl;
> -	u16 pos = PCI_CFG_SPACE_SIZE;
> -
> -	/* minimum 8 bytes per capability */
> -	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> -
>  	if (dev->cfg_size <= PCI_CFG_SPACE_SIZE)
>  		return 0;
>  
> -	if (start)
> -		pos = start;
> -
> -	if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
> -		return 0;
> -
> -	/*
> -	 * 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;
> -
> -		if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
> -			break;
> -	}
> -
> -	return 0;
> +	return PCI_FIND_NEXT_EXT_CAPABILITY(pci_bus_read_config, start, cap,
> +					    dev->bus, dev->devfn);
>  }
>  EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
>  
> @@ -649,7 +598,7 @@ EXPORT_SYMBOL_GPL(pci_get_dsn);
>  
>  static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>  {
> -	int rc, ttl = PCI_FIND_CAP_TTL;
> +	int rc;
>  	u8 cap, mask;
>  
>  	if (ht_cap == HT_CAPTYPE_SLAVE || ht_cap == HT_CAPTYPE_HOST)
> @@ -658,7 +607,7 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>  		mask = HT_5BIT_CAP_MASK;
>  
>  	pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn, pos,
> -				      PCI_CAP_ID_HT, &ttl);
> +				      PCI_CAP_ID_HT);
>  	while (pos) {
>  		rc = pci_read_config_byte(dev, pos + 3, &cap);
>  		if (rc != PCIBIOS_SUCCESSFUL)
> @@ -669,7 +618,7 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>  
>  		pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn,
>  					      pos + PCI_CAP_LIST_NEXT,
> -					      PCI_CAP_ID_HT, &ttl);
> +					      PCI_CAP_ID_HT);
>  	}
>  
>  	return 0;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 5e1477d6e254..f9cf45026e6e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -2,6 +2,8 @@
>  #ifndef DRIVERS_PCI_H
>  #define DRIVERS_PCI_H
>  
> +#include <linux/align.h>
> +#include <linux/bitfield.h>
>  #include <linux/pci.h>
>  
>  struct pcie_tlp_log;
> @@ -91,6 +93,90 @@ bool pcie_cap_has_rtctl(const struct pci_dev *dev);
>  int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
>  			u32 *val);
>  
> +/* Standard Capability finder */
> +/**
> + * PCI_FIND_NEXT_CAP_TTL - Find a PCI standard capability
> + * @read_cfg: Function pointer for reading PCI config space
> + * @start: Starting position to begin search
> + * @cap: Capability ID to find
> + * @args: Arguments to pass to read_cfg function
> + *
> + * Iterates through the capability list in PCI config space to find
> + * the specified capability. Implements TTL (time-to-live) protection

to find @cap.

> + * against infinite loops.
> + *
> + * Returns: Position of the capability if found, 0 otherwise.
> + */
> +#define PCI_FIND_NEXT_CAP_TTL(read_cfg, start, cap, args...)		\
> +({									\
> +	int __ttl = PCI_FIND_CAP_TTL;					\
> +	u8 __id, __found_pos = 0;					\
> +	u8 __pos = (start);						\
> +	u16 __ent;							\
> +									\
> +	read_cfg(args, __pos, 1, (u32 *)&__pos);			\
> +									\
> +	while (__ttl--) {						\
> +		if (__pos < PCI_STD_HEADER_SIZEOF)			\
> +			break;						\
> +									\
> +		__pos = ALIGN_DOWN(__pos, 4);				\
> +		read_cfg(args, __pos, 2, (u32 *)&__ent);		\
> +									\
> +		__id = FIELD_GET(PCI_CAP_ID_MASK, __ent);		\
> +		if (__id == 0xff)					\
> +			break;						\
> +									\
> +		if (__id == (cap)) {					\
> +			__found_pos = __pos;				\
> +			break;						\
> +		}							\
> +									\
> +		__pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, __ent);	\
> +	}								\
> +	__found_pos;							\
> +})
> +
> +/* Extended Capability finder */
> +/**
> + * PCI_FIND_NEXT_EXT_CAPABILITY - Find a PCI extended capability
> + * @read_cfg: Function pointer for reading PCI config space
> + * @start: Starting position to begin search (0 for initial search)
> + * @cap: Extended capability ID to find
> + * @args: Arguments to pass to read_cfg function
> + *
> + * Searches the extended capability space in PCI config registers
> + * for the specified capability. Implements TTL protection against

for @cap.

> + * infinite loops using a calculated maximum search count.
> + *
> + * Returns: Position of the capability if found, 0 otherwise.
> + */
> +#define PCI_FIND_NEXT_EXT_CAPABILITY(read_cfg, start, cap, args...)		\
> +({										\
> +	u16 __pos = (start) ?: PCI_CFG_SPACE_SIZE;				\
> +	u16 __found_pos = 0;							\
> +	int __ttl, __ret;							\
> +	u32 __header;								\
> +										\
> +	__ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;		\
> +	while (__ttl-- > 0 && __pos >= PCI_CFG_SPACE_SIZE) {			\
> +		__ret = read_cfg(args, __pos, 4, &__header);			\
> +		if (__ret != PCIBIOS_SUCCESSFUL)				\
> +			break;							\
> +										\
> +		if (__header == 0)						\
> +			break;							\
> +										\
> +		if (PCI_EXT_CAP_ID(__header) == (cap) && __pos != start) {	\
> +			__found_pos = __pos;					\
> +			break;							\
> +		}								\
> +										\
> +		__pos = PCI_EXT_CAP_NEXT(__header);				\
> +	}									\
> +	__found_pos;								\
> +})
> +
>  /* Functions internal to the PCI core code */
>  
>  #ifdef CONFIG_DMI
> 

-- 
 i.

Re: [PATCH v12 4/6] PCI: dwc: Use common PCI host bridge APIs for finding the capabilities

[Ilpo Järvinen]

On Thu, 15 May 2025, Hans Zhang wrote:

> Use the PCI core is now exposing generic macros for the host bridges to
> search for the PCIe capabilities, make use of them in the DWC driver.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> Changes since v11:
> - Resolve compilation errors. s/dw_pcie_read_dbi/dw_pcie_read*_dbi
> 
> Changes since v10:
> - None
> 
> Changes since v9:
> - Resolved [v9 4/6] compilation error.
>   The latest 6.15 rc1 merge __dw_pcie_find_vsec_capability, which uses 
>   dw_pcie_find_next_ext_capability.
> 
> Changes since v8:
> - None
> 
> Changes since v7:
> - Resolve compilation errors.
> 
> Changes since v6:
> https://lore.kernel.org/linux-pci/20250323164852.430546-3-18255117159@163.com/
> 
> - The patch commit message were modified.
> 
> Changes since v5:
> https://lore.kernel.org/linux-pci/20250321163803.391056-3-18255117159@163.com/
> 
> - Kconfig add "select PCI_HOST_HELPERS"
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 81 ++++----------------
>  1 file changed, 14 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 97d76d3dc066..7939411a24eb 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -205,83 +205,30 @@ void dw_pcie_version_detect(struct dw_pcie *pci)
>  		pci->type = ver;
>  }
>  
> -/*
> - * 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 __dw_pcie_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> -				  u8 cap)
> +static int dw_pcie_read_cfg(void *priv, int where, int size, u32 *val)
>  {
> -	u8 cap_id, next_cap_ptr;
> -	u16 reg;
> -
> -	if (!cap_ptr)
> -		return 0;
> +	struct dw_pcie *pci = priv;
>  
> -	reg = dw_pcie_readw_dbi(pci, cap_ptr);
> -	cap_id = (reg & 0x00ff);
> -
> -	if (cap_id > PCI_CAP_ID_MAX)
> -		return 0;
> -
> -	if (cap_id == cap)
> -		return cap_ptr;
> +	if (size == 4)
> +		*val = dw_pcie_readl_dbi(pci, where);
> +	else if (size == 2)
> +		*val = dw_pcie_readw_dbi(pci, where);
> +	else if (size == 1)
> +		*val = dw_pcie_readb_dbi(pci, where);

Maybe here as well return error if the given size is invalid.
>  
> -	next_cap_ptr = (reg & 0xff00) >> 8;
> -	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
> +	return PCIBIOS_SUCCESSFUL;
>  }
>  
>  u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap)
>  {
> -	u8 next_cap_ptr;
> -	u16 reg;
> -
> -	reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
> -	next_cap_ptr = (reg & 0x00ff);
> -
> -	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
> +	return PCI_FIND_NEXT_CAP_TTL(dw_pcie_read_cfg, PCI_CAPABILITY_LIST, cap,
> +				     pci);
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_find_capability);
>  
> -static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, 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 = dw_pcie_readl_dbi(pci, pos);
> -	/*
> -	 * 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 = dw_pcie_readl_dbi(pci, pos);
> -	}
> -
> -	return 0;
> -}
> -
>  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)
>  {
> -	return dw_pcie_find_next_ext_capability(pci, 0, cap);
> +	return PCI_FIND_NEXT_EXT_CAPABILITY(dw_pcie_read_cfg, 0, cap, pci);
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);
>  
> @@ -294,8 +241,8 @@ static u16 __dw_pcie_find_vsec_capability(struct dw_pcie *pci, u16 vendor_id,
>  	if (vendor_id != dw_pcie_readw_dbi(pci, PCI_VENDOR_ID))
>  		return 0;
>  
> -	while ((vsec = dw_pcie_find_next_ext_capability(pci, vsec,
> -						       PCI_EXT_CAP_ID_VNDR))) {
> +	while ((vsec = PCI_FIND_NEXT_EXT_CAPABILITY(
> +			dw_pcie_read_cfg, vsec, PCI_EXT_CAP_ID_VNDR, pci))) {

Start the arguments from the first line and align the continuations to (.

>  		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
>  		if (PCI_VNDR_HEADER_ID(header) == vsec_id)
>  			return vsec;
> 

-- 
 i.

Re: [PATCH v12 6/6] PCI: cadence: Use cdns_pcie_find_*capability to avoid hardcode.

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 7641 bytes --]

On Thu, 15 May 2025, Hans Zhang wrote:

> The PCIe capability/extended capability offsets are not guaranteed to be
> the same across all SoCs integrating the Cadence PCIe IP. Hence, use the
> cdns_pcie_find_{ext}_capability() APIs for finding them.

A minor point perhaps, but IMO, controller drivers should use the core's 
capability search regardless of the offset being same or not. :-)

> This avoids hardcoding the offsets in the driver.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> Changes since v8 ~ v11:
> - None
> 
> Changes since v7:
> - Resolve compilation errors.
> 
> Changes since v6:
> https://lore.kernel.org/linux-pci/20250323164852.430546-4-18255117159@163.com/
> 
> - The patch commit message were modified.
> 
> Changes since v5:
> https://lore.kernel.org/linux-pci/20250321163803.391056-4-18255117159@163.com
> 
> - Kconfig add "select PCI_HOST_HELPERS"
> ---
>  .../pci/controller/cadence/pcie-cadence-ep.c  | 40 +++++++++++--------
>  drivers/pci/controller/cadence/pcie-cadence.h |  5 ---
>  2 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> index 599ec4b1223e..5c4b2151d181 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> @@ -19,12 +19,13 @@
>  
>  static u8 cdns_pcie_get_fn_from_vfn(struct cdns_pcie *pcie, u8 fn, u8 vfn)
>  {
> -	u32 cap = CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET;
>  	u32 first_vf_offset, stride;
> +	u16 cap;
>  
>  	if (vfn == 0)
>  		return fn;
>  
> +	cap = cdns_pcie_find_ext_capability(pcie, PCI_EXT_CAP_ID_SRIOV);
>  	first_vf_offset = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_SRIOV_VF_OFFSET);
>  	stride = cdns_pcie_ep_fn_readw(pcie, fn, cap +  PCI_SRIOV_VF_STRIDE);
>  	fn = fn + first_vf_offset + ((vfn - 1) * stride);
> @@ -36,10 +37,11 @@ static int cdns_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
>  				     struct pci_epf_header *hdr)
>  {
>  	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
> -	u32 cap = CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET;
>  	struct cdns_pcie *pcie = &ep->pcie;
>  	u32 reg;
> +	u16 cap;
>  
> +	cap = cdns_pcie_find_ext_capability(pcie, PCI_EXT_CAP_ID_SRIOV);
>  	if (vfn > 1) {
>  		dev_err(&epc->dev, "Only Virtual Function #1 has deviceID\n");
>  		return -EINVAL;
> @@ -224,9 +226,10 @@ static int cdns_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn, u8 mmc)
>  {
>  	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct cdns_pcie *pcie = &ep->pcie;
> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>  	u16 flags;
> +	u8 cap;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>  	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>  
>  	/*
> @@ -246,9 +249,10 @@ static int cdns_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)
>  {
>  	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct cdns_pcie *pcie = &ep->pcie;
> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>  	u16 flags, mme;
> +	u8 cap;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>  	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>  
>  	/* Validate that the MSI feature is actually enabled. */
> @@ -269,9 +273,10 @@ static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
>  {
>  	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct cdns_pcie *pcie = &ep->pcie;
> -	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
>  	u32 val, reg;
> +	u8 cap;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
>  	func_no = cdns_pcie_get_fn_from_vfn(pcie, func_no, vfunc_no);
>  
>  	reg = cap + PCI_MSIX_FLAGS;
> @@ -290,9 +295,10 @@ static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,
>  {
>  	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct cdns_pcie *pcie = &ep->pcie;
> -	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
>  	u32 val, reg;
> +	u8 cap;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
>  	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>  
>  	reg = cap + PCI_MSIX_FLAGS;
> @@ -378,11 +384,11 @@ static int cdns_pcie_ep_send_msi_irq(struct cdns_pcie_ep *ep, u8 fn, u8 vfn,
>  				     u8 interrupt_num)
>  {
>  	struct cdns_pcie *pcie = &ep->pcie;
> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>  	u16 flags, mme, data, data_mask;
> -	u8 msi_count;
>  	u64 pci_addr, pci_addr_mask = 0xff;
> +	u8 msi_count, cap;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>  	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>  
>  	/* Check whether the MSI feature has been enabled by the PCI host. */
> @@ -430,14 +436,14 @@ static int cdns_pcie_ep_map_msi_irq(struct pci_epc *epc, u8 fn, u8 vfn,
>  				    u32 *msi_addr_offset)
>  {
>  	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>  	struct cdns_pcie *pcie = &ep->pcie;
>  	u64 pci_addr, pci_addr_mask = 0xff;
>  	u16 flags, mme, data, data_mask;
> -	u8 msi_count;
> +	u8 msi_count, cap;
>  	int ret;
>  	int i;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>  	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>  
>  	/* Check whether the MSI feature has been enabled by the PCI host. */
> @@ -480,16 +486,16 @@ static int cdns_pcie_ep_map_msi_irq(struct pci_epc *epc, u8 fn, u8 vfn,
>  static int cdns_pcie_ep_send_msix_irq(struct cdns_pcie_ep *ep, u8 fn, u8 vfn,
>  				      u16 interrupt_num)
>  {
> -	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
>  	u32 tbl_offset, msg_data, reg;
>  	struct cdns_pcie *pcie = &ep->pcie;
>  	struct pci_epf_msix_tbl *msix_tbl;
>  	struct cdns_pcie_epf *epf;
>  	u64 pci_addr_mask = 0xff;
>  	u64 msg_addr;
> +	u8 bir, cap;
>  	u16 flags;
> -	u8 bir;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
>  	epf = &ep->epf[fn];
>  	if (vfn > 0)
>  		epf = &epf->epf[vfn - 1];
> @@ -563,7 +569,9 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
>  	int max_epfs = sizeof(epc->function_num_map) * 8;
>  	int ret, epf, last_fn;
>  	u32 reg, value;
> +	u8 cap;
>  
> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_EXP);
>  	/*
>  	 * BIT(0) is hardwired to 1, hence function 0 is always enabled
>  	 * and can't be disabled anyway.
> @@ -587,12 +595,10 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
>  				continue;
>  
>  			value = cdns_pcie_ep_fn_readl(pcie, epf,
> -					CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET +
> -					PCI_EXP_DEVCAP);
> +						      cap + PCI_EXP_DEVCAP);
>  			value &= ~PCI_EXP_DEVCAP_FLR;
> -			cdns_pcie_ep_fn_writel(pcie, epf,
> -					CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET +
> -					PCI_EXP_DEVCAP, value);
> +			cdns_pcie_ep_fn_writel(pcie, epf, cap + PCI_EXP_DEVCAP,
> +					       value);
>  		}
>  	}
>  
> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> index 0a4a8bfd3174..e7c108f6e0b2 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence.h
> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
> @@ -125,11 +125,6 @@
>   */
>  #define CDNS_PCIE_EP_FUNC_BASE(fn)	(((fn) << 12) & GENMASK(19, 12))
>  
> -#define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET	0x90
> -#define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET	0xb0
> -#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET	0xc0
> -#define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET	0x200
> -
>  /*
>   * Endpoint PF Registers
>   */
> 

Nice to see these go away.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH v12 1/6] PCI: Introduce generic bus config read helper function

[Hans Zhang]

On 2025/6/3 17:18, Ilpo Järvinen wrote:
> On Thu, 15 May 2025, Hans Zhang wrote:
> 
>> The primary PCI config space accessors are tied to the size of the read
>> (byte/word/dword). Upcoming refactoring of PCI capability discovery logic
>> requires passing a config accessor function that must be able to perform
>> read with different sizes.
>>
>> Add any size config space read accessor pci_bus_read_config() to allow
>> giving it as the config space accessor to the upcoming PCI capability
>> discovery macro.
>>
>> Reconstructs the PCI function discovery logic to prepare for unified
>> configuration of access modes. No function changes are intended.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> Changes since v9 ~ v11:
>> - None
>>
>> Changes since v8:
>> - The new split is patch 1/6.
>> - The patch commit message were modified.
>> ---
>>   drivers/pci/access.c | 17 +++++++++++++++++
>>   drivers/pci/pci.h    |  2 ++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
>> index b123da16b63b..603332658ab3 100644
>> --- a/drivers/pci/access.c
>> +++ b/drivers/pci/access.c
>> @@ -85,6 +85,23 @@ EXPORT_SYMBOL(pci_bus_write_config_byte);
>>   EXPORT_SYMBOL(pci_bus_write_config_word);
>>   EXPORT_SYMBOL(pci_bus_write_config_dword);
>>   
>> +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
>> +			u32 *val)
>> +{
>> +	struct pci_bus *bus = priv;
>> +	int ret;
>> +
>> +	if (size == 1)
>> +		ret = pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
>> +	else if (size == 2)
>> +		ret = pci_bus_read_config_word(bus, devfn, where, (u16 *)val);
>> +	else
>> +		ret = pci_bus_read_config_dword(bus, devfn, where, val);
> 
> Perhaps this should check also size == 4 and return
> PCIBIOS_BAD_REGISTER_NUMBER in case size is wrong.
> 

Dear Ilpo,

Thank you very much for your reply. Will change.

I will send the next version after v6.16.

Best regards,
Hans

>> +
>> +	return ret;
> 
> 
>> +}
>> +EXPORT_SYMBOL_GPL(pci_bus_read_config);
> 
> Does this even need to be exported? Isn't the capability search always
> built in?
> 
>>   int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
>>   			    int where, int size, u32 *val)
>>   {
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index b81e99cd4b62..5e1477d6e254 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -88,6 +88,8 @@ extern bool pci_early_dump;
>>   bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
>>   bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
>>   bool pcie_cap_has_rtctl(const struct pci_dev *dev);
>> +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
>> +			u32 *val);
>>   
>>   /* Functions internal to the PCI core code */
>>   
>>
> 

Re: [PATCH v12 1/6] PCI: Introduce generic bus config read helper function

[Hans Zhang]

On 2025/6/3 17:21, Ilpo Järvinen wrote:
> On Tue, 3 Jun 2025, Ilpo Järvinen wrote:
> 
>> On Thu, 15 May 2025, Hans Zhang wrote:
>>
>>> The primary PCI config space accessors are tied to the size of the read
>>> (byte/word/dword). Upcoming refactoring of PCI capability discovery logic
>>> requires passing a config accessor function that must be able to perform
>>> read with different sizes.
>>>
>>> Add any size config space read accessor pci_bus_read_config() to allow
>>> giving it as the config space accessor to the upcoming PCI capability
>>> discovery macro.
>>>
>>> Reconstructs the PCI function discovery logic to prepare for unified
>>> configuration of access modes. No function changes are intended.
>>>
>>> Signed-off-by: Hans Zhang <18255117159@163.com>
>>> ---
>>> Changes since v9 ~ v11:
>>> - None
>>>
>>> Changes since v8:
>>> - The new split is patch 1/6.
>>> - The patch commit message were modified.
>>> ---
>>>   drivers/pci/access.c | 17 +++++++++++++++++
>>>   drivers/pci/pci.h    |  2 ++
>>>   2 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
>>> index b123da16b63b..603332658ab3 100644
>>> --- a/drivers/pci/access.c
>>> +++ b/drivers/pci/access.c
>>> @@ -85,6 +85,23 @@ EXPORT_SYMBOL(pci_bus_write_config_byte);
>>>   EXPORT_SYMBOL(pci_bus_write_config_word);
>>>   EXPORT_SYMBOL(pci_bus_write_config_dword);
>>>   
>>> +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
>>> +			u32 *val)
>>> +{
>>> +	struct pci_bus *bus = priv;
>>> +	int ret;
>>> +
>>> +	if (size == 1)
>>> +		ret = pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
>>> +	else if (size == 2)
>>> +		ret = pci_bus_read_config_word(bus, devfn, where, (u16 *)val);
>>> +	else
>>> +		ret = pci_bus_read_config_dword(bus, devfn, where, val);
>>
>> Perhaps this should check also size == 4 and return
>> PCIBIOS_BAD_REGISTER_NUMBER in case size is wrong.
>>
>>> +
>>> +	return ret;
> 
> I'd also forgo ret variable and return directly.


Dear Ilpo,

Will delete ret.

Best regards,
Hans

> 
>>> +}
>>> +EXPORT_SYMBOL_GPL(pci_bus_read_config);
>>
>> Does this even need to be exported? Isn't the capability search always
>> built in?
>>
>>>   int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
>>>   			    int where, int size, u32 *val)
>>>   {
>>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>>> index b81e99cd4b62..5e1477d6e254 100644
>>> --- a/drivers/pci/pci.h
>>> +++ b/drivers/pci/pci.h
>>> @@ -88,6 +88,8 @@ extern bool pci_early_dump;
>>>   bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
>>>   bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
>>>   bool pcie_cap_has_rtctl(const struct pci_dev *dev);
>>> +int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
>>> +			u32 *val);
>>>   
>>>   /* Functions internal to the PCI core code */
>>>   
>>>
>>
>>
> 

Re: [PATCH v12 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability

[Hans Zhang]

On 2025/6/3 17:30, Ilpo Järvinen wrote:
> On Thu, 15 May 2025, Hans Zhang wrote:
> 
>> Refactor the __pci_find_next_cap_ttl() to improve code clarity:
>> - Replace magic number 0x40 with PCI_STD_HEADER_SIZEOF.
>> - Use ALIGN_DOWN() for position alignment instead of manual bitmask.
>> - Extract PCI capability fields via FIELD_GET() with standardized masks.
>> - Add necessary headers (linux/align.h).
>>
>> The changes are purely non-functional cleanups, ensuring behavior remains
>> identical to the original implementation.
> 
> If you want a simpler wording for this, this is often used:
> 
> No functional changes intended.

Dear Ilpo,

Will change.

> 
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> Changes since v11:
>> - None
>>
>> Changes since v10:
>> - Remove #include <uapi/linux/pci_regs.h> and add macro definition comments.
>>
>> Changes since v9:
>> - None
>>
>> Changes since v8:
>> - Split into patch 1/6, patch 2/6.
>> - The
>> ---
>>   drivers/pci/pci.c             | 9 +++++----
>>   include/uapi/linux/pci_regs.h | 2 ++
>>   2 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index e77d5b53c0ce..27d2adb18a30 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -9,6 +9,7 @@
>>    */
>>   
>>   #include <linux/acpi.h>
>> +#include <linux/align.h>
>>   #include <linux/kernel.h>
>>   #include <linux/delay.h>
>>   #include <linux/dmi.h>
>> @@ -432,17 +433,17 @@ static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>>   	pci_bus_read_config_byte(bus, devfn, pos, &pos);
>>   
>>   	while ((*ttl)--) {
>> -		if (pos < 0x40)
>> +		if (pos < PCI_STD_HEADER_SIZEOF)
>>   			break;
>> -		pos &= ~3;
>> +		pos = ALIGN_DOWN(pos, 4);
>>   		pci_bus_read_config_word(bus, devfn, pos, &ent);
>>   
>> -		id = ent & 0xff;
>> +		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
>>   		if (id == 0xff)
>>   			break;
>>   		if (id == cap)
>>   			return pos;
>> -		pos = (ent >> 8);
>> +		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
>>   	}
>>   	return 0;
>>   }
>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>> index ba326710f9c8..35051f9ac16a 100644
>> --- a/include/uapi/linux/pci_regs.h
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -206,6 +206,8 @@
>>   /* 0x48-0x7f reserved */
>>   
>>   /* Capability lists */
>> +#define PCI_CAP_ID_MASK		0x00ff	/* Capability ID mask */
>> +#define PCI_CAP_LIST_NEXT_MASK	0xff00	/* Next Capability Pointer mask */
>>   
>>   #define PCI_CAP_LIST_ID		0	/* Capability ID */
> 
> I'd add those here with the extra space before name and add empty line in
> between them and the capability id list.
> 

Will change.


Best regards,
Hans

>>   #define  PCI_CAP_ID_PM		0x01	/* Power Management */
>>
> 

Re: [PATCH v12 3/6] PCI: Refactor capability search into common macros

[Hans Zhang]

On 2025/6/3 17:38, Ilpo Järvinen wrote:
> On Thu, 15 May 2025, Hans Zhang wrote:
> 
>> The PCI Capability search functionality is duplicated across the PCI core
>> and several controller drivers. The core's current implementation requires
>> fully initialized PCI device and bus structures, which prevents controller
>> drivers from using it during early initialization phases before these
>> structures are available.
>>
>> Move the Capability search logic into a header-based macro that accepts a
>> config space accessor function as an argument. This enables controller
>> drivers to perform Capability discovery using their early access
>> mechanisms prior to full device initialization while sharing the
>> Capability search code.
>>
>> Convert the existing PCI core Capability search implementation to use this
>> new macro. Controller drivers can later use the same macros with their
>> early access mechanisms while maintaining the existing protection against
>> infinite loops through preserved TTL checks.
>>
>> The ttl parameter was originally an additional safeguard to prevent
>> infinite loops in corrupted config space.  However, the
>> PCI_FIND_NEXT_CAP_TTL macro already enforces a TTL limit internally.
> 
> PCI_FIND_NEXT_CAP_TTL()

Dear Ilpo,

Will change.

> 
>> Removing redundant ttl handling simplifies the interface while maintaining
>> the safety guarantee. This aligns with the macro's design intent of
>> encapsulating TTL management.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> Changes since v11:
>> - Add #include <linux/bitfield.h>, solve the compilation warnings caused by the subsequent patch calls.
>>
>> Changes since v10:
>> - Remove #include <uapi/linux/pci_regs.h>.
>> - The patch commit message were modified.
>>
>> Changes since v9:
>> - None
>>
>> Changes since v8:
>> - The patch commit message were modified.
>> ---
>>   drivers/pci/pci.c | 69 +++++--------------------------------
>>   drivers/pci/pci.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 95 insertions(+), 60 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 27d2adb18a30..271d922abdcc 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -9,7 +9,6 @@
>>    */
>>   
>>   #include <linux/acpi.h>
>> -#include <linux/align.h>
>>   #include <linux/kernel.h>
>>   #include <linux/delay.h>
>>   #include <linux/dmi.h>
>> @@ -425,35 +424,16 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>>   }
>>   
>>   static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>> -				  u8 pos, int cap, int *ttl)
>> +				  u8 pos, int cap)
>>   {
>> -	u8 id;
>> -	u16 ent;
>> -
>> -	pci_bus_read_config_byte(bus, devfn, pos, &pos);
>> -
>> -	while ((*ttl)--) {
>> -		if (pos < PCI_STD_HEADER_SIZEOF)
>> -			break;
>> -		pos = ALIGN_DOWN(pos, 4);
>> -		pci_bus_read_config_word(bus, devfn, pos, &ent);
>> -
>> -		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
>> -		if (id == 0xff)
>> -			break;
>> -		if (id == cap)
>> -			return pos;
>> -		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
>> -	}
>> -	return 0;
>> +	return PCI_FIND_NEXT_CAP_TTL(pci_bus_read_config, pos, cap, bus,
>> +				     devfn);
>>   }

Will delete __pci_find_next_cap_ttl function.

>>   
>>   static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
>>   			      u8 pos, int cap)
>>   {
>> -	int ttl = PCI_FIND_CAP_TTL;
>> -
>> -	return __pci_find_next_cap_ttl(bus, devfn, pos, cap, &ttl);
>> +	return __pci_find_next_cap_ttl(bus, devfn, pos, cap);
>>   }
> 
> Please just get rid of the ttl variant, use PCI_FIND_NEXT_CAP_TTL()
> directly here, and adjust the other callers of the ttl variable to call
> this one instead.
> 

Will change.

>>   
>>   u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap)
>> @@ -554,42 +534,11 @@ EXPORT_SYMBOL(pci_bus_find_capability);
>>    */
>>   u16 pci_find_next_ext_capability(struct pci_dev *dev, u16 start, int cap)
>>   {
>> -	u32 header;
>> -	int ttl;
>> -	u16 pos = PCI_CFG_SPACE_SIZE;
>> -
>> -	/* minimum 8 bytes per capability */
>> -	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
>> -
>>   	if (dev->cfg_size <= PCI_CFG_SPACE_SIZE)
>>   		return 0;
>>   
>> -	if (start)
>> -		pos = start;
>> -
>> -	if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
>> -		return 0;
>> -
>> -	/*
>> -	 * 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;
>> -
>> -		if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
>> -			break;
>> -	}
>> -
>> -	return 0;
>> +	return PCI_FIND_NEXT_EXT_CAPABILITY(pci_bus_read_config, start, cap,
>> +					    dev->bus, dev->devfn);
>>   }
>>   EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
>>   
>> @@ -649,7 +598,7 @@ EXPORT_SYMBOL_GPL(pci_get_dsn);
>>   
>>   static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>>   {
>> -	int rc, ttl = PCI_FIND_CAP_TTL;
>> +	int rc;
>>   	u8 cap, mask;
>>   
>>   	if (ht_cap == HT_CAPTYPE_SLAVE || ht_cap == HT_CAPTYPE_HOST)
>> @@ -658,7 +607,7 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>>   		mask = HT_5BIT_CAP_MASK;
>>   
>>   	pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn, pos,

Will use PCI_FIND_NEXT_CAP_TTL().

>> -				      PCI_CAP_ID_HT, &ttl);
>> +				      PCI_CAP_ID_HT);
>>   	while (pos) {
>>   		rc = pci_read_config_byte(dev, pos + 3, &cap);
>>   		if (rc != PCIBIOS_SUCCESSFUL)
>> @@ -669,7 +618,7 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>>   
>>   		pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn,

Will use PCI_FIND_NEXT_CAP_TTL().

>>   					      pos + PCI_CAP_LIST_NEXT,
>> -					      PCI_CAP_ID_HT, &ttl);
>> +					      PCI_CAP_ID_HT);
>>   	}
>>   
>>   	return 0;
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 5e1477d6e254..f9cf45026e6e 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -2,6 +2,8 @@
>>   #ifndef DRIVERS_PCI_H
>>   #define DRIVERS_PCI_H
>>   
>> +#include <linux/align.h>
>> +#include <linux/bitfield.h>
>>   #include <linux/pci.h>
>>   
>>   struct pcie_tlp_log;
>> @@ -91,6 +93,90 @@ bool pcie_cap_has_rtctl(const struct pci_dev *dev);
>>   int pci_bus_read_config(void *priv, unsigned int devfn, int where, u32 size,
>>   			u32 *val);
>>   
>> +/* Standard Capability finder */
>> +/**
>> + * PCI_FIND_NEXT_CAP_TTL - Find a PCI standard capability
>> + * @read_cfg: Function pointer for reading PCI config space
>> + * @start: Starting position to begin search
>> + * @cap: Capability ID to find
>> + * @args: Arguments to pass to read_cfg function
>> + *
>> + * Iterates through the capability list in PCI config space to find
>> + * the specified capability. Implements TTL (time-to-live) protection
> 
> to find @cap.

Will change.

> 
>> + * against infinite loops.
>> + *
>> + * Returns: Position of the capability if found, 0 otherwise.
>> + */
>> +#define PCI_FIND_NEXT_CAP_TTL(read_cfg, start, cap, args...)		\
>> +({									\
>> +	int __ttl = PCI_FIND_CAP_TTL;					\
>> +	u8 __id, __found_pos = 0;					\
>> +	u8 __pos = (start);						\
>> +	u16 __ent;							\
>> +									\
>> +	read_cfg(args, __pos, 1, (u32 *)&__pos);			\
>> +									\
>> +	while (__ttl--) {						\
>> +		if (__pos < PCI_STD_HEADER_SIZEOF)			\
>> +			break;						\
>> +									\
>> +		__pos = ALIGN_DOWN(__pos, 4);				\
>> +		read_cfg(args, __pos, 2, (u32 *)&__ent);		\
>> +									\
>> +		__id = FIELD_GET(PCI_CAP_ID_MASK, __ent);		\
>> +		if (__id == 0xff)					\
>> +			break;						\
>> +									\
>> +		if (__id == (cap)) {					\
>> +			__found_pos = __pos;				\
>> +			break;						\
>> +		}							\
>> +									\
>> +		__pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, __ent);	\
>> +	}								\
>> +	__found_pos;							\
>> +})
>> +
>> +/* Extended Capability finder */
>> +/**
>> + * PCI_FIND_NEXT_EXT_CAPABILITY - Find a PCI extended capability
>> + * @read_cfg: Function pointer for reading PCI config space
>> + * @start: Starting position to begin search (0 for initial search)
>> + * @cap: Extended capability ID to find
>> + * @args: Arguments to pass to read_cfg function
>> + *
>> + * Searches the extended capability space in PCI config registers
>> + * for the specified capability. Implements TTL protection against
> 
> for @cap.

Will change.

Best regards,
Hans

> 
>> + * infinite loops using a calculated maximum search count.
>> + *
>> + * Returns: Position of the capability if found, 0 otherwise.
>> + */
>> +#define PCI_FIND_NEXT_EXT_CAPABILITY(read_cfg, start, cap, args...)		\
>> +({										\
>> +	u16 __pos = (start) ?: PCI_CFG_SPACE_SIZE;				\
>> +	u16 __found_pos = 0;							\
>> +	int __ttl, __ret;							\
>> +	u32 __header;								\
>> +										\
>> +	__ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;		\
>> +	while (__ttl-- > 0 && __pos >= PCI_CFG_SPACE_SIZE) {			\
>> +		__ret = read_cfg(args, __pos, 4, &__header);			\
>> +		if (__ret != PCIBIOS_SUCCESSFUL)				\
>> +			break;							\
>> +										\
>> +		if (__header == 0)						\
>> +			break;							\
>> +										\
>> +		if (PCI_EXT_CAP_ID(__header) == (cap) && __pos != start) {	\
>> +			__found_pos = __pos;					\
>> +			break;							\
>> +		}								\
>> +										\
>> +		__pos = PCI_EXT_CAP_NEXT(__header);				\
>> +	}									\
>> +	__found_pos;								\
>> +})
>> +
>>   /* Functions internal to the PCI core code */
>>   
>>   #ifdef CONFIG_DMI
>>
> 

Re: [PATCH v12 4/6] PCI: dwc: Use common PCI host bridge APIs for finding the capabilities

[Hans Zhang]

On 2025/6/3 17:42, Ilpo Järvinen wrote:
> On Thu, 15 May 2025, Hans Zhang wrote:
> 
>> Use the PCI core is now exposing generic macros for the host bridges to
>> search for the PCIe capabilities, make use of them in the DWC driver.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> Changes since v11:
>> - Resolve compilation errors. s/dw_pcie_read_dbi/dw_pcie_read*_dbi
>>
>> Changes since v10:
>> - None
>>
>> Changes since v9:
>> - Resolved [v9 4/6] compilation error.
>>    The latest 6.15 rc1 merge __dw_pcie_find_vsec_capability, which uses
>>    dw_pcie_find_next_ext_capability.
>>
>> Changes since v8:
>> - None
>>
>> Changes since v7:
>> - Resolve compilation errors.
>>
>> Changes since v6:
>> https://lore.kernel.org/linux-pci/20250323164852.430546-3-18255117159@163.com/
>>
>> - The patch commit message were modified.
>>
>> Changes since v5:
>> https://lore.kernel.org/linux-pci/20250321163803.391056-3-18255117159@163.com/
>>
>> - Kconfig add "select PCI_HOST_HELPERS"
>> ---
>>   drivers/pci/controller/dwc/pcie-designware.c | 81 ++++----------------
>>   1 file changed, 14 insertions(+), 67 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 97d76d3dc066..7939411a24eb 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -205,83 +205,30 @@ void dw_pcie_version_detect(struct dw_pcie *pci)
>>   		pci->type = ver;
>>   }
>>   
>> -/*
>> - * 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 __dw_pcie_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
>> -				  u8 cap)
>> +static int dw_pcie_read_cfg(void *priv, int where, int size, u32 *val)
>>   {
>> -	u8 cap_id, next_cap_ptr;
>> -	u16 reg;
>> -
>> -	if (!cap_ptr)
>> -		return 0;
>> +	struct dw_pcie *pci = priv;
>>   
>> -	reg = dw_pcie_readw_dbi(pci, cap_ptr);
>> -	cap_id = (reg & 0x00ff);
>> -
>> -	if (cap_id > PCI_CAP_ID_MAX)
>> -		return 0;
>> -
>> -	if (cap_id == cap)
>> -		return cap_ptr;
>> +	if (size == 4)
>> +		*val = dw_pcie_readl_dbi(pci, where);
>> +	else if (size == 2)
>> +		*val = dw_pcie_readw_dbi(pci, where);
>> +	else if (size == 1)
>> +		*val = dw_pcie_readb_dbi(pci, where);
> 
> Maybe here as well return error if the given size is invalid.

Dear Ilpo.

Will add return PCIBIOS_BAD_REGISTER_NUMBER.

>>   
>> -	next_cap_ptr = (reg & 0xff00) >> 8;
>> -	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
>> +	return PCIBIOS_SUCCESSFUL;
>>   }
>>   
>>   u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap)
>>   {
>> -	u8 next_cap_ptr;
>> -	u16 reg;
>> -
>> -	reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
>> -	next_cap_ptr = (reg & 0x00ff);
>> -
>> -	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
>> +	return PCI_FIND_NEXT_CAP_TTL(dw_pcie_read_cfg, PCI_CAPABILITY_LIST, cap,
>> +				     pci);
>>   }
>>   EXPORT_SYMBOL_GPL(dw_pcie_find_capability);
>>   
>> -static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, 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 = dw_pcie_readl_dbi(pci, pos);
>> -	/*
>> -	 * 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 = dw_pcie_readl_dbi(pci, pos);
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>>   u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)
>>   {
>> -	return dw_pcie_find_next_ext_capability(pci, 0, cap);
>> +	return PCI_FIND_NEXT_EXT_CAPABILITY(dw_pcie_read_cfg, 0, cap, pci);
>>   }
>>   EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);
>>   
>> @@ -294,8 +241,8 @@ static u16 __dw_pcie_find_vsec_capability(struct dw_pcie *pci, u16 vendor_id,
>>   	if (vendor_id != dw_pcie_readw_dbi(pci, PCI_VENDOR_ID))
>>   		return 0;
>>   
>> -	while ((vsec = dw_pcie_find_next_ext_capability(pci, vsec,
>> -						       PCI_EXT_CAP_ID_VNDR))) {
>> +	while ((vsec = PCI_FIND_NEXT_EXT_CAPABILITY(
>> +			dw_pcie_read_cfg, vsec, PCI_EXT_CAP_ID_VNDR, pci))) {
> 
> Start the arguments from the first line and align the continuations to (.

Will change.

Best regards,
Hans

> 
>>   		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
>>   		if (PCI_VNDR_HEADER_ID(header) == vsec_id)
>>   			return vsec;
>>
> 

Re: [PATCH v12 6/6] PCI: cadence: Use cdns_pcie_find_*capability to avoid hardcode.

[Hans Zhang]

On 2025/6/3 17:49, Ilpo Järvinen wrote:
> On Thu, 15 May 2025, Hans Zhang wrote:
> 
>> The PCIe capability/extended capability offsets are not guaranteed to be
>> the same across all SoCs integrating the Cadence PCIe IP. Hence, use the
>> cdns_pcie_find_{ext}_capability() APIs for finding them.
> 
> A minor point perhaps, but IMO, controller drivers should use the core's
> capability search regardless of the offset being same or not. :-)

Dear Ilpo,

I agree. In the future code iterations, we are committed to finding 
better methods.

> 
>> This avoids hardcoding the offsets in the driver.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> Changes since v8 ~ v11:
>> - None
>>
>> Changes since v7:
>> - Resolve compilation errors.
>>
>> Changes since v6:
>> https://lore.kernel.org/linux-pci/20250323164852.430546-4-18255117159@163.com/
>>
>> - The patch commit message were modified.
>>
>> Changes since v5:
>> https://lore.kernel.org/linux-pci/20250321163803.391056-4-18255117159@163.com
>>
>> - Kconfig add "select PCI_HOST_HELPERS"
>> ---
>>   .../pci/controller/cadence/pcie-cadence-ep.c  | 40 +++++++++++--------
>>   drivers/pci/controller/cadence/pcie-cadence.h |  5 ---
>>   2 files changed, 23 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
>> index 599ec4b1223e..5c4b2151d181 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
>> +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
>> @@ -19,12 +19,13 @@
>>   
>>   static u8 cdns_pcie_get_fn_from_vfn(struct cdns_pcie *pcie, u8 fn, u8 vfn)
>>   {
>> -	u32 cap = CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET;
>>   	u32 first_vf_offset, stride;
>> +	u16 cap;
>>   
>>   	if (vfn == 0)
>>   		return fn;
>>   
>> +	cap = cdns_pcie_find_ext_capability(pcie, PCI_EXT_CAP_ID_SRIOV);
>>   	first_vf_offset = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_SRIOV_VF_OFFSET);
>>   	stride = cdns_pcie_ep_fn_readw(pcie, fn, cap +  PCI_SRIOV_VF_STRIDE);
>>   	fn = fn + first_vf_offset + ((vfn - 1) * stride);
>> @@ -36,10 +37,11 @@ static int cdns_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
>>   				     struct pci_epf_header *hdr)
>>   {
>>   	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>> -	u32 cap = CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET;
>>   	struct cdns_pcie *pcie = &ep->pcie;
>>   	u32 reg;
>> +	u16 cap;
>>   
>> +	cap = cdns_pcie_find_ext_capability(pcie, PCI_EXT_CAP_ID_SRIOV);
>>   	if (vfn > 1) {
>>   		dev_err(&epc->dev, "Only Virtual Function #1 has deviceID\n");
>>   		return -EINVAL;
>> @@ -224,9 +226,10 @@ static int cdns_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn, u8 mmc)
>>   {
>>   	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>>   	struct cdns_pcie *pcie = &ep->pcie;
>> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>>   	u16 flags;
>> +	u8 cap;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>>   	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>>   
>>   	/*
>> @@ -246,9 +249,10 @@ static int cdns_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)
>>   {
>>   	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>>   	struct cdns_pcie *pcie = &ep->pcie;
>> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>>   	u16 flags, mme;
>> +	u8 cap;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>>   	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>>   
>>   	/* Validate that the MSI feature is actually enabled. */
>> @@ -269,9 +273,10 @@ static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
>>   {
>>   	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>>   	struct cdns_pcie *pcie = &ep->pcie;
>> -	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
>>   	u32 val, reg;
>> +	u8 cap;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
>>   	func_no = cdns_pcie_get_fn_from_vfn(pcie, func_no, vfunc_no);
>>   
>>   	reg = cap + PCI_MSIX_FLAGS;
>> @@ -290,9 +295,10 @@ static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,
>>   {
>>   	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>>   	struct cdns_pcie *pcie = &ep->pcie;
>> -	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
>>   	u32 val, reg;
>> +	u8 cap;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
>>   	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>>   
>>   	reg = cap + PCI_MSIX_FLAGS;
>> @@ -378,11 +384,11 @@ static int cdns_pcie_ep_send_msi_irq(struct cdns_pcie_ep *ep, u8 fn, u8 vfn,
>>   				     u8 interrupt_num)
>>   {
>>   	struct cdns_pcie *pcie = &ep->pcie;
>> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>>   	u16 flags, mme, data, data_mask;
>> -	u8 msi_count;
>>   	u64 pci_addr, pci_addr_mask = 0xff;
>> +	u8 msi_count, cap;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>>   	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>>   
>>   	/* Check whether the MSI feature has been enabled by the PCI host. */
>> @@ -430,14 +436,14 @@ static int cdns_pcie_ep_map_msi_irq(struct pci_epc *epc, u8 fn, u8 vfn,
>>   				    u32 *msi_addr_offset)
>>   {
>>   	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
>> -	u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
>>   	struct cdns_pcie *pcie = &ep->pcie;
>>   	u64 pci_addr, pci_addr_mask = 0xff;
>>   	u16 flags, mme, data, data_mask;
>> -	u8 msi_count;
>> +	u8 msi_count, cap;
>>   	int ret;
>>   	int i;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSI);
>>   	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
>>   
>>   	/* Check whether the MSI feature has been enabled by the PCI host. */
>> @@ -480,16 +486,16 @@ static int cdns_pcie_ep_map_msi_irq(struct pci_epc *epc, u8 fn, u8 vfn,
>>   static int cdns_pcie_ep_send_msix_irq(struct cdns_pcie_ep *ep, u8 fn, u8 vfn,
>>   				      u16 interrupt_num)
>>   {
>> -	u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
>>   	u32 tbl_offset, msg_data, reg;
>>   	struct cdns_pcie *pcie = &ep->pcie;
>>   	struct pci_epf_msix_tbl *msix_tbl;
>>   	struct cdns_pcie_epf *epf;
>>   	u64 pci_addr_mask = 0xff;
>>   	u64 msg_addr;
>> +	u8 bir, cap;
>>   	u16 flags;
>> -	u8 bir;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_MSIX);
>>   	epf = &ep->epf[fn];
>>   	if (vfn > 0)
>>   		epf = &epf->epf[vfn - 1];
>> @@ -563,7 +569,9 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
>>   	int max_epfs = sizeof(epc->function_num_map) * 8;
>>   	int ret, epf, last_fn;
>>   	u32 reg, value;
>> +	u8 cap;
>>   
>> +	cap = cdns_pcie_find_capability(pcie, PCI_CAP_ID_EXP);
>>   	/*
>>   	 * BIT(0) is hardwired to 1, hence function 0 is always enabled
>>   	 * and can't be disabled anyway.
>> @@ -587,12 +595,10 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
>>   				continue;
>>   
>>   			value = cdns_pcie_ep_fn_readl(pcie, epf,
>> -					CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET +
>> -					PCI_EXP_DEVCAP);
>> +						      cap + PCI_EXP_DEVCAP);
>>   			value &= ~PCI_EXP_DEVCAP_FLR;
>> -			cdns_pcie_ep_fn_writel(pcie, epf,
>> -					CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET +
>> -					PCI_EXP_DEVCAP, value);
>> +			cdns_pcie_ep_fn_writel(pcie, epf, cap + PCI_EXP_DEVCAP,
>> +					       value);
>>   		}
>>   	}
>>   
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
>> index 0a4a8bfd3174..e7c108f6e0b2 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence.h
>> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
>> @@ -125,11 +125,6 @@
>>    */
>>   #define CDNS_PCIE_EP_FUNC_BASE(fn)	(((fn) << 12) & GENMASK(19, 12))
>>   
>> -#define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET	0x90
>> -#define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET	0xb0
>> -#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET	0xc0
>> -#define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET	0x200
>> -
>>   /*
>>    * Endpoint PF Registers
>>    */
>>
> 
> Nice to see these go away.
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Thank you for your review.

Best regards,
Hans

>