From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A4781329C73 for ; Wed, 11 Mar 2026 10:00:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773223257; cv=none; b=tdS3CJ/IbZH4ealbyxoRlZMVGn1W53GI77LCCO2/2zO9f2j/3Tapou7JSiwyRVqpAhJyhqiT4Co7TnSTPNC4IAn0aLlU8gDri3LJrHMFkzFN45k94OFOMyCMqYYcqc4pyR1VMG4LkMK+PkqjWS6nCA3ulUd8K1G0Fo9MYzVRqEM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773223257; c=relaxed/simple; bh=th6/Spfb/htH3UowzS/TbF+AxDPJCsaUEzoaTbrPt1g=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ouilpuig4LJpkyHDMp9uqkdyGySlgNLFcHUJpXGqvf4PWQ+xUoyop1TryUnwkzBNElBYCJD2QjFitjyR679mmkdwnlWGBF5DN0Ram3bPQv0LL8r9VhrVfe8ovUWJXL1m8V/gaDFxU3Bn/VGKFDlT1gDS64I0kfS5FIJXE/EeNKk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=BTzqvP99; arc=none smtp.client-ip=209.85.221.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="BTzqvP99" Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-439c944bb62so5281908f8f.3 for ; Wed, 11 Mar 2026 03:00:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1773223253; x=1773828053; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Ow79JTabgZIRsJS6+UL0N2v2U8DPJz+PJynXqjT3UBM=; b=BTzqvP99hY7y1BtIfL8xE2gpnIbkV1/GC6zg3quknDk3bOlJNaC3SxWj9FbeCyIriU /ZbtxJV9Cn/QnKZLotWU6AmYtA5srXFtM8dAUM6CWCX/cHNUkNeuiM17qkqw37Hd5LPT fw13La8XrK8i7ePY5pQf8INC6pVkWIZbBxMGXFJFf7GekQ1jLEhzQ9x5vg+644Z9LiTY LGRFy8HUtKshdPzGTO7TXAxOg3lV/Wf1Pzdwpx42NTbt+SSN8XSKJDUXXzMvrcDh+kAs KeVuJL6igohdE/JbBFafwmgpihAYvbXStSmHB+4Ngpy9mBjgMdMk/CGt1hhQUewZrZ/D 5cqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1773223253; x=1773828053; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Ow79JTabgZIRsJS6+UL0N2v2U8DPJz+PJynXqjT3UBM=; b=BfUztTpnYNKLXaiNXs9Kn3TvZwY2ioDuPlpXYnxBHszF/hd2b0OnwHkdPTpNlqNP0J ldNcVTRzFdXkFMEmRfGgLTvXwCzxBki+BPQwceYnhNuVIx+ONhnnJcokb47B2bT+FtN8 aYAmIeFGG1NNOvJYtWJkmEs7QEjKXkvTBL2JBDeMNsB1w6o2EkCImLugi+0JEMxw34bg LEic2yNMH6U91cIwksSFPeThapQuu40O+idmwAQOaI1NGUhhYk3KdoCbSeVMUM35xH6X wJA99N6CmsuvidIbshNNkNo0r3tDuuvuhsfK7cAayrN8MPFPAVisBz8AUvAtEPpvi93s tIqA== X-Forwarded-Encrypted: i=1; AJvYcCWjS71RR3qdMxA8Q51TSC8iqpvRyhz1+PMp/zxmI2NJEz0HVdGRHoRvPdL+hcjLQH8ZHycG+YcUgR9RoTg=@vger.kernel.org X-Gm-Message-State: AOJu0YyBgUN7WqchWjxc9y5CYJObBrJPdYzWizp0wNNYK/3TM1nS+OTn HoVhT94Z3rnyZU9haSUExBCfebNiqDmeSVHMPJ9kNV47cUoRAND+rXu4zaYzG/FONFI= X-Gm-Gg: ATEYQzyaqtyPf1zDVo+ur3mB9YXCxTaYc7ZiXdngB6k+p/z65IKo34DIJn21XsCGd7o sHiN54XEisg2FBKE8wI65Ui8OOa8YWcFXNWDPthtmf/M8exTQFP579R1uQF82S7LR6AqPkRYzF1 HqnDAlPfBSwyTL5zC32emim2OcjT0Ha+hbkQLejcYPzNz7+BspxUZV7XADAxThVJ4dBHD9ZAaQ0 Kq8Sb2MoCnePRIiubZjQxOdph/zKHTpyrWBwvQFxOSg/+eor+fXops3x4+BBV1XFhCBPHi9GVLs eP6oa1mfb/zQ62uCZfCk+EZHW0UQ7Mo1cLB1gg0+UJfBmNi9Er7v5HPkzxcqheqOz29W6IS3D2D OikeSCuTJFlEd+zczpfCK7DFXOyrYdKmx2Q9qm5hACmKJ8MtAFRkAKqxFoxbdgCXVAqel0Ttp3G mR3MZkiLB03FXxkQT0C7eMk/pLh39R1+22voA= X-Received: by 2002:a05:6000:2f85:b0:439:c38e:66cc with SMTP id ffacd0b85a97d-439f821e5aemr3551932f8f.46.1773223249530; Wed, 11 Mar 2026 03:00:49 -0700 (PDT) Received: from linaro.org ([2a02:2454:ff23:4441:1c2c:7aff:fe45:362e]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-439f81acc22sm5146729f8f.16.2026.03.11.03.00.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Mar 2026 03:00:48 -0700 (PDT) Date: Wed, 11 Mar 2026 11:00:37 +0100 From: Stephan Gerhold To: Bartosz Golaszewski Cc: Vinod Koul , Jonathan Corbet , Thara Gopinath , Herbert Xu , "David S. Miller" , Udit Tiwari , Daniel Perez-Zoghbi , Md Sadre Alam , Dmitry Baryshkov , Peter Ujfalusi , Michal Simek , Frank Li , dmaengine@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-crypto@vger.kernel.org, linux-arm-kernel@lists.infradead.org, brgl@kernel.org, Bartosz Golaszewski Subject: Re: [PATCH v12 05/12] dmaengine: qcom: bam_dma: add support for BAM locking Message-ID: References: <20260310-qcom-qce-cmd-descr-v12-0-398f37f26ef0@oss.qualcomm.com> <20260310-qcom-qce-cmd-descr-v12-5-398f37f26ef0@oss.qualcomm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260310-qcom-qce-cmd-descr-v12-5-398f37f26ef0@oss.qualcomm.com> On Tue, Mar 10, 2026 at 04:44:19PM +0100, Bartosz Golaszewski wrote: > Add support for BAM pipe locking. To that end: when starting DMA on an RX > channel - prepend the existing queue of issued descriptors with an > additional "dummy" command descriptor with the LOCK bit set. Once the > transaction is done (no more issued descriptors), issue one more dummy > descriptor with the UNLOCK bit. > > We *must* wait until the transaction is signalled as done because we > must not perform any writes into config registers while the engine is > busy. > > [...] > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c > index 83491e7c2f17d8c9d12a1a055baea7e3a0a75a53..627c85a2df4dcdbac247d831a4aef047c2189456 100644 > --- a/drivers/dma/qcom/bam_dma.c > +++ b/drivers/dma/qcom/bam_dma.c > [...] > +static int bam_do_setup_pipe_lock(struct bam_chan *bchan, bool lock) > +{ > + struct bam_device *bdev = bchan->bdev; > + const struct bam_device_data *bdata = bdev->dev_data; > + struct bam_async_desc *lock_desc; > + struct bam_cmd_element *ce; > + struct scatterlist *sgl; > + unsigned long flag; > + > + lockdep_assert_held(&bchan->vc.lock); > + > + if (!bdata->pipe_lock_supported || !bchan->scratchpad_addr || > + bchan->slave.direction != DMA_MEM_TO_DEV) > + return 0; > + > + if (lock) { > + sgl = &bchan->lock_sg; > + ce = &bchan->lock_ce; > + flag = DESC_FLAG_LOCK; > + } else { > + sgl = &bchan->unlock_sg; > + ce = &bchan->unlock_ce; > + flag = DESC_FLAG_UNLOCK; > + } > + > + lock_desc = bam_make_lock_desc(bchan, sgl, ce, flag); > + if (!lock_desc) > + return -ENOMEM; > + > + if (lock) > + list_add(&lock_desc->vd.node, &bchan->vc.desc_issued); > + else > + list_add_tail(&lock_desc->vd.node, &bchan->vc.desc_issued); > + > + bchan->locked = lock; > + > + return 0; > +} > + > +static int bam_setup_pipe_lock(struct bam_chan *bchan) > +{ > + return bam_do_setup_pipe_lock(bchan, true); > +} > + > +static int bam_setup_pipe_unlock(struct bam_chan *bchan) > +{ > + return bam_do_setup_pipe_lock(bchan, false); > +} > + > /** > * bam_start_dma - start next transaction > * @bchan: bam dma channel > @@ -1121,6 +1266,7 @@ static void bam_dma_work(struct work_struct *work) > struct bam_device *bdev = from_work(bdev, work, work); > struct bam_chan *bchan; > unsigned int i; > + int ret; > > /* go through the channels and kick off transactions */ > for (i = 0; i < bdev->num_channels; i++) { > @@ -1128,6 +1274,13 @@ static void bam_dma_work(struct work_struct *work) > > guard(spinlock_irqsave)(&bchan->vc.lock); > > + if (list_empty(&bchan->vc.desc_issued) && bchan->locked) { > + ret = bam_setup_pipe_unlock(bchan); > + if (ret) > + dev_err(bchan->vc.chan.slave, > + "Failed to set up the pipe unlock descriptor\n"); > + } > + > if (!list_empty(&bchan->vc.desc_issued) && !IS_BUSY(bchan)) > bam_start_dma(bchan); > } 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. 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]. The chapter in the APQ8016E TRM kind of contradicts itself sometimes, but there is this sentence for example: "On the data descriptor preceding command descriptor, NWD bit must be asserted in order to assure that all the data has been transferred and the peripheral is ready to be re-configured." Thanks, Stephan [1]: https://git.codelinaro.org/clo/la/platform/vendor/qcom/opensource/securemsm-kernel/-/blob/fa55a96773d3fbfcd96beb2965efcaaae5697816/crypto-qti/qce50.c#L5361-5362