On Wed, Apr 09, 2025 at 12:59:14PM -0400, Yury Norov wrote:
> 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 <eleanor15x@xxxxxxxxx>
> > Signed-off-by: Yu-Chun Lin <eleanor15x@xxxxxxxxx>
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@xxxxxxxxx>
> > ---
> > 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.
>
Ack.
Will do in next version.
Regards,
Kuan-Wei
> > +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
