[v4 0/4] Introduce generic capability search functions

[v4 0/4] Introduce generic capability search functions

[Hans Zhang]

1. Introduce generic capability search functions.
2. Replace dw_pcie_find_capability() and dw_pcie_find_ext_capability().
3. Add configuration space capability search API.
4. Use cdns_pcie_find_*capability to find capability offset instead of
hardcore.

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 (4):
  PCI: Introduce generic capability search functions
  PCI: dwc: Replace dw_pcie_find_capability() and
    dw_pcie_find_ext_capability()
  PCI: cadence: Add configuration space capability search API
  PCI: cadence: Use cdns_pcie_find_*capability to find capability offset
    instead of hardcore

 .../pci/controller/cadence/pcie-cadence-ep.c  | 40 +++++----
 drivers/pci/controller/cadence/pcie-cadence.c | 25 ++++++
 drivers/pci/controller/cadence/pcie-cadence.h |  8 +-
 drivers/pci/controller/dwc/pcie-designware.c  | 71 ++--------------
 drivers/pci/pci.c                             | 83 +++++++++++++++++++
 include/linux/pci.h                           | 14 +++-
 6 files changed, 152 insertions(+), 89 deletions(-)


base-commit: a1cffe8cc8aef85f1b07c4464f0998b9785b795a
-- 
2.25.1

[v4 1/4] PCI: Introduce generic capability search functions

[Hans Zhang]

Existing controller drivers (e.g., DWC, custom out-of-tree drivers)
duplicate logic for scanning PCI capability lists. This creates
maintenance burdens and risks inconsistencies.

To resolve this:

1. Add pci_generic_find_capability() and pci_generic_find_ext_capability()
in drivers/pci/pci.c, accepting controller-specific read functions
and device data as parameters.

2. Refactor dwc_pcie_find_capability() and similar functions to utilize
these new generic interfaces.

3. Update out-of-tree drivers to leverage the common implementation,
eliminating code duplication.

This approach:
- Centralizes critical PCI capability scanning logic
- Allows flexible adaptation to varied hardware access methods
- Reduces future maintenance overhead
- Aligns with kernel code reuse best practices

Tested with DWC PCIe controller and CDNS PCIe drivers.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v3:
https://lore.kernel.org/linux-pci/20250321040358.360755-2-18255117159@163.com/

- Resolved compilation error.
---
 drivers/pci/pci.c   | 83 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h | 14 +++++++-
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 869d204a70a3..3c381827a3a0 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -612,6 +612,89 @@ u16 pci_find_ext_capability(struct pci_dev *dev, int cap)
 }
 EXPORT_SYMBOL_GPL(pci_find_ext_capability);
 
+/*
+ * These interfaces resemble the pci_find_*capability() interfaces, but these
+ * are for configuring host controllers, which are bridges *to* PCI devices but
+ * are not PCI devices themselves.
+ */
+static u8 __pci_generic_find_next_cap(void *priv, pci_generic_read_cfg read_cfg,
+				      u8 cap_ptr, u8 cap)
+{
+	u8 cap_id, next_cap_ptr;
+	u16 reg;
+
+	if (!cap_ptr)
+		return 0;
+
+	reg = read_cfg(priv, cap_ptr, 2);
+	cap_id = (reg & 0x00ff);
+
+	if (cap_id > PCI_CAP_ID_MAX)
+		return 0;
+
+	if (cap_id == cap)
+		return cap_ptr;
+
+	next_cap_ptr = (reg & 0xff00) >> 8;
+	return __pci_generic_find_next_cap(priv, read_cfg, next_cap_ptr, cap);
+}
+
+u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
+			       u8 cap)
+{
+	u8 next_cap_ptr;
+	u16 reg;
+
+	reg = read_cfg(priv, PCI_CAPABILITY_LIST, 2);
+	next_cap_ptr = (reg & 0x00ff);
+
+	return __pci_generic_find_next_cap(priv, read_cfg, next_cap_ptr, cap);
+}
+EXPORT_SYMBOL_GPL(pci_generic_find_capability);
+
+static u16 pci_generic_find_next_ext_capability(void *priv,
+						pci_generic_read_cfg read_cfg,
+						u16 start, u8 cap)
+{
+	u32 header;
+	int ttl;
+	int pos = PCI_CFG_SPACE_SIZE;
+
+	/* minimum 8 bytes per capability */
+	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
+
+	if (start)
+		pos = start;
+
+	header = read_cfg(priv, pos, 4);
+	/*
+	 * If we have no capabilities, this is indicated by cap ID,
+	 * cap version and next pointer all being 0.
+	 */
+	if (header == 0)
+		return 0;
+
+	while (ttl-- > 0) {
+		if (PCI_EXT_CAP_ID(header) == cap && pos != start)
+			return pos;
+
+		pos = PCI_EXT_CAP_NEXT(header);
+		if (pos < PCI_CFG_SPACE_SIZE)
+			break;
+
+		header = read_cfg(priv, pos, 4);
+	}
+
+	return 0;
+}
+
+u16 pci_generic_find_ext_capability(void *priv, pci_generic_read_cfg read_cfg,
+				    u8 cap)
+{
+	return pci_generic_find_next_ext_capability(priv, read_cfg, 0, cap);
+}
+EXPORT_SYMBOL_GPL(pci_generic_find_ext_capability);
+
 /**
  * pci_get_dsn - Read and return the 8-byte Device Serial Number
  * @dev: PCI device to query
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 47b31ad724fa..800da2cb884d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1205,6 +1205,11 @@ u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
 u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap);
 u16 pci_find_ext_capability(struct pci_dev *dev, int cap);
 u16 pci_find_next_ext_capability(struct pci_dev *dev, u16 pos, int cap);
+typedef u32 (*pci_generic_read_cfg)(void *priv, int where, int size);
+u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
+			       u8 cap);
+u16 pci_generic_find_ext_capability(void *priv, pci_generic_read_cfg read_cfg,
+				    u8 cap);
 struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
 u16 pci_find_vsec_capability(struct pci_dev *dev, u16 vendor, int cap);
 u16 pci_find_dvsec_capability(struct pci_dev *dev, u16 vendor, u16 dvsec);
@@ -2012,7 +2017,14 @@ static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post, int cap)
 { return 0; }
 static inline u16 pci_find_ext_capability(struct pci_dev *dev, int cap)
 { return 0; }
-
+typedef u32 (*pci_generic_read_cfg)(void *priv, int where, int size);
+static u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
+				      u8 cap)
+{ return 0; }
+static u16 pci_generic_find_ext_capability(void *priv,
+					   pci_generic_read_cfg read_cfg,
+					   u8 cap)
+{ return 0; }
 static inline u64 pci_get_dsn(struct pci_dev *dev)
 { return 0; }
 
-- 
2.25.1

Re: [v4 1/4] PCI: Introduce generic capability search functions

[kernel test robot]

Hi Hans,

kernel test robot noticed the following build warnings:

[auto build test WARNING on a1cffe8cc8aef85f1b07c4464f0998b9785b795a]

url:    https://github.com/intel-lab-lkp/linux/commits/Hans-Zhang/PCI-Introduce-generic-capability-search-functions/20250321-182140
base:   a1cffe8cc8aef85f1b07c4464f0998b9785b795a
patch link:    https://lore.kernel.org/r/20250321101710.371480-2-18255117159%40163.com
patch subject: [v4 1/4] PCI: Introduce generic capability search functions
config: arc-randconfig-002-20250321 (https://download.01.org/0day-ci/archive/20250321/202503212059.oxvxSlCc-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250321/202503212059.oxvxSlCc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503212059.oxvxSlCc-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/nvme/host/nvme.h:11,
                    from drivers/nvme/host/fc.c:13:
>> include/linux/pci.h:2024:12: warning: 'pci_generic_find_ext_capability' defined but not used [-Wunused-function]
    2024 | static u16 pci_generic_find_ext_capability(void *priv,
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/pci.h:2021:11: warning: 'pci_generic_find_capability' defined but not used [-Wunused-function]
    2021 | static u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
         |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/pci_generic_find_ext_capability +2024 include/linux/pci.h

  1999	
  2000	static inline void pci_set_master(struct pci_dev *dev) { }
  2001	static inline void pci_clear_master(struct pci_dev *dev) { }
  2002	static inline int pci_enable_device(struct pci_dev *dev) { return -EIO; }
  2003	static inline void pci_disable_device(struct pci_dev *dev) { }
  2004	static inline int pcim_enable_device(struct pci_dev *pdev) { return -EIO; }
  2005	static inline int pci_assign_resource(struct pci_dev *dev, int i)
  2006	{ return -EBUSY; }
  2007	static inline int __must_check __pci_register_driver(struct pci_driver *drv,
  2008							     struct module *owner,
  2009							     const char *mod_name)
  2010	{ return 0; }
  2011	static inline int pci_register_driver(struct pci_driver *drv)
  2012	{ return 0; }
  2013	static inline void pci_unregister_driver(struct pci_driver *drv) { }
  2014	static inline u8 pci_find_capability(struct pci_dev *dev, int cap)
  2015	{ return 0; }
  2016	static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post, int cap)
  2017	{ return 0; }
  2018	static inline u16 pci_find_ext_capability(struct pci_dev *dev, int cap)
  2019	{ return 0; }
  2020	typedef u32 (*pci_generic_read_cfg)(void *priv, int where, int size);
> 2021	static u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
  2022					      u8 cap)
  2023	{ return 0; }
> 2024	static u16 pci_generic_find_ext_capability(void *priv,
  2025						   pci_generic_read_cfg read_cfg,
  2026						   u8 cap)
  2027	{ return 0; }
  2028	static inline u64 pci_get_dsn(struct pci_dev *dev)
  2029	{ return 0; }
  2030	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [v4 1/4] PCI: Introduce generic capability search functions

[Manivannan Sadhasivam]

On Fri, Mar 21, 2025 at 06:17:07PM +0800, Hans Zhang wrote:
> Existing controller drivers (e.g., DWC, custom out-of-tree drivers)
> duplicate logic for scanning PCI capability lists. This creates
> maintenance burdens and risks inconsistencies.
> 
> To resolve this:
> 
> 1. Add pci_generic_find_capability() and pci_generic_find_ext_capability()
> in drivers/pci/pci.c, accepting controller-specific read functions
> and device data as parameters.
> 

I'd reword pci_generic* as pci_host_bridge* to reflect the fact that these APIs
are meant for host bridges.

> 2. Refactor dwc_pcie_find_capability() and similar functions to utilize
> these new generic interfaces.
> 

This is not part of this patch. So should be dropped.

> 3. Update out-of-tree drivers to leverage the common implementation,
> eliminating code duplication.
> 

This also.

> This approach:
> - Centralizes critical PCI capability scanning logic
> - Allows flexible adaptation to varied hardware access methods
> - Reduces future maintenance overhead
> - Aligns with kernel code reuse best practices
> 
> Tested with DWC PCIe controller and CDNS PCIe drivers.
> 

This tested info is also not required since the DWC and CDNS changes are not
part of this patch.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [v4 1/4] PCI: Introduce generic capability search functions

[Hans Zhang]

On 2025/3/21 21:00, Manivannan Sadhasivam wrote:
> On Fri, Mar 21, 2025 at 06:17:07PM +0800, Hans Zhang wrote:
>> Existing controller drivers (e.g., DWC, custom out-of-tree drivers)
>> duplicate logic for scanning PCI capability lists. This creates
>> maintenance burdens and risks inconsistencies.
>>
>> To resolve this:
>>
>> 1. Add pci_generic_find_capability() and pci_generic_find_ext_capability()
>> in drivers/pci/pci.c, accepting controller-specific read functions
>> and device data as parameters.
>>
> 
> I'd reword pci_generic* as pci_host_bridge* to reflect the fact that these APIs
> are meant for host bridges.
> 

Hi Mani,

Thanks your for reply. Will change.

>> 2. Refactor dwc_pcie_find_capability() and similar functions to utilize
>> these new generic interfaces.
>>
> 
> This is not part of this patch. So should be dropped.

Will change.

>> 3. Update out-of-tree drivers to leverage the common implementation,
>> eliminating code duplication.
>>
> 
> This also.

Will change.

>> This approach:
>> - Centralizes critical PCI capability scanning logic
>> - Allows flexible adaptation to varied hardware access methods
>> - Reduces future maintenance overhead
>> - Aligns with kernel code reuse best practices
>>
>> Tested with DWC PCIe controller and CDNS PCIe drivers.
>>
> 
> This tested info is also not required since the DWC and CDNS changes are not
> part of this patch.

Will change.

Best regards,
Hans

[v4 2/4] PCI: dwc: Replace dw_pcie_find_capability() and dw_pcie_find_ext_capability()

[Hans Zhang]

Replace duplicate logic code. Use a common interface and provide callback
functions.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 71 ++------------------
 1 file changed, 5 insertions(+), 66 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 145e7f579072..2ebebedbf18d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -203,83 +203,22 @@ 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 u32 dwc_pcie_read_cfg(void *priv, int where, int size)
 {
-	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;
+	struct dw_pcie *pci = priv;
 
-	if (cap_id == cap)
-		return cap_ptr;
-
-	next_cap_ptr = (reg & 0xff00) >> 8;
-	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
+	return dw_pcie_read_dbi(pci, where, size);
 }
 
 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_generic_find_capability(pci, dwc_pcie_read_cfg, cap);
 }
 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_generic_find_ext_capability(pci, dwc_pcie_read_cfg, cap);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);
 
-- 
2.25.1

Re: [v4 2/4] PCI: dwc: Replace dw_pcie_find_capability() and dw_pcie_find_ext_capability()

[Manivannan Sadhasivam]

On Fri, Mar 21, 2025 at 06:17:08PM +0800, Hans Zhang wrote:
> Replace duplicate logic code. Use a common interface and provide callback
> functions.
> 

I'd reword the subject and description as:

```
PCI: dwc: Use common PCI host bridge APIs for finding the capabilities

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

Patch LGTM!

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [v4 2/4] PCI: dwc: Replace dw_pcie_find_capability() and dw_pcie_find_ext_capability()

[Hans Zhang]

On 2025/3/21 21:05, Manivannan Sadhasivam wrote:
> On Fri, Mar 21, 2025 at 06:17:08PM +0800, Hans Zhang wrote:
>> Replace duplicate logic code. Use a common interface and provide callback
>> functions.
>>
> 
> I'd reword the subject and description as:
> 
> ```
> PCI: dwc: Use common PCI host bridge APIs for finding the capabilities
> 
> Since the PCI core is now exposing generic APIs for the host bridges to
> search for the PCIe capabilities, make use of them in the DWC driver.
> ```

Hi Mani,

Thank you very much for your advice. Will change.

Best regards,
Hans

> Patch LGTM!
> 
> - Mani
> 

[v4 3/4] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

Add configuration space capability search API using struct cdns_pcie*
pointer.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v2~v3:
https://lore.kernel.org/linux-pci/20250308133903.322216-1-18255117159@163.com/
https://lore.kernel.org/linux-pci/20250321040358.360755-4-18255117159@163.com/

- Introduce generic capability search functions

Changes since v1:
https://lore.kernel.org/linux-pci/20250123070935.1810110-1-18255117159@163.com

- Added calling the new API in PCI-Cadence ep.c.
- Add a commit message reason for adding the API.
---
 drivers/pci/controller/cadence/pcie-cadence.c | 25 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
index 204e045aed8c..ea9bfbe76cd6 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.c
+++ b/drivers/pci/controller/cadence/pcie-cadence.c
@@ -8,6 +8,31 @@
 
 #include "pcie-cadence.h"
 
+static u32 cdns_pcie_read_cfg(void *priv, int where, int size)
+{
+	struct cdns_pcie *pcie = priv;
+	u32 val;
+
+	if (size == 4)
+		val = readl(pcie->reg_base + where);
+	else if (size == 2)
+		val = readw(pcie->reg_base + where);
+	else if (size == 1)
+		val = readb(pcie->reg_base + where);
+
+	return val;
+}
+
+u8 cdns_pcie_find_capability(struct cdns_pcie *pcie, u8 cap)
+{
+	return pci_generic_find_capability(pcie, cdns_pcie_read_cfg, cap);
+}
+
+u16 cdns_pcie_find_ext_capability(struct cdns_pcie *pcie, u8 cap)
+{
+	return pci_generic_find_ext_capability(pcie, cdns_pcie_read_cfg, cap);
+}
+
 void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie)
 {
 	u32 delay = 0x3;
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index f5eeff834ec1..6f4981fccb94 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -557,6 +557,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

Re: [v4 3/4] PCI: cadence: Add configuration space capability search API

[Manivannan Sadhasivam]

On Fri, Mar 21, 2025 at 06:17:09PM +0800, Hans Zhang wrote:
> Add configuration space capability search API using struct cdns_pcie*
> pointer.
> 

Please reuse the subject and description I provided in previous patch.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [v4 3/4] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/21 21:06, Manivannan Sadhasivam wrote:
> On Fri, Mar 21, 2025 at 06:17:09PM +0800, Hans Zhang wrote:
>> Add configuration space capability search API using struct cdns_pcie*
>> pointer.
>>
> 
> Please reuse the subject and description I provided in previous patch.
> 

Hi Mani,

Thanks your for reply. Will change.

Best regards,
Hans

[v4 4/4] PCI: cadence: Use cdns_pcie_find_*capability to find capability offset instead of hardcore

[Hans Zhang]

The offset address of capability or extended capability designed by
different SOC design companies may not be the same. Therefore, a flexible
public API is required to find the offset address of a capability or
extended capability in the configuration space.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 .../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 e0cc4560dfde..aea53ddcaf9b 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;
@@ -379,11 +385,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. */
@@ -431,14 +437,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. */
@@ -481,16 +487,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];
@@ -564,7 +570,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.
@@ -588,12 +596,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 6f4981fccb94..d0fcf1b3549c 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: [v4 4/4] PCI: cadence: Use cdns_pcie_find_*capability to find capability offset instead of hardcore

[Manivannan Sadhasivam]

On Fri, Mar 21, 2025 at 06:17:10PM +0800, Hans Zhang wrote:
> The offset address of capability or extended capability designed by
> different SOC design companies may not be the same. Therefore, a flexible
> public API is required to find the offset address of a capability or
> extended capability in the configuration space.
> 

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.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [v4 4/4] PCI: cadence: Use cdns_pcie_find_*capability to find capability offset instead of hardcore

[Hans Zhang]

On 2025/3/21 21:18, Manivannan Sadhasivam wrote:
> On Fri, Mar 21, 2025 at 06:17:10PM +0800, Hans Zhang wrote:
>> The offset address of capability or extended capability designed by
>> different SOC design companies may not be the same. Therefore, a flexible
>> public API is required to find the offset address of a capability or
>> extended capability in the configuration space.
>>
> 
> 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.
> 

Hi Mani,

Thank you very much for your advice. Will change.

Best regards,
Hans

> - Mani
>