On Wed, Apr 09, 2025 at 11:43:44PM +0800, Kuan-Wei Chiu wrote:
> Redesign the parity8() helper as parity_odd(), changing its input type
> from u8 to u64 to support broader use cases and its return type from
> int to bool to clearly reflect the function's binary output. The
> function now returns true for odd parity and false for even parity,
> making its behavior more intuitive based on the name.
>
> Also mark the function with __attribute_const__ to enable better
> compiler optimization, as the result depends solely on its input and
> has no side effects.
>
> While more efficient implementations may exist, further optimization is
> postponed until a use case in performance-critical paths arises.
>
> Co-developed-by: Yu-Chun Lin <eleanor...@gmail.com>
> Signed-off-by: Yu-Chun Lin <eleanor...@gmail.com>
> Signed-off-by: Kuan-Wei Chiu <visitor...@gmail.com>
> ---
> arch/x86/kernel/bootflag.c | 4 ++--
> drivers/hwmon/spd5118.c | 2 +-
> drivers/i3c/master/dw-i3c-master.c | 2 +-
> drivers/i3c/master/i3c-master-cdns.c | 2 +-
> drivers/i3c/master/mipi-i3c-hci/dat_v1.c | 2 +-
> include/linux/bitops.h | 19 ++++++++++++-------
> 6 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kernel/bootflag.c b/arch/x86/kernel/bootflag.c
> index 73274d76ce16..86aae4b2bfd5 100644
> --- a/arch/x86/kernel/bootflag.c
> +++ b/arch/x86/kernel/bootflag.c
> @@ -26,7 +26,7 @@ static void __init sbf_write(u8 v)
> unsigned long flags;
>
> if (sbf_port != -1) {
> - if (!parity8(v))
> + if (!parity_odd(v))
> v ^= SBF_PARITY;
>
> printk(KERN_INFO "Simple Boot Flag at 0x%x set to 0x%x\n",
> @@ -57,7 +57,7 @@ static bool __init sbf_value_valid(u8 v)
> {
> if (v & SBF_RESERVED) /* Reserved bits */
> return false;
> - if (!parity8(v))
> + if (!parity_odd(v))
> return false;
>
> return true;
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 358152868d96..15761f2ca4e9 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -298,7 +298,7 @@ static umode_t spd5118_is_visible(const void *_data, enum
> hwmon_sensor_types typ
> */
> static bool spd5118_vendor_valid(u8 bank, u8 id)
> {
> - if (parity8(bank) == 0 || parity8(id) == 0)
> + if (!parity_odd(bank) || !parity_odd(id))
> return false;
>
> id &= 0x7f;
> diff --git a/drivers/i3c/master/dw-i3c-master.c
> b/drivers/i3c/master/dw-i3c-master.c
> index 611c22b72c15..dc61d87fcd94 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -867,7 +867,7 @@ static int dw_i3c_master_daa(struct i3c_master_controller
> *m)
> master->devs[pos].addr = ret;
> last_addr = ret;
>
> - ret |= parity8(ret) ? 0 : BIT(7);
> + ret |= parity_odd(ret) ? 0 : BIT(7);
>
> writel(DEV_ADDR_TABLE_DYNAMIC_ADDR(ret),
> master->regs +
> diff --git a/drivers/i3c/master/i3c-master-cdns.c
> b/drivers/i3c/master/i3c-master-cdns.c
> index fd3752cea654..df14f978a388 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -889,7 +889,7 @@ static u32 prepare_rr0_dev_address(u32 addr)
> ret |= (addr & GENMASK(9, 7)) << 6;
>
> /* RR0[0] = ~XOR(addr[6:0]) */
> - ret |= parity8(addr & 0x7f) ? 0 : BIT(0);
> + ret |= parity_odd(addr & 0x7f) ? 0 : BIT(0);
>
> return ret;
> }
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> index 85c4916972e4..d692a299607d 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> @@ -114,7 +114,7 @@ static void hci_dat_v1_set_dynamic_addr(struct i3c_hci
> *hci,
> dat_w0 = dat_w0_read(dat_idx);
> dat_w0 &= ~(DAT_0_DYNAMIC_ADDRESS | DAT_0_DYNADDR_PARITY);
> dat_w0 |= FIELD_PREP(DAT_0_DYNAMIC_ADDRESS, address) |
> - (parity8(address) ? 0 : DAT_0_DYNADDR_PARITY);
> + (parity_odd(address) ? 0 : DAT_0_DYNADDR_PARITY);
> dat_w0_write(dat_idx, dat_w0);
> }
>
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index c1cb53cf2f0f..7c4c8afccef1 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -230,35 +230,40 @@ static inline int get_count_order_long(unsigned long l)
> }
>
> /**
> - * parity8 - get the parity of an u8 value
> - * @value: the value to be examined
> + * parity_odd - get the parity of an u64 value
> + * @val: the value to be examined
> *
> - * Determine the parity of the u8 argument.
> + * Determine the parity of the u64 argument.
> *
> * Returns:
> - * 0 for even parity, 1 for odd parity
> + * false for even parity, true for odd parity
> *
> * Note: This function informs you about the current parity. Example to bail
> * out when parity is odd:
> *
> - * if (parity8(val) == 1)
> + * if (parity_odd(val))
> * 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 (parity8(val) == 0)
> + * if (!parity_odd(val))
> * val ^= BIT(7);
> */
> -static inline int parity8(u8 val)
> +#ifndef parity_odd
Please don't add this guard. We've got no any arch implementations
so far, and this is a dead code. Those adding arch code will also
add the ifdefery.
> +static inline __attribute_const__ bool parity_odd(u64 val)
> {
> /*
> * One explanation of this algorithm:
> * https://funloop.org/codex/problem/parity/README.html
> */
> + val ^= val >> 32;
> + val ^= val >> 16;
> + val ^= val >> 8;
> val ^= val >> 4;
> return (0x6996 >> (val & 0xf)) & 1;
> }
> +#endif
>
> /**
> * __ffs64 - find first set bit in a 64 bit word
> --
> 2.34.1
