* [renesas-drivers:sh-pfc-for-v5.10 5/8] drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y
@ 2020-09-23 11:47 kernel test robot
2020-09-24 8:01 ` Geert Uytterhoeven
0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2020-09-23 11:47 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: kbuild-all, linux-renesas-soc, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 4134 bytes --]
Hi Kuninori,
First bad commit (maybe != root cause):
tree: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git sh-pfc-for-v5.10
head: 46d5baf0885ac8e1a799bdc4dc396dab403ccdf9
commit: 8449bfa9e6a9f7ec9b9037a2e61784140b673823 [5/8] pinctrl: sh-pfc: Collect Renesas related CONFIGs in one place
config: c6x-randconfig-s031-20200923 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-201-g24bdaac6-dirty
git checkout 8449bfa9e6a9f7ec9b9037a2e61784140b673823
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=c6x
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/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y
drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:189:52: sparse: sparse: dubious: x | !y
# https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?id=8449bfa9e6a9f7ec9b9037a2e61784140b673823
git remote add renesas-drivers https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
git fetch --no-tags renesas-drivers sh-pfc-for-v5.10
git checkout 8449bfa9e6a9f7ec9b9037a2e61784140b673823
vim +183 drivers/pinctrl/sh-pfc/pinctrl-rzn1.c
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 174
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 175 static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 176 {
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 177 /*
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 178 * The pinmux configuration is locked by writing the physical address of
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 179 * the status_protect register to itself. It is unlocked by writing the
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 180 * address | 1.
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 181 */
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 182 if (lock & LOCK_LEVEL1) {
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 @183 u32 val = ipctl->lev1_protect_phys | !(value & LOCK_LEVEL1);
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 184
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 185 writel(val, &ipctl->lev1->status_protect);
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 186 }
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 187
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 188 if (lock & LOCK_LEVEL2) {
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 189 u32 val = ipctl->lev2_protect_phys | !(value & LOCK_LEVEL2);
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 190
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 191 writel(val, &ipctl->lev2->status_protect);
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 192 }
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 193 }
4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 194
:::::: The code at line 183 was first introduced by commit
:::::: 4e53b5004745ef26a37bca4933b2d3ea71313f2a pinctrl: renesas: Renesas RZ/N1 pinctrl driver
:::::: TO: Phil Edworthy <phil.edworthy@renesas.com>
:::::: CC: Geert Uytterhoeven <geert+renesas@glider.be>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30267 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [renesas-drivers:sh-pfc-for-v5.10 5/8] drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y 2020-09-23 11:47 [renesas-drivers:sh-pfc-for-v5.10 5/8] drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y kernel test robot @ 2020-09-24 8:01 ` Geert Uytterhoeven 2020-09-24 19:17 ` Luc Van Oostenryck 0 siblings, 1 reply; 3+ messages in thread From: Geert Uytterhoeven @ 2020-09-24 8:01 UTC (permalink / raw) To: kernel test robot Cc: Kuninori Morimoto, kbuild-all, Linux-Renesas, Gareth Williams, Luc Van Oostenryck Hi Kernel test robot, CC Gareth, Luc, On Wed, Sep 23, 2020 at 1:48 PM kernel test robot <lkp@intel.com> wrote: > First bad commit (maybe != root cause): > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git sh-pfc-for-v5.10 > head: 46d5baf0885ac8e1a799bdc4dc396dab403ccdf9 > commit: 8449bfa9e6a9f7ec9b9037a2e61784140b673823 [5/8] pinctrl: sh-pfc: Collect Renesas related CONFIGs in one place > config: c6x-randconfig-s031-20200923 (attached as .config) > compiler: c6x-elf-gcc (GCC) 9.3.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # apt-get install sparse > # sparse version: v0.6.2-201-g24bdaac6-dirty > git checkout 8449bfa9e6a9f7ec9b9037a2e61784140b673823 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=c6x > > 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/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y > drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:189:52: sparse: sparse: dubious: x | !y I believe the code is correct (see below). > # https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?id=8449bfa9e6a9f7ec9b9037a2e61784140b673823 > git remote add renesas-drivers https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > git fetch --no-tags renesas-drivers sh-pfc-for-v5.10 > git checkout 8449bfa9e6a9f7ec9b9037a2e61784140b673823 > vim +183 drivers/pinctrl/sh-pfc/pinctrl-rzn1.c > > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 174 > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 175 static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value) > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 176 { > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 177 /* > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 178 * The pinmux configuration is locked by writing the physical address of The comments seem to be wrong, though: "unlocked" > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 179 * the status_protect register to itself. It is unlocked by writing the "locked". Also, the function seems to be used in the opposite way than the variables are named, which confuses the reader: rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, LOCK_LEVEL1); /* unlock level 1 before use */ writel(l1, &ipctl->lev1->conf[pin]); rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, 0); /* lock level 1 again */ But the actual operation is correct, and matches the hardware manual ;-) > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 180 * address | 1. > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 181 */ > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 182 if (lock & LOCK_LEVEL1) { > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 @183 u32 val = ipctl->lev1_protect_phys | !(value & LOCK_LEVEL1); IMHO this is correct: ipctl->lev1_protect_phys is to be ORed with 0 or 1, depending on whether LOCK_LEVEL1 is set or nor. Is there a way to inform sparse this code is correct? > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 184 > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 185 writel(val, &ipctl->lev1->status_protect); > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 186 } > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 187 > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 188 if (lock & LOCK_LEVEL2) { > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 189 u32 val = ipctl->lev2_protect_phys | !(value & LOCK_LEVEL2); Likewise. > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 190 > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 191 writel(val, &ipctl->lev2->status_protect); > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 192 } > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 193 } > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 194 > > :::::: The code at line 183 was first introduced by commit > :::::: 4e53b5004745ef26a37bca4933b2d3ea71313f2a pinctrl: renesas: Renesas RZ/N1 pinctrl driver > > :::::: TO: Phil Edworthy <phil.edworthy@renesas.com> > :::::: CC: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org -- Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [renesas-drivers:sh-pfc-for-v5.10 5/8] drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y 2020-09-24 8:01 ` Geert Uytterhoeven @ 2020-09-24 19:17 ` Luc Van Oostenryck 0 siblings, 0 replies; 3+ messages in thread From: Luc Van Oostenryck @ 2020-09-24 19:17 UTC (permalink / raw) To: Geert Uytterhoeven Cc: kernel test robot, Kuninori Morimoto, kbuild-all, Linux-Renesas, Gareth Williams Hi, On Thu, Sep 24, 2020 at 10:01:57AM +0200, Geert Uytterhoeven wrote: > Hi Kernel test robot, > > CC Gareth, Luc, > > > sparse warnings: (new ones prefixed by >>) > > > > >> drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y > > drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:189:52: sparse: sparse: dubious: x | !y > > I believe the code is correct (see below). > > > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 180 * address | 1. > > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 181 */ > > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 182 if (lock & LOCK_LEVEL1) { > > 4e53b5004745ef drivers/pinctrl/pinctrl-rzn1.c Phil Edworthy 2018-09-26 @183 u32 val = ipctl->lev1_protect_phys | !(value & LOCK_LEVEL1); > > IMHO this is correct: ipctl->lev1_protect_phys is to be ORed with 0 or 1, > depending on whether LOCK_LEVEL1 is set or nor. > > Is there a way to inform sparse this code is correct? Not really, at least no kind of annotation to suppress the warning (and I don't think it would be a good idea to have one). But of course, since the warning is about the possible error of mixing a bitwise and with a logical not, you can rewrite the expression as: u32 val = ipctl->lev1_protect_phys | ((value & LOCK_LEVEL1) == 0); or, better; u32 val = ipctl->lev1_protect_phys | ((value & LOCK_LEVEL1) ? 0 1); But I understand quite well why people would prefer the simpler, more idiomatic !(value & LOCK_LEVEL1). Best regards, -- Luc ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-24 19:17 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-23 11:47 [renesas-drivers:sh-pfc-for-v5.10 5/8] drivers/pinctrl/sh-pfc/pinctrl-rzn1.c:183:52: sparse: sparse: dubious: x | !y kernel test robot 2020-09-24 8:01 ` Geert Uytterhoeven 2020-09-24 19:17 ` Luc Van Oostenryck
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).