From: "Nuno Sá" <noname.nuno@gmail.com>
To: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>,
nuno.sa@analog.com, linux-iio@vger.kernel.org,
David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>
Subject: Re: [PATCH v2] iio: buffer-dmaengine: Add support for cyclic DMA transfers
Date: Mon, 20 Jul 2026 14:43:03 +0100 [thread overview]
Message-ID: <al4kJW8nDllWrBBB@nsa> (raw)
In-Reply-To: <20260719030346.6fb9670e@jic23-huawei>
On Sun, Jul 19, 2026 at 03:03:46AM +0100, Jonathan Cameron wrote:
> On Sun, 19 Jul 2026 03:00:57 +0100
> Jonathan Cameron <jonathan.cameron@oss.qualcomm.com> wrote:
>
> > On Wed, 15 Jul 2026 19:09:42 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> >
> > > On Wed, Jul 15, 2026 at 05:13:32PM +0200, Nuno Sá wrote:
> > > > On Wed, Jul 15, 2026 at 05:23:13PM +0300, Andy Shevchenko wrote:
> > > > > On Wed, Jul 15, 2026 at 01:24:53PM +0200, Nuno Sá via B4 Relay wrote:
> > >
> > > ...
> > >
> > > > > > static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > > > > > struct iio_dma_buffer_block *block)
> > > > >
> > > > > > + if (block->cyclic)
> > > > > > + flags = DMA_PREP_REPEAT;
> > > > > > + else
> > > > > > + flags = DMA_PREP_INTERRUPT;
> > > > > > +
> > > > > > + /*
> > > > > > + * A new transfer may need to end an already active cyclic transfer
> > > > > > + * before it can run, so always set the EOT flag.
> > > > > > + */
> > > > > > desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
> > > > > > vecs, nents, dma_dir,
> > > > > > - DMA_PREP_INTERRUPT);
> > > > > > + flags | DMA_PREP_LOAD_EOT);
> > > > >
> > > > > Logically I would expect this to be a separate line with the above comment.
> > > >
> > > > Hmm I can agree to some extent. I guess it makes sense, yes. OTOH, it´s close
> > > > enough to be clear what's happening.
> > >
> > > Close, but not enough in my opinion. This currently is spread over pure flags
> > > manipulation and API calls.
> > >
> > > > Anyways, can spin v3 if you feel
> > > > strong about it (will anyways wait for Jonathan to take a look to make
> > > > sure this the only pending concern).
> > >
> > > Not so strong, but lean towards my version. Let's wait for his review and
> > > decide if it will be tweaked whilst applying or next version.
> > >
> > Applied with this tweak. Hopefully I didn't mess it up!
>
> I'll leave it on tree for now but I forgot to check sashiko and
> it asks about plausible sounding races. Please take a look and
> let me know if they are real and an update is needed:
> https://sashiko.dev/#/patchset/20260715-iio-dma-cyclic-v2-1-268a3a28ff84%40analog.com
"
Does this check allow a non-cyclic transfer to be submitted while a cyclic
transfer is already active?
Since block->cyclic is false for a non-cyclic transfer, this safety check
is bypassed."
The above is the whole point of DMA_PREP_LOAD_EOT (so we can gracefully
"terminate" a cyclic transfer)
I now see that our current cap will never allow for a new cyclic to be
submitted (unless we disable + enable the buffer) but I would keep things simple
for now (it was also the whole point of having a cyclic cap in the first
place).
"If a non-cyclic file I/O transfer is submitted instead, it lacks the
DMA_PREP_LOAD_EOT flag, which could cause it to hang forever while the DMA
engine continuously repeats the preceding cyclic transfer."
Not sure where AI got the above from. The DMA_PREP_LOAD_EOT is always
set (even for the file IO based approach).
So unless I'm missing nothing, I think we are fine.
- Nuno Sá
>
> >
> > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > index 639bac135539..ecc02a427b92 100644
> > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > @@ -132,9 +132,10 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > * A new transfer may need to end an already active cyclic transfer
> > * before it can run, so always set the EOT flag.
> > */
> > + flags |= DMA_PREP_LOAD_EOT;
> > desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
> > vecs, nents, dma_dir,
> > - flags | DMA_PREP_LOAD_EOT);
> > + flags);
> > kfree(vecs);
> > } else {
> > max_size = min(block->size, dmaengine_buffer->max_size);
> >
> > > > > /*
> > > > > * ...comment...
> > > > > */
> > > > > flags |= DMA_PREP_LOAD_EOT;
> > > > > ...
> > > > > flags);
> > > > >
> > > > > > kfree(vecs);
> > >
> >
> >
>
prev parent reply other threads:[~2026-07-20 13:41 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-15 11:24 [PATCH v2] iio: buffer-dmaengine: Add support for cyclic DMA transfers Nuno Sá via B4 Relay
2026-07-15 11:24 ` Nuno Sá
2026-07-15 14:23 ` Andy Shevchenko
2026-07-15 15:13 ` Nuno Sá
2026-07-15 16:09 ` Andy Shevchenko
2026-07-19 2:00 ` Jonathan Cameron
2026-07-19 2:03 ` Jonathan Cameron
2026-07-20 13:43 ` Nuno Sá [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=al4kJW8nDllWrBBB@nsa \
--to=noname.nuno@gmail.com \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jonathan.cameron@oss.qualcomm.com \
--cc=linux-iio@vger.kernel.org \
--cc=nuno.sa@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.