[kbuild-all] 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)

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(a)linux.intel.com&gt; 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(a)intel.com&gt; 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.