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
