Re: [RFC PATCH v2 2/2] iio: accel: msa311: use regmap_reg_field_* functions

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 13 Nov 2026 00:55:46 -0600
Victor Chernyakin <chernyakin.victor.j@outlook.com> wrote:

> Simplify the driver by using the new regmap_reg_field_* functions.
> regmap_reg_field_FOO is equivalent to regmap_field_FOO, but takes
> a reg_field directly instead of a regmap_field.

Hi Victor,

In general this seems like a reasonable idea.

One minor suggestion inline on dropping some comments that
add little value and reordering where the defines are to
give better grouping with what they are about.

When sending a small series, just send the whole lot to the list
and all people on the thread.  I had to go find the context
to understand why this was an RFC.

> 
> Tested by compiling.
I'd drop that below the ---

Thanks,

Jonathan

> 
> Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
> ---
>  drivers/iio/accel/msa311.c | 298 +++++++++++++------------------------
>  1 file changed, 106 insertions(+), 192 deletions(-)
> 
> diff --git a/drivers/iio/accel/msa311.c b/drivers/iio/accel/msa311.c
> index caf9b1b6397f..63c795d6bc99 100644
> --- a/drivers/iio/accel/msa311.c
> +++ b/drivers/iio/accel/msa311.c
> @@ -75,137 +75,85 @@
>  #define MSA311_OFFSET_Y_REG       0x39
>  #define MSA311_OFFSET_Z_REG       0x3A
>  

> +/* Soft_Reset */

These comments don't add any value because the register name is over here...

Obviously one set of them in he original code were equally pointless!

I'd be tempted to format this whole thing differently to keep register and
register field definitions together:

#define MSA311_SOFT_RESET_REG     0x00
#define MSA311_F_SOFT_RESET_I2C	REG_FIELD(MSA311_SOFT_RESET_REG, 2, 2)
#define MSA311_F_SOFT_RESET_SPI	REG_FIELD(MSA311_SOFT_RESET_REG, 5, 5)

#define MSA311_PARTID_REG         0x01
#define MSA311_ACC_X_REG          0x02
#define MSA311_ACC_Y_REG          0x04
#define MSA311_ACC_Z_REG          0x06

#define MSA311_MOTION_INT_REG     0x09
#define MSA311_F_ORIENT_INT	REG_FIELD(MSA311_MOTION_INT_REG, 6, 6)
#define MSA311_F_S_TAP_INT	REG_FIELD(MSA311_MOTION_INT_REG, 5, 5)
#define MSA311_F_D_TAP_INT	REG_FIELD(MSA311_MOTION_INT_REG, 4, 4)
#define MSA311_F_ACTIVE_INT	REG_FIELD(MSA311_MOTION_INT_REG, 2, 2)
#define MSA311_F_FREEFALL_INT	REG_FIELD(MSA311_MOTION_INT_REG, 0, 0)

#define MSA311_DATA_INT_REG       0x0A
#define MSA311_F_NEW_DATA_INT	REG_FIELD(MSA311_DATA_INT_REG, 0, 0)

Then even less need for comments.

> +#define MSA311_F_SOFT_RESET_I2C	REG_FIELD(MSA311_SOFT_RESET_REG, 2, 2)
> +#define MSA311_F_SOFT_RESET_SPI	REG_FIELD(MSA311_SOFT_RESET_REG, 5, 5)