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

nasser <[email protected]>
Newsgroups org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel
Message-ID <CANpqX3aC0a6Daps-Kgc2gXmjKdhSWAYW0J_T1BVuMGNru=ZVgQ@mail.gmail.com>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.