[RFC PATCH 0/2] regmap: add APIs to bypass regmap_field and work with reg_field directly

[RFC PATCH 0/2] regmap: add APIs to bypass regmap_field and work with reg_field directly

[Victor Chernyakin]

Working with the existing regmap_field_* APIs requires you to define
a reg_field, then use one of the regmap_field_*_alloc functions to turn
it into a dynamically allocated regmap_field. But reg_field and
regmap_field are practically the same struct; all the *_alloc functions do
is shuffle fields around. So this RFC proposes adding functions that
cut out the middleman and take reg_field directly. Patch 1 adds two
of them: regmap_reg_field_read and regmap_reg_field_write.

Patch 2 ports a driver to use these new functions. The result is
shorter and simpler, since all the code to perform the allocations
and manage the resulting regmap_fields goes away. In theory,
there should also be a performance benefit (we're doing fewer allocations
and have less duplicated state), but I haven't benchmarked it, so take that
with a grain of salt.

If we like this approach, I can post a v2 that adds regmap_reg_field_*
analogs for all the regmap_field_* functions, not just the two you see here.


This is not actually a new idea! Both it and the current *_alloc approach
were suggested as alternatives right when the regmap_field_* APIs were
being added [1]. The *_alloc approach was chosen without much
discussion, but I think (and Patch 2 demonstrates, I hope) that
this approach is the cleaner one.

[1] https://lore.kernel.org/20130604210137.GV31367@sirena.org.uk

Victor Chernyakin (2):
  regmap: add regmap_reg_field_* APIs
  iio: accel: msa311: use regmap_reg_field_* functions

 drivers/iio/accel/msa311.c | 297 +++++++++++++------------------------
 include/linux/regmap.h     |  61 ++++++--
 2 files changed, 158 insertions(+), 200 deletions(-)

-- 
2.53.0

[RFC PATCH 1/2] regmap: add regmap_reg_field_* APIs

[Victor Chernyakin]

These functions are equivalent to their regmap_field_* counterparts,
but allow working with reg_field directly, without having to
allocate and manage regmap_fields.

Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
---
 include/linux/regmap.h | 61 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 10 deletions(-)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index c8aebd148e08..b4131495d993 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -11,6 +11,7 @@
  */
 
+#include <linux/bitfield.h>
 #include <linux/bug.h>
 #include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/err.h>
@@ -1463,18 +1464,18 @@ struct reg_field {
 	unsigned int id_offset;
 };
 
-#define REG_FIELD(_reg, _lsb, _msb) {		\
-				.reg = _reg,	\
-				.lsb = _lsb,	\
-				.msb = _msb,	\
+#define REG_FIELD(_reg, _lsb, _msb) (struct reg_field){	\
+				.reg = _reg,		\
+				.lsb = _lsb,		\
+				.msb = _msb,		\
 				}
 
-#define REG_FIELD_ID(_reg, _lsb, _msb, _size, _offset) {	\
-				.reg = _reg,			\
-				.lsb = _lsb,			\
-				.msb = _msb,			\
-				.id_size = _size,		\
-				.id_offset = _offset,		\
+#define REG_FIELD_ID(_reg, _lsb, _msb, _size, _offset) (struct reg_field){ \
+				.reg = _reg,				   \
+				.lsb = _lsb,				   \
+				.msb = _msb,				   \
+				.id_size = _size,			   \
+				.id_offset = _offset,			   \
 				}
 
 struct regmap_field *regmap_field_alloc(struct regmap *regmap,
@@ -1583,6 +1584,30 @@ regmap_fields_force_update_bits(struct regmap_field *field, unsigned int id,
 					      NULL, false, true);
 }
 
+static inline int
+regmap_reg_field_read(struct regmap *regmap, struct reg_field field,
+		      unsigned int *val)
+{
+	int ret;
+
+	ret = regmap_read(regmap, field.reg, val);
+	if (ret)
+		return ret;
+
+	*val = field_get(GENMASK(field.msb, field.lsb), *val);
+
+	return ret;
+}
+
+static inline int
+regmap_reg_field_write(struct regmap *regmap, struct reg_field field,
+		       unsigned int val)
+{
+	return regmap_update_bits(regmap, field.reg,
+				  GENMASK(field.msb, field.lsb),
+				  val << field.lsb);
+}
+
 /**
  * struct regmap_irq_type - IRQ type definitions.
  *
@@ -2071,6 +2096,22 @@ regmap_fields_force_update_bits(struct regmap_field *field, unsigned int id,
 	return -EINVAL;
 }
 
+static inline int
+regmap_reg_field_read(struct regmap *regmap, struct reg_field field,
+		      unsigned int *val)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
+static inline int
+regmap_reg_field_write(struct regmap *regmap, struct reg_field field,
+		       unsigned int val)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
 static inline int regmap_get_val_bytes(struct regmap *map)
 {
 	WARN_ONCE(1, "regmap API is disabled");
-- 
2.53.0

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

[Victor Chernyakin]

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.

Tested by compiling.

Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
---
 drivers/iio/accel/msa311.c | 297 +++++++++++++------------------------
 1 file changed, 107 insertions(+), 190 deletions(-)

diff --git a/drivers/iio/accel/msa311.c b/drivers/iio/accel/msa311.c
index caf9b1b6397f..191109ae92b1 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
 
-enum msa311_fields {
-	/* Soft_Reset */
-	F_SOFT_RESET_I2C, F_SOFT_RESET_SPI,
-	/* Motion_Interrupt */
-	F_ORIENT_INT, F_S_TAP_INT, F_D_TAP_INT, F_ACTIVE_INT, F_FREEFALL_INT,
-	/* Data_Interrupt */
-	F_NEW_DATA_INT,
-	/* Tap_Active_Status */
-	F_TAP_SIGN, F_TAP_FIRST_X, F_TAP_FIRST_Y, F_TAP_FIRST_Z, F_ACTV_SIGN,
-	F_ACTV_FIRST_X, F_ACTV_FIRST_Y, F_ACTV_FIRST_Z,
-	/* Orientation_Status */
-	F_ORIENT_Z, F_ORIENT_X_Y,
-	/* Range */
-	F_FS,
-	/* ODR */
-	F_X_AXIS_DIS, F_Y_AXIS_DIS, F_Z_AXIS_DIS, F_ODR,
-	/* Power Mode/Bandwidth */
-	F_PWR_MODE, F_LOW_POWER_BW,
-	/* Swap_Polarity */
-	F_X_POLARITY, F_Y_POLARITY, F_Z_POLARITY, F_X_Y_SWAP,
-	/* Int_Set_0 */
-	F_ORIENT_INT_EN, F_S_TAP_INT_EN, F_D_TAP_INT_EN, F_ACTIVE_INT_EN_Z,
-	F_ACTIVE_INT_EN_Y, F_ACTIVE_INT_EN_X,
-	/* Int_Set_1 */
-	F_NEW_DATA_INT_EN, F_FREEFALL_INT_EN,
-	/* Int_Map_0 */
-	F_INT1_ORIENT, F_INT1_S_TAP, F_INT1_D_TAP, F_INT1_ACTIVE,
-	F_INT1_FREEFALL,
-	/* Int_Map_1 */
-	F_INT1_NEW_DATA,
-	/* Int_Config */
-	F_INT1_OD, F_INT1_LVL,
-	/* Int_Latch */
-	F_RESET_INT, F_LATCH_INT,
-	/* Freefall_Hy */
-	F_FREEFALL_MODE, F_FREEFALL_HY,
-	/* Active_Dur */
-	F_ACTIVE_DUR,
-	/* Tap_Dur */
-	F_TAP_QUIET, F_TAP_SHOCK, F_TAP_DUR,
-	/* Tap_Th */
-	F_TAP_TH,
-	/* Orient_Hy */
-	F_ORIENT_HYST, F_ORIENT_BLOCKING, F_ORIENT_MODE,
-	/* Z_Block */
-	F_Z_BLOCKING,
-	/* End of register map */
-	F_MAX_FIELDS,
-};
-
-static const struct reg_field msa311_reg_fields[] = {
-	/* Soft_Reset */
-	[F_SOFT_RESET_I2C] = REG_FIELD(MSA311_SOFT_RESET_REG, 2, 2),
-	[F_SOFT_RESET_SPI] = REG_FIELD(MSA311_SOFT_RESET_REG, 5, 5),
-	/* Motion_Interrupt */
-	[F_ORIENT_INT] = REG_FIELD(MSA311_MOTION_INT_REG, 6, 6),
-	[F_S_TAP_INT] = REG_FIELD(MSA311_MOTION_INT_REG, 5, 5),
-	[F_D_TAP_INT] = REG_FIELD(MSA311_MOTION_INT_REG, 4, 4),
-	[F_ACTIVE_INT] = REG_FIELD(MSA311_MOTION_INT_REG, 2, 2),
-	[F_FREEFALL_INT] = REG_FIELD(MSA311_MOTION_INT_REG, 0, 0),
-	/* Data_Interrupt */
-	[F_NEW_DATA_INT] = REG_FIELD(MSA311_DATA_INT_REG, 0, 0),
-	/* Tap_Active_Status */
-	[F_TAP_SIGN] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 7, 7),
-	[F_TAP_FIRST_X] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 6, 6),
-	[F_TAP_FIRST_Y] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 5, 5),
-	[F_TAP_FIRST_Z] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 4, 4),
-	[F_ACTV_SIGN] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 3, 3),
-	[F_ACTV_FIRST_X] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 2, 2),
-	[F_ACTV_FIRST_Y] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 1, 1),
-	[F_ACTV_FIRST_Z] = REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 0, 0),
-	/* Orientation_Status */
-	[F_ORIENT_Z] = REG_FIELD(MSA311_ORIENT_STS_REG, 6, 6),
-	[F_ORIENT_X_Y] = REG_FIELD(MSA311_ORIENT_STS_REG, 4, 5),
-	/* Range */
-	[F_FS] = REG_FIELD(MSA311_RANGE_REG, 0, 1),
-	/* ODR */
-	[F_X_AXIS_DIS] = REG_FIELD(MSA311_ODR_REG, 7, 7),
-	[F_Y_AXIS_DIS] = REG_FIELD(MSA311_ODR_REG, 6, 6),
-	[F_Z_AXIS_DIS] = REG_FIELD(MSA311_ODR_REG, 5, 5),
-	[F_ODR] = REG_FIELD(MSA311_ODR_REG, 0, 3),
-	/* Power Mode/Bandwidth */
-	[F_PWR_MODE] = REG_FIELD(MSA311_PWR_MODE_REG, 6, 7),
-	[F_LOW_POWER_BW] = REG_FIELD(MSA311_PWR_MODE_REG, 1, 4),
-	/* Swap_Polarity */
-	[F_X_POLARITY] = REG_FIELD(MSA311_SWAP_POLARITY_REG, 3, 3),
-	[F_Y_POLARITY] = REG_FIELD(MSA311_SWAP_POLARITY_REG, 2, 2),
-	[F_Z_POLARITY] = REG_FIELD(MSA311_SWAP_POLARITY_REG, 1, 1),
-	[F_X_Y_SWAP] = REG_FIELD(MSA311_SWAP_POLARITY_REG, 0, 0),
-	/* Int_Set_0 */
-	[F_ORIENT_INT_EN] = REG_FIELD(MSA311_INT_SET_0_REG, 6, 6),
-	[F_S_TAP_INT_EN] = REG_FIELD(MSA311_INT_SET_0_REG, 5, 5),
-	[F_D_TAP_INT_EN] = REG_FIELD(MSA311_INT_SET_0_REG, 4, 4),
-	[F_ACTIVE_INT_EN_Z] = REG_FIELD(MSA311_INT_SET_0_REG, 2, 2),
-	[F_ACTIVE_INT_EN_Y] = REG_FIELD(MSA311_INT_SET_0_REG, 1, 1),
-	[F_ACTIVE_INT_EN_X] = REG_FIELD(MSA311_INT_SET_0_REG, 0, 0),
-	/* Int_Set_1 */
-	[F_NEW_DATA_INT_EN] = REG_FIELD(MSA311_INT_SET_1_REG, 4, 4),
-	[F_FREEFALL_INT_EN] = REG_FIELD(MSA311_INT_SET_1_REG, 3, 3),
-	/* Int_Map_0 */
-	[F_INT1_ORIENT] = REG_FIELD(MSA311_INT_MAP_0_REG, 6, 6),
-	[F_INT1_S_TAP] = REG_FIELD(MSA311_INT_MAP_0_REG, 5, 5),
-	[F_INT1_D_TAP] = REG_FIELD(MSA311_INT_MAP_0_REG, 4, 4),
-	[F_INT1_ACTIVE] = REG_FIELD(MSA311_INT_MAP_0_REG, 2, 2),
-	[F_INT1_FREEFALL] = REG_FIELD(MSA311_INT_MAP_0_REG, 0, 0),
-	/* Int_Map_1 */
-	[F_INT1_NEW_DATA] = REG_FIELD(MSA311_INT_MAP_1_REG, 0, 0),
-	/* Int_Config */
-	[F_INT1_OD] = REG_FIELD(MSA311_INT_CONFIG_REG, 1, 1),
-	[F_INT1_LVL] = REG_FIELD(MSA311_INT_CONFIG_REG, 0, 0),
-	/* Int_Latch */
-	[F_RESET_INT] = REG_FIELD(MSA311_INT_LATCH_REG, 7, 7),
-	[F_LATCH_INT] = REG_FIELD(MSA311_INT_LATCH_REG, 0, 3),
-	/* Freefall_Hy */
-	[F_FREEFALL_MODE] = REG_FIELD(MSA311_FREEFALL_HY_REG, 2, 2),
-	[F_FREEFALL_HY] = REG_FIELD(MSA311_FREEFALL_HY_REG, 0, 1),
-	/* Active_Dur */
-	[F_ACTIVE_DUR] = REG_FIELD(MSA311_ACTIVE_DUR_REG, 0, 1),
-	/* Tap_Dur */
-	[F_TAP_QUIET] = REG_FIELD(MSA311_TAP_DUR_REG, 7, 7),
-	[F_TAP_SHOCK] = REG_FIELD(MSA311_TAP_DUR_REG, 6, 6),
-	[F_TAP_DUR] = REG_FIELD(MSA311_TAP_DUR_REG, 0, 2),
-	/* Tap_Th */
-	[F_TAP_TH] = REG_FIELD(MSA311_TAP_TH_REG, 0, 4),
-	/* Orient_Hy */
-	[F_ORIENT_HYST] = REG_FIELD(MSA311_ORIENT_HY_REG, 4, 6),
-	[F_ORIENT_BLOCKING] = REG_FIELD(MSA311_ORIENT_HY_REG, 2, 3),
-	[F_ORIENT_MODE] = REG_FIELD(MSA311_ORIENT_HY_REG, 0, 1),
-	/* Z_Block */
-	[F_Z_BLOCKING] = REG_FIELD(MSA311_Z_BLOCK_REG, 0, 3),
-};
+/* Soft_Reset */
+#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)
+/* Motion_Interrupt */
+#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)
+/* Data_Interrupt */
+#define MSA311_F_NEW_DATA_INT	REG_FIELD(MSA311_DATA_INT_REG, 0, 0)
+/* Tap_Active_Status */
+#define MSA311_F_TAP_SIGN	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 7, 7)
+#define MSA311_F_TAP_FIRST_X	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 6, 6)
+#define MSA311_F_TAP_FIRST_Y	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 5, 5)
+#define MSA311_F_TAP_FIRST_Z	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 4, 4)
+#define MSA311_F_ACTV_SIGN	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 3, 3)
+#define MSA311_F_ACTV_FIRST_X	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 2, 2)
+#define MSA311_F_ACTV_FIRST_Y	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 1, 1)
+#define MSA311_F_ACTV_FIRST_Z	REG_FIELD(MSA311_TAP_ACTIVE_STS_REG, 0, 0)
+/* Orientation_Status */
+#define MSA311_F_ORIENT_Z	REG_FIELD(MSA311_ORIENT_STS_REG, 6, 6)
+#define MSA311_F_ORIENT_X_Y	REG_FIELD(MSA311_ORIENT_STS_REG, 4, 5)
+/* Range */
+#define MSA311_F_FS		REG_FIELD(MSA311_RANGE_REG, 0, 1)
+/* ODR */
+#define MSA311_F_X_AXIS_DIS	REG_FIELD(MSA311_ODR_REG, 7, 7)
+#define MSA311_F_Y_AXIS_DIS	REG_FIELD(MSA311_ODR_REG, 6, 6)
+#define MSA311_F_Z_AXIS_DIS	REG_FIELD(MSA311_ODR_REG, 5, 5)
+#define MSA311_F_ODR		REG_FIELD(MSA311_ODR_REG, 0, 3)
+/* Power Mode/Bandwidth */
+#define MSA311_F_PWR_MODE	REG_FIELD(MSA311_PWR_MODE_REG, 6, 7)
+#define MSA311_F_LOW_POWER_BW	REG_FIELD(MSA311_PWR_MODE_REG, 1, 4)
+/* Swap_Polarity */
+#define MSA311_F_X_POLARITY	REG_FIELD(MSA311_SWAP_POLARITY_REG, 3, 3)
+#define MSA311_F_Y_POLARITY	REG_FIELD(MSA311_SWAP_POLARITY_REG, 2, 2)
+#define MSA311_F_Z_POLARITY	REG_FIELD(MSA311_SWAP_POLARITY_REG, 1, 1)
+#define MSA311_F_X_Y_SWAP	REG_FIELD(MSA311_SWAP_POLARITY_REG, 0, 0)
+/* Int_Set_0 */
+#define MSA311_F_ORIENT_INT_EN		REG_FIELD(MSA311_INT_SET_0_REG, 6, 6)
+#define MSA311_F_S_TAP_INT_EN		REG_FIELD(MSA311_INT_SET_0_REG, 5, 5)
+#define MSA311_F_D_TAP_INT_EN		REG_FIELD(MSA311_INT_SET_0_REG, 4, 4)
+#define MSA311_F_ACTIVE_INT_EN_Z	REG_FIELD(MSA311_INT_SET_0_REG, 2, 2)
+#define MSA311_F_ACTIVE_INT_EN_Y	REG_FIELD(MSA311_INT_SET_0_REG, 1, 1)
+#define MSA311_F_ACTIVE_INT_EN_X	REG_FIELD(MSA311_INT_SET_0_REG, 0, 0)
+/* Int_Set_1 */
+#define MSA311_F_NEW_DATA_INT_EN	REG_FIELD(MSA311_INT_SET_1_REG, 4, 4)
+#define MSA311_F_FREEFALL_INT_EN	REG_FIELD(MSA311_INT_SET_1_REG, 3, 3)
+/* Int_Map_0 */
+#define MSA311_F_INT1_ORIENT	REG_FIELD(MSA311_INT_MAP_0_REG, 6, 6)
+#define MSA311_F_INT1_S_TAP	REG_FIELD(MSA311_INT_MAP_0_REG, 5, 5)
+#define MSA311_F_INT1_D_TAP	REG_FIELD(MSA311_INT_MAP_0_REG, 4, 4)
+#define MSA311_F_INT1_ACTIVE	REG_FIELD(MSA311_INT_MAP_0_REG, 2, 2)
+#define MSA311_F_INT1_FREEFALL	REG_FIELD(MSA311_INT_MAP_0_REG, 0, 0)
+/* Int_Map_1 */
+#define MSA311_F_INT1_NEW_DATA	REG_FIELD(MSA311_INT_MAP_1_REG, 0, 0)
+/* Int_Config */
+#define MSA311_F_INT1_OD	REG_FIELD(MSA311_INT_CONFIG_REG, 1, 1)
+#define MSA311_F_INT1_LVL	REG_FIELD(MSA311_INT_CONFIG_REG, 0, 0)
+/* Int_Latch */
+#define MSA311_F_RESET_INT	REG_FIELD(MSA311_INT_LATCH_REG, 7, 7)
+#define MSA311_F_LATCH_INT	REG_FIELD(MSA311_INT_LATCH_REG, 0, 3)
+/* Freefall_Hy */
+#define MSA311_F_FREEFALL_MODE	REG_FIELD(MSA311_FREEFALL_HY_REG, 2, 2)
+#define MSA311_F_FREEFALL_HY	REG_FIELD(MSA311_FREEFALL_HY_REG, 0, 1)
+/* Active_Dur */
+#define MSA311_F_ACTIVE_DUR	REG_FIELD(MSA311_ACTIVE_DUR_REG, 0, 1)
+/* Tap_Dur */
+#define MSA311_F_TAP_QUIET	REG_FIELD(MSA311_TAP_DUR_REG, 7, 7)
+#define MSA311_F_TAP_SHOCK	REG_FIELD(MSA311_TAP_DUR_REG, 6, 6)
+#define MSA311_F_TAP_DUR	REG_FIELD(MSA311_TAP_DUR_REG, 0, 2)
+/* Tap_Th */
+#define MSA311_F_TAP_TH		REG_FIELD(MSA311_TAP_TH_REG, 0, 4)
+/* Orient_Hy */
+#define MSA311_F_ORIENT_HYST		REG_FIELD(MSA311_ORIENT_HY_REG, 4, 6)
+#define MSA311_F_ORIENT_BLOCKING	REG_FIELD(MSA311_ORIENT_HY_REG, 2, 3)
+#define MSA311_F_ORIENT_MODE		REG_FIELD(MSA311_ORIENT_HY_REG, 0, 1)
+/* Z_Block */
+#define MSA311_F_Z_BLOCKING	REG_FIELD(MSA311_Z_BLOCK_REG, 0, 3)
 
 #define MSA311_WHO_AM_I 0x13
 
@@ -334,11 +282,7 @@ static const struct regmap_config msa311_regmap_config = {
 	.cache_type = REGCACHE_MAPLE,
 };
 
-#define MSA311_GENMASK(field) ({                \
-	typeof(&(msa311_reg_fields)[0]) _field; \
-	_field = &msa311_reg_fields[(field)];   \
-	GENMASK(_field->msb, _field->lsb);      \
-})
+#define MSA311_GENMASK(field) GENMASK((field).msb, (field).lsb)
 
 /**
  * struct msa311_priv - MSA311 internal private state
@@ -354,7 +298,6 @@ static const struct regmap_config msa311_regmap_config = {
  */
 struct msa311_priv {
 	struct regmap *regs;
-	struct regmap_field *fields[F_MAX_FIELDS];
 
 	struct device *dev;
 	struct mutex lock;
@@ -410,7 +353,7 @@ static int msa311_get_odr(struct msa311_priv *msa311, unsigned int *odr)
 {
 	int err;
 
-	err = regmap_field_read(msa311->fields[F_ODR], odr);
+	err = regmap_reg_field_read(msa311->regs, MSA311_F_ODR, odr);
 	if (err)
 		return err;
 
@@ -453,7 +396,7 @@ static int msa311_set_odr(struct msa311_priv *msa311, unsigned int odr)
 	bool good_odr;
 	int err;
 
-	err = regmap_field_read(msa311->fields[F_PWR_MODE], &pwr_mode);
+	err = regmap_reg_field_read(msa311->regs, MSA311_F_PWR_MODE, &pwr_mode);
 	if (err)
 		return err;
 
@@ -472,7 +415,7 @@ static int msa311_set_odr(struct msa311_priv *msa311, unsigned int odr)
 		return -EINVAL;
 	}
 
-	return regmap_field_write(msa311->fields[F_ODR], odr);
+	return regmap_reg_field_write(msa311->regs, MSA311_F_ODR, odr);
 }
 
 /**
@@ -538,11 +481,11 @@ static int msa311_set_pwr_mode(struct msa311_priv *msa311, unsigned int mode)
 
 	dev_dbg(dev, "transition to %s mode\n", msa311_pwr_modes[mode]);
 
-	err = regmap_field_read(msa311->fields[F_PWR_MODE], &prev_mode);
+	err = regmap_reg_field_read(msa311->regs, MSA311_F_PWR_MODE, &prev_mode);
 	if (err)
 		return err;
 
-	err = regmap_field_write(msa311->fields[F_PWR_MODE], mode);
+	err = regmap_reg_field_write(msa311->regs, MSA311_F_PWR_MODE, mode);
 	if (err)
 		return err;
 
@@ -634,7 +577,7 @@ static int msa311_read_scale(struct iio_dev *indio_dev, int *val, int *val2)
 	int err;
 
 	mutex_lock(&msa311->lock);
-	err = regmap_field_read(msa311->fields[F_FS], &fs);
+	err = regmap_reg_field_read(msa311->regs, MSA311_F_FS, &fs);
 	mutex_unlock(&msa311->lock);
 	if (err) {
 		dev_err(dev, "can't get actual scale (%pe)\n", ERR_PTR(err));
@@ -734,7 +677,7 @@ static int msa311_write_scale(struct iio_dev *indio_dev, int val, int val2)
 		/* Do not check msa311_fs_table[fs].integral, it's always 0 */
 		if (val2 == msa311_fs_table[fs].microfract) {
 			mutex_lock(&msa311->lock);
-			err = regmap_field_write(msa311->fields[F_FS], fs);
+			err = regmap_reg_field_write(msa311->regs, MSA311_F_FS, fs);
 			mutex_unlock(&msa311->lock);
 			break;
 		}
@@ -863,7 +806,7 @@ static int msa311_set_new_data_trig_state(struct iio_trigger *trig, bool state)
 	int err;
 
 	mutex_lock(&msa311->lock);
-	err = regmap_field_write(msa311->fields[F_NEW_DATA_INT_EN], state);
+	err = regmap_reg_field_write(msa311->regs, MSA311_F_NEW_DATA_INT_EN, state);
 	mutex_unlock(&msa311->lock);
 	if (err)
 		dev_err(dev,
@@ -934,8 +877,8 @@ static irqreturn_t msa311_irq_thread(int irq, void *p)
 	 * specification it's cleared automatically after a fixed time.
 	 * So just check that is enabled by driver logic.
 	 */
-	err = regmap_field_read(msa311->fields[F_NEW_DATA_INT_EN],
-				&new_data_int_enabled);
+	err = regmap_reg_field_read(msa311->regs, MSA311_F_NEW_DATA_INT_EN,
+				    &new_data_int_enabled);
 
 	mutex_unlock(&msa311->lock);
 	if (err) {
@@ -995,8 +938,8 @@ static int msa311_soft_reset(struct msa311_priv *msa311)
 	int err;
 
 	err = regmap_write(msa311->regs, MSA311_SOFT_RESET_REG,
-			   MSA311_GENMASK(F_SOFT_RESET_I2C) |
-			   MSA311_GENMASK(F_SOFT_RESET_SPI));
+			   MSA311_GENMASK(MSA311_F_SOFT_RESET_I2C) |
+			   MSA311_GENMASK(MSA311_F_SOFT_RESET_SPI));
 	if (err)
 		return dev_err_probe(dev, err, "can't soft reset all logic\n");
 
@@ -1029,9 +972,9 @@ static int msa311_chip_init(struct msa311_priv *msa311)
 
 	/* Disable all axes by default */
 	err = regmap_clear_bits(msa311->regs, MSA311_ODR_REG,
-				MSA311_GENMASK(F_X_AXIS_DIS) |
-				MSA311_GENMASK(F_Y_AXIS_DIS) |
-				MSA311_GENMASK(F_Z_AXIS_DIS));
+				MSA311_GENMASK(MSA311_F_X_AXIS_DIS) |
+				MSA311_GENMASK(MSA311_F_Y_AXIS_DIS) |
+				MSA311_GENMASK(MSA311_F_Z_AXIS_DIS));
 	if (err)
 		return dev_err_probe(dev, err, "can't enable all axes\n");
 
@@ -1074,30 +1017,30 @@ static int msa311_setup_interrupts(struct msa311_priv *msa311)
 		return dev_err_probe(dev, err,
 				     "can't register newdata trigger\n");
 
-	err = regmap_field_write(msa311->fields[F_INT1_OD],
-				 MSA311_INT1_OD_PUSH_PULL);
+	err = regmap_reg_field_write(msa311->regs, MSA311_F_INT1_OD,
+				     MSA311_INT1_OD_PUSH_PULL);
 	if (err)
 		return dev_err_probe(dev, err,
 				     "can't enable push-pull interrupt\n");
 
-	err = regmap_field_write(msa311->fields[F_INT1_LVL],
-				 MSA311_INT1_LVL_HIGH);
+	err = regmap_reg_field_write(msa311->regs, MSA311_F_INT1_LVL,
+				     MSA311_INT1_LVL_HIGH);
 	if (err)
 		return dev_err_probe(dev, err,
 				     "can't set active interrupt level\n");
 
-	err = regmap_field_write(msa311->fields[F_LATCH_INT],
-				 MSA311_LATCH_INT_LATCHED);
+	err = regmap_reg_field_write(msa311->regs, MSA311_F_LATCH_INT,
+				     MSA311_LATCH_INT_LATCHED);
 	if (err)
 		return dev_err_probe(dev, err,
 				     "can't latch interrupt\n");
 
-	err = regmap_field_write(msa311->fields[F_RESET_INT], 1);
+	err = regmap_reg_field_write(msa311->regs, MSA311_F_RESET_INT, 1);
 	if (err)
 		return dev_err_probe(dev, err,
 				     "can't reset interrupt\n");
 
-	err = regmap_field_write(msa311->fields[F_INT1_NEW_DATA], 1);
+	err = regmap_reg_field_write(msa311->regs, MSA311_F_INT1_NEW_DATA, 1);
 	if (err)
 		return dev_err_probe(dev, err,
 				     "can't map new data interrupt\n");
@@ -1105,33 +1048,6 @@ static int msa311_setup_interrupts(struct msa311_priv *msa311)
 	return 0;
 }
 
-static int msa311_regmap_init(struct msa311_priv *msa311)
-{
-	struct regmap_field **fields = msa311->fields;
-	struct device *dev = msa311->dev;
-	struct i2c_client *i2c = to_i2c_client(dev);
-	struct regmap *regmap;
-	int i;
-
-	regmap = devm_regmap_init_i2c(i2c, &msa311_regmap_config);
-	if (IS_ERR(regmap))
-		return dev_err_probe(dev, PTR_ERR(regmap),
-				     "failed to register i2c regmap\n");
-
-	msa311->regs = regmap;
-
-	for (i = 0; i < F_MAX_FIELDS; i++) {
-		fields[i] = devm_regmap_field_alloc(dev,
-						    msa311->regs,
-						    msa311_reg_fields[i]);
-		if (IS_ERR(msa311->fields[i]))
-			return dev_err_probe(dev, PTR_ERR(msa311->fields[i]),
-					     "can't alloc field[%d]\n", i);
-	}
-
-	return 0;
-}
-
 static void msa311_powerdown(void *msa311)
 {
 	msa311_set_pwr_mode(msa311, MSA311_PWR_MODE_SUSPEND);
@@ -1152,9 +1068,10 @@ static int msa311_probe(struct i2c_client *i2c)
 	msa311->dev = dev;
 	i2c_set_clientdata(i2c, indio_dev);
 
-	err = msa311_regmap_init(msa311);
-	if (err)
-		return err;
+	msa311->regs = devm_regmap_init_i2c(i2c, &msa311_regmap_config);
+	if (IS_ERR(msa311->regs))
+		return dev_err_probe(dev, PTR_ERR(msa311->regs),
+				     "failed to register i2c regmap\n");
 
 	mutex_init(&msa311->lock);
 
-- 
2.53.0

Re: [RFC PATCH 0/2] regmap: add APIs to bypass regmap_field and work with reg_field directly

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

On Wed, Nov 04, 2026 at 06:09:48PM -0600, Victor Chernyakin wrote:

> This is not actually a new idea! Both it and the current *_alloc approach
> were suggested as alternatives right when the regmap_field_* APIs were
> being added [1]. The *_alloc approach was chosen without much
> discussion, but I think (and Patch 2 demonstrates, I hope) that
> this approach is the cleaner one.

The demonstration in patch 2 would be a lot clearer had I been sent
patch 2...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC PATCH 0/2] regmap: add APIs to bypass regmap_field and work with reg_field directly

[Victor Chernyakin]

> > This is not actually a new idea! Both it and the current *_alloc approach
> > were suggested as alternatives right when the regmap_field_* APIs were
> > being added [1]. The *_alloc approach was chosen without much
> > discussion, but I think (and Patch 2 demonstrates, I hope) that
> > this approach is the cleaner one.
> 
> The demonstration in patch 2 would be a lot clearer had I been sent
> patch 2...

That's what I get for just blindly using get_maintainer.pl, sorry
about that. Sent it to you now.

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

[Andy Shevchenko]

On Wed, Nov 04, 2026 at 06:09:50PM -0600, Victor Chernyakin 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.

I have just proposed REG_FIELD_GENMASK() macro, this change may rely on it
20261105103533.1426533-1-andriy.shevchenko@linux.intel.com if it comes through.

-- 
With Best Regards,
Andy Shevchenko