Re: [PATCH v1 2/2] iio: adc: add MAX40080 current-sense amplifier driver

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sat, Jul 04, 2026 at 10:32:32PM +0600, Siratul Islam wrote:
> On 26/07/04 03:05PM, Andy Shevchenko wrote:
> > On Sat, Jul 04, 2026 at 01:42:39AM +0600, Siratul Islam wrote:
> > > On Fri, 2026-07-03 at 13:29 +0300, Stefan Popa wrote:

...

> > > This can fit in 1 line.
> > > static int max40080_update_bits(struct max40080_state *st, u8 reg, u16 mask, u16 val)
> > 
> > Here...
> Keeping this function single line results in 86 cols, which is just a
> little more than 80. So I thought splitting here was avoidable. But if
> you think it's an acceptable split then so be it.

Split is a safe choice. No split is a slippery slope depending on some factors.

...

> > > > +static int max40080_get_range(struct max40080_state *st, unsigned int *range)
> > > > +{
> > > > +	int tmp;
> > > > +
> > > > +	tmp = i2c_smbus_read_word_data(st->client, MAX40080_REG_CFG);
> > > I think tmp can be initialized, since it is only assigned once.
> > 
> > I don't get this comment. You mean switching to ret?
> > 
> I meant tmp is assigned only once so the indirection here, i.e.
> declaring and assigning in two steps doesn't buy us anything. Instead,
> it could be initialized like "int tmp = i2c_smbus_read_word_data()."

Ah, definitely no to this suggestion. It makes code harder to maintain
and the pattern you proposed is actually discouraged. You can search in
mail archive and find like ~1-2 year old message from me with the detailed
explanation why.

> > > > +	if (tmp < 0)
> > > > +		return tmp;
> > > > +
> > > > +	*range = FIELD_GET(MAX40080_RANGE_MSK, tmp);
> > > > +
> > > > +	return 0;
> > > > +}

-- 
With Best Regards,
Andy Shevchenko