From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B1601C02198 for ; Fri, 14 Feb 2025 15:45:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Content-Transfer-Encoding:Content-Type:MIME-Version:References:In-Reply-To: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+UBhTHpXQ+XdamCLSlWS9cI3aGJ7aKK8OHT/Zn/M/Bw=; b=yubDfS+BYqKl0s ZrOlu5na3YThM2GJU19oxyBs6mtf3df2Y0QWRF0YZRCLv5R+afSj/EPmdyLQCJbbk0X/TbHrjHNQ/ M6S+/6coKGG+hmM4JUQTita/9fZHpwMBxsRJgS+Jz27UzTOUdo2VY+6+IjQbCFteF38wVdTbfD6Qo ha0QSUyfTueZSIRHfYRjU9gFgUw1OAAzTyumf9NXYGRf+ekGlV8Ng9S6k1sK9cE8P4xTX8Ary7FPK mVqko/gAnFXljEmgcE9OFoF0P6APGnEiXHylHsv7ERQY03trG3zlxiH6U7YyC+OGK6vR8x2crs0KI Q8I44dw5oKDmkjmvXYmQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tixs9-0000000FNaN-1EDL; Fri, 14 Feb 2025 15:44:57 +0000 Received: from nyc.source.kernel.org ([2604:1380:45d1:ec00::3]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tixhe-0000000FLCR-1cVE for linux-arm-kernel@lists.infradead.org; Fri, 14 Feb 2025 15:34:08 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id E2035A42DD9; Fri, 14 Feb 2025 15:32:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6E762C4CED1; Fri, 14 Feb 2025 15:34:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1739547244; bh=afzjkoDNR+5URzZhBvSQ4ZnpfxUfaPdGqiY8US/VboU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ECrCiL7DBdPP6UFFz6PjonfFVlEUbl56ymR9CSiZzxpxCifwC27Tw8p1u6IDaNAcz XemUEQwq0pq9I6NzQ57R87EtLHA58AoZUYXKU4c/a4+0bKli0nC2pWWJ0CaTzucIWf zEEnZzErfP0wK5sPyu2mi2qclLmZogSRA/rGjbulFjwx2g9cjB/YQIACh2RMxA/mmG B0Uv3e4Yr3cTk/XVs+liLdVf+fr5YxFQXWCevR4NaUolQnSHhBUYWJJ4xYbbe3NeAW MF9KLwHoWvD7iUCoAoTnAelZhfw/GRKpfZBbyWOre6DjfvmbKONn7GTj2BMeFEUPTK 6xY1KtkqdoRBw== Date: Fri, 14 Feb 2025 07:34:02 -0800 From: Jakub Kicinski To: Geert Uytterhoeven Subject: Re: [PATCH treewide v3 2/4] bitfield: Add non-constant field_{prep,get}() helpers Message-ID: <20250214073402.0129e259@kernel.org> In-Reply-To: <2d30e5ffe70ce35f952b7d497d2959391fbf0580.1739540679.git.geert+renesas@glider.be> References: <2d30e5ffe70ce35f952b7d497d2959391fbf0580.1739540679.git.geert+renesas@glider.be> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250214_073406_556745_EF28F8B9 X-CRM114-Status: GOOD ( 18.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Giovanni Cabiddu , Alexandre Belloni , Crt Mori , linux-aspeed@lists.ozlabs.org, linux-iio@vger.kernel.org, Michael Turquette , Rasmus Villemoes , Jaroslav Kysela , Claudiu Beznea , Shan-Chun Hung , linux-clk@vger.kernel.org, Lars-Peter Clausen , Herbert Xu , Bartosz Golaszewski , Takashi Iwai , qat-linux@intel.com, Joel Stanley , Andrew Jeffery , Linus Walleij , Jacky Huang , Yury Norov , linux-sound@vger.kernel.org, linux-gpio@vger.kernel.org, Jonathan Cameron , Alex Elder , Vincent Mailhol , linux-arm-kernel@lists.infradead.org, David Laight , Stephen Boyd , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-crypto@vger.kernel.org, Johannes Berg , David Miller , Jonathan Cameron Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 14 Feb 2025 14:55:51 +0100 Geert Uytterhoeven wrote: > The existing FIELD_{GET,PREP}() macros are limited to compile-time > constants. However, it is very common to prepare or extract bitfield > elements where the bitfield mask is not a compile-time constant. > > To avoid this limitation, the AT91 clock driver and several other > drivers already have their own non-const field_{prep,get}() macros. > Make them available for general use by consolidating them in > , and improve them slightly: > 1. Avoid evaluating macro parameters more than once, > 2. Replace "ffs() - 1" by "__ffs()", > 3. Support 64-bit use on 32-bit architectures. > > This is deliberately not merged into the existing FIELD_{GET,PREP}() > macros, as people expressed the desire to keep stricter variants for > increased safety, or for performance critical paths. I really really think that people should just use the static inline helpers if the field is not constant. And we should do something like below so that people can actually find them. diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h index 63928f173223..e02afcd7aeee 100644 --- a/include/linux/bitfield.h +++ b/include/linux/bitfield.h @@ -156,6 +156,80 @@ (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \ }) +/** + * u32_encode_bits() - prepare a u32 bitfield element (non-const) + * @v: value to put in the field + * @field: shifted mask defining the field's length and position + * + * Equivalent of FIELD_PREP() for u32, field does not have to be constant. + * + * Note that the helper is available for other field widths (generated below). + */ +static __always_inline __u32 u32_encode_bits(u32 v, u32 field) +{ + if (__builtin_constant_p(v) && (v & ~field_mask(field))) + __field_overflow(); + return ((v & field_mask(field)) * field_multiplier(field)); +} + +/** + * u32_replace_bits() - change a u32 bitfield element (non-const) + * @old: old u32 value to modify + * @val: value to put in the field + * @field: shifted mask defining the field's length and position + * + * Remove the current contents of the @field in @old and set it to @new. + * + * Note that the helper is available for other field widths (generated below). + */ +static __always_inline __u32 u32_replace_bits(__u32 old, u32 val, u32 field) +{ + return (old & ~(field)) | u32_encode_bits(val, field); +} + +/** + * u32_get_bits() - get u32 bitfield element (non-const) + * @v: value to extract the field from + * @field: shifted mask defining the field's length and position + * + * Extract the value of the field and shift it down. + * + * Note that the helper is available for other field widths (generated below). + */ +static __always_inline u32 u32_get_bits(__u32 v, u32 field) +{ + return ((v) & field) / field_multiplier(field); +} + +static __always_inline void u32p_replace_bits(__u32 * p, u32 val, u32 field) +{ + * p = ( * p & ~(field)) | u32_encode_bits(val, field); +} + +static __always_inline __le32 le32_encode_bits(u32 v, u32 field) +{ + if (__builtin_constant_p(v) && (v & ~field_mask(field))) + __field_overflow(); + return cpu_to_le32((v & field_mask(field)) * field_multiplier(field)); +} + +static __always_inline __le32 le32_replace_bits(__le32 old, u32 val, u32 field) +{ + return (old & ~cpu_to_le32(field)) | le32_encode_bits(val, field); +} + +static __always_inline void le32p_replace_bits(__le32 * p, u32 val, u32 field) +{ + * p = ( * p & ~cpu_to_le32(field)) | le32_encode_bits(val, field); +} + +static __always_inline u32 le32_get_bits(__le32 v, u32 field) +{ + return (le32_to_cpu(v) & field) / field_multiplier(field); +} + +/* Auto-generate bit ops for other field width and endian combination */ + extern void __compiletime_error("value doesn't fit into mask") __field_overflow(void); extern void __compiletime_error("bad bitfield mask") @@ -198,7 +272,7 @@ static __always_inline base type##_get_bits(__##type v, base field) \ ____MAKE_OP(u##size,u##size,,) ____MAKE_OP(u8,u8,,) __MAKE_OP(16) -__MAKE_OP(32) +____MAKE_OP(be32,u32,cpu_to_be32,be32_to_cpu) /* Other 32b types open coded */ __MAKE_OP(64) #undef __MAKE_OP #undef ____MAKE_OP