| Message ID | 20241206074456.17401-2-shradha.t@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for RAS DES feature in PCIe DW | expand |
Hi Shradha, kernel test robot noticed the following build warnings: [auto build test WARNING on pci/next] [also build test WARNING on pci/for-linus mani-mhi/mhi-next linus/master v6.13-rc1 next-20241205] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Shradha-Todi/PCI-dwc-Add-support-for-vendor-specific-capability-search/20241206-163620 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20241206074456.17401-2-shradha.t%40samsung.com patch subject: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search config: i386-buildonly-randconfig-006 (https://download.01.org/0day-ci/archive/20241206/202412061940.WSlxZ8i1-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241206/202412061940.WSlxZ8i1-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/202412061940.WSlxZ8i1-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/pci/controller/dwc/pcie-designware.c: In function 'dw_pcie_find_vsec_capability': >> drivers/pci/controller/dwc/pcie-designware.c:285:16: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 285 | while (vsec = dw_pcie_find_next_ext_capability(pci, vsec, | ^~~~ vim +285 drivers/pci/controller/dwc/pcie-designware.c 279 280 u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap) 281 { 282 u16 vsec = 0; 283 u32 header; 284 > 285 while (vsec = dw_pcie_find_next_ext_capability(pci, vsec, 286 PCI_EXT_CAP_ID_VNDR)) { 287 header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER); 288 if (PCI_VNDR_HEADER_ID(header) == vsec_cap) 289 return vsec; 290 } 291 292 return 0; 293 } 294 EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability); 295
Hi Shradha,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus mani-mhi/mhi-next linus/master v6.13-rc1 next-20241205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shradha-Todi/PCI-dwc-Add-support-for-vendor-specific-capability-search/20241206-163620
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20241206074456.17401-2-shradha.t%40samsung.com
patch subject: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
config: arm64-randconfig-003-20241206 (https://download.01.org/0day-ci/archive/20241206/202412062046.0XR2e2V6-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241206/202412062046.0XR2e2V6-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/202412062046.0XR2e2V6-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/pci/controller/dwc/pcie-designware.c:15:
In file included from include/linux/dma/edma.h:13:
In file included from include/linux/dmaengine.h:12:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2223:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/dwc/pcie-designware.c:285:14: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
285 | while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
| ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
286 | PCI_EXT_CAP_ID_VNDR)) {
| ~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/dwc/pcie-designware.c:285:14: note: place parentheses around the assignment to silence this warning
285 | while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
| ^
| (
286 | PCI_EXT_CAP_ID_VNDR)) {
|
| )
drivers/pci/controller/dwc/pcie-designware.c:285:14: note: use '==' to turn this assignment into an equality comparison
285 | while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
| ^
| ==
5 warnings generated.
vim +285 drivers/pci/controller/dwc/pcie-designware.c
279
280 u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
281 {
282 u16 vsec = 0;
283 u32 header;
284
> 285 while (vsec = dw_pcie_find_next_ext_capability(pci, vsec,
286 PCI_EXT_CAP_ID_VNDR)) {
287 header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
288 if (PCI_VNDR_HEADER_ID(header) == vsec_cap)
289 return vsec;
290 }
291
292 return 0;
293 }
294 EXPORT_SYMBOL_GPL(dw_pcie_find_vsec_capability);
295
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" and a "u16 vsec_cap". (pci_find_vsec_capability() takes an "int cap", but I don't think that's quite right). > +{ > + 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 >
> -----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 > >
On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote: > > -----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). For VSEC, the vendor that matters is the one identified at 0x0 in config space. That's why pci_find_vsec_capability() checks the supplied "vendor" against "dev->vendor". > 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. In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom, etc., even though it may contain Synopsys DWC IP. Each vendor assigns VSEC IDs independently, so VSEC ID 0x2 may mean something different to Samsung than it does to NVIDIA or Qcom. PCIe r6.0, sec 7.9.5 has the details, but the important part is this: With a PCI Express Function, the structure and definition of the vendor-specific Registers area is determined by the vendor indicated by the Vendor ID field located at byte offset 00h in PCI-compatible Configuration Space. There IS a separate DVSEC ("Designated Vendor-Specific") Capability; see sec 7.9.6. That one does include a DVSEC Vendor ID in the Capability itself, and this would make more sense for this situation. If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for RAS, then devices from Samsung, NVIDIA, Qcom, etc., could advertise a DVSEC Capability that contained a DVSEC Vendor ID of PCI_VENDOR_ID_SYNOPSYS with DVSEC ID 0x2, and all those devices could easily locate it. Unfortunately Samsung et al used VSEC instead of DVSEC, so we're stuck with having to specify the device vendor and the VSEC ID assigned by that vendor, and those VSEC IDs might be different per vendor. Bjorn
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) +{ + 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);
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(+)