Re: [PATCH 4/4] iio: dac: ad5686: update device list and file header comments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Kconfig help text introduces typos and omits supported devices.

Pre-existing issues:
- [High] The I2C read operation uses overlapping memory in the same cacheline for concurrent DMA_TO_DEVICE and DMA_FROM_DEVICE mappings.
--

--- Patch [4]: [PATCH 4/4] iio: dac: ad5686: update device list and file header comments ---
commit 815b7c6dbcb143b832135213c1aa2cb0c1357c58
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

iio: dac: ad5686: update device list and file header comments

Update device list entries in Kconfig and remove device description from
file headers. One can rely on the ID table(s) to check for supported
devices. Adjust comment header accordingly and update Copyright notice in
comment file headers with the current year.

> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index d6d560c09e250..ba2065241af31 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -251,9 +251,18 @@ config AD5686_SPI
>  	depends on SPI
>  	select AD5686
>  	help
> -	  Say yes here to build support for Analog Devices AD5672R, AD5674R,
> -	  AD5676, AD5676R, AD5679R, AD5684, AD5684R, AD5684R, AD5685R, AD5686,
> -	  AD5686R Voltage Output Digital to Analog Converter.
> +	  Say yes here to build support for Analog Devices Voltage Output
> +	  Digital to Analog Converters:
> +	  - Single-channel:
> +	    AD5310R, AD5681R, AD5682R, AD5683R, AD5683R

[Severity: Low]
Is AD5683R intentionally duplicated here? It looks like AD5683 might have
been intended instead, since it is supported by the driver but missing from
this list.

[ ... ]

> @@ -263,10 +272,18 @@ config AD5696_I2C
>  	depends on I2C
>  	select AD5686
>  	help
> -	  Say yes here to build support for Analog Devices AD5311R, AD5337,
> -	  AD5338R, AD5671R, AD5673R, AD5675R, AD5677R, AD5691R, AD5692R, AD5693,
> -	  AD5693R, AD5694, AD5694R, AD5695R, AD5696, and AD5696R Digital to
> -	  Analog converters.
> +	  Say yes here to build support for Analog Devices Voltage Output
> +	  Digital to Analog Converters:
> +	  - Single-channel:
> +	    AD5311R, AD5691R, AD5692R, AD5693, AD5693R
> +	  - Dual-channel:
> +	    AD5338R, AD5697R

[Severity: Low]
Does this list omit AD5337R? It appears to be present in the driver's ID
table but is missing from this updated text.

[ ... ]

> diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
> index 7254bb976388c..19605274e7877 100644
> --- a/drivers/iio/dac/ad5696-i2c.c
> +++ b/drivers/iio/dac/ad5696-i2c.c
> @@ -1,10 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * AD5338R, AD5671R, AD5673R, AD5675R, AD5677R, AD5691R, AD5692R, AD5693,
> - * AD5693R, AD5694, AD5694R, AD5695R, AD5696, AD5696R
> - * Digital to analog converters driver
> + * I2C driver for AD5696 and similar Digital to Analog Converters
>   *
> - * Copyright 2018 Analog Devices Inc.
> + * Copyright 2018-2026 Analog Devices Inc.
>   */

[Severity: High]
This is a pre-existing issue, but can the I2C read operation cause data
corruption on non-coherent architectures?

Looking at ad5686_i2c_read() in this file, the I2C message array provides
overlapping buffers within the exact same union:

	struct i2c_msg msg[2] = {
		{
			.addr = i2c->addr,
			.flags = i2c->flags,
			.len = 3,
			.buf = &st->data[0].d8[1],
		},
		{
			.addr = i2c->addr,
			.flags = i2c->flags | I2C_M_RD,
			.len = 2,
			.buf = (char *)&st->data[0].d16,
		},
	};

When i2c_transfer() is backed by a DMA-enabled I2C controller, both messages
are concurrently mapped for DMA (one for DMA_TO_DEVICE, one for
DMA_FROM_DEVICE). Mapping overlapping memory in the same cacheline with
conflicting DMA directions violates coherency rules, which might lead to the
RX buffer invalidation discarding the TX buffer's data.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719-ad5686-extend-dev-support-v1-0-841ec75cf5d8@analog.com?part=4