Re: [PATCH RFT v2 1/5] bitops: add generic parity calculation for u8

From: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: David Laight <david.laight.linux@gmail.com>
Cc: linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Guenter Roeck <linux@roeck-us.net>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Yury Norov <yury.norov@gmail.com>,
	Kuan-Wei Chiu <visitorckw@gmail.com>
Subject: Re: [PATCH RFT v2 1/5] bitops: add generic parity calculation for u8
Date: Thu, 2 Jan 2025 09:55:38 +0100	[thread overview]
Message-ID: <Z3ZUit3usbDSblTb@shikoro> (raw)
In-Reply-To: <20241229111129.2602d219@dsl-u17-10>

[-- Attachment #1.1: Type: text/plain, Size: 1095 bytes --]

> > +/**
> > + * parity8 - get the parity of an u8 value
> > + * @value: the value to be examined
> > + *
> > + * Determine the parity of the u8 argument.
> > + *
> > + * Returns:
> > + * 0 for even parity, 1 for odd parity
> 
> I think I'd return 0x80 for even and 0 for odd.

This would be a very non-intuitive result which makes code more
complicated to read. And increases chances that people get it wrong.

> That just need the 'magic constant' changing and masking with 0x80.

Not all users will have the parity in bit 7, some have it in bit 0, some
have it in a different register even. So, returning 0x80 would be a
micro-optimization for some cases in a code path which is not hot.

> Also rename to parity8_even() - since it returns non-zero for even parity.

The name has been agreed on with the maintainer of bitmap.h already.

> (and I'm sorry, but IMHO 0x80 is better than BIT(7))

No need to feel sorry, but it is really your HO. We have reasons for
switching from 0xXY to BIT() tree-wide. You might want to check commit
messages of such changes.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 111 bytes --]

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