diff mbox series

PCI/VPD: Use unaligned access helpers in pci_vpd_read

Message ID 6edebb53-b714-3205-6266-d02416fd3cfe@gmail.com
State New
Headers show
Series PCI/VPD: Use unaligned access helpers in pci_vpd_read | expand

Commit Message

Heiner Kallweit May 1, 2021, 1:43 p.m. UTC 
With this patch we avoid some unnecessary looping and make the code
better readable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/pci/vpd.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

kernel test robot May 1, 2021, 4:52 p.m. UTC | #1 
Hi Heiner,

I love your patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.12 next-20210430]
[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/Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-randconfig-s002-20210501 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/3115b0380e42b10762f7eee96f10b5a02cb4d2d5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
        git checkout 3115b0380e42b10762f7eee96f10b5a02cb4d2d5
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386 

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/vpd.c: In function 'pci_vpd_read':
>> drivers/pci/vpd.c:224:4: error: implicit declaration of function 'put_unaligned_le32' [-Werror=implicit-function-declaration]
     224 |    put_unaligned_le32(val, buf);
         |    ^~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/put_unaligned_le32 +224 drivers/pci/vpd.c

   168	
   169	static ssize_t pci_vpd_read(struct pci_dev *dev, loff_t pos, size_t count,
   170				    void *buf)
   171	{
   172		struct pci_vpd *vpd = dev->vpd;
   173		int ret;
   174		loff_t end = pos + count;
   175	
   176		if (pos < 0)
   177			return -EINVAL;
   178	
   179		if (!vpd->valid) {
   180			vpd->valid = 1;
   181			vpd->len = pci_vpd_size(dev, vpd->len);
   182		}
   183	
   184		if (vpd->len == 0)
   185			return -EIO;
   186	
   187		if (pos > vpd->len)
   188			return 0;
   189	
   190		if (end > vpd->len) {
   191			end = vpd->len;
   192			count = end - pos;
   193		}
   194	
   195		if (mutex_lock_killable(&vpd->lock))
   196			return -EINTR;
   197	
   198		ret = pci_vpd_wait(dev);
   199		if (ret < 0)
   200			goto out;
   201	
   202		while (pos < end) {
   203			unsigned int len, skip;
   204			u32 val;
   205	
   206			ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
   207							 pos & ~3);
   208			if (ret < 0)
   209				break;
   210			vpd->busy = 1;
   211			vpd->flag = PCI_VPD_ADDR_F;
   212			ret = pci_vpd_wait(dev);
   213			if (ret < 0)
   214				break;
   215	
   216			ret = pci_user_read_config_dword(dev, vpd->cap + PCI_VPD_DATA, &val);
   217			if (ret < 0)
   218				break;
   219	
   220			skip = pos & 3;
   221			len = min_t(unsigned int, 4 - skip, end - pos);
   222	
   223			if (len == 4)  {
 > 224				put_unaligned_le32(val, buf);
   225			} else {
   226				u8 tmpbuf[4];
   227	
   228				put_unaligned_le32(val, tmpbuf);
   229				memcpy(buf, tmpbuf + skip, len);
   230			}
   231	
   232			buf += len;
   233			pos += len;
   234		}
   235	out:
   236		mutex_unlock(&vpd->lock);
   237		return ret ? ret : count;
   238	}
   239	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot May 1, 2021, 5:23 p.m. UTC | #2 
Hi Heiner,

I love your patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.12 next-20210430]
[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/Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: powerpc-randconfig-r012-20210501 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8f5a2a5836cc8e4c1def2bdeb022e7b496623439)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/0day-ci/linux/commit/3115b0380e42b10762f7eee96f10b5a02cb4d2d5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
        git checkout 3115b0380e42b10762f7eee96f10b5a02cb4d2d5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=powerpc 

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/vpd.c:224:4: error: implicit declaration of function 'put_unaligned_le32' [-Werror,-Wimplicit-function-declaration]
                           put_unaligned_le32(val, buf);
                           ^
   drivers/pci/vpd.c:228:4: error: implicit declaration of function 'put_unaligned_le32' [-Werror,-Wimplicit-function-declaration]
                           put_unaligned_le32(val, tmpbuf);
                           ^
   2 errors generated.


vim +/put_unaligned_le32 +224 drivers/pci/vpd.c

   168	
   169	static ssize_t pci_vpd_read(struct pci_dev *dev, loff_t pos, size_t count,
   170				    void *buf)
   171	{
   172		struct pci_vpd *vpd = dev->vpd;
   173		int ret;
   174		loff_t end = pos + count;
   175	
   176		if (pos < 0)
   177			return -EINVAL;
   178	
   179		if (!vpd->valid) {
   180			vpd->valid = 1;
   181			vpd->len = pci_vpd_size(dev, vpd->len);
   182		}
   183	
   184		if (vpd->len == 0)
   185			return -EIO;
   186	
   187		if (pos > vpd->len)
   188			return 0;
   189	
   190		if (end > vpd->len) {
   191			end = vpd->len;
   192			count = end - pos;
   193		}
   194	
   195		if (mutex_lock_killable(&vpd->lock))
   196			return -EINTR;
   197	
   198		ret = pci_vpd_wait(dev);
   199		if (ret < 0)
   200			goto out;
   201	
   202		while (pos < end) {
   203			unsigned int len, skip;
   204			u32 val;
   205	
   206			ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
   207							 pos & ~3);
   208			if (ret < 0)
   209				break;
   210			vpd->busy = 1;
   211			vpd->flag = PCI_VPD_ADDR_F;
   212			ret = pci_vpd_wait(dev);
   213			if (ret < 0)
   214				break;
   215	
   216			ret = pci_user_read_config_dword(dev, vpd->cap + PCI_VPD_DATA, &val);
   217			if (ret < 0)
   218				break;
   219	
   220			skip = pos & 3;
   221			len = min_t(unsigned int, 4 - skip, end - pos);
   222	
   223			if (len == 4)  {
 > 224				put_unaligned_le32(val, buf);
   225			} else {
   226				u8 tmpbuf[4];
   227	
   228				put_unaligned_le32(val, tmpbuf);
   229				memcpy(buf, tmpbuf + skip, len);
   230			}
   231	
   232			buf += len;
   233			pos += len;
   234		}
   235	out:
   236		mutex_unlock(&vpd->lock);
   237		return ret ? ret : count;
   238	}
   239	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Heiner Kallweit May 1, 2021, 7:53 p.m. UTC | #3 
On 01.05.2021 18:52, kernel test robot wrote:
> Hi Heiner,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on pci/next]
> [also build test ERROR on v5.12 next-20210430]
> [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/Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
> config: i386-randconfig-s002-20210501 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce:
>         # apt-get install sparse
>         # sparse version: v0.6.3-341-g8af24329-dirty
>         # https://github.com/0day-ci/linux/commit/3115b0380e42b10762f7eee96f10b5a02cb4d2d5
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
>         git checkout 3115b0380e42b10762f7eee96f10b5a02cb4d2d5
>         # save the attached .config to linux build tree
>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386 
> 
> 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/vpd.c: In function 'pci_vpd_read':
>>> drivers/pci/vpd.c:224:4: error: implicit declaration of function 'put_unaligned_le32' [-Werror=implicit-function-declaration]
>      224 |    put_unaligned_le32(val, buf);
>          |    ^~~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors
> 
> 
> vim +/put_unaligned_le32 +224 drivers/pci/vpd.c
> 
>    168	
>    169	static ssize_t pci_vpd_read(struct pci_dev *dev, loff_t pos, size_t count,
>    170				    void *buf)
>    171	{
>    172		struct pci_vpd *vpd = dev->vpd;
>    173		int ret;
>    174		loff_t end = pos + count;
>    175	
>    176		if (pos < 0)
>    177			return -EINVAL;
>    178	
>    179		if (!vpd->valid) {
>    180			vpd->valid = 1;
>    181			vpd->len = pci_vpd_size(dev, vpd->len);
>    182		}
>    183	
>    184		if (vpd->len == 0)
>    185			return -EIO;
>    186	
>    187		if (pos > vpd->len)
>    188			return 0;
>    189	
>    190		if (end > vpd->len) {
>    191			end = vpd->len;
>    192			count = end - pos;
>    193		}
>    194	
>    195		if (mutex_lock_killable(&vpd->lock))
>    196			return -EINTR;
>    197	
>    198		ret = pci_vpd_wait(dev);
>    199		if (ret < 0)
>    200			goto out;
>    201	
>    202		while (pos < end) {
>    203			unsigned int len, skip;
>    204			u32 val;
>    205	
>    206			ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
>    207							 pos & ~3);
>    208			if (ret < 0)
>    209				break;
>    210			vpd->busy = 1;
>    211			vpd->flag = PCI_VPD_ADDR_F;
>    212			ret = pci_vpd_wait(dev);
>    213			if (ret < 0)
>    214				break;
>    215	
>    216			ret = pci_user_read_config_dword(dev, vpd->cap + PCI_VPD_DATA, &val);
>    217			if (ret < 0)
>    218				break;
>    219	
>    220			skip = pos & 3;
>    221			len = min_t(unsigned int, 4 - skip, end - pos);
>    222	
>    223			if (len == 4)  {
>  > 224				put_unaligned_le32(val, buf);
>    225			} else {
>    226				u8 tmpbuf[4];
>    227	
>    228				put_unaligned_le32(val, tmpbuf);
>    229				memcpy(buf, tmpbuf + skip, len);
>    230			}
>    231	
>    232			buf += len;
>    233			pos += len;
>    234		}
>    235	out:
>    236		mutex_unlock(&vpd->lock);
>    237		return ret ? ret : count;
>    238	}
>    239	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 

It depends on "PCI/VPD: Use unaligned access helper in pci_vpd_write"
which takes care of including asm/unaligned.h
Heiner Kallweit May 7, 2021, 9:18 p.m. UTC | #4 
On 01.05.2021 21:53, Heiner Kallweit wrote:
> On 01.05.2021 18:52, kernel test robot wrote:
>> Hi Heiner,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on pci/next]
>> [also build test ERROR on v5.12 next-20210430]
>> [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/Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
>> config: i386-randconfig-s002-20210501 (attached as .config)
>> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>> reproduce:
>>         # apt-get install sparse
>>         # sparse version: v0.6.3-341-g8af24329-dirty
>>         # https://github.com/0day-ci/linux/commit/3115b0380e42b10762f7eee96f10b5a02cb4d2d5
>>         git remote add linux-review https://github.com/0day-ci/linux
>>         git fetch --no-tags linux-review Heiner-Kallweit/PCI-VPD-Use-unaligned-access-helpers-in-pci_vpd_read/20210501-214553
>>         git checkout 3115b0380e42b10762f7eee96f10b5a02cb4d2d5
>>         # save the attached .config to linux build tree
>>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386 
>>
>> 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/vpd.c: In function 'pci_vpd_read':
>>>> drivers/pci/vpd.c:224:4: error: implicit declaration of function 'put_unaligned_le32' [-Werror=implicit-function-declaration]
>>      224 |    put_unaligned_le32(val, buf);
>>          |    ^~~~~~~~~~~~~~~~~~
>>    cc1: some warnings being treated as errors
>>
>>
>> vim +/put_unaligned_le32 +224 drivers/pci/vpd.c
>>
>>    168	
>>    169	static ssize_t pci_vpd_read(struct pci_dev *dev, loff_t pos, size_t count,
>>    170				    void *buf)
>>    171	{
>>    172		struct pci_vpd *vpd = dev->vpd;
>>    173		int ret;
>>    174		loff_t end = pos + count;
>>    175	
>>    176		if (pos < 0)
>>    177			return -EINVAL;
>>    178	
>>    179		if (!vpd->valid) {
>>    180			vpd->valid = 1;
>>    181			vpd->len = pci_vpd_size(dev, vpd->len);
>>    182		}
>>    183	
>>    184		if (vpd->len == 0)
>>    185			return -EIO;
>>    186	
>>    187		if (pos > vpd->len)
>>    188			return 0;
>>    189	
>>    190		if (end > vpd->len) {
>>    191			end = vpd->len;
>>    192			count = end - pos;
>>    193		}
>>    194	
>>    195		if (mutex_lock_killable(&vpd->lock))
>>    196			return -EINTR;
>>    197	
>>    198		ret = pci_vpd_wait(dev);
>>    199		if (ret < 0)
>>    200			goto out;
>>    201	
>>    202		while (pos < end) {
>>    203			unsigned int len, skip;
>>    204			u32 val;
>>    205	
>>    206			ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
>>    207							 pos & ~3);
>>    208			if (ret < 0)
>>    209				break;
>>    210			vpd->busy = 1;
>>    211			vpd->flag = PCI_VPD_ADDR_F;
>>    212			ret = pci_vpd_wait(dev);
>>    213			if (ret < 0)
>>    214				break;
>>    215	
>>    216			ret = pci_user_read_config_dword(dev, vpd->cap + PCI_VPD_DATA, &val);
>>    217			if (ret < 0)
>>    218				break;
>>    219	
>>    220			skip = pos & 3;
>>    221			len = min_t(unsigned int, 4 - skip, end - pos);
>>    222	
>>    223			if (len == 4)  {
>>  > 224				put_unaligned_le32(val, buf);
>>    225			} else {
>>    226				u8 tmpbuf[4];
>>    227	
>>    228				put_unaligned_le32(val, tmpbuf);
>>    229				memcpy(buf, tmpbuf + skip, len);
>>    230			}
>>    231	
>>    232			buf += len;
>>    233			pos += len;
>>    234		}
>>    235	out:
>>    236		mutex_unlock(&vpd->lock);
>>    237		return ret ? ret : count;
>>    238	}
>>    239	
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>>
> 
> It depends on "PCI/VPD: Use unaligned access helper in pci_vpd_write"
> which takes care of including asm/unaligned.h
> 
Please disregard this patch, I'll send a slightly improved v2.
diff mbox series

Patch

diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
index 741e7a505..0752505d7 100644
--- a/drivers/pci/vpd.c
+++ b/drivers/pci/vpd.c
@@ -163,12 +163,11 @@  static int pci_vpd_wait(struct pci_dev *dev)
 }
 
 static ssize_t pci_vpd_read(struct pci_dev *dev, loff_t pos, size_t count,
-			    void *arg)
+			    void *buf)
 {
 	struct pci_vpd *vpd = dev->vpd;
 	int ret;
 	loff_t end = pos + count;
-	u8 *buf = arg;
 
 	if (pos < 0)
 		return -EINVAL;
@@ -197,8 +196,8 @@  static ssize_t pci_vpd_read(struct pci_dev *dev, loff_t pos, size_t count,
 		goto out;
 
 	while (pos < end) {
+		unsigned int len, skip;
 		u32 val;
-		unsigned int i, skip;
 
 		ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
 						 pos & ~3);
@@ -215,14 +214,19 @@  static ssize_t pci_vpd_read(struct pci_dev *dev, loff_t pos, size_t count,
 			break;
 
 		skip = pos & 3;
-		for (i = 0;  i < sizeof(u32); i++) {
-			if (i >= skip) {
-				*buf++ = val;
-				if (++pos == end)
-					break;
-			}
-			val >>= 8;
+		len = min_t(unsigned int, 4 - skip, end - pos);
+
+		if (len == 4)  {
+			put_unaligned_le32(val, buf);
+		} else {
+			u8 tmpbuf[4];
+
+			put_unaligned_le32(val, tmpbuf);
+			memcpy(buf, tmpbuf + skip, len);
 		}
+
+		buf += len;
+		pos += len;
 	}
 out:
 	mutex_unlock(&vpd->lock);