On 3/6/2025 11:29 PM, Andrew Lunn wrote:
Thanks for the suggestion. Just to clarify, we preferred
u32p_replace_bits() over FIELD_PREP() because the former does
a clear-and-set operation against a given mask, where as with
FIELD_PREP(), we need to clear the bits first before we use the
macro and then set it. Due to this, we preferred using
u32_replace_bits() since it made the macro definitions to modify
the registers simpler. Given this, would it be acceptable to
document u32p_replace_bits() better, as it is already being used
by other drivers as well?
I suggest you submit a patch to those who maintain that file and see
what they say.
But maybe also look at how others are using u32p_replace_bits() and
should it be wrapped up in a macro? FIELD_MOD()? These macros do a lot
of build time checking that you are not overflowing the type. It would
be good to have that to catch bugs at build time, rather than years
later at runtime.
Andrew
OK, understand. I will submit the patch by adding the FIELD_MODIFY()
with required build time checking included.
Below is a draft of the macro, please take a look if possible before
it is posted to maintainers. I will update the driver to use this macro
if it can be accepted. Thanks.
/**
* FIELD_MODIFY() - modify a bitfield element
* @_mask: shifted mask defining the field's length and position
* @_reg_p: point to the memory that should be updated
* @_val: value to store in the bitfield
*
* FIELD_MODIFY() modifies the set of bits in @_reg_p specified
* by @_mask, by replacing them with the bitfield value passed
* in as @_val.
*/
#define FIELD_MODIFY(_mask, _reg_p, _val) \
({ \
__BF_FIELD_CHECK(_mask, *_reg_p, _val, "FIELD_MODIFY:
"); \
*_reg_p &= ~(_mask); \
*_reg_p |= (_val) << __bf_shf(_mask) & (_mask); \
})
