Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

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

On Tue, May 12, 2026 at 07:12:57AM +0000, Stan, Liviu wrote:
> On Mon, Apr 27, 2026 Andy Shevchenko wrote:

...

> > >  #define LTC2983_CHAN_START_ADDR(chan) \
> > >  			(((chan - 1) * 4) +
> > LTC2983_CHAN_ASSIGN_START_REG)
> > > -#define LTC2983_CHAN_RES_ADDR(chan) \
> > > -			(((chan - 1) * 4) + LTC2983_TEMP_RES_START_REG)
> > > +#define LTC2983_CHAN_RES_ADDR(chan, base) \
> > > +			((((chan) - 1) * 4) + (base))
> > 
> > For the sake of consistency I would see (base) also to be in the _START_ADDR()
> > macro.
> 
> I said I would change this in v2, but on second look, I think it would be better
> to keep LTC2983_CHAN_START_ADDR without a (base) parameter. The base
> parameter in LTC2983_CHAN_RES_ADDR exists because the ADT7604 adds a
> second result register bank, so the base genuinely varies. For channel assignment
> there is only one bank, so adding a base parameter would make the macro look
> configurable when it isn't and force callers to always pass
> LTC2983_CHAN_ASSIGN_START_REG.

Do the names of the definitions _START_ADDR and _RES_ADDR come directly from
the datasheet?  Also, given the above explanation I would see rather (bank)
than (base) there. With this it makes less attractive for a change that I
suggested earlier.

> Happy to change if you still prefer consistency.

With current names they sound like they are semantically tighten, when in
practice it's not so. There are options:
- move to (bank) and leave as currently done
- synchronise them and use (base) in both cases
- rename one or the other to be different by the name, so less confusion is
added

Your choice needs to be based on the datasheet explanation for these registers.

-- 
With Best Regards,
Andy Shevchenko