Re: [PATCH] iio: adc: mcp320x: refactor driver to use bitfield API

Jonathan Cameron
2026-04-21T15:51:43Z

On Mon, 20 Apr 2026 18:49:18 -0300
Gustavo Pagnotta Faria <gustavo.pagnotta@ime.usp.br> wrote:

> Update the mcp320x driver to use the standard Linux bitfield
> API (<linux/bitfield.h>) instead of manual bitwise shifts and masks.
>
> This replaces the hardcoded shift operations in the TX data
> preparation (mcp320x_channel_to_tx_data) with FIELD_PREP()
> and replaces the manual masking in the RX data extraction
> (mcp320x_adc_conversion) with FIELD_GET(). Explicit masks
> using GENMASK() and BIT() were also introduced for both transmit
> configurations and receive extractions.
>
> Signed-off-by: Gustavo Pagnotta Faria <gustavo.pagnotta@ime.usp.br>
> Co-developed-by: Eduardo Augusto <eduardoaugustoabc@ime.usp.br>
> Signed-off-by: Eduardo Augusto <eduardoaugustoabc@ime.usp.br>
> Co-developed-by: Christian Barry <christian.barry@ime.usp.br>
> Signed-off-by: Christian Barry <christian.barry@ime.usp.br>

> @@ -140,26 +163,27 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
> if (ret < 0)
> return ret;
>
> + u16 raw16 = be16_to_cpup((__be16 *)adc->rx_buf);

Sashiko pointed out that rx_buf is unaligned so you need an unaligned accessor here.
https://sashiko.dev/#/patchset/20260420215021.14112-1-gustavo.pagnotta%40ime.usp.br
> +
> switch (device_index) {
> case mcp3001:

> @@ -170,17 +194,17 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
> if (!(adc->spi->mode & SPI_CPOL))
> raw <<= 1; /* strip Data Ready bit in SPI mode 0,0 */
>
> + raw = FIELD_GET(MCP355X_DATA_MASK, raw);
> /*
> * If the input is within -vref and vref, bit 21 is the sign.
> * Up to 12% overrange or underrange are allowed, in which case
> * bit 23 is the sign and bit 0 to 21 is the value.
> */
> - raw >>= 8;
> - if (raw & BIT(22) && raw & BIT(23))
> + if ((raw & MCP355X_OVR) && (raw & MCP355X_SGN))
> return -EIO; /* cannot have overrange AND underrange */
> - else if (raw & BIT(22))
> - raw &= ~BIT(22); /* overrange */
> - else if (raw & BIT(23) || raw & BIT(21))
> + else if (raw & MCP355X_OVR)
> + raw &= MCP355X_OVR; /* overrange */
&= ~MCP355X_OVR; ?

Sashiko wins again.

J
> + else if ((raw & MCP355X_SGN) || (raw & BIT(21)))
> raw |= GENMASK(31, 22); /* underrange or negative */
>
> *val = (s32)raw;