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

["Siratul Islam" <siratul.islam@linux.dev>]

July 4, 2026 at 11:05 PM, "Andy Shevchenko" <andriy.shevchenko@intel.com mailto:andriy.shevchenko@intel.com?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?
> > 
> > > > + if (tmp < 0)
> >  > > + return tmp;
> >  > > +
> >  > > + *range = FIELD_GET(MAX40080_RANGE_MSK, tmp);
> >  > > +
> >  > > + return 0;
> >  > > +}
> > 
> -- 
> With Best Regards,
> Andy Shevchenko
>