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

[PATCH v15 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` and
`PCI_FIND_NEXT_EXT_CAP`), 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.

---
Dear Niklas and Gerd,

Can you test this series of patches on the s390?

Thank you very much.
---

---
Changes since v14:
- Delete the first patch in the v14 series.
- The functions that can obtain the values of the registers u8/u16/u32 are
  directly called in PCI_FIND_NEXT_CAP() and PCI_FIND_NEXT_EXT_CAP().
  Use the pci_bus_read_config_byte/word/dword function directly.
- Delete dw_pcie_read_cfg and redefine three functions: dw_pcie_read_cfg_byte/word/dword.
- Delete cdns_pcie_read_cfg and redefine three functions: cdns_pcie_read_cfg_byte/word/dword.

Changes since v13:
- Split patch 3/6 into two patches for searching standard and extended capability. (Bjorn)
- Optimize the code based on the review comments from Bjorn.
- Patch 5/7 and 6/7 use simplified macro definitions: PCI_FIND_NEXT_CAP(), PCI_FIND_NEXT_EXT_CAP().
- The other patches have not been modified.

Changes since v12:
- Modify some commit messages, code format issues, and optimize the function return values.

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: Clean up __pci_find_next_cap_ttl() readability
  PCI: Refactor capability search into PCI_FIND_NEXT_CAP()
  PCI: Refactor extended capability search into PCI_FIND_NEXT_EXT_CAP()
  PCI: dwc: Use PCI core APIs to find capabilities
  PCI: cadence: Use PCI core APIs to find capabilities
  PCI: cadence: Use cdns_pcie_find_*capability() to avoid hardcoding
    offsets

 .../pci/controller/cadence/pcie-cadence-ep.c  | 38 +++++----
 drivers/pci/controller/cadence/pcie-cadence.c | 14 +++
 drivers/pci/controller/cadence/pcie-cadence.h | 39 +++++++--
 drivers/pci/controller/dwc/pcie-designware.c  | 77 ++---------------
 drivers/pci/controller/dwc/pcie-designware.h  | 21 +++++
 drivers/pci/pci.c                             | 76 +++--------------
 drivers/pci/pci.h                             | 85 +++++++++++++++++++
 include/uapi/linux/pci_regs.h                 |  3 +
 8 files changed, 194 insertions(+), 159 deletions(-)


base-commit: 8742b2d8935f476449ef37e263bc4da3295c7b58
-- 
2.25.1

[PATCH v15 1/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).

No functional changes intended.

Signed-off-by: Hans Zhang <18255117159@163.com>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
---
 drivers/pci/pci.c             | 9 +++++----
 include/uapi/linux/pci_regs.h | 3 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b0f4d98036cd..40a5c87d9a6b 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 f5b17745de60..1bba99b46227 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -207,6 +207,9 @@
 
 /* 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 */
 #define  PCI_CAP_ID_AGP		0x02	/* Accelerated Graphics Port */
-- 
2.25.1

[PATCH v15 2/6] PCI: Refactor capability search into PCI_FIND_NEXT_CAP()

[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 PCI_FIND_NEXT_CAP() 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
PCI_FIND_NEXT_CAP().  Controller drivers can later use this with their
early access mechanisms while maintaining the existing protection against
infinite loops through preserved TTL checks.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/pci.c | 42 ++++++++----------------------------------
 drivers/pci/pci.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 40a5c87d9a6b..ac2658d946ea 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>
@@ -424,36 +423,10 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 	return 1;
 }
 
-static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
-				  u8 pos, int cap, int *ttl)
-{
-	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;
-}
-
 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(pci_bus_read_config, pos, cap, bus, devfn);
 }
 
 u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap)
@@ -649,7 +622,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)
@@ -657,8 +630,8 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
 	else
 		mask = HT_5BIT_CAP_MASK;
 
-	pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn, pos,
-				      PCI_CAP_ID_HT, &ttl);
+	pos = PCI_FIND_NEXT_CAP(pci_bus_read_config, pos,
+				PCI_CAP_ID_HT, dev->bus, dev->devfn);
 	while (pos) {
 		rc = pci_read_config_byte(dev, pos + 3, &cap);
 		if (rc != PCIBIOS_SUCCESSFUL)
@@ -667,9 +640,10 @@ static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
 		if ((cap & mask) == ht_cap)
 			return pos;
 
-		pos = __pci_find_next_cap_ttl(dev->bus, dev->devfn,
-					      pos + PCI_CAP_LIST_NEXT,
-					      PCI_CAP_ID_HT, &ttl);
+		pos = PCI_FIND_NEXT_CAP(pci_bus_read_config,
+					pos + PCI_CAP_LIST_NEXT,
+					PCI_CAP_ID_HT, dev->bus,
+					dev->devfn);
 	}
 
 	return 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 34f65d69662e..81580987509f 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;
@@ -88,6 +90,49 @@ 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);
 
+/* Standard Capability finder */
+/**
+ * PCI_FIND_NEXT_CAP - 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
+ *
+ * Search the capability list in PCI config space to find @cap.
+ * Implements TTL (time-to-live) protection against infinite loops.
+ *
+ * Return: Position of the capability if found, 0 otherwise.
+ */
+#define PCI_FIND_NEXT_CAP(read_cfg, start, cap, args...)		\
+({									\
+	int __ttl = PCI_FIND_CAP_TTL;					\
+	u8 __id, __found_pos = 0;					\
+	u8 __pos = (start);						\
+	u16 __ent;							\
+									\
+	read_cfg##_byte(args, __pos, &__pos);				\
+									\
+	while (__ttl--) {						\
+		if (__pos < PCI_STD_HEADER_SIZEOF)			\
+			break;						\
+									\
+		__pos = ALIGN_DOWN(__pos, 4);				\
+		read_cfg##_word(args, __pos, &__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;							\
+})
+
 /* Functions internal to the PCI core code */
 
 #ifdef CONFIG_DMI
-- 
2.25.1

[PATCH v15 3/6] PCI: Refactor extended capability search into PCI_FIND_NEXT_EXT_CAP()

[Hans Zhang]

Move the extended Capability search logic into a PCI_FIND_NEXT_EXT_CAP()
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 extended Capability search implementation to
use PCI_FIND_NEXT_EXT_CAP().

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/pci.c | 35 ++---------------------------------
 drivers/pci/pci.h | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ac2658d946ea..e698278229f2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -527,42 +527,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_CAP(pci_bus_read_config, start, cap,
+				     dev->bus, dev->devfn);
 }
 EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 81580987509f..7fb44faf2c44 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -133,6 +133,46 @@ bool pcie_cap_has_rtctl(const struct pci_dev *dev);
 	__found_pos;							\
 })
 
+/* Extended Capability finder */
+/**
+ * PCI_FIND_NEXT_EXT_CAP - 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
+ *
+ * Search the extended capability list in PCI config space to find @cap.
+ * Implements TTL protection against infinite loops using a calculated
+ * maximum search count.
+ *
+ * Return: Position of the capability if found, 0 otherwise.
+ */
+#define PCI_FIND_NEXT_EXT_CAP(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##_dword(args, __pos, &__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 v15 4/6] PCI: dwc: Use PCI core APIs to find capabilities

[Hans Zhang]

The PCI core now provides generic PCI_FIND_NEXT_CAP() and
PCI_FIND_NEXT_EXT_CAP() macros to search for PCI capabilities, using a
config accessor we supply; use them in the DWC driver.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 77 ++------------------
 drivers/pci/controller/dwc/pcie-designware.h | 21 ++++++
 2 files changed, 26 insertions(+), 72 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 89aad5a08928..5fe0744d4235 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -213,83 +213,16 @@ 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)
-{
-	u8 cap_id, next_cap_ptr;
-	u16 reg;
-
-	if (!cap_ptr)
-		return 0;
-
-	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;
-
-	next_cap_ptr = (reg & 0xff00) >> 8;
-	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
-}
-
 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(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_CAP(dw_pcie_read_cfg, 0, cap, pci);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);
 
@@ -302,8 +235,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_CAP(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;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 00f52d472dcd..b5e7e18138a6 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -609,6 +609,27 @@ static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
 	dw_pcie_write_dbi2(pci, reg, 0x4, val);
 }
 
+static inline int dw_pcie_read_cfg_byte(struct dw_pcie *pci, int where,
+					u8 *val)
+{
+	*val = dw_pcie_readb_dbi(pci, where);
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static inline int dw_pcie_read_cfg_word(struct dw_pcie *pci, int where,
+					u16 *val)
+{
+	*val = dw_pcie_readw_dbi(pci, where);
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static inline int dw_pcie_read_cfg_dword(struct dw_pcie *pci, int where,
+					 u32 *val)
+{
+	*val = dw_pcie_readl_dbi(pci, where);
+	return PCIBIOS_SUCCESSFUL;
+}
+
 static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
 						     u8 func_no)
 {
-- 
2.25.1

[PATCH v15 5/6] PCI: cadence: Use PCI core APIs to find capabilities

[Hans Zhang]

The PCI core now provides generic PCI_FIND_NEXT_CAP() and
PCI_FIND_NEXT_EXT_CAP() macros to search for PCI capabilities, using a
config accessor we supply; use them in the CDNS driver.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/cadence/pcie-cadence.c | 14 ++++++++
 drivers/pci/controller/cadence/pcie-cadence.h | 34 +++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
index 70a19573440e..c45585ae1746 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.c
+++ b/drivers/pci/controller/cadence/pcie-cadence.c
@@ -8,6 +8,20 @@
 #include <linux/of.h>
 
 #include "pcie-cadence.h"
+#include "../../pci.h"
+
+u8 cdns_pcie_find_capability(struct cdns_pcie *pcie, u8 cap)
+{
+	return PCI_FIND_NEXT_CAP(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_CAP(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 1d81c4bf6c6d..71e203de1087 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -367,6 +367,37 @@ 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 int cdns_pcie_read_cfg_byte(struct cdns_pcie *pcie, int where,
+					  u8 *val)
+{
+	*val = cdns_pcie_readb(pcie, where);
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static inline int cdns_pcie_read_cfg_word(struct cdns_pcie *pcie, int where,
+					  u16 *val)
+{
+	*val = cdns_pcie_readw(pcie, where);
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static inline int cdns_pcie_read_cfg_dword(struct cdns_pcie *pcie, int where,
+					   u32 *val)
+{
+	*val = cdns_pcie_readl(pcie, where);
+	return PCIBIOS_SUCCESSFUL;
+}
+
 static inline u32 cdns_pcie_read_sz(void __iomem *addr, int size)
 {
 	void __iomem *aligned_addr = PTR_ALIGN_DOWN(addr, 0x4);
@@ -536,6 +567,9 @@ static inline void cdns_pcie_ep_disable(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 v15 6/6] PCI: cadence: Use cdns_pcie_find_*capability() to avoid hardcoding offsets

[Hans Zhang]

The PCI 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>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../pci/controller/cadence/pcie-cadence-ep.c  | 38 +++++++++++--------
 drivers/pci/controller/cadence/pcie-cadence.h |  5 ---
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 77c5a19b2ab1..5529ed84649f 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -21,12 +21,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);
@@ -38,10 +39,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;
@@ -227,9 +229,10 @@ static int cdns_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn, u8 nr_irqs)
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 	struct cdns_pcie *pcie = &ep->pcie;
 	u8 mmc = order_base_2(nr_irqs);
-	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);
 
 	/*
@@ -249,9 +252,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_MSIX);
 	fn = cdns_pcie_get_fn_from_vfn(pcie, fn, vfn);
 
 	/* Validate that the MSI feature is actually enabled. */
@@ -272,9 +276,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;
@@ -292,9 +297,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;
@@ -380,11 +386,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. */
@@ -432,14 +438,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. */
@@ -482,16 +488,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];
@@ -565,7 +571,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.
@@ -589,12 +597,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);
+					       cap + PCI_EXP_DEVCAP, value);
 		}
 	}
 
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 71e203de1087..84686b1493f2 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 v15 0/6] Refactor capability search into common macros

[Niklas Schnelle]

On Wed, 2025-08-13 at 22:45 +0800, 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` and
> `PCI_FIND_NEXT_EXT_CAP`), 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.
> 
> ---
> Dear Niklas and Gerd,
> 
> Can you test this series of patches on the s390?
> 
> Thank you very much.

Hi Hans, I gave this series a try on top of v6.17-rc1 on s390
and a bunch of PCI devices including the mlx5 cards that Gerd we
originally saw issues with. All looks well now. So feel free to
add as appropriate:

Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>

Thanks,
Niklas

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

[Hans Zhang]

On 2025/8/14 16:32, Niklas Schnelle wrote:
> On Wed, 2025-08-13 at 22:45 +0800, 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` and
>> `PCI_FIND_NEXT_EXT_CAP`), 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.
>>
>> ---
>> Dear Niklas and Gerd,
>>
>> Can you test this series of patches on the s390?
>>
>> Thank you very much.
> 
> Hi Hans, I gave this series a try on top of v6.17-rc1 on s390
> and a bunch of PCI devices including the mlx5 cards that Gerd we
> originally saw issues with. All looks well now. So feel free to
> add as appropriate:
> 
> Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 

Hi Niklas,

Thank you very much for your test. Let's see what others think.

Best regards,
Hans

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

[Bjorn Helgaas]

On Wed, Aug 13, 2025 at 10:45:23PM +0800, 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` and
> `PCI_FIND_NEXT_EXT_CAP`), 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.
> 
> ---
> Dear Niklas and Gerd,
> 
> Can you test this series of patches on the s390?
> 
> Thank you very much.
> ---
> 
> ---
> Changes since v14:
> - Delete the first patch in the v14 series.
> - The functions that can obtain the values of the registers u8/u16/u32 are
>   directly called in PCI_FIND_NEXT_CAP() and PCI_FIND_NEXT_EXT_CAP().
>   Use the pci_bus_read_config_byte/word/dword function directly.
> - Delete dw_pcie_read_cfg and redefine three functions: dw_pcie_read_cfg_byte/word/dword.
> - Delete cdns_pcie_read_cfg and redefine three functions: cdns_pcie_read_cfg_byte/word/dword.
> 
> Changes since v13:
> - Split patch 3/6 into two patches for searching standard and extended capability. (Bjorn)
> - Optimize the code based on the review comments from Bjorn.
> - Patch 5/7 and 6/7 use simplified macro definitions: PCI_FIND_NEXT_CAP(), PCI_FIND_NEXT_EXT_CAP().
> - The other patches have not been modified.
> 
> Changes since v12:
> - Modify some commit messages, code format issues, and optimize the function return values.
> 
> 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: Clean up __pci_find_next_cap_ttl() readability
>   PCI: Refactor capability search into PCI_FIND_NEXT_CAP()
>   PCI: Refactor extended capability search into PCI_FIND_NEXT_EXT_CAP()
>   PCI: dwc: Use PCI core APIs to find capabilities
>   PCI: cadence: Use PCI core APIs to find capabilities
>   PCI: cadence: Use cdns_pcie_find_*capability() to avoid hardcoding
>     offsets
> 
>  .../pci/controller/cadence/pcie-cadence-ep.c  | 38 +++++----
>  drivers/pci/controller/cadence/pcie-cadence.c | 14 +++
>  drivers/pci/controller/cadence/pcie-cadence.h | 39 +++++++--
>  drivers/pci/controller/dwc/pcie-designware.c  | 77 ++---------------
>  drivers/pci/controller/dwc/pcie-designware.h  | 21 +++++
>  drivers/pci/pci.c                             | 76 +++--------------
>  drivers/pci/pci.h                             | 85 +++++++++++++++++++
>  include/uapi/linux/pci_regs.h                 |  3 +
>  8 files changed, 194 insertions(+), 159 deletions(-)

I applied this on pci/capability-search for v6.18, thanks!

Niklas, I added your Tested-by, omitting the dwc and cadence patches
because I think you tested s390 and probably didn't exercise dwc or
cadence.  Thanks very much to you and Gerd for finding the issue and
testing the resolution!

Bjorn

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

[Niklas Schnelle]

On Thu, 2025-08-14 at 15:25 -0500, Bjorn Helgaas wrote:
> On Wed, Aug 13, 2025 at 10:45:23PM +0800, 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` and
> > `PCI_FIND_NEXT_EXT_CAP`), 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.
> > 
> > ---
> > Dear Niklas and Gerd,
> > 
> > Can you test this series of patches on the s390?
> > 
> > Thank you very much.
> > ---
> > 
> > ---
> > Changes since v14:
> > - Delete the first patch in the v14 series.
> > - The functions that can obtain the values of the registers u8/u16/u32 are
> >   directly called in PCI_FIND_NEXT_CAP() and PCI_FIND_NEXT_EXT_CAP().
> >   Use the pci_bus_read_config_byte/word/dword function directly.
> > - Delete dw_pcie_read_cfg and redefine three functions: dw_pcie_read_cfg_byte/word/dword.
> > - Delete cdns_pcie_read_cfg and redefine three functions: cdns_pcie_read_cfg_byte/word/dword.
> > 
> > Changes since v13:
> > - Split patch 3/6 into two patches for searching standard and extended capability. (Bjorn)
> > - Optimize the code based on the review comments from Bjorn.
> > - Patch 5/7 and 6/7 use simplified macro definitions: PCI_FIND_NEXT_CAP(), PCI_FIND_NEXT_EXT_CAP().
> > - The other patches have not been modified.
> > 
> > Changes since v12:
> > - Modify some commit messages, code format issues, and optimize the function return values.
> > 
> > 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: Clean up __pci_find_next_cap_ttl() readability
> >   PCI: Refactor capability search into PCI_FIND_NEXT_CAP()
> >   PCI: Refactor extended capability search into PCI_FIND_NEXT_EXT_CAP()
> >   PCI: dwc: Use PCI core APIs to find capabilities
> >   PCI: cadence: Use PCI core APIs to find capabilities
> >   PCI: cadence: Use cdns_pcie_find_*capability() to avoid hardcoding
> >     offsets
> > 
> >  .../pci/controller/cadence/pcie-cadence-ep.c  | 38 +++++----
> >  drivers/pci/controller/cadence/pcie-cadence.c | 14 +++
> >  drivers/pci/controller/cadence/pcie-cadence.h | 39 +++++++--
> >  drivers/pci/controller/dwc/pcie-designware.c  | 77 ++---------------
> >  drivers/pci/controller/dwc/pcie-designware.h  | 21 +++++
> >  drivers/pci/pci.c                             | 76 +++--------------
> >  drivers/pci/pci.h                             | 85 +++++++++++++++++++
> >  include/uapi/linux/pci_regs.h                 |  3 +
> >  8 files changed, 194 insertions(+), 159 deletions(-)
> 
> I applied this on pci/capability-search for v6.18, thanks!
> 
> Niklas, I added your Tested-by, omitting the dwc and cadence patches
> because I think you tested s390 and probably didn't exercise dwc or
> cadence.  Thanks very much to you and Gerd for finding the issue and
> testing the resolution!
> 
> Bjorn

Thanks, yes leaving out dwc and cadence makes sense. Though I do often
also test on my private x86 systems this one was s390 only. Since I
have you here and as you applied this one now and Lukas PCI/ERR stuff
yesterday, is it possible that my series titled "PCI/ERR: s390/pci: Use
pci_uevent_ers() in PCI recovery" somehow fell through your mail
filters maybe due to not having any "PCI:" in a subject?

Thanks,
Niklas

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

[Bjorn Helgaas]

On Thu, Aug 14, 2025 at 10:37:03PM +0200, Niklas Schnelle wrote:
> On Thu, 2025-08-14 at 15:25 -0500, Bjorn Helgaas wrote:
> ...

> > I applied this on pci/capability-search for v6.18, thanks!
> > 
> > Niklas, I added your Tested-by, omitting the dwc and cadence patches
> > because I think you tested s390 and probably didn't exercise dwc or
> > cadence.  Thanks very much to you and Gerd for finding the issue and
> > testing the resolution!
> 
> Thanks, yes leaving out dwc and cadence makes sense. Though I do often
> also test on my private x86 systems this one was s390 only. Since I
> have you here and as you applied this one now and Lukas PCI/ERR stuff
> yesterday, is it possible that my series titled "PCI/ERR: s390/pci: Use
> pci_uevent_ers() in PCI recovery" somehow fell through your mail
> filters maybe due to not having any "PCI:" in a subject?

Nope, I saw it and am actually looking at it right now :)

I see there's some conversation about Lukas's series, so I expect some
minor rebasing, if only to add Reviewed-by and such.  Nothing unusual;
I treat these branches as drafts until the merge window.

Bjorn

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

[Hans Zhang]

On 2025/8/15 04:25, Bjorn Helgaas wrote:
> On Wed, Aug 13, 2025 at 10:45:23PM +0800, 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` and
>> `PCI_FIND_NEXT_EXT_CAP`), 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.
>>
>> ---
>> Dear Niklas and Gerd,
>>
>> Can you test this series of patches on the s390?
>>
>> Thank you very much.
>> ---
>>
>> ---
>> Changes since v14:
>> - Delete the first patch in the v14 series.
>> - The functions that can obtain the values of the registers u8/u16/u32 are
>>    directly called in PCI_FIND_NEXT_CAP() and PCI_FIND_NEXT_EXT_CAP().
>>    Use the pci_bus_read_config_byte/word/dword function directly.
>> - Delete dw_pcie_read_cfg and redefine three functions: dw_pcie_read_cfg_byte/word/dword.
>> - Delete cdns_pcie_read_cfg and redefine three functions: cdns_pcie_read_cfg_byte/word/dword.
>>
>> Changes since v13:
>> - Split patch 3/6 into two patches for searching standard and extended capability. (Bjorn)
>> - Optimize the code based on the review comments from Bjorn.
>> - Patch 5/7 and 6/7 use simplified macro definitions: PCI_FIND_NEXT_CAP(), PCI_FIND_NEXT_EXT_CAP().
>> - The other patches have not been modified.
>>
>> Changes since v12:
>> - Modify some commit messages, code format issues, and optimize the function return values.
>>
>> 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: Clean up __pci_find_next_cap_ttl() readability
>>    PCI: Refactor capability search into PCI_FIND_NEXT_CAP()
>>    PCI: Refactor extended capability search into PCI_FIND_NEXT_EXT_CAP()
>>    PCI: dwc: Use PCI core APIs to find capabilities
>>    PCI: cadence: Use PCI core APIs to find capabilities
>>    PCI: cadence: Use cdns_pcie_find_*capability() to avoid hardcoding
>>      offsets
>>
>>   .../pci/controller/cadence/pcie-cadence-ep.c  | 38 +++++----
>>   drivers/pci/controller/cadence/pcie-cadence.c | 14 +++
>>   drivers/pci/controller/cadence/pcie-cadence.h | 39 +++++++--
>>   drivers/pci/controller/dwc/pcie-designware.c  | 77 ++---------------
>>   drivers/pci/controller/dwc/pcie-designware.h  | 21 +++++
>>   drivers/pci/pci.c                             | 76 +++--------------
>>   drivers/pci/pci.h                             | 85 +++++++++++++++++++
>>   include/uapi/linux/pci_regs.h                 |  3 +
>>   8 files changed, 194 insertions(+), 159 deletions(-)
> 
> I applied this on pci/capability-search for v6.18, thanks!
> 
> Niklas, I added your Tested-by, omitting the dwc and cadence patches
> because I think you tested s390 and probably didn't exercise dwc or
> cadence.  Thanks very much to you and Gerd for finding the issue and
> testing the resolution!

Dear Bjorn,

I have tested it on our company's own EVB board and it works normally. 
(Cadence PCIe 4.0 IP)

And it was tested on the RK3588 and worked normally. (Synopsys PCIe 3.0 IP)

Of course, it would be great if others were willing to give it a try.

Best regards,
Hans

Re: [PATCH v15 1/6] PCI: Clean up __pci_find_next_cap_ttl() readability

[Gerd Bayer]

On Wed, 2025-08-13 at 22:45 +0800, 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).
> 
> No functional changes intended.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
>  drivers/pci/pci.c             | 9 +++++----
>  include/uapi/linux/pci_regs.h | 3 +++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b0f4d98036cd..40a5c87d9a6b 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 f5b17745de60..1bba99b46227 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -207,6 +207,9 @@
>  
>  /* 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 */
>  #define  PCI_CAP_ID_AGP		0x02	/* Accelerated Graphics Port */

Hi Hans,

I like your approach to replace the magic numbers here. If you went
further to replace the single pci_bus_read_config_word() with two
single-byte reads at the appropriate places - for CAP_ID and
CAP_LIST_NEXT - you could even go with the already existing offset
defines PCI_CAP_LIST_ID and PCI_CAP_LIST_NEXT from pci_regs.h.

But that might be a more intricate change and involves more HW accesses
than what it's worth.

So feel free to add my
Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>

Thanks,
Gerd

Re: [PATCH v15 1/6] PCI: Clean up __pci_find_next_cap_ttl() readability

[Ilpo Järvinen]

On Wed, 20 Aug 2025, Gerd Bayer wrote:

> On Wed, 2025-08-13 at 22:45 +0800, 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).
> > 
> > No functional changes intended.
> > 
> > Signed-off-by: Hans Zhang <18255117159@163.com>
> > Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> > ---
> >  drivers/pci/pci.c             | 9 +++++----
> >  include/uapi/linux/pci_regs.h | 3 +++
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index b0f4d98036cd..40a5c87d9a6b 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 f5b17745de60..1bba99b46227 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -207,6 +207,9 @@
> >  
> >  /* 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 */
> >  #define  PCI_CAP_ID_AGP		0x02	/* Accelerated Graphics Port */
> 
> Hi Hans,
> 
> I like your approach to replace the magic numbers here. If you went
> further to replace the single pci_bus_read_config_word() with two
> single-byte reads at the appropriate places - for CAP_ID and
> CAP_LIST_NEXT - you could even go with the already existing offset
> defines PCI_CAP_LIST_ID and PCI_CAP_LIST_NEXT from pci_regs.h.
>
> But that might be a more intricate change and involves more HW accesses
> than what it's worth.

Hi,

As you noted, it'll be less efficient so it's undesirable to split the 
read.

It's somewhat problematic that some of the defines in 
include/uapi/linux/pci_regs.h cannot be easily used efficiently with 
multi-byte reads leading to use of literals in code.

IMO, adding the multi-byte masks like this Hans' change is IMO the correct 
way to address it.

> So feel free to add my
> Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>
> 
> Thanks,
> Gerd
> 

-- 
 i.