[PATCH v4 0/2] iio: adc: mcp3422: apply bit manipulation macros

[PATCH v4 0/2] iio: adc: mcp3422: apply bit manipulation macros

[Marcelo Machado Lage]

This patch set rewrites several open-coded bit manipulation parts of
the MCP3421/2/3/4/5/6/7/8 driver code using bits.h and bitfield.h
macros.

Marcelo Machado Lage (2):
  iio: adc: mcp3422: rewrite mask macros with help of bits.h APIs
  iio: adc: mcp3422: write bit operations using bitfield.h APIs

 drivers/iio/adc/mcp3422.c | 65 ++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 38 deletions(-)

-- 
2.34.1

[PATCH v4 1/2] iio: adc: mcp3422: rewrite mask macros with help of bits.h APIs

[Marcelo Machado Lage]

Rewrite MCP3422_CHANNEL_MASK, MCP3422_SRATE_MASK, MCP3422_PGA_MASK
and MCP3422_CONT_SAMPLING using GENMASK() and BIT() macros from
bits.h.

The other macros MCP3422_SRATE_{240, 60, 15, 3} were not changed
because they are also used as array indices.

Signed-off-by: Marcelo Machado Lage <marcelomlage@usp.br>
Co-developed-by: Vinicius Lira <vinilira@usp.br>
Signed-off-by: Vinicius Lira <vinilira@usp.br>
---

v1 -> v2:
 - Split this change from the rest of the patch, following a request by Andy Shevchenko
v2 -> v3:
 - Sort marks in logical order, following a request by David Lechner
 - Delete the "/* Masks */" comment to avoid confusion, following a request by Jonathan Cameron
v3 -> v4:
 - Correct a typo in the commit message, following a request by Jonathan Cameron
 - Rephrase the Subject, following a request by Andy Shevchenko

v1: https://lore.kernel.org/linux-iio/20260417005041.484742-1-marcelomlage@usp.br/
v2: https://lore.kernel.org/linux-iio/20260417165747.507487-1-marcelomlage@usp.br/
v3: https://lore.kernel.org/linux-iio/20260420191455.529923-1-marcelomlage@usp.br/

 drivers/iio/adc/mcp3422.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
index 50834fdcf738..0cd0e7d39e39 100644
--- a/drivers/iio/adc/mcp3422.c
+++ b/drivers/iio/adc/mcp3422.c
@@ -13,6 +13,7 @@
  * voltage unit is nV.
  */
 
+#include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
@@ -24,10 +25,10 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-/* Masks */
-#define MCP3422_CHANNEL_MASK	0x60
-#define MCP3422_PGA_MASK	0x03
-#define MCP3422_SRATE_MASK	0x0C
+#define MCP3422_CHANNEL_MASK	GENMASK(6, 5)
+#define MCP3422_SRATE_MASK	GENMASK(3, 2)
+#define MCP3422_PGA_MASK	GENMASK(1, 0)
+
 #define MCP3422_SRATE_240	0x0
 #define MCP3422_SRATE_60	0x1
 #define MCP3422_SRATE_15	0x2
@@ -36,7 +37,7 @@
 #define MCP3422_PGA_2	1
 #define MCP3422_PGA_4	2
 #define MCP3422_PGA_8	3
-#define MCP3422_CONT_SAMPLING	0x10
+#define MCP3422_CONT_SAMPLING	BIT(4)
 
 #define MCP3422_CHANNEL(config)	(((config) & MCP3422_CHANNEL_MASK) >> 5)
 #define MCP3422_PGA(config)	((config) & MCP3422_PGA_MASK)
-- 
2.34.1

[PATCH v4 2/2] iio: adc: mcp3422: write bit operations using bitfield.h APIs

[Marcelo Machado Lage]

Replace manual bit manipulations with FIELD_GET(), FIELD_PREP() and
FIELD_MODIFY() calls. The resulting code is more readable and
maintainable, and 6 macros previously defined in the header are not
needed anymore.

Signed-off-by: Marcelo Machado Lage <marcelomlage@usp.br>
Co-developed-by: Vinicius Lira <vinilira@usp.br>
Signed-off-by: Vinicius Lira <vinilira@usp.br>
---

v1 -> v2:
 - Remove 3 further macros still present in v1 and replace their usages by inline calls to FIELD_GET(), following a request by Jonathan Comeron
v2 -> v3:
 - Apply stylistic changes to the definition block inside mcp3422_read_raw(), following a request by Andy Shevchenko
v3 -> v4:
 - Fix operator style in line breaks inside mcp3422_probe(), following a request by Andy Shevchenko

v1: https://lore.kernel.org/linux-iio/20260417005041.484742-1-marcelomlage@usp.br/
v2: https://lore.kernel.org/linux-iio/20260417165747.507487-1-marcelomlage@usp.br/
v3: https://lore.kernel.org/linux-iio/20260420191455.529923-1-marcelomlage@usp.br/

 drivers/iio/adc/mcp3422.c | 54 +++++++++++++++------------------------
 1 file changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
index 0cd0e7d39e39..0cbd7a874798 100644
--- a/drivers/iio/adc/mcp3422.c
+++ b/drivers/iio/adc/mcp3422.c
@@ -13,6 +13,7 @@
  * voltage unit is nV.
  */
 
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
@@ -39,14 +40,6 @@
 #define MCP3422_PGA_8	3
 #define MCP3422_CONT_SAMPLING	BIT(4)
 
-#define MCP3422_CHANNEL(config)	(((config) & MCP3422_CHANNEL_MASK) >> 5)
-#define MCP3422_PGA(config)	((config) & MCP3422_PGA_MASK)
-#define MCP3422_SAMPLE_RATE(config)	(((config) & MCP3422_SRATE_MASK) >> 2)
-
-#define MCP3422_CHANNEL_VALUE(value) (((value) << 5) & MCP3422_CHANNEL_MASK)
-#define MCP3422_PGA_VALUE(value) ((value) & MCP3422_PGA_MASK)
-#define MCP3422_SAMPLE_RATE_VALUE(value) ((value << 2) & MCP3422_SRATE_MASK)
-
 #define MCP3422_CHAN(_index) \
 	{ \
 		.type = IIO_VOLTAGE, \
@@ -109,7 +102,7 @@ static int mcp3422_update_config(struct mcp3422 *adc, u8 newconfig)
 static int mcp3422_read(struct mcp3422 *adc, int *value, u8 *config)
 {
 	int ret = 0;
-	u8 sample_rate = MCP3422_SAMPLE_RATE(adc->config);
+	u8 sample_rate = FIELD_GET(MCP3422_SRATE_MASK, adc->config);
 	u8 buf[4] = {0, 0, 0, 0};
 	u32 temp;
 
@@ -137,18 +130,18 @@ static int mcp3422_read_channel(struct mcp3422 *adc,
 
 	mutex_lock(&adc->lock);
 
-	if (req_channel != MCP3422_CHANNEL(adc->config)) {
+	if (req_channel != FIELD_GET(MCP3422_CHANNEL_MASK, adc->config)) {
 		config = adc->config;
-		config &= ~MCP3422_CHANNEL_MASK;
-		config |= MCP3422_CHANNEL_VALUE(req_channel);
-		config &= ~MCP3422_PGA_MASK;
-		config |= MCP3422_PGA_VALUE(adc->pga[req_channel]);
+
+		FIELD_MODIFY(MCP3422_CHANNEL_MASK, &config, req_channel);
+		FIELD_MODIFY(MCP3422_PGA_MASK, &config, adc->pga[req_channel]);
+
 		ret = mcp3422_update_config(adc, config);
 		if (ret < 0) {
 			mutex_unlock(&adc->lock);
 			return ret;
 		}
-		msleep(mcp3422_read_times[MCP3422_SAMPLE_RATE(adc->config)]);
+		msleep(mcp3422_read_times[FIELD_GET(MCP3422_SRATE_MASK, adc->config)]);
 	}
 
 	ret = mcp3422_read(adc, value, &config);
@@ -164,9 +157,8 @@ static int mcp3422_read_raw(struct iio_dev *iio,
 {
 	struct mcp3422 *adc = iio_priv(iio);
 	int err;
-
-	u8 sample_rate = MCP3422_SAMPLE_RATE(adc->config);
-	u8 pga		 = MCP3422_PGA(adc->config);
+	u8 sample_rate = FIELD_GET(MCP3422_SRATE_MASK, adc->config);
+	u8 pga = FIELD_GET(MCP3422_PGA_MASK, adc->config);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -182,7 +174,7 @@ static int mcp3422_read_raw(struct iio_dev *iio,
 		return IIO_VAL_INT_PLUS_NANO;
 
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		*val1 = mcp3422_sample_rates[MCP3422_SAMPLE_RATE(adc->config)];
+		*val1 = mcp3422_sample_rates[FIELD_GET(MCP3422_SRATE_MASK, adc->config)];
 		return IIO_VAL_INT;
 
 	default:
@@ -200,7 +192,7 @@ static int mcp3422_write_raw(struct iio_dev *iio,
 	u8 temp;
 	u8 config = adc->config;
 	u8 req_channel = channel->channel;
-	u8 sample_rate = MCP3422_SAMPLE_RATE(config);
+	u8 sample_rate = FIELD_GET(MCP3422_SRATE_MASK, config);
 	u8 i;
 
 	switch (mask) {
@@ -212,10 +204,8 @@ static int mcp3422_write_raw(struct iio_dev *iio,
 			if (val2 == mcp3422_scales[sample_rate][i]) {
 				adc->pga[req_channel] = i;
 
-				config &= ~MCP3422_CHANNEL_MASK;
-				config |= MCP3422_CHANNEL_VALUE(req_channel);
-				config &= ~MCP3422_PGA_MASK;
-				config |= MCP3422_PGA_VALUE(adc->pga[req_channel]);
+				FIELD_MODIFY(MCP3422_CHANNEL_MASK, &config, req_channel);
+				FIELD_MODIFY(MCP3422_PGA_MASK, &config, adc->pga[req_channel]);
 
 				return mcp3422_update_config(adc, config);
 			}
@@ -242,10 +232,8 @@ static int mcp3422_write_raw(struct iio_dev *iio,
 			return -EINVAL;
 		}
 
-		config &= ~MCP3422_CHANNEL_MASK;
-		config |= MCP3422_CHANNEL_VALUE(req_channel);
-		config &= ~MCP3422_SRATE_MASK;
-		config |= MCP3422_SAMPLE_RATE_VALUE(temp);
+		FIELD_MODIFY(MCP3422_CHANNEL_MASK, &config, req_channel);
+		FIELD_MODIFY(MCP3422_SRATE_MASK, &config, temp);
 
 		return mcp3422_update_config(adc, config);
 
@@ -284,7 +272,7 @@ static ssize_t mcp3422_show_scales(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct mcp3422 *adc = iio_priv(dev_to_iio_dev(dev));
-	u8 sample_rate = MCP3422_SAMPLE_RATE(adc->config);
+	u8 sample_rate = FIELD_GET(MCP3422_SRATE_MASK, adc->config);
 
 	return sprintf(buf, "0.%09u 0.%09u 0.%09u 0.%09u\n",
 		mcp3422_scales[sample_rate][0],
@@ -377,10 +365,10 @@ static int mcp3422_probe(struct i2c_client *client)
 	}
 
 	/* meaningful default configuration */
-	config = (MCP3422_CONT_SAMPLING
-		| MCP3422_CHANNEL_VALUE(0)
-		| MCP3422_PGA_VALUE(MCP3422_PGA_1)
-		| MCP3422_SAMPLE_RATE_VALUE(MCP3422_SRATE_240));
+	config = MCP3422_CONT_SAMPLING |
+		 FIELD_PREP(MCP3422_CHANNEL_MASK, 0) |
+		 FIELD_PREP(MCP3422_PGA_MASK, MCP3422_PGA_1) |
+		 FIELD_PREP(MCP3422_SRATE_MASK, MCP3422_SRATE_240);
 	err = mcp3422_update_config(adc, config);
 	if (err < 0)
 		return err;
-- 
2.34.1

Re: [PATCH v4 0/2] iio: adc: mcp3422: apply bit manipulation macros

[Andy Shevchenko]

On Wed, Apr 29, 2026 at 07:43:59PM -0300, Marcelo Machado Lage wrote:
> This patch set rewrites several open-coded bit manipulation parts of
> the MCP3421/2/3/4/5/6/7/8 driver code using bits.h and bitfield.h
> macros.

Both LGTM now
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/2] iio: adc: mcp3422: apply bit manipulation macros

[Jonathan Cameron]

On Thu, 30 Apr 2026 09:11:31 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Apr 29, 2026 at 07:43:59PM -0300, Marcelo Machado Lage wrote:
> > This patch set rewrites several open-coded bit manipulation parts of
> > the MCP3421/2/3/4/5/6/7/8 driver code using bits.h and bitfield.h
> > macros.  
> 
> Both LGTM now
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
Nice.  Applied to the testing branch of iio.git.

FWIW Sashiko had a field day on 'whilst I was here' things in this driver:
https://sashiko.dev/#/patchset/20260429224401.11818-1-marcelomlage%40usp.br

Note that the DMA one is wrong. Some of the others might or might not be correct.
Maybe some stuff to look at if you want to do more work on this driver, though
you'll probably need to work out some way of testing fixes for the more complex
ones.

Thanks,

Jonathan

Re: [PATCH v4 0/2] iio: adc: mcp3422: apply bit manipulation macros

[Marcelo Machado Lage]

On Tue, 5 May 2026 at 13:22, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 30 Apr 2026 09:11:31 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>
> > On Wed, Apr 29, 2026 at 07:43:59PM -0300, Marcelo Machado Lage wrote:
> > > This patch set rewrites several open-coded bit manipulation parts of
> > > the MCP3421/2/3/4/5/6/7/8 driver code using bits.h and bitfield.h
> > > macros.
> >
> > Both LGTM now
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> >
> Nice.  Applied to the testing branch of iio.git.

Thanks for the reviews! We're happy to see our patch applied!

> FWIW Sashiko had a field day on 'whilst I was here' things in this driver:
> https://sashiko.dev/#/patchset/20260429224401.11818-1-marcelomlage%40usp.br
>
> Note that the DMA one is wrong. Some of the others might or might not be correct.
> Maybe some stuff to look at if you want to do more work on this driver, though
> you'll probably need to work out some way of testing fixes for the more complex
> ones.

Thanks for pointing out Sashiko's thoughts on this patch.
Unfortunately, we do not have access to the device for testing complex fixes,
but we'll definitely study its suggestions for next contributions.

Best regards,
Marcelo