[v2] PCI: cadence: Add configuration space capability search API

[v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

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

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>
---
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.
---
 .../pci/controller/cadence/pcie-cadence-ep.c  | 40 ++++++----
 drivers/pci/controller/cadence/pcie-cadence.c | 80 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h |  8 +-
 3 files changed, 106 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.c b/drivers/pci/controller/cadence/pcie-cadence.c
index 204e045aed8c..ebb4a0130145 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.c
+++ b/drivers/pci/controller/cadence/pcie-cadence.c
@@ -8,6 +8,86 @@
 
 #include "pcie-cadence.h"
 
+/*
+ * These interfaces resemble the pci_find_*capability() interfaces, but these
+ * are for configuring host controllers, which are bridges *to* PCI devices but
+ * are not PCI devices themselves.
+ */
+static u8 __cdns_pcie_find_next_cap(struct cdns_pcie *pcie, u8 cap_ptr,
+				    u8 cap)
+{
+	u8 cap_id, next_cap_ptr;
+	u16 reg;
+
+	if (!cap_ptr)
+		return 0;
+
+	reg = cdns_pcie_readl(pcie, 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 __cdns_pcie_find_next_cap(pcie, next_cap_ptr, cap);
+}
+
+u8 cdns_pcie_find_capability(struct cdns_pcie *pcie, u8 cap)
+{
+	u8 next_cap_ptr;
+	u16 reg;
+
+	reg = cdns_pcie_readl(pcie, PCI_CAPABILITY_LIST);
+	next_cap_ptr = (reg & 0x00ff);
+
+	return __cdns_pcie_find_next_cap(pcie, next_cap_ptr, cap);
+}
+EXPORT_SYMBOL_GPL(cdns_pcie_find_capability);
+
+static u16 cdns_pcie_find_next_ext_capability(struct cdns_pcie *pcie,
+					      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 = cdns_pcie_readl(pcie, 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 = cdns_pcie_readl(pcie, pos);
+	}
+
+	return 0;
+}
+
+u16 cdns_pcie_find_ext_capability(struct cdns_pcie *pcie, u8 cap)
+{
+	return cdns_pcie_find_next_ext_capability(pcie, 0, cap);
+}
+EXPORT_SYMBOL_GPL(cdns_pcie_find_ext_capability);
+
 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..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
  */
@@ -557,6 +552,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,

base-commit: 76544811c850a1f4c055aa182b513b7a843868ea
-- 
2.25.1

Re: [v2] PCI: cadence: Add configuration space capability search API

[Siddharth Vadapalli]

On Sat, Mar 08, 2025 at 09:39:03PM +0800, Hans Zhang wrote:
> Add configuration space capability search API using struct cdns_pcie*
> pointer.
> 
> 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>
> ---
> 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.

In reply to your v1 patch, you have mentioned the following:
"Our controller driver currently has no plans for upstream and needs to
wait for notification from the boss."
at:
https://lore.kernel.org/linux-pci/fcfd4827-4d9e-4bcd-b1d0-8f9e349a6be7@163.com/

Since you have posted this patch, does it mean that you will be
upstreaming your driver as well? If not, we still end up in the same
situation as earlier where the Upstream Linux has APIs to support a
Downstream driver.

Bjorn indicated the above already at:
https://lore.kernel.org/linux-pci/20250123170831.GA1226684@bhelgaas/
and you did agree to do so. But this patch has no reference to the
upstream driver series which shall be making use of the APIs in this
patch.

Regards,
Siddharth.

Re: [v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/9 10:38, Siddharth Vadapalli wrote:
> On Sat, Mar 08, 2025 at 09:39:03PM +0800, Hans Zhang wrote:
>> Add configuration space capability search API using struct cdns_pcie*
>> pointer.
>>
>> 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>
>> ---
>> 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.
> 
> In reply to your v1 patch, you have mentioned the following:
> "Our controller driver currently has no plans for upstream and needs to
> wait for notification from the boss."
> at:
> https://lore.kernel.org/linux-pci/fcfd4827-4d9e-4bcd-b1d0-8f9e349a6be7@163.com/
> 
> Since you have posted this patch, does it mean that you will be
> upstreaming your driver as well? If not, we still end up in the same
> situation as earlier where the Upstream Linux has APIs to support a
> Downstream driver.
> 
> Bjorn indicated the above already at:
> https://lore.kernel.org/linux-pci/20250123170831.GA1226684@bhelgaas/
> and you did agree to do so. But this patch has no reference to the
> upstream driver series which shall be making use of the APIs in this
> patch.

Hi Siddharth,


Bjorn:
   If/when you upstream code that needs this interface, include this
   patch as part of the series.  As Siddharth pointed out, we avoid
   merging code that has no upstream users.


Hans: This user is: pcie-cadence-ep.c. I think this is an optimization 
of Cadence common code. I think this is an optimization of Cadence 
common code. Siddharth, what do you think?


Best regards,
Hans

Re: [v2] PCI: cadence: Add configuration space capability search API

[Siddharth Vadapalli]

On Sun, Mar 09, 2025 at 11:18:21AM +0800, Hans Zhang wrote:
> 
> 
> On 2025/3/9 10:38, Siddharth Vadapalli wrote:
> > On Sat, Mar 08, 2025 at 09:39:03PM +0800, Hans Zhang wrote:
> > > Add configuration space capability search API using struct cdns_pcie*
> > > pointer.
> > > 
> > > 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>
> > > ---
> > > 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.
> > 
> > In reply to your v1 patch, you have mentioned the following:
> > "Our controller driver currently has no plans for upstream and needs to
> > wait for notification from the boss."
> > at:
> > https://lore.kernel.org/linux-pci/fcfd4827-4d9e-4bcd-b1d0-8f9e349a6be7@163.com/
> > 
> > Since you have posted this patch, does it mean that you will be
> > upstreaming your driver as well? If not, we still end up in the same
> > situation as earlier where the Upstream Linux has APIs to support a
> > Downstream driver.
> > 
> > Bjorn indicated the above already at:
> > https://lore.kernel.org/linux-pci/20250123170831.GA1226684@bhelgaas/
> > and you did agree to do so. But this patch has no reference to the
> > upstream driver series which shall be making use of the APIs in this
> > patch.
> 
> Hi Siddharth,
> 
> 
> Bjorn:
>   If/when you upstream code that needs this interface, include this
>   patch as part of the series.  As Siddharth pointed out, we avoid
>   merging code that has no upstream users.
> 
> 
> Hans: This user is: pcie-cadence-ep.c. I think this is an optimization of
> Cadence common code. I think this is an optimization of Cadence common code.
> Siddharth, what do you think?

This seems to be an extension of the driver rather than an optimization.
At first glance, though it seems like this patch is enabling code-reuse,
it is actually attempting to walk through the config space registers to
identify a capability. Prior to this patch, those offsets were hard-coded,
saving the trouble of having to walk through the capability pointers to
arrive at the capability.

This patch will affect the following functions:
01. cdns_pcie_get_fn_from_vfn()
02. cdns_pcie_ep_write_header()
03. cdns_pcie_ep_set_msi()
04. cdns_pcie_ep_get_msi()
05. cdns_pcie_ep_get_msix()
06. cdns_pcie_ep_set_msix()
07. cdns_pcie_ep_send_msi_irq()
08. cdns_pcie_ep_map_msi_irq()
09. cdns_pcie_ep_send_msix_irq()
10. cdns_pcie_ep_start()
which will now take longer to get to the capability whose offset was
known. I understand that you wish to extend these functions to support
your SoC where the offsets don't match the hard-coded ones.

I will let Bjorn and others share their views on this patch.

Regards,
Siddharth.

Re: [v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/9 13:48, Siddharth Vadapalli wrote:
> On Sun, Mar 09, 2025 at 11:18:21AM +0800, Hans Zhang wrote:
>>
>>
>> On 2025/3/9 10:38, Siddharth Vadapalli wrote:
>>> On Sat, Mar 08, 2025 at 09:39:03PM +0800, Hans Zhang wrote:
>>>> Add configuration space capability search API using struct cdns_pcie*
>>>> pointer.
>>>>
>>>> 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>
>>>> ---
>>>> 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.
>>>
>>> In reply to your v1 patch, you have mentioned the following:
>>> "Our controller driver currently has no plans for upstream and needs to
>>> wait for notification from the boss."
>>> at:
>>> https://lore.kernel.org/linux-pci/fcfd4827-4d9e-4bcd-b1d0-8f9e349a6be7@163.com/
>>>
>>> Since you have posted this patch, does it mean that you will be
>>> upstreaming your driver as well? If not, we still end up in the same
>>> situation as earlier where the Upstream Linux has APIs to support a
>>> Downstream driver.
>>>
>>> Bjorn indicated the above already at:
>>> https://lore.kernel.org/linux-pci/20250123170831.GA1226684@bhelgaas/
>>> and you did agree to do so. But this patch has no reference to the
>>> upstream driver series which shall be making use of the APIs in this
>>> patch.
>>
>> Hi Siddharth,
>>
>>
>> Bjorn:
>>    If/when you upstream code that needs this interface, include this
>>    patch as part of the series.  As Siddharth pointed out, we avoid
>>    merging code that has no upstream users.
>>
>>
>> Hans: This user is: pcie-cadence-ep.c. I think this is an optimization of
>> Cadence common code. I think this is an optimization of Cadence common code.
>> Siddharth, what do you think?
> 
> This seems to be an extension of the driver rather than an optimization.
> At first glance, though it seems like this patch is enabling code-reuse,
> it is actually attempting to walk through the config space registers to
> identify a capability. Prior to this patch, those offsets were hard-coded,
> saving the trouble of having to walk through the capability pointers to
> arrive at the capability.

Hi Siddharth,

Prior to this patch, I don't think hard-coded is that reasonable. 
Because the SOC design of each company does not guarantee that the 
offset of each capability is the same. This parameter can be configured 
when selecting PCIe configuration options. The current code that just 
happens to hit the offset address is the same.

You can refer to the pci_find_*capability() or dw_pcie_find_*capability 
API. The meaning of their appearance is the same as what I said, and the 
design of each company may be different.

Best regards,
Hans

> 
> This patch will affect the following functions:
> 01. cdns_pcie_get_fn_from_vfn()
> 02. cdns_pcie_ep_write_header()
> 03. cdns_pcie_ep_set_msi()
> 04. cdns_pcie_ep_get_msi()
> 05. cdns_pcie_ep_get_msix()
> 06. cdns_pcie_ep_set_msix()
> 07. cdns_pcie_ep_send_msi_irq()
> 08. cdns_pcie_ep_map_msi_irq()
> 09. cdns_pcie_ep_send_msix_irq()
> 10. cdns_pcie_ep_start()
> which will now take longer to get to the capability whose offset was
> known. I understand that you wish to extend these functions to support
> your SoC where the offsets don't match the hard-coded ones.
>
In my opinion, hard-coded is not universal.

> I will let Bjorn and others share their views on this patch.
> 
> Regards,
> Siddharth.

Re: [v2] PCI: cadence: Add configuration space capability search API

[Siddharth Vadapalli]

On Sun, Mar 09, 2025 at 05:49:09PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/3/9 13:48, Siddharth Vadapalli wrote:
> > On Sun, Mar 09, 2025 at 11:18:21AM +0800, Hans Zhang wrote:
> > > 
> > > 
> > > On 2025/3/9 10:38, Siddharth Vadapalli wrote:
> > > > On Sat, Mar 08, 2025 at 09:39:03PM +0800, Hans Zhang wrote:
> > > > > Add configuration space capability search API using struct cdns_pcie*
> > > > > pointer.
> > > > > 
> > > > > 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>
> > > > > ---
> > > > > 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.
> > > > 
> > > > In reply to your v1 patch, you have mentioned the following:
> > > > "Our controller driver currently has no plans for upstream and needs to
> > > > wait for notification from the boss."
> > > > at:
> > > > https://lore.kernel.org/linux-pci/fcfd4827-4d9e-4bcd-b1d0-8f9e349a6be7@163.com/
> > > > 
> > > > Since you have posted this patch, does it mean that you will be
> > > > upstreaming your driver as well? If not, we still end up in the same
> > > > situation as earlier where the Upstream Linux has APIs to support a
> > > > Downstream driver.
> > > > 
> > > > Bjorn indicated the above already at:
> > > > https://lore.kernel.org/linux-pci/20250123170831.GA1226684@bhelgaas/
> > > > and you did agree to do so. But this patch has no reference to the
> > > > upstream driver series which shall be making use of the APIs in this
> > > > patch.
> > > 
> > > Hi Siddharth,
> > > 
> > > 
> > > Bjorn:
> > >    If/when you upstream code that needs this interface, include this
> > >    patch as part of the series.  As Siddharth pointed out, we avoid
> > >    merging code that has no upstream users.
> > > 
> > > 
> > > Hans: This user is: pcie-cadence-ep.c. I think this is an optimization of
> > > Cadence common code. I think this is an optimization of Cadence common code.
> > > Siddharth, what do you think?
> > 
> > This seems to be an extension of the driver rather than an optimization.
> > At first glance, though it seems like this patch is enabling code-reuse,
> > it is actually attempting to walk through the config space registers to
> > identify a capability. Prior to this patch, those offsets were hard-coded,
> > saving the trouble of having to walk through the capability pointers to
> > arrive at the capability.
> 
> Hi Siddharth,
> 
> Prior to this patch, I don't think hard-coded is that reasonable. Because
> the SOC design of each company does not guarantee that the offset of each
> capability is the same. This parameter can be configured when selecting PCIe
> configuration options. The current code that just happens to hit the offset
> address is the same.

1. You are modifying the driver for the Cadence PCIe Controller IP and
not the one for your SoC (a.k.a the application/glue/wrapper driver).
2. The offsets are tied to the Controller IP and not to your SoC. Any
differences that arise due to IP Integration into your SoC should be
handled in the Glue driver (the one which you haven't upstreamed yet).
3. If the offsets in the Controller IP itself have changed, this
indicates a different IP version which has nothing to do with the SoC
that you are using.

Please clarify whether you are facing problems with the offsets due to a
difference in the IP Controller Version, or due to the way in which the IP
was integrated into your SoC.

> 
> You can refer to the pci_find_*capability() or dw_pcie_find_*capability API.
> The meaning of their appearance is the same as what I said, and the design
> of each company may be different.

These APIs exits since there are multiple users with different Controllers
(in the case of pci_find_*capability()) or different versions of the
Controller (in the case of dw_pcie_find_*capability) due to which we cannot
hard-code offsets. In the case of the Cadence PCIe Controller, I see only
one user in Upstream Linux at the moment which happens to be the
"pci-j721e.c" driver. Maybe there are other users, but the offsets seem
to be compatible with their SoCs in that case.

Regards,
Siddharth.

Re: [v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/9 18:02, Siddharth Vadapalli wrote:
>> Hi Siddharth,
>>
>> Prior to this patch, I don't think hard-coded is that reasonable. Because
>> the SOC design of each company does not guarantee that the offset of each
>> capability is the same. This parameter can be configured when selecting PCIe
>> configuration options. The current code that just happens to hit the offset
>> address is the same.
> 
> 1. You are modifying the driver for the Cadence PCIe Controller IP and
> not the one for your SoC (a.k.a the application/glue/wrapper driver).
> 2. The offsets are tied to the Controller IP and not to your SoC. Any
> differences that arise due to IP Integration into your SoC should be
> handled in the Glue driver (the one which you haven't upstreamed yet).
> 3. If the offsets in the Controller IP itself have changed, this
> indicates a different IP version which has nothing to do with the SoC
> that you are using.
> 
> Please clarify whether you are facing problems with the offsets due to a
> difference in the IP Controller Version, or due to the way in which the IP
> was integrated into your SoC.
> 

Hi Siddharth,

I have consulted several PCIe RTL designers in the past two days. They 
told me that the controller IP of Synopsys or Cadence can be configured 
with the offset address of the capability. I don't think it has anything 
to do with SOC, it's just a feature of PCIe controller IP. In 
particular, the number of extended capability is relatively large. When 
RTL is generated, one more configuration may cause the offset addresses 
of extended capability to be different. Therefore, it is unreasonable to 
assign all the offset addresses in advance.

Here, I want to make Cadence PCIe common driver more general. When we 
keep developing new SoCs, the capability or extended capability offset 
address may eventually be inconsistent.

Thank you very much Siddharth for discussing this patch with me. I would 
like to know what other maintainers have to say about this.

>>
>> You can refer to the pci_find_*capability() or dw_pcie_find_*capability API.
>> The meaning of their appearance is the same as what I said, and the design
>> of each company may be different.
> 
> These APIs exits since there are multiple users with different Controllers
> (in the case of pci_find_*capability()) or different versions of the
> Controller (in the case of dw_pcie_find_*capability) due to which we cannot
> hard-code offsets. In the case of the Cadence PCIe Controller, I see only
> one user in Upstream Linux at the moment which happens to be the
> "pci-j721e.c" driver. Maybe there are other users, but the offsets seem
> to be compatible with their SoCs in that case.

Yes. Cadence definitely has a lot of customers, and from what I 
understand, not every customer wants to go upstream on their drive. Most 
of these customers are just local modifications, probably mimicking the 
API that the dwc driver already uses. We are, for example.

Best regards,
Hans

Re: [v2] PCI: cadence: Add configuration space capability search API

[Manivannan Sadhasivam]

On Mon, Mar 10, 2025 at 11:09:54PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/3/9 18:02, Siddharth Vadapalli wrote:
> > > Hi Siddharth,
> > > 
> > > Prior to this patch, I don't think hard-coded is that reasonable. Because
> > > the SOC design of each company does not guarantee that the offset of each
> > > capability is the same. This parameter can be configured when selecting PCIe
> > > configuration options. The current code that just happens to hit the offset
> > > address is the same.
> > 
> > 1. You are modifying the driver for the Cadence PCIe Controller IP and
> > not the one for your SoC (a.k.a the application/glue/wrapper driver).
> > 2. The offsets are tied to the Controller IP and not to your SoC. Any
> > differences that arise due to IP Integration into your SoC should be
> > handled in the Glue driver (the one which you haven't upstreamed yet).
> > 3. If the offsets in the Controller IP itself have changed, this
> > indicates a different IP version which has nothing to do with the SoC
> > that you are using.
> > 
> > Please clarify whether you are facing problems with the offsets due to a
> > difference in the IP Controller Version, or due to the way in which the IP
> > was integrated into your SoC.
> > 
> 
> Hi Siddharth,
> 
> I have consulted several PCIe RTL designers in the past two days. They told
> me that the controller IP of Synopsys or Cadence can be configured with the
> offset address of the capability. I don't think it has anything to do with
> SOC, it's just a feature of PCIe controller IP. In particular, the number of
> extended capability is relatively large. When RTL is generated, one more
> configuration may cause the offset addresses of extended capability to be
> different. Therefore, it is unreasonable to assign all the offset addresses
> in advance.
> 
> Here, I want to make Cadence PCIe common driver more general. When we keep
> developing new SoCs, the capability or extended capability offset address
> may eventually be inconsistent.
> 
> Thank you very much Siddharth for discussing this patch with me. I would
> like to know what other maintainers have to say about this.
> 

Even though this patch is mostly for an out of tree controller driver which is
not going to be upstreamed, the patch itself is serving some purpose. I really
like to avoid the hardcoded offsets wherever possible. So I'm in favor of this
patch.

However, these newly introduced functions are a duplicated version of DWC
functions. So we will end up with duplicated functions in multiple places. I'd
like them to be moved (both this and DWC) to drivers/pci/pci.c if possible. The
generic function *_find_capability() can accept the controller specific readl/
readw APIs and the controller specific private data.

- Mani

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

Re: [v2] PCI: cadence: Add configuration space capability search API

[Bjorn Helgaas]

On Fri, Mar 14, 2025 at 06:35:11PM +0530, Manivannan Sadhasivam wrote:
> ...

> Even though this patch is mostly for an out of tree controller
> driver which is not going to be upstreamed, the patch itself is
> serving some purpose. I really like to avoid the hardcoded offsets
> wherever possible. So I'm in favor of this patch.
> 
> However, these newly introduced functions are a duplicated version
> of DWC functions. So we will end up with duplicated functions in
> multiple places. I'd like them to be moved (both this and DWC) to
> drivers/pci/pci.c if possible. The generic function
> *_find_capability() can accept the controller specific readl/ readw
> APIs and the controller specific private data.

I agree, it would be really nice to share this code.

It looks a little messy to deal with passing around pointers to
controller read ops, and we'll still end up with a lot of duplicated
code between __pci_find_next_cap() and __cdns_pcie_find_next_cap(),
etc.

Maybe someday we'll make a generic way to access non-PCI "config"
space like this host controller space and PCIe RCRBs.

Or if you add interfaces that accept read/write ops, maybe the
existing pci_find_capability() etc could be refactored on top of them
by passing in pci_bus_read_config_word() as the accessor.

Bjorn

Re: [v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/15 04:31, Bjorn Helgaas wrote:
> On Fri, Mar 14, 2025 at 06:35:11PM +0530, Manivannan Sadhasivam wrote:
>> ...
> 
>> Even though this patch is mostly for an out of tree controller
>> driver which is not going to be upstreamed, the patch itself is
>> serving some purpose. I really like to avoid the hardcoded offsets
>> wherever possible. So I'm in favor of this patch.
>>
>> However, these newly introduced functions are a duplicated version
>> of DWC functions. So we will end up with duplicated functions in
>> multiple places. I'd like them to be moved (both this and DWC) to
>> drivers/pci/pci.c if possible. The generic function
>> *_find_capability() can accept the controller specific readl/ readw
>> APIs and the controller specific private data.
> 
> I agree, it would be really nice to share this code.
> 
> It looks a little messy to deal with passing around pointers to
> controller read ops, and we'll still end up with a lot of duplicated
> code between __pci_find_next_cap() and __cdns_pcie_find_next_cap(),
> etc.
> 
> Maybe someday we'll make a generic way to access non-PCI "config"
> space like this host controller space and PCIe RCRBs.
> 
> Or if you add interfaces that accept read/write ops, maybe the
> existing pci_find_capability() etc could be refactored on top of them
> by passing in pci_bus_read_config_word() as the accessor.
> 

Hi Bjorn,

I have already replied to an email, please help review whether it is 
appropriate.

Best regards,
Hans

Re: [v2] PCI: cadence: Add configuration space capability search API

[Bjorn Helgaas]

On Sat, Mar 15, 2025 at 08:06:52AM +0800, Hans Zhang wrote:
> On 2025/3/15 04:31, Bjorn Helgaas wrote:
> > On Fri, Mar 14, 2025 at 06:35:11PM +0530, Manivannan Sadhasivam wrote:
> > > ...
> > 
> > > Even though this patch is mostly for an out of tree controller
> > > driver which is not going to be upstreamed, the patch itself is
> > > serving some purpose. I really like to avoid the hardcoded offsets
> > > wherever possible. So I'm in favor of this patch.
> > > 
> > > However, these newly introduced functions are a duplicated version
> > > of DWC functions. So we will end up with duplicated functions in
> > > multiple places. I'd like them to be moved (both this and DWC) to
> > > drivers/pci/pci.c if possible. The generic function
> > > *_find_capability() can accept the controller specific readl/ readw
> > > APIs and the controller specific private data.
> > 
> > I agree, it would be really nice to share this code.
> > 
> > It looks a little messy to deal with passing around pointers to
> > controller read ops, and we'll still end up with a lot of duplicated
> > code between __pci_find_next_cap() and __cdns_pcie_find_next_cap(),
> > etc.
> > 
> > Maybe someday we'll make a generic way to access non-PCI "config"
> > space like this host controller space and PCIe RCRBs.
> > 
> > Or if you add interfaces that accept read/write ops, maybe the
> > existing pci_find_capability() etc could be refactored on top of them
> > by passing in pci_bus_read_config_word() as the accessor.
> 
> I have already replied to an email, please help review whether it is
> appropriate.

URL to the email on lore.kernel.org?

If you mean this:
https://lore.kernel.org/r/3cadf8d5-c4d8-4941-ae2e-8b00ceb83a8f@163.com,
just post it as a v3 patch with the usual commit log and motivation
for the change, and it will automatically get picked up in patchwork
so it doesn't get forgotten.

Right now the urgency seems fairly low since (IIUC) it doesn't fix an
existing bug in the upstream code.

Bjorn

Re: [v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/21 05:28, Bjorn Helgaas wrote:
> On Sat, Mar 15, 2025 at 08:06:52AM +0800, Hans Zhang wrote:
>> On 2025/3/15 04:31, Bjorn Helgaas wrote:
>>> On Fri, Mar 14, 2025 at 06:35:11PM +0530, Manivannan Sadhasivam wrote:
>>>> ...
>>>
>>>> Even though this patch is mostly for an out of tree controller
>>>> driver which is not going to be upstreamed, the patch itself is
>>>> serving some purpose. I really like to avoid the hardcoded offsets
>>>> wherever possible. So I'm in favor of this patch.
>>>>
>>>> However, these newly introduced functions are a duplicated version
>>>> of DWC functions. So we will end up with duplicated functions in
>>>> multiple places. I'd like them to be moved (both this and DWC) to
>>>> drivers/pci/pci.c if possible. The generic function
>>>> *_find_capability() can accept the controller specific readl/ readw
>>>> APIs and the controller specific private data.
>>>
>>> I agree, it would be really nice to share this code.
>>>
>>> It looks a little messy to deal with passing around pointers to
>>> controller read ops, and we'll still end up with a lot of duplicated
>>> code between __pci_find_next_cap() and __cdns_pcie_find_next_cap(),
>>> etc.
>>>
>>> Maybe someday we'll make a generic way to access non-PCI "config"
>>> space like this host controller space and PCIe RCRBs.
>>>
>>> Or if you add interfaces that accept read/write ops, maybe the
>>> existing pci_find_capability() etc could be refactored on top of them
>>> by passing in pci_bus_read_config_word() as the accessor.
>>
>> I have already replied to an email, please help review whether it is
>> appropriate.
> 
> URL to the email on lore.kernel.org?
> 
> If you mean this:
> https://lore.kernel.org/r/3cadf8d5-c4d8-4941-ae2e-8b00ceb83a8f@163.com,
> just post it as a v3 patch with the usual commit log and motivation
> for the change, and it will automatically get picked up in patchwork
> so it doesn't get forgotten.
> 
> Right now the urgency seems fairly low since (IIUC) it doesn't fix an
> existing bug in the upstream code.

Hi Bjorn,

Thanks your for reply. I will submit v3 patch.

Best regards,
Hans

Re: [v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/14 21:05, Manivannan Sadhasivam wrote:
> On Mon, Mar 10, 2025 at 11:09:54PM +0800, Hans Zhang wrote:
>>
>>
>> On 2025/3/9 18:02, Siddharth Vadapalli wrote:
>>>> Hi Siddharth,
>>>>
>>>> Prior to this patch, I don't think hard-coded is that reasonable. Because
>>>> the SOC design of each company does not guarantee that the offset of each
>>>> capability is the same. This parameter can be configured when selecting PCIe
>>>> configuration options. The current code that just happens to hit the offset
>>>> address is the same.
>>>
>>> 1. You are modifying the driver for the Cadence PCIe Controller IP and
>>> not the one for your SoC (a.k.a the application/glue/wrapper driver).
>>> 2. The offsets are tied to the Controller IP and not to your SoC. Any
>>> differences that arise due to IP Integration into your SoC should be
>>> handled in the Glue driver (the one which you haven't upstreamed yet).
>>> 3. If the offsets in the Controller IP itself have changed, this
>>> indicates a different IP version which has nothing to do with the SoC
>>> that you are using.
>>>
>>> Please clarify whether you are facing problems with the offsets due to a
>>> difference in the IP Controller Version, or due to the way in which the IP
>>> was integrated into your SoC.
>>>
>>
>> Hi Siddharth,
>>
>> I have consulted several PCIe RTL designers in the past two days. They told
>> me that the controller IP of Synopsys or Cadence can be configured with the
>> offset address of the capability. I don't think it has anything to do with
>> SOC, it's just a feature of PCIe controller IP. In particular, the number of
>> extended capability is relatively large. When RTL is generated, one more
>> configuration may cause the offset addresses of extended capability to be
>> different. Therefore, it is unreasonable to assign all the offset addresses
>> in advance.
>>
>> Here, I want to make Cadence PCIe common driver more general. When we keep
>> developing new SoCs, the capability or extended capability offset address
>> may eventually be inconsistent.
>>
>> Thank you very much Siddharth for discussing this patch with me. I would
>> like to know what other maintainers have to say about this.
>>
> 
> Even though this patch is mostly for an out of tree controller driver which is
> not going to be upstreamed, the patch itself is serving some purpose. I really
> like to avoid the hardcoded offsets wherever possible. So I'm in favor of this
> patch.
> 
> However, these newly introduced functions are a duplicated version of DWC
> functions. So we will end up with duplicated functions in multiple places. I'd
> like them to be moved (both this and DWC) to drivers/pci/pci.c if possible. The
> generic function *_find_capability() can accept the controller specific readl/
> readw APIs and the controller specific private data.


Hi Mani,

Thanks Mani for the idea. Please check whether the following patches are 
OK. If yes, I will submit a series of patches again.

I have tested on RK3588 development board, as well as cdns platform is OK.

diff --git a/drivers/pci/controller/cadence/pcie-cadence.c 
b/drivers/pci/controller/cadence/pcie-cadence.c
index 204e045aed8c..e1ce3ad612c9 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_find_cap(struct cdns_pcie *pcie, int cap)
+{
+	return pci_generic_find_capability(pcie, cdns_pcie_read_cfg, cap);
+}
+
+u16 cdns_find_ext_cap(struct cdns_pcie *pcie, int 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..e90464c4a660 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_find_cap(struct cdns_pcie *pcie, int cap);
+u16 cdns_find_ext_cap(struct cdns_pcie *pcie, int 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,
diff --git a/drivers/pci/controller/dwc/pcie-designware.c 
b/drivers/pci/controller/dwc/pcie-designware.c
index 145e7f579072..4b645ad58b35 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -283,6 +283,22 @@ u16 dw_pcie_find_ext_capability(struct dw_pcie 
*pci, u8 cap)
  }
  EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);

+static u32 dwc_pcie_read_cfg(void *priv, int where, int size)
+{
+	struct dw_pcie *pcie = priv;
+	return dw_pcie_read_dbi(pcie, where, size);
+}
+
+u8 dwc_find_cap(struct dw_pcie *pcie, int cap)
+{
+	return pci_generic_find_capability(pcie, dwc_pcie_read_cfg, cap);
+}
+
+u16 dwc_find_ext_cap(struct dw_pcie *pcie, int cap)
+{
+	return pci_generic_find_ext_capability(pcie, dwc_pcie_read_cfg, cap);
+}
+
  int dw_pcie_read(void __iomem *addr, int size, u32 *val)
  {
  	if (!IS_ALIGNED((uintptr_t)addr, size)) {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h 
b/drivers/pci/controller/dwc/pcie-designware.h
index 501d9ddfea16..260c6672a4f5 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -479,6 +479,9 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
  u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);

+u8 dwc_find_cap(struct dw_pcie *pcie, int cap);
+u16 dwc_find_ext_cap(struct dw_pcie *pcie, int cap);
+
  int dw_pcie_read(void __iomem *addr, int size, u32 *val);
  int dw_pcie_write(void __iomem *addr, int size, u32 val);

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 869d204a70a3..4376bee65cc6 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -612,6 +612,84 @@ u16 pci_find_ext_capability(struct pci_dev *dev, 
int cap)
  }
  EXPORT_SYMBOL_GPL(pci_find_ext_capability);

+static u8 __pci_generic_find_next_cap(void *priv, pci_generic_read_cfg 
read_cfg,
+				      u8 cap_ptr, int 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,
+			       int 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,
+				    int 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..63744f3de3a4 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,
+			       int cap);
+u16 pci_generic_find_ext_capability(void *priv, pci_generic_read_cfg 
read_cfg,
+				    int 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,13 @@ 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);
+u8 pci_generic_find_capability(void *priv, pci_generic_read_cfg read_cfg,
+			       int cap)
+{ return 0; }
+u16 pci_generic_find_ext_capability(void *priv, pci_generic_read_cfg 
read_cfg,
+				    int cap)
+{ return 0; }
  static inline u64 pci_get_dsn(struct pci_dev *dev)
  { return 0; }


Best regards,
Hans