Re: [PATCH v7 2/7] iio: dac: ad5686: refactor command/data macros

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] DMA API Cacheline Sharing Violation in SPI Read
--

--- Patch [2]: [PATCH v7 2/7] iio: dac: ad5686: refactor command/data macros ---
commit 7ffa72bdbbf5952d81c8c2ac98d1aa1023cddf2f
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
iio: dac: ad5686: refactor command/data macros

This commit refactors bit shifting macros in the AD5686 driver to use
FIELD_PREP(). It hardens the code to prepare for upcoming triggered buffer
support by ensuring bit masking is applied correctly.

Link: https://lore.kernel.org/all/20260628143026.EC6CA1F000E9@smtp.kernel.org/#t

> diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
> index 6b6ef1d7071fe..d3b64b4361d51 100644
> --- a/drivers/iio/dac/ad5686-spi.c
> +++ b/drivers/iio/dac/ad5686-spi.c
[ ... ]
> @@ -81,9 +82,9 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
>  		return -EINVAL;
>  	}
>  
> -	st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> -				      AD5686_ADDR(addr));
> -	st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
> +	st->data[0].d32 = cpu_to_be32(FIELD_PREP(AD5686_CMD_MSK, cmd) |
> +				      FIELD_PREP(AD5686_ADDR_MSK, addr));
> +	st->data[1].d32 = cpu_to_be32(FIELD_PREP(AD5686_CMD_MSK, AD5686_CMD_NOOP));
>  
>  	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
>  	if (ret < 0)

[Severity: High]
This isn't a bug introduced by this patch, but is it possible this setup
causes a DMA API cacheline sharing violation during SPI reads?

In ad5686_spi_read(), the SPI transfers use adjacent elements from the
st->data array:

    struct spi_transfer t[] = {
        {
            .tx_buf = &st->data[0].d8[1],
            .len = 3,
            .cs_change = 1,
        }, {
            .tx_buf = &st->data[1].d8[1],
            .rx_buf = &st->data[2].d8[1],
            .len = 3,
        },
    };

Since st->data elements are 4-byte unions, they reside within the same
CPU cacheline. When spi_sync_transfer() maps st->data[0] and st->data[1]
as DMA_TO_DEVICE and st->data[2] as DMA_FROM_DEVICE simultaneously, does
this create overlapping mappings that violate the DMA API?

This could trigger CONFIG_DMA_API_DEBUG warnings for overlapping
mappings, and on non-coherent architectures, could there be a risk of
memory corruption when DMA_FROM_DEVICE cache invalidations interact
with active transfers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ad5686-new-features-v7-0-1bcc8c280e4d@analog.com?part=2