RE: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

["Shradha Todi" <shradha.t@samsung.com>]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 06 December 2024 21:43
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; manivannan.sadhasivam@linaro.org; lpieralisi@kernel.org;
> kw@linux.com; robh@kernel.org; bhelgaas@google.com; jingoohan1@gmail.com; Jonathan.Cameron@huawei.com;
> fan.ni@samsung.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com; quic_nitegupt@quicinc.com;
> quic_krichai@quicinc.com; gost.dev@samsung.com
> Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> 
> On Fri, Dec 06, 2024 at 01:14:55PM +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 6d6cbc8b5b2c..41230c5e4a53 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -277,6 +277,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)
> 
> To make sure that we find a VSEC ID that corresponds to the expected vendor, I think this interface needs to be the
same
> as pci_find_vsec_capability().  In particular, it needs to take a "u16 vendor"

As per my understanding, Synopsys is the vendor here when we talk about vsec capabilities.
VSEC cap IDs are fixed for each vendor (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP,
0x4 is always PTM responder and so on).
So no matter if the DWC IP is being integrated by Samsung, NVDIA or Qcom, the vendor specific CAP IDs will
remain constant. Now since this function is being written as part of designware file, the control will reach here
only when the PCIe IP is DWC. So, we don't really require a vendor ID to be checked here. EG: If 0x2 VSEC ID is present
in any DWC controller, it means RAS is supported. Please correct me if I'm wrong.

>and a "u16 vsec_cap".
> 
> (pci_find_vsec_capability() takes an "int cap", but I don't think that's quite right).
> 

It should be u16 vsec_cap. You're right. I will fix this in the next patchset.

Shradha

> > +{
> > +	u16 vsec = 0;
> > +	u32 header;
> > +
> > +	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);
> > +
> >  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)  {
> >  	return dw_pcie_find_next_ext_capability(pci, 0, cap); diff --git
> > a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index 347ab74ac35a..98a057820bc7 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -476,6 +476,7 @@ 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);
> > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap);
> >
> >  int dw_pcie_read(void __iomem *addr, int size, u32 *val);  int
> > dw_pcie_write(void __iomem *addr, int size, u32 val);
> > --
> > 2.17.1
> >