On Sun, 29 Dec 2024 11:49:55 +0100
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> Hi Wolfram,
>
> On Sun, Dec 29, 2024 at 11:13 AM Wolfram Sang
> <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx
>
> 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
>
