| Message ID | 1c1492558c1a72b64bb26f7a44c4e69fff0e6b44.1679149543.git.william.gray@linaro.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Refactor 104-quad-8 to match device operations | expand |
On Sat, 2023-03-18 at 14:59 +0000, William Breathitt Gray wrote: > It is a common code pattern to modify a bitfield by masking the field > and performing a bitwise OR with the respective FIELD_PREP. Wrap such a > task into a macro by introducing FIELD_MODIFY() which modifies the field > specified by a mask from a bitfield by putting a val in the field. So I have no objection to adding this and you using FIELD_* macros, but just wanted to say that personally I've come to prefer the typed versions declared later in the fiel, and there we have <type>_replace_bits() already. Hmm. And now that I mentioned that, maybe that means FIELD_REPLACE() would be nicer as a name? johannes
On Mon, Mar 20, 2023 at 09:50:35AM +0100, Johannes Berg wrote: > On Sat, 2023-03-18 at 14:59 +0000, William Breathitt Gray wrote: > > It is a common code pattern to modify a bitfield by masking the field > > and performing a bitwise OR with the respective FIELD_PREP. Wrap such a > > task into a macro by introducing FIELD_MODIFY() which modifies the field > > specified by a mask from a bitfield by putting a val in the field. > > So I have no objection to adding this and you using FIELD_* macros, but > just wanted to say that personally I've come to prefer the typed > versions declared later in the fiel, and there we have > <type>_replace_bits() already. > > Hmm. And now that I mentioned that, maybe that means FIELD_REPLACE() > would be nicer as a name? +1 here with the similar thoughts. One thing I hate about macros like above mentioned is that Elixir or similar code browsing tools can't find. In net there are specific #if 0 ... #endif sections for mitigating that. Shouldn't we add the similar into bitfield.h?
On Mon, Mar 20, 2023 at 09:50:35AM +0100, Johannes Berg wrote: > On Sat, 2023-03-18 at 14:59 +0000, William Breathitt Gray wrote: > > It is a common code pattern to modify a bitfield by masking the field > > and performing a bitwise OR with the respective FIELD_PREP. Wrap such a > > task into a macro by introducing FIELD_MODIFY() which modifies the field > > specified by a mask from a bitfield by putting a val in the field. > > So I have no objection to adding this and you using FIELD_* macros, but > just wanted to say that personally I've come to prefer the typed > versions declared later in the fiel, and there we have > <type>_replace_bits() already. > > Hmm. And now that I mentioned that, maybe that means FIELD_REPLACE() > would be nicer as a name? > > johannes Perhaps I can convert all of these FIELD_GET(), FIELD_MODIFY(), and FIELD_GET() to the equivalent of u8_get_bits(), u8p_replace_bits(), and u8_encode_bits(). If that works, then I'll just drop the FIELD_MODIFY() patch in the v2 patchset submission. William Breathitt Gray
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h index ebfa12f69501..b06a98f0a87f 100644 --- a/include/linux/bitfield.h +++ b/include/linux/bitfield.h @@ -38,8 +38,7 @@ * FIELD_PREP(REG_FIELD_D, 0x40); * * Modify: - * reg &= ~REG_FIELD_C; - * reg |= FIELD_PREP(REG_FIELD_C, c); + * reg = FIELD_MODIFY(REG_FIELD_C, reg, c); */ #define __bf_shf(x) (__builtin_ffsll(x) - 1) @@ -155,6 +154,21 @@ (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \ }) +/** + * FIELD_MODIFY() - modify a bitfield element + * @_mask: shifted mask defining the field's length and position + * @_reg: value of entire bitfield + * @_val: value to put in the field + * + * FIELD_MODIFY() modifies the field specified by @_mask from the + * bitfield passed in as @_reg by putting @val in the field. + */ +#define FIELD_MODIFY(_mask, _reg, _val) \ + ({ \ + __BF_FIELD_CHECK(_mask, _reg, _val, "FIELD_MODIFY: "); \ + (typeof(_mask))(((_reg) & ~(_mask)) | FIELD_PREP(_mask, _val)); \ + }) + extern void __compiletime_error("value doesn't fit into mask") __field_overflow(void); extern void __compiletime_error("bad bitfield mask")
It is a common code pattern to modify a bitfield by masking the field and performing a bitwise OR with the respective FIELD_PREP. Wrap such a task into a macro by introducing FIELD_MODIFY() which modifies the field specified by a mask from a bitfield by putting a val in the field. Signed-off-by: William Breathitt Gray <william.gray@linaro.org> --- include/linux/bitfield.h | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)