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

[David Laight <david.laight.linux@gmail.com>]

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

> Hi Wolfram,
> 
> On Sun, Dec 29, 2024 at 11:13 AM Wolfram Sang
> <wsa+renesas@sang-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@sang-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@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
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c