linux-kernel - Re: [PATCH RFT v2 5/5] i3c: cdns: use get_parity8 helper instead of open coding it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241229113311.3b8f70d6@dsl-u17-10>
Date: Sun, 29 Dec 2024 11:33:11 +0000
From: David Laight <david.laight.linux@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Wolfram Sang <wsa+renesas@...g-engineering.com>,
 linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org, Rasmus Villemoes
 <linux@...musvillemoes.dk>, Przemysław Gaj
 <pgaj@...ence.com>, Alexandre Belloni <alexandre.belloni@...tlin.com>
Subject: Re: [PATCH RFT v2 5/5] i3c: cdns: use get_parity8 helper instead of
 open coding it

On Sun, 29 Dec 2024 11:49:55 +0100
Geert Uytterhoeven <geert@...ux-m68k.org> wrote:

> Hi Wolfram,
> 
> On Sun, Dec 29, 2024 at 11:13 AM Wolfram Sang
> <wsa+renesas@...g-engineering.com> wrote:
> > The kernel has now a generic helper for getting parity with easier to
> > understand semantics. Make use of it.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@...g-engineering.com>  
> 
> Thanks for your patch!
> 
> > --- a/drivers/i3c/master/i3c-master-cdns.c
> > +++ b/drivers/i3c/master/i3c-master-cdns.c
> > @@ -889,8 +889,7 @@ static u32 prepare_rr0_dev_address(u32 addr)
> >         ret |= (addr & GENMASK(9, 7)) << 6;
> >
> >         /* RR0[0] = ~XOR(addr[6:0]) */
> > -       if (!(hweight8(addr & 0x7f) & 1))
> > -               ret |= 1;
> > +       ret |= parity8(addr & 0x7f) ? 0 : BIT(0);  
> 
> Perhaps keep the if()-construct, to better match the example in the
> documentation in 1/5?

That line is hard to read, with parity8() returning 1 for 'odd' it could be:
	ret |= parity8(addr & 0x7f) ^ 1;
But:
	if (!parity8(addr & 0x7f))
		ret |= 1;
is probably easier to read.
But I'd change the name to parity8_odd() for clarity.
(or _even and return 0x80 for even)

	David

> 
> >
> >         return ret;
> >  }  
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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
>