Re: [PATCH 1/3] PCI: dwc: Add support for vendor specific capability search

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Jun 25, 2024 at 03:08:11PM +0530, Shradha Todi wrote:
> Add vendor specific extended configuration space capability search API
> using struct dw_pcie pointer for DW controllers.
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 250cf7f40b85..b74e4a97558e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -275,6 +275,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
>  	return 0;
>  }
>  
> +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
> +{
> +	u16 vsec = 0;
> +	u32 header;

IIUC, any use of PCI_EXT_CAP_ID_VNDR should check dev->vendor, as
pci_find_vsec_capability() does.

You only know how "vsec" works if you also know the vendor who defined
vsec.

Do you expect DW_PCIE_RAS_DES_CAP to be present only in devices with
Vendor ID of PCI_VENDOR_ID_SYNOPSYS?

I was hoping that this was generic DesignWare functionality that might
be present in devices from other vendors that incorporated the DWC IP.
Those devices would likely have other Vendor IDs.  In that case, a
DVSEC (not VSEC) capability might be more appropriate.

> +	while ((vsec = dw_pcie_find_next_ext_capability(pci, vsec,
> +					PCI_EXT_CAP_ID_VNDR))) {
> +		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
> +		if (PCI_VNDR_HEADER_ID(header) == vsec_cap)
> +			return vsec;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability);