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

[Jonathan Cameron <jic23@kernel.org>]

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;