diff mbox series

PCI: add helper function to find DVSEC

Message ID 160409768616.919324.13994867117217584719.stgit@djiang5-desk3.ch.intel.com
State New
Headers show
Series PCI: add helper function to find DVSEC | expand

Commit Message

Dave Jiang Oct. 30, 2020, 10:42 p.m. UTC 
Add function that searches for DVSEC and returns the offset in PCI
configuration space for the interested DVSEC capability.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
---

The patch has dependency on David Box’s dvsec definition patch:
https://lore.kernel.org/linux-pci/bc5f059c5bae957daebde699945c80808286bf45.camel@linux.intel.com/T/#m1d0dc12e3b2c739e2c37106a45f325bb8f001774

 drivers/pci/pci.c   |   30 ++++++++++++++++++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 33 insertions(+)

Comments

Randy Dunlap Oct. 31, 2020, 5:35 a.m. UTC | #1 
On 10/30/20 3:42 PM, Dave Jiang wrote:
> Add function that searches for DVSEC and returns the offset in PCI
> configuration space for the interested DVSEC capability.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
> ---
> 
> The patch has dependency on David Box’s dvsec definition patch:
> https://lore.kernel.org/linux-pci/bc5f059c5bae957daebde699945c80808286bf45.camel@linux.intel.com/T/#m1d0dc12e3b2c739e2c37106a45f325bb8f001774
> 
>  drivers/pci/pci.c   |   30 ++++++++++++++++++++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6d4d5a2f923d..49e57b831509 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -589,6 +589,36 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap)
>  }
>  EXPORT_SYMBOL_GPL(pci_find_ext_capability);
>  
> +/**
> + * pci_find_dvsec - return position of DVSEC with provided vendor and DVSEC ID
> + * @dev: the PCI device
> + * @vendor: vendor for the DVSEC
> + * @id: the DVSEC capibility ID

                     capability

> + *
> + * Return the offset of DVSEC on success or -ENOTSUPP if not found

    * Return: the offset of DVSEC on success or -ENOTSUPP if not found

> + */
> +int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id)
> +{
> +	u16 dev_vendor, dev_id;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DVSEC);
> +	if (!pos)
> +		return -ENOTSUPP;
> +
> +	while (pos) {
> +		pci_read_config_word(dev, pos + PCI_DVSEC_HEADER1, &dev_vendor);
> +		pci_read_config_word(dev, pos + PCI_DVSEC_HEADER2, &dev_id);
> +		if (dev_vendor == vendor && dev_id == id)
> +			return pos;
> +
> +		pos = pci_find_next_ext_capability(dev, pos, PCI_EXT_CAP_ID_DVSEC);
> +	}
> +
> +	return -ENOTSUPP;
> +}
> +EXPORT_SYMBOL_GPL(pci_find_dvsec);
> +
>  /**
>   * 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 22207a79762c..6c692d32c82a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1069,6 +1069,7 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap);
>  int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
>  int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
>  int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
> +int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id);
>  struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
>  
>  u64 pci_get_dsn(struct pci_dev *dev);
> @@ -1726,6 +1727,8 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post,
>  { return 0; }
>  static inline int pci_find_ext_capability(struct pci_dev *dev, int cap)
>  { return 0; }
> +static inline int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id)
> +{ return 0; }

Why shouldn't this return -ENOTSUPP instead of 0?

>  
>  static inline u64 pci_get_dsn(struct pci_dev *dev)
>  { return 0; }
> 
>
kernel test robot Nov. 1, 2020, 5:54 p.m. UTC | #2 
Hi Dave,

I love your patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.10-rc1 next-20201030]
[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]

url:    https://github.com/0day-ci/linux/commits/Dave-Jiang/PCI-add-helper-function-to-find-DVSEC/20201031-220304
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/5c1551e498323db6a917599f6f79f37080c22825
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dave-Jiang/PCI-add-helper-function-to-find-DVSEC/20201031-220304
        git checkout 5c1551e498323db6a917599f6f79f37080c22825
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/pci/pci.c: In function 'pci_find_dvsec':
>> drivers/pci/pci.c:605:37: error: 'PCI_EXT_CAP_ID_DVSEC' undeclared (first use in this function); did you mean 'PCI_EXT_CAP_ID_DPC'?
     605 |  pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DVSEC);
         |                                     ^~~~~~~~~~~~~~~~~~~~
         |                                     PCI_EXT_CAP_ID_DPC
   drivers/pci/pci.c:605:37: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/pci/pci.c:610:35: error: 'PCI_DVSEC_HEADER1' undeclared (first use in this function); did you mean 'PCI_VSEC_HDR'?
     610 |   pci_read_config_word(dev, pos + PCI_DVSEC_HEADER1, &dev_vendor);
         |                                   ^~~~~~~~~~~~~~~~~
         |                                   PCI_VSEC_HDR
>> drivers/pci/pci.c:611:35: error: 'PCI_DVSEC_HEADER2' undeclared (first use in this function); did you mean 'PCI_VSEC_HDR'?
     611 |   pci_read_config_word(dev, pos + PCI_DVSEC_HEADER2, &dev_id);
         |                                   ^~~~~~~~~~~~~~~~~
         |                                   PCI_VSEC_HDR

vim +605 drivers/pci/pci.c

   591	
   592	/**
   593	 * pci_find_dvsec - return position of DVSEC with provided vendor and DVSEC ID
   594	 * @dev: the PCI device
   595	 * @vendor: vendor for the DVSEC
   596	 * @id: the DVSEC capibility ID
   597	 *
   598	 * Return the offset of DVSEC on success or -ENOTSUPP if not found
   599	 */
   600	int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id)
   601	{
   602		u16 dev_vendor, dev_id;
   603		int pos;
   604	
 > 605		pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DVSEC);
   606		if (!pos)
   607			return -ENOTSUPP;
   608	
   609		while (pos) {
 > 610			pci_read_config_word(dev, pos + PCI_DVSEC_HEADER1, &dev_vendor);
 > 611			pci_read_config_word(dev, pos + PCI_DVSEC_HEADER2, &dev_id);
   612			if (dev_vendor == vendor && dev_id == id)
   613				return pos;
   614	
   615			pos = pci_find_next_ext_capability(dev, pos, PCI_EXT_CAP_ID_DVSEC);
   616		}
   617	
   618		return -ENOTSUPP;
   619	}
   620	EXPORT_SYMBOL_GPL(pci_find_dvsec);
   621	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Dave Jiang Nov. 2, 2020, 3:50 p.m. UTC | #3 
On 10/30/2020 10:35 PM, Randy Dunlap wrote:
> On 10/30/20 3:42 PM, Dave Jiang wrote:
>> Add function that searches for DVSEC and returns the offset in PCI
>> configuration space for the interested DVSEC capability.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
>> ---
>>
>> The patch has dependency on David Box’s dvsec definition patch:
>> https://lore.kernel.org/linux-pci/bc5f059c5bae957daebde699945c80808286bf45.camel@linux.intel.com/T/#m1d0dc12e3b2c739e2c37106a45f325bb8f001774
>>
>>   drivers/pci/pci.c   |   30 ++++++++++++++++++++++++++++++
>>   include/linux/pci.h |    3 +++
>>   2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 6d4d5a2f923d..49e57b831509 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -589,6 +589,36 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap)
>>   }
>>   EXPORT_SYMBOL_GPL(pci_find_ext_capability);
>>   
>> +/**
>> + * pci_find_dvsec - return position of DVSEC with provided vendor and DVSEC ID
>> + * @dev: the PCI device
>> + * @vendor: vendor for the DVSEC
>> + * @id: the DVSEC capibility ID
> 
>                       capability
> 
>> + *
>> + * Return the offset of DVSEC on success or -ENOTSUPP if not found
> 
>      * Return: the offset of DVSEC on success or -ENOTSUPP if not found
> 
>> + */
>> +int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id)
>> +{
>> +	u16 dev_vendor, dev_id;
>> +	int pos;
>> +
>> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DVSEC);
>> +	if (!pos)
>> +		return -ENOTSUPP;
>> +
>> +	while (pos) {
>> +		pci_read_config_word(dev, pos + PCI_DVSEC_HEADER1, &dev_vendor);
>> +		pci_read_config_word(dev, pos + PCI_DVSEC_HEADER2, &dev_id);
>> +		if (dev_vendor == vendor && dev_id == id)
>> +			return pos;
>> +
>> +		pos = pci_find_next_ext_capability(dev, pos, PCI_EXT_CAP_ID_DVSEC);
>> +	}
>> +
>> +	return -ENOTSUPP;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_find_dvsec);
>> +
>>   /**
>>    * 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 22207a79762c..6c692d32c82a 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1069,6 +1069,7 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap);
>>   int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
>>   int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
>>   int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
>> +int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id);
>>   struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
>>   
>>   u64 pci_get_dsn(struct pci_dev *dev);
>> @@ -1726,6 +1727,8 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post,
>>   { return 0; }
>>   static inline int pci_find_ext_capability(struct pci_dev *dev, int cap)
>>   { return 0; }
>> +static inline int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id)
>> +{ return 0; }
> 
> Why shouldn't this return -ENOTSUPP instead of 0?

Actually looking at the other find cap functions. Should I be returning 0 for 
all failures?

> 
>>   
>>   static inline u64 pci_get_dsn(struct pci_dev *dev)
>>   { return 0; }
>>
>>
>
diff mbox series

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6d4d5a2f923d..49e57b831509 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -589,6 +589,36 @@  int pci_find_ext_capability(struct pci_dev *dev, int cap)
 }
 EXPORT_SYMBOL_GPL(pci_find_ext_capability);
 
+/**
+ * pci_find_dvsec - return position of DVSEC with provided vendor and DVSEC ID
+ * @dev: the PCI device
+ * @vendor: vendor for the DVSEC
+ * @id: the DVSEC capibility ID
+ *
+ * Return the offset of DVSEC on success or -ENOTSUPP if not found
+ */
+int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id)
+{
+	u16 dev_vendor, dev_id;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DVSEC);
+	if (!pos)
+		return -ENOTSUPP;
+
+	while (pos) {
+		pci_read_config_word(dev, pos + PCI_DVSEC_HEADER1, &dev_vendor);
+		pci_read_config_word(dev, pos + PCI_DVSEC_HEADER2, &dev_id);
+		if (dev_vendor == vendor && dev_id == id)
+			return pos;
+
+		pos = pci_find_next_ext_capability(dev, pos, PCI_EXT_CAP_ID_DVSEC);
+	}
+
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(pci_find_dvsec);
+
 /**
  * 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 22207a79762c..6c692d32c82a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1069,6 +1069,7 @@  int pci_find_ext_capability(struct pci_dev *dev, int cap);
 int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
 int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
 int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
+int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id);
 struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
 
 u64 pci_get_dsn(struct pci_dev *dev);
@@ -1726,6 +1727,8 @@  static inline int pci_find_next_capability(struct pci_dev *dev, u8 post,
 { return 0; }
 static inline int pci_find_ext_capability(struct pci_dev *dev, int cap)
 { return 0; }
+static inline int pci_find_dvsec(struct pci_dev *dev, u16 vendor, u16 id)
+{ return 0; }
 
 static inline u64 pci_get_dsn(struct pci_dev *dev)
 { return 0; }