[RFT,1/5] bitops: add generic parity calculation for u8

Commit Message

Wolfram Sang Dec. 20, 2024, 11:29 a.m. UTC

There are multiple open coded implementations for getting the parity of
a byte in the kernel, even using different approaches. Take the pretty
efficient version from SPD5118 driver and make it generally available by
putting it into the bitops header. As long as there is just one parity
calculation helper, the creation of a distinct 'parity.h' header was
discarded. Also, the usage of hweight8() for architectures having a
popcnt instruction is postponed until a use case within hot paths is
desired. The motivation for this patch is the frequent use of odd parity
in the I3C specification and to simplify drivers there.

Changes compared to the original SPD5118 version are the addition of
kernel documentation, switching the return type from bool to int, and
renaming the argument of the function.

Signed-off-by: Wolfram Sang <[e-mail]>
Tested-by: Guenter Roeck <[e-mail]>
Reviewed-by: Geert Uytterhoeven <[e-mail]>
Acked-by: Yury Norov <[e-mail]>
Reviewed-by: Kuan-Wei Chiu <[e-mail]>
Tested-by: Kuan-Wei Chiu <[e-mail]>
---
 include/linux/bitops.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Rasmus Villemoes Dec. 20, 2024, 11:50 a.m. UTC | #1

> +/**
> + * get_parity8 - get the parity of an u8 value

I know it's prototypical bikeshedding, but what's with the "get_"
prefix? Certainly the purpose of any pure function is to "get" the
result of some computation. We don't have "get_strlen()".

Please either just "parity8", or if you want to emphasize that this
belongs in the bit-munging family, "bit_parity8".

> + * @value: the value to be examined
> + *
> + * Determine the parity of the u8 argument.
> + *
> + * Returns:
> + * 0 for even parity, 1 for odd parity
> + *
> + * Note: This function informs you about the current parity. Example to bail
> + * out when parity is odd:
> + *
> + *     if (get_parity8(val) == 1)
> + *             return -EBADMSG;
> + *
> + * If you need to calculate a parity bit, you need to draw the conclusion from
> + * this result yourself. Example to enforce odd parity, parity bit is bit 7:
> + *
> + *     if (get_parity8(val) == 0)
> + *             val |= BIT(7);

Shouldn't that be ^= in general? Of course in some particular
application one may have constructed the value in the lower 7 bits and
"know" that bit 7 is currently clear.

Rasmus

Patch

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index ba35bbf07798..4ed430934ffc 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -229,6 +229,37 @@  static inline int get_count_order_long(unsigned long l)
 	return (int)fls_long(--l);
 }
 
+/**
+ * get_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
+ *
+ * Note: This function informs you about the current parity. Example to bail
+ * out when parity is odd:
+ *
+ *	if (get_parity8(val) == 1)
+ *		return -EBADMSG;
+ *
+ * If you need to calculate a parity bit, you need to draw the conclusion from
+ * this result yourself. Example to enforce odd parity, parity bit is bit 7:
+ *
+ *	if (get_parity8(val) == 0)
+ *		val |= BIT(7);
+ */
+static inline int get_parity8(u8 val)
+{
+	/*
+	 * One explanation of this algorithm:
+	 * https://funloop.org/codex/problem/parity/README.html
+	 */
+	val ^= val >> 4;
+	return (0x6996 >> (val & 0xf)) & 1;
+}
+
 /**
  * __ffs64 - find first set bit in a 64 bit word
  * @word: The 64 bit word