* [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).