* [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces)
@ 2022-02-11 21:24 kernel test robot
2022-02-14 12:27 ` Jan Dąbroś
0 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2022-02-11 21:24 UTC (permalink / raw)
To: Jan Dabros
Cc: kbuild-all, Linux Memory Management List, Wolfram Sang, Andy Shevchenko
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: 6d9bd4ad4ca08b1114e814c2c42383b8b13be631
commit: 91560fe37f81ba8145427477a39cea88f4422385 [4897/5417] i2c: designware: Add AMD PSP I2C bus support
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220212/202202120520.NbWJGvF2-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=91560fe37f81ba8145427477a39cea88f4422385
git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
git fetch --no-tags linux-next master
git checkout 91560fe37f81ba8145427477a39cea88f4422385
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/i2c/busses/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got unsigned int * @@
drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: expected void const volatile [noderef] __iomem *addr
drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: got unsigned int *
vim +165 drivers/i2c/busses/i2c-designware-amdpsp.c
159
160 /* Helper to verify status returned by PSP */
161 static int check_i2c_req_sts(struct psp_i2c_req *req)
162 {
163 int status;
164
> 165 status = readl(&req->hdr.status);
166
167 switch (status) {
168 case PSP_I2C_REQ_STS_OK:
169 return 0;
170 case PSP_I2C_REQ_STS_BUS_BUSY:
171 return -EBUSY;
172 case PSP_I2C_REQ_STS_INV_PARAM:
173 default:
174 return -EIO;
175 };
176 }
177
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) 2022-02-11 21:24 [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) kernel test robot @ 2022-02-14 12:27 ` Jan Dąbroś 2022-02-14 13:27 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Jan Dąbroś @ 2022-02-14 12:27 UTC (permalink / raw) To: kernel test robot Cc: kbuild-all, Linux Memory Management List, Wolfram Sang, Andy Shevchenko pt., 11 lut 2022 o 22:24 kernel test robot <lkp@intel.com> napisał(a): > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > head: 6d9bd4ad4ca08b1114e814c2c42383b8b13be631 > commit: 91560fe37f81ba8145427477a39cea88f4422385 [4897/5417] i2c: designware: Add AMD PSP I2C bus support > config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220212/202202120520.NbWJGvF2-lkp@intel.com/config) > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > reproduce: > # apt-get install sparse > # sparse version: v0.6.4-dirty > # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=91560fe37f81ba8145427477a39cea88f4422385 > git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git > git fetch --no-tags linux-next master > git checkout 91560fe37f81ba8145427477a39cea88f4422385 > # save the config file to linux build tree > mkdir build_dir > make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/i2c/busses/ > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > > sparse warnings: (new ones prefixed by >>) > >> drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got unsigned int * @@ > drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: expected void const volatile [noderef] __iomem *addr > drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: got unsigned int * > > vim +165 drivers/i2c/busses/i2c-designware-amdpsp.c > > 159 > 160 /* Helper to verify status returned by PSP */ > 161 static int check_i2c_req_sts(struct psp_i2c_req *req) > 162 { > 163 int status; > 164 > > 165 status = readl(&req->hdr.status); Actually the above error points to something hidden but important - for reading from command-response buffer, we shouldn't use __iomem specifier (nor readl() family of functions) since this is normal memory - however updated by PSP. Thus I will refactor this to use 'volatile u32 *' and reading status by de-referencing pointer. Best Regards, Jan > 166 > 167 switch (status) { > 168 case PSP_I2C_REQ_STS_OK: > 169 return 0; > 170 case PSP_I2C_REQ_STS_BUS_BUSY: > 171 return -EBUSY; > 172 case PSP_I2C_REQ_STS_INV_PARAM: > 173 default: > 174 return -EIO; > 175 }; > 176 } > 177 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) 2022-02-14 12:27 ` Jan Dąbroś @ 2022-02-14 13:27 ` Andy Shevchenko 2022-02-14 13:59 ` Jan Dąbroś 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2022-02-14 13:27 UTC (permalink / raw) To: Jan Dąbroś Cc: kernel test robot, kbuild-all, Linux Memory Management List, Wolfram Sang On Mon, Feb 14, 2022 at 01:27:35PM +0100, Jan Dąbroś wrote: > pt., 11 lut 2022 o 22:24 kernel test robot <lkp@intel.com> napisał(a): > > 159 > > 160 /* Helper to verify status returned by PSP */ > > 161 static int check_i2c_req_sts(struct psp_i2c_req *req) > > 162 { > > 163 int status; > > 164 > > > 165 status = readl(&req->hdr.status); > > Actually the above error points to something hidden but important - > for reading from command-response buffer, we shouldn't use __iomem > specifier (nor readl() family of functions) since this is normal > memory - however updated by PSP. Thus I will refactor this to use > 'volatile u32 *' and reading status by de-referencing pointer. Not sure volatile is a good idea. Perhaps READ_ONCE() is what you need. Is this a system memory? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) 2022-02-14 13:27 ` Andy Shevchenko @ 2022-02-14 13:59 ` Jan Dąbroś 2022-02-14 14:43 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Jan Dąbroś @ 2022-02-14 13:59 UTC (permalink / raw) To: Andy Shevchenko Cc: kernel test robot, kbuild-all, Linux Memory Management List, Wolfram Sang pon., 14 lut 2022 o 14:28 Andy Shevchenko <andriy.shevchenko@linux.intel.com> napisał(a): > > On Mon, Feb 14, 2022 at 01:27:35PM +0100, Jan Dąbroś wrote: > > pt., 11 lut 2022 o 22:24 kernel test robot <lkp@intel.com> napisał(a): > > > > 159 > > > 160 /* Helper to verify status returned by PSP */ > > > 161 static int check_i2c_req_sts(struct psp_i2c_req *req) > > > 162 { > > > 163 int status; > > > 164 > > > > 165 status = readl(&req->hdr.status); > > > > Actually the above error points to something hidden but important - > > for reading from command-response buffer, we shouldn't use __iomem > > specifier (nor readl() family of functions) since this is normal > > memory - however updated by PSP. Thus I will refactor this to use > > 'volatile u32 *' and reading status by de-referencing pointer. > > Not sure volatile is a good idea. Perhaps READ_ONCE() is what you need. > Is this a system memory? Yes, this is system memory. Actually looking at asm-generic/rwonce.h: #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x)) it is more-less based on volatile, so that compiler will not be able to (among others) optimize out such reads of memory which may be changed outside of the scope of "program". I believe that I will get the same outcome from using READ_ONCE and explicit volatile, is the first way preferred in the kernel? Best Regards, Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) 2022-02-14 13:59 ` Jan Dąbroś @ 2022-02-14 14:43 ` Andy Shevchenko 2022-02-16 14:06 ` Jan Dąbroś 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2022-02-14 14:43 UTC (permalink / raw) To: Jan Dąbroś Cc: kernel test robot, kbuild-all, Linux Memory Management List, Wolfram Sang On Mon, Feb 14, 2022 at 02:59:34PM +0100, Jan Dąbroś wrote: > pon., 14 lut 2022 o 14:28 Andy Shevchenko > <andriy.shevchenko@linux.intel.com> napisał(a): > > > > On Mon, Feb 14, 2022 at 01:27:35PM +0100, Jan Dąbroś wrote: > > > pt., 11 lut 2022 o 22:24 kernel test robot <lkp@intel.com> napisał(a): > > > > > > 159 > > > > 160 /* Helper to verify status returned by PSP */ > > > > 161 static int check_i2c_req_sts(struct psp_i2c_req *req) > > > > 162 { > > > > 163 int status; > > > > 164 > > > > > 165 status = readl(&req->hdr.status); > > > > > > Actually the above error points to something hidden but important - > > > for reading from command-response buffer, we shouldn't use __iomem > > > specifier (nor readl() family of functions) since this is normal > > > memory - however updated by PSP. Thus I will refactor this to use > > > 'volatile u32 *' and reading status by de-referencing pointer. > > > > Not sure volatile is a good idea. Perhaps READ_ONCE() is what you need. > > Is this a system memory? > > Yes, this is system memory. > > Actually looking at asm-generic/rwonce.h: > #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x)) > it is more-less based on volatile, so that compiler will not be able > to (among others) optimize out such reads of memory which may be > changed outside of the scope of "program". > > I believe that I will get the same outcome from using READ_ONCE and > explicit volatile, is the first way preferred in the kernel? READ_ONCE() may be different on different arches. I believe that's why it's preferred. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) 2022-02-14 14:43 ` Andy Shevchenko @ 2022-02-16 14:06 ` Jan Dąbroś 0 siblings, 0 replies; 6+ messages in thread From: Jan Dąbroś @ 2022-02-16 14:06 UTC (permalink / raw) To: Andy Shevchenko Cc: kernel test robot, kbuild-all, Linux Memory Management List, Wolfram Sang pon., 14 lut 2022 o 15:44 Andy Shevchenko <andriy.shevchenko@linux.intel.com> napisał(a): > > On Mon, Feb 14, 2022 at 02:59:34PM +0100, Jan Dąbroś wrote: > > pon., 14 lut 2022 o 14:28 Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> napisał(a): > > > > > > On Mon, Feb 14, 2022 at 01:27:35PM +0100, Jan Dąbroś wrote: > > > > pt., 11 lut 2022 o 22:24 kernel test robot <lkp@intel.com> napisał(a): > > > > > > > > 159 > > > > > 160 /* Helper to verify status returned by PSP */ > > > > > 161 static int check_i2c_req_sts(struct psp_i2c_req *req) > > > > > 162 { > > > > > 163 int status; > > > > > 164 > > > > > > 165 status = readl(&req->hdr.status); > > > > > > > > Actually the above error points to something hidden but important - > > > > for reading from command-response buffer, we shouldn't use __iomem > > > > specifier (nor readl() family of functions) since this is normal > > > > memory - however updated by PSP. Thus I will refactor this to use > > > > 'volatile u32 *' and reading status by de-referencing pointer. > > > > > > Not sure volatile is a good idea. Perhaps READ_ONCE() is what you need. > > > Is this a system memory? > > > > Yes, this is system memory. > > > > Actually looking at asm-generic/rwonce.h: > > #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x)) > > it is more-less based on volatile, so that compiler will not be able > > to (among others) optimize out such reads of memory which may be > > changed outside of the scope of "program". > > > > I believe that I will get the same outcome from using READ_ONCE and > > explicit volatile, is the first way preferred in the kernel? > > READ_ONCE() may be different on different arches. I believe that's why > it's preferred. OK, I see. Let me send a commit with this change this week. Best Regards, Jan > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-16 14:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-11 21:24 [linux-next:master 4897/5417] drivers/i2c/busses/i2c-designware-amdpsp.c:165:25: sparse: sparse: incorrect type in argument 1 (different address spaces) kernel test robot 2022-02-14 12:27 ` Jan Dąbroś 2022-02-14 13:27 ` Andy Shevchenko 2022-02-14 13:59 ` Jan Dąbroś 2022-02-14 14:43 ` Andy Shevchenko 2022-02-16 14:06 ` Jan Dąbroś
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).