Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <54bfff70-938f-16e1-198d-47ed9ba95db4@gmail.com>
Date:   Thu, 23 Jun 2022 19:14:48 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH v5 2/2] iio: adc: ad4130: add AD4130 driver
Content-Language: en-US
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     Jonathan Cameron <jic23@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-iio <linux-iio@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        Cosmin Tanislav <cosmin.tanislav@analog.com>
References: <20220620162059.1097264-1-cosmin.tanislav@analog.com>
 <20220620162059.1097264-3-cosmin.tanislav@analog.com>
 <CAHp75VcBJkQ+CwyoDaTJ_AD+mv9d0tEd_txqHwkPRy4-xvnyKg@mail.gmail.com>
 <2aa93eab-de6d-866b-a829-36b47ff00982@gmail.com>
 <CAHp75Vc_fcAP6gGwMkYZUoMM6jKeUoQr8J+zYCUz8inSHnTF_w@mail.gmail.com>
From:   Cosmin Tanislav <demonsingur@gmail.com>
In-Reply-To: <CAHp75Vc_fcAP6gGwMkYZUoMM6jKeUoQr8J+zYCUz8inSHnTF_w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 6/23/22 18:39, Andy Shevchenko wrote:
> On Thu, Jun 23, 2022 at 5:27 PM Cosmin Tanislav <demonsingur@gmail.com> wrote:
>> On 6/20/22 21:29, Andy Shevchenko wrote:
>>> On Mon, Jun 20, 2022 at 6:27 PM Cosmin Tanislav <demonsingur@gmail.com> wrote:
> 
> ...
> 
>>>> +       /*
>>>> +        * DMA (thus cache coherency maintenance) requires the
>>>> +        * transfer buffers to live in their own cache lines.
>>>> +        */
>>>
>>> This is a good comment, but what fields does it apply to?
>>
>> Whatever is below it, grouped together. This is not hard to
>> understand.
> 
> It's hard to understand what exactly is DMA-aware here. I see only one
> buffer that is aligned properly for DMA, the rest are not, except the
> case if all of them are going in one DMA transaction. Is this the case
> here?
> 
>>>> +       u8                      reset_buf[AD4130_RESET_BUF_SIZE] __aligned(IIO_DMA_MINALIGN);
> 
> This is aligned.
> 
>>>> +       u8                      reg_write_tx_buf[4];
> 
> This one is aligned + offset (== AD4130_RESET_BUF_SIZE + 0).
> 
>>>> +       u8                      reg_read_tx_buf[1];
> 
> This one is aligned + offset (== AD4130_RESET_BUF_SIZE + 0 + 4).
> 
>>>> +       u8                      reg_read_rx_buf[3];
> 
> This one is aligned + offset (== AD4130_RESET_BUF_SIZE + 0 + 4 + 1).
> And this is Rx.
> 
>>>> +       u8                      fifo_tx_buf[2];
> 
> Here is Tx again which is most likely is not aligned...
> 
>>>> +       u8                      fifo_rx_buf[AD4130_FIFO_SIZE *
>>>> +                                           AD4130_FIFO_MAX_SAMPLE_SIZE];
>>>> +};
> 

This has been mentioned before by Jonathan as a reply to V6 of my
AD74413R driver.

 > I'm surprised I didn't mention this before but you only need to 
ensure  > that any memory used for DMA is not in a cacheline with memory 
used
 > for other things that might change concurrently.

To my understanding, as long as the DMA buffers will all be accessed by
the same DMA-compatible SPI controller, you only need to align them so
they're not in the same cacheline with memory that will not be accessed
by the SPI controller.