Re: [PATCH v3 2/2] iio: adc: mcp3422: write bit operations with bitfield.h macros

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 20 Apr 2026 23:24:51 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Apr 20, 2026 at 04:14:55PM -0300, Marcelo Machado Lage wrote:
> > 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.  
> 
> ...
> 
> >  	/* 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));  
> 
> While at it, please fix the style by making the operator to be the last on
> the respective lines.
> 
> 	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));
> 
> But, to me sounds better if you just resplit it it to
> 
> 	config = MCP3422_CONT_SAMPLING;
> 	FIELD_MODIFY(...);
> 	FIELD_MODIFY(...);
> 	FIELD_MODIFY(...);
> 
To me FIELD_MODIFY() kind of hints it might already be set but in this
case the whole value is being written.  So maybe just reorganize
as per Andy's first suggestion above.