Re: [PATCH v9 03/11] dmaengine: qcom: bam_dma: implement support for BAM locking

[Bartosz Golaszewski]

On Thu, 19 Feb 2026 13:12:09 +0100, Manivannan Sadhasivam
<mani@xxxxxxxxxx> said:
> On Fri, Jan 09, 2026 at 03:15:38PM +0100, Bartosz Golaszewski wrote:
>> On Fri, Jan 9, 2026 at 3:27 AM Vinod Koul <vkoul@xxxxxxxxxx> wrote:
>> >
>> > >
>> > > We need an API because we send a locking descriptor, then a regular
>> > > descriptor (or descriptors) for the actual transaction(s) and then an
>> > > unlocking descriptor. It's a thing the user of the DMA engine needs to
>> > > decide on, not the DMA engine itself.
>> >
>> > I think downstream sends lock descriptor always. What is the harm in
>> > doing that every time if we go down that path?
>>
>> No, in downstream it too depends on the user setting the right bits.
>> Currently the only user of the BAM locking downstream is the NAND
>> driver. I don't think the code where the crypto driver uses it is
>> public yet.
>>
>> And yes, there is harm - it slightly impacts performance. For QCE it
>> doesn't really matter as any users wanting to offload skcipher or SHA
>> are better off using the Arm Crypto Extensions anyway as they are
>> faster by an order of magnitude (!). It's also the default upstream,
>> where the priorities are set such that the ARM CEs are preferred over
>> the QCE. QCE however, is able to coordinate with the TrustZone and
>> will be used to support the DRM use-cases.
>>
>> I prefer to avoid impacting any other users of BAM DMA.
>>
>
> Sorry for jumping late. But I disagree with the argument that the client drivers
> have to set the LOCK/UNLOCK bit. These bits are specific to BAM DMA IP for
> serializing the command descriptors from multiple entities. So DMA clients like
> Crypto/NAND have no business in setting this flag. It is the job of the BAM
> dmaengine driver to set/unset it at the start and end of the descriptor chain.
>

But what if a given client does not need locking? We don't want to enable it
for everyone - as I explained before.

>> > Reg Dmitry question above, this is dma hw capability, how will client
>> > know if it has to lock on older rev of hardware or not...?
>> >
>> > > Also: only the crypto engine needs it for now, not all the other users
>> > > of the BAM engine.
>> >
>>
>> Trying to set the lock/unlock bits will make
>> dmaengine_desc_attach_metadata() fail if HW does not support it.
>>
>
> The BAM dmaengine driver *must* know based on the IP version whether it supports
> the LOCK/UNLOCK bits or not, not the client drivers. How can the client drivers
> know about the BAM DMA IP capability?
>

FYI: the current version of this is v10[1].

In it (and in this one too but let's discuss the current one) the BAM driver
*does* know *based on IP version* whether is supports locking or not. The client
requests a lock but this will fail if the BAM does not support it. The
client does
not check the BAM IP revision. So yes: it's the BAM driver that's in charge.

> For all these reasons, BAM driver should handle the locking mechanism internaly.
> This will allow the client drivers to work without any modifications.
>

Ok, I'm open to alternatives but please help me figure out the "hows": How do
you tell the BAM driver that the client needs (or does not) locking? How do
you handle the case where we need to lock the BAM, send an arbitrary number
of descriptors from the client and then unlock it? How can the BAM know *when*
to lock/unlock?

> FWIW, NAND driver too is impacted by this missing feature in the BAM driver as
> both Modem and Linux tries to driver BAM and currently Linux BAM driver doesn't
> set these bits leading to crashes.
>

Yes, downstream handles that in a very dirty way (same as downstream QCE).

Bart

[1] https://lore.kernel.org/all/20251219-qcom-qce-cmd-descr-v10-0-ff7e4bf7dad4@xxxxxxxxxxxxxxxx/