Re: [PATCH v2 1/7] iio: temperature: ltc2983: Fix macro parenthesization and rename

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Undocumented API change: addition of the `base` parameter to the `LTC2983_RESULT_ADDR` macro.
- [Low] Compilation failure in `ltc2983_chan_read()` due to undeclared identifier `base_reg`.
--

commit 24bf0bfe00d6f9aa08599e0c812d5f43a2500269
Author: Liviu Stan <liviu.stan@analog.com>

iio: temperature: ltc2983: Fix macro parenthesization and rename

This commit adds parentheses around the chan parameter in macro definitions
to prevent expansion issues and renames the macros to better reflect the
datasheet names.

>     Wrap the 'chan' parameter in LTC2983_CHAN_START_ADDR() and
>     LTC2983_CHAN_RES_ADDR() with parentheses to prevent potential
>     macro argument expansion issues. Also rename LTC2983_CHAN_START_ADDR
>     to LTC2983_CHAN_ASSIGN_ADDR and LTC2983_CHAN_RES_ADDR to
>     LTC2983_RESULT_ADDR, to better reflect the datasheet names and avoid
>     them being confused as related.

Could the commit message be updated to mention the addition of the base
parameter to LTC2983_RESULT_ADDR?

The commit message describes renaming the macro and adding parentheses, but
fails to mention that the macro was also modified to take a new base
parameter. This undocumented change obscures the intent behind the
refactoring.

> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index 38e6f8dfd3b80..a292274e6ff07 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -56,10 +56,10 @@
>  #define LTC2983_EEPROM_WRITE_TIME_MS		2600
>  #define LTC2983_EEPROM_READ_TIME_MS		20
>  
> -#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_ASSIGN_ADDR(chan) \
> +			((((chan) - 1) * 4) + LTC2983_CHAN_ASSIGN_START_REG)
> +#define LTC2983_RESULT_ADDR(chan, base) \
> +			((((chan) - 1) * 4) + (base))
>  #define LTC2983_THERMOCOUPLE_DIFF_MASK		BIT(3)
>  #define LTC2983_THERMOCOUPLE_SGL(x) \
>  				FIELD_PREP(LTC2983_THERMOCOUPLE_DIFF_MASK, x)

[ ... ]

> @@ -1197,7 +1197,7 @@ static int ltc2983_chan_read(struct ltc2983_data *st,
>  	}
>  
>  	/* read the converted data */
> -	ret = regmap_bulk_read(st->regmap, LTC2983_CHAN_RES_ADDR(sensor->chan),
> +	ret = regmap_bulk_read(st->regmap, LTC2983_RESULT_ADDR(sensor->chan, base_reg),
>  			       &st->temp, sizeof(st->temp));
>  	if (ret)
>  		return ret;

Does this cause a compilation failure because base_reg is undeclared in
ltc2983_chan_read()?

It appears the fix is introduced later in the series in commit 7aff450fb9c7b
("iio: temperature: ltc2983: Add support for ADT7604"), which adds base_reg as
a function parameter. However, each patch in a series must compile
independently.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514144712.64374-1-liviu.stan@analog.com?part=1