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

[Andy Shevchenko]

On Sat, Jul 4, 2026 at 8:30 PM Siratul Islam <siratul.islam@xxxxxxxxx> wrote:
> July 4, 2026 at 11:05 PM, "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxx mailto:andriy.shevchenko@xxxxxxxxx?to=%22Andy%20Shevchenko%22%20%3Candriy.shevchenko%40intel.com%3E > wrote:
> > 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:

...

> > > > > +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.
> >
> Thanks! I'll look into it. Learning a lot of stuff. But I'm trying to understand where to draw the line though.
> Like "struct xxx* data = iio_priv(indio_dev);" and "s64 ts = iio_get_time_ns(indio_dev);" are pretty common.
> Do these calls make a special case for it?

When the assigned value is coupled with a (validation) check,
definitely do not assign and define simultaneously. When it's used as
a counter, it's better to assign it closer to the user (meaning the
counters that can't be folded into a for-loop). This is the rough rule
of thumb. Plus use common sense.

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

-- 
With Best Regards,
Andy Shevchenko