Re: [PATCH v12 05/12] dmaengine: qcom: bam_dma: add support for BAM locking

Stephan Gerhold
2026-03-11T11:25:25Z

On Wed, Mar 11, 2026 at 03:32:38AM -0700, Bartosz Golaszewski wrote:
> On Wed, Mar 11, 2026 at 11:00 AM Stephan Gerhold
> <stephan.gerhold@linaro.org> wrote:
> >
> > I'm not entirely sure if this actually guarantees waiting with the
> > unlock until the transaction is "done", for two reasons:
> >
> > 1. &bchan->vc.desc_issued looks like a "TODO" list for descriptors we
> > haven't fully managed to squeeze into the BAM FIFO yet. It doesn't
> > tell you which descriptors have been consumed and finished
> > processing inside the FIFO.
> >
> > Consider e.g. the following case: The client has issued a number of
> > descriptors, they all fit into the FIFO. The first descriptor has a
> > callback assigned, so we ask the BAM to send us an interrupt when it
> > has been consumed. We get the interrupt for the first descriptor and
> > process_channel_irqs() marks it as completed, the rest of the
> > descriptors are still pending. &bchan->vc.desc_issued is empty, so
> > you queue the unlock command before the rest of the descriptors have
> > finished.
> >
>
> Thanks for looking into it. Good catch, I think you're right.
>
> > 2. From reading the BAM chapter in the APQ8016E TRM I get the
> > impression that by default an interrupt for a descriptor just tells
> > you that the descriptor was consumed by the BAM (and forwarded to
> > the peripheral). If you want to guarantee that the transaction is
> > actually done on the peripheral side before allowing writes into
> > config registers, you would need to set the NWD (Notify When Done)
> > bit (aka DMA_PREP_FENCE) on the last descriptor before the unlock
> > command.
> >
> > NWD seems to stall descriptor processing until the peripheral
> > signals completion, so this might allow you to immediately queue the
> > unlock command like in v11. The downside is that you would need to
> > make assumptions about the set of commands submitted by the client
> > again... The downstream driver seems to set NWD on the data
> > descriptor immediately before the UNLOCK command [1].
> >
>
> If what we have in the queue is:
>
> [DATA] [DATA] [DATA] [CMD]
>
> And we want to extend it with LOCK/UNLOCK like so:
>
> [LOCK] [DATA] [DATA] [DATA] [CMD] [UNLOCK]
>
> Should the NWD go with the last DATA descriptor or the last descriptor period
> whether data or command?
>
> It's, again, not very clear from reading tha part.
>

I'm not sure, my impression is that the exact behavior of NWD is quite
specific to the actual peripheral (i.e. QCE, QPIC NAND, etc). In the
downstream drivers:

- QCE seems to add NWD to the last data descriptor before the UNLOCK
(as I wrote, it seems to queue command descriptors before data).

- QPIC NAND has a dedicated "cmd" pipe that doesn't get any data
descriptors, it specifies NWD always for the EXEC_CMD register write,
which isn't even the last descriptor. This is also done in mainline
already (see NAND_BAM_NWD in qcom_write_reg_dma() [1]).

It is possible that NWD works only when attached to certain descriptors
(when there is an actual operation running that gets completed by a
certain descriptor), so we might not be able to simply add NWD to the
last descriptor. :/

I suppose you could argue that "make sure engine does not get
re-configured while busy" is a requirement of QCE and not BAM, so
perhaps it would be easiest and safest if you just add DMA_PREP_FENCE to
the right descriptor inside the QCE driver. qcom_nandc has that already.

Thanks,
Stephan

[1]: https://elixir.bootlin.com/linux/v7.0-rc3/source/drivers/mtd/nand/qpic_common.c#L484