Re: [PATCH v3 2/2] staging: iio: ad7816: avoid DMA from stack in spi_read

[nasser]

Hi  Jonathan


Thanks for the review.

> The ad7816_spi_read() path performs a sequence of SPI transfers and GPIO
> state changes that must not be interleaved with another read operation.
>
> Split the two ad7816 out as a series on their own.

Sure, I will split these two patches into a separate series in v4.

> +     mutex_lock(&chip->lock);
>
> Use guard(mutex)(&chip->lock);
> and adjust the rest as appropriate, including adding cleanup.h if it
> isn't already there.

Thanks for pointing this out. I will update the code to use guard(mutex)
and devm_mutex_init() to handle the cleanup automatically.

> Move the buffer into struct ad7816_chip_info to provide storage with an
> appropriate lifetime for DMA, align it with
>
> Why is lifetime relevant here?  The on stack data was fine lifetime wise.

Right, I'll update the commit message to focus on cache coherency .

> Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
>
> No blank lines in commit block. Sometimes I just fix these up when picking
> patches up, but sometimes I get grumpy and bounce them back to submitter
> to fix up in a new version.

Noted. I will make sure to remove the blank line between the tags.

I will send v4 shortly.

Thanks,
Abdelnasser