Re: [PATCH v2 2/3] iio: adc: ltc2497: add LTC2499 internal temperature channel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver attempts to use a DMA-safe buffer to bypass I2C bounce buffering, but calls the ordinary `i2c_master_send()` API instead of `i2c_master_send_dmasafe()`.
--

--- Patch [2]: [PATCH v2 2/3] iio: adc: ltc2497: add LTC2499 internal temperature channel ---
commit cea285d89524ad50d9ef052e9f3bd1971904c7b9
Author: Andrei Stancovici <andrei.stancovici@analog.com>

iio: adc: ltc2497: add LTC2499 internal temperature channel

This commit adds support for the LTC2499 internal temperature channel by
providing raw, scale, and offset values. It implements two-byte I2C commands
using a DMA-safe buffer to switch between the internal PTAT sensor and external
voltage inputs.

> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c

[ ... ]

> @@ -85,6 +85,40 @@ static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata,
>  			return 0;
>  	}
>  
> +	/*
> +	 * Parts with the internal PTAT sensor (LTC2499) latch their converter
> +	 * configuration via a second command byte and only re-evaluate it when
> +	 * that byte has EN2 set; a single byte, or a second byte with EN2 = 0,
> +	 * means "keep previous". A one-byte channel select therefore cannot pull
> +	 * the device back out of temperature mode, so a voltage read after a
> +	 * temperature read would keep returning the PTAT result. Always drive the
> +	 * second byte with EN2 set on these parts: IM = 1 for a temperature read,
> +	 * EN2 alone (IM = 0) to (re)select an external input. FA = FB = 0 keeps
> +	 * the power-on simultaneous 50/60Hz rejection, whose worst-case
> +	 * conversion time the driver's wait already covers.
> +	 *
> +	 * The two bytes are assembled in the DMA-safe st->data buffer rather than
> +	 * on the stack, so the pointer handed to i2c_master_send() stays valid on
> +	 * adapters that DMA the transfer (e.g. with CONFIG_VMAP_STACK).
> +	 */
> +	if (ddata->chip_info->has_temp) {
> +		if (address == LTC2497_TEMP_ADDR) {
> +			st->data.d8[0] = LTC2497_ENABLE | LTC2497_CONFIG_DEFAULT;
> +			st->data.d8[1] = LTC2499_EN2 | LTC2499_IM;
> +		} else {
> +			st->data.d8[0] = LTC2497_ENABLE | address;
> +			st->data.d8[1] = LTC2499_EN2;
> +		}
> +
> +		ret = i2c_master_send(st->client, (char *)st->data.d8, 2);

[Severity: Medium]
The comment above states the two bytes are assembled in a DMA-safe buffer to
ensure the pointer stays valid on adapters that DMA the transfer. Since
ordinary i2c_master_send() does not set the I2C_M_DMA_SAFE flag, won't the
I2C core automatically allocate a temporary bounce buffer and copy the data
anyway?

Would it be better to use i2c_master_send_dmasafe() here so the I2C core knows
it can safely use this buffer directly without bouncing?

> +		if (ret < 0) {
> +			dev_err(&st->client->dev, "i2c transfer failed: %pe\n",
> +				ERR_PTR(ret));
> +			return ret;
> +		}
> +		return 0;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813160139.70000-1-andrei.stancovici@analog.com?part=2