* [PATCH] iio: adc: mcp3422: write bit operations with bitfield macros
@ 2026-04-17 0:50 Marcelo Machado Lage
2026-04-17 8:29 ` Andy Shevchenko
2026-04-17 13:26 ` Jonathan Cameron
0 siblings, 2 replies; 3+ messages in thread
From: Marcelo Machado Lage @ 2026-04-17 0:50 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy
Cc: Marcelo Machado Lage, Vinicius Lira, linux-iio
Replace manual bit manipulations with GENMASK(), FIELD_GET(),
FIELD_PREP() and FIELD_MODIFY() calls. The resulting code is more
readable and maintainable, and some 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>
---
drivers/iio/adc/mcp3422.c | 43 ++++++++++++++++-----------------------
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
index 50834fdcf738..d93036bf9a85 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/err.h>
#include <linux/i2c.h>
#include <linux/module.h>
@@ -25,9 +26,9 @@
#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_PGA_MASK GENMASK(1, 0)
+#define MCP3422_SRATE_MASK GENMASK(3, 2)
#define MCP3422_SRATE_240 0x0
#define MCP3422_SRATE_60 0x1
#define MCP3422_SRATE_15 0x2
@@ -38,13 +39,9 @@
#define MCP3422_PGA_8 3
#define MCP3422_CONT_SAMPLING 0x10
-#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_CHANNEL(config) FIELD_GET(MCP3422_CHANNEL_MASK, config)
+#define MCP3422_PGA(config) FIELD_GET(MCP3422_PGA_MASK, config)
+#define MCP3422_SAMPLE_RATE(config) FIELD_GET(MCP3422_SRATE_MASK, config)
#define MCP3422_CHAN(_index) \
{ \
@@ -138,10 +135,10 @@ static int mcp3422_read_channel(struct mcp3422 *adc,
if (req_channel != MCP3422_CHANNEL(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);
@@ -211,10 +208,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);
}
@@ -241,10 +236,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);
@@ -377,9 +370,9 @@ 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));
+ | 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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] iio: adc: mcp3422: write bit operations with bitfield macros
2026-04-17 0:50 [PATCH] iio: adc: mcp3422: write bit operations with bitfield macros Marcelo Machado Lage
@ 2026-04-17 8:29 ` Andy Shevchenko
2026-04-17 13:26 ` Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-04-17 8:29 UTC (permalink / raw)
To: Marcelo Machado Lage
Cc: jic23, dlechner, nuno.sa, andy, Vinicius Lira, linux-iio
On Thu, Apr 16, 2026 at 09:50:40PM -0300, Marcelo Machado Lage wrote:
> Replace manual bit manipulations with GENMASK(), FIELD_GET(),
> FIELD_PREP() and FIELD_MODIFY() calls. The resulting code is more
> readable and maintainable, and some macros previously defined in the
> header are not needed anymore.
...
> -#define MCP3422_CHANNEL_MASK 0x60
> -#define MCP3422_PGA_MASK 0x03
> -#define MCP3422_SRATE_MASK 0x0C
> +#define MCP3422_CHANNEL_MASK GENMASK(6, 5)
> +#define MCP3422_PGA_MASK GENMASK(1, 0)
> +#define MCP3422_SRATE_MASK GENMASK(3, 2)
Please, split this change and also add now missing bits.h
(bitfield.h doesn't cover it).
While doing that, convert all eligible constants.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: adc: mcp3422: write bit operations with bitfield macros
2026-04-17 0:50 [PATCH] iio: adc: mcp3422: write bit operations with bitfield macros Marcelo Machado Lage
2026-04-17 8:29 ` Andy Shevchenko
@ 2026-04-17 13:26 ` Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-04-17 13:26 UTC (permalink / raw)
To: Marcelo Machado Lage
Cc: jic23, dlechner, nuno.sa, andy, Vinicius Lira, linux-iio
On Thu, 16 Apr 2026 21:50:40 -0300
Marcelo Machado Lage <marcelomlage@usp.br> wrote:
> Replace manual bit manipulations with GENMASK(), FIELD_GET(),
> FIELD_PREP() and FIELD_MODIFY() calls. The resulting code is more
> readable and maintainable, and some 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>
> ---
> drivers/iio/adc/mcp3422.c | 43 ++++++++++++++++-----------------------
> 1 file changed, 18 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
> index 50834fdcf738..d93036bf9a85 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/err.h>
> #include <linux/i2c.h>
> #include <linux/module.h>
> @@ -25,9 +26,9 @@
> #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_PGA_MASK GENMASK(1, 0)
> +#define MCP3422_SRATE_MASK GENMASK(3, 2)
> #define MCP3422_SRATE_240 0x0
> #define MCP3422_SRATE_60 0x1
> #define MCP3422_SRATE_15 0x2
> @@ -38,13 +39,9 @@
> #define MCP3422_PGA_8 3
> #define MCP3422_CONT_SAMPLING 0x10
>
> -#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_CHANNEL(config) FIELD_GET(MCP3422_CHANNEL_MASK, config)
> +#define MCP3422_PGA(config) FIELD_GET(MCP3422_PGA_MASK, config)
> +#define MCP3422_SAMPLE_RATE(config) FIELD_GET(MCP3422_SRATE_MASK, config)
Drop these 3 macros and use FIELD_GET() inline instead just as you've done for the prep side.
>
> #define MCP3422_CHAN(_index) \
> { \
> @@ -138,10 +135,10 @@ static int mcp3422_read_channel(struct mcp3422 *adc,
>
> if (req_channel != MCP3422_CHANNEL(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);
> @@ -211,10 +208,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);
> }
> @@ -241,10 +236,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);
>
> @@ -377,9 +370,9 @@ 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));
> + | 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;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-17 13:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 0:50 [PATCH] iio: adc: mcp3422: write bit operations with bitfield macros Marcelo Machado Lage
2026-04-17 8:29 ` Andy Shevchenko
2026-04-17 13:26 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox