* [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
From: Hao Jia @ 2026-07-17 8:51 UTC (permalink / raw)
To: akpm, tj, hannes, shakeel.butt, mhocko, yosry, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin
Cc: linux-mm, linux-kernel, linux-doc, Hao Jia
From: Hao Jia <jiahao1@lixiang.com>
This series fixes and improves the zswap global shrinker (shrink_worker()):
Patch 1: Fix missing global shrinker when memory cgroup is disabled.
Patch 2: Extend shrink_memcg() to support batch writeback and update its
return value semantics, thereby improving the writeback efficiency
in the shrink_worker() path.
v1->v2:
- Add a reschedule check to the -ENOENT return path in shrink_memcg() to
handle the theoretical issue of prolonged heavy concurrent zswap stores.
- Remove the shrink_memcg() return value changes part, and include a more
detailed test report in the commit message.
[v1] https://lore.kernel.org/all/20260714081510.16895-1-jiahao.kernel@gmail.com
Hao Jia (2):
mm/zswap: Fix global shrinker when memory cgroup is disabled
mm/zswap: Support batch writeback in shrink_memcg()
mm/zswap.c | 60 ++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 47 insertions(+), 13 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
From: Hao Jia @ 2026-07-17 8:46 UTC (permalink / raw)
To: Yosry Ahmed, Nhat Pham
Cc: Andrew Morton, tj, hannes, shakeel.butt, mhocko, mkoutny,
chengming.zhou, muchun.song, roman.gushchin, linux-mm,
linux-kernel, linux-doc, Hao Jia, stable
In-Reply-To: <CAO9r8zMy3C=tTLsyv_WkAS8Fd_Z3uSCmX4cekPLo8KkNb9bkCA@mail.gmail.com>
On 2026/7/17 08:08, Yosry Ahmed wrote:
> On Thu, Jul 16, 2026 at 11:19 AM Nhat Pham <nphamcs@gmail.com> wrote:
>>
>> On Wed, Jul 15, 2026 at 7:21 PM Hao Jia <jiahao.kernel@gmail.com> wrote:
>>>
>>>
>>>
>>> On 2026/7/16 00:13, Yosry Ahmed wrote:
>>>> On Wed, Jul 15, 2026 at 5:31 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 2026/7/15 10:31, Andrew Morton wrote:
>>>>>> On Tue, 14 Jul 2026 09:52:59 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
>>>>>>
>>>>>>>> When memory cgroup is disabled, mem_cgroup_iter() always returns NULL.
>>>>>>>> Therefore, the global shrinker shrink_worker() always takes the !memcg
>>>>>>>> branch. After MAX_RECLAIM_RETRIES empty walks, the worker simply gives up,
>>>>>>>> so it fails to write back anything.
>>>>>>>>
>>>>>>>> Therefore, when memory cgroup is disabled, fall through with the !memcg
>>>>>>>> branch and shrink the root memcg directly.
>>>>>>>>
>>>>>>>> With memcg disabled, shrink_memcg() only returns -ENOENT when the root
>>>>>>>> LRU is empty, which means the total pages are already below thr. The
>>>>>>>> loop then safely bails out via the zswap_total_pages() <= thr check.
>>>>>>>> For any other return value from shrink_memcg(), the loop is guaranteed
>>>>>>>> to terminate, either after MAX_RECLAIM_RETRIES failures or once the
>>>>>>>> threshold is met.
>>>>>>>>
>>>>>>>> Fixes: a65b0e7607cc ("zswap: make shrinking memcg-aware")
>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>> Suggested-by: Nhat Pham <nphamcs@gmail.com>
>>>>>>>> Acked-by: Nhat Pham <nphamcs@gmail.com>
>>>>>>>> Acked-by: Yosry Ahmed <yosry@kernel.org>
>>>>>>>> Reported-by: Yosry Ahmed <yosry@kernel.org>
>>>>>>>> Closes: https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com
>>>>>>>> Signed-off-by: Hao Jia <jiahao1@lixiang.com>
>>>>>>>
>>>>>>> Patch 2 doesn't really depend on this one, right?
>>>>>>>
>>>>>>> If that's the case I think this can (and should be) picked up
>>>>>>> separately as a hotfix. Andrew, WDYT?
>>>>>>
>>>>>> Please update the changelog to clearly describe the userspace-visible
>>>>>> effects of the bug, thanks.
>>>>>
>>>>> I am not entirely sure if my understanding is correct here, but maybe I
>>>>> should add something like this to the commit message?
>>>>>
>>>>> When cgroup_disable=memory is used (or with CONFIG_MEMCG=n), the global
>>>>> shrinker fails to write back any pages. Consequently, the zswap pool
>>>>> fills up to its limit and rejects further storage, preventing memory
>>>>> pressure from being offloaded to the backing swap device.
>>>>
>>>> I think you can simply write that zswap writeback when the limit is
>>>> hit is broken when memcg is disabled.
>>> Will do. Thanks!
>>>
>>>>
>>>>>
>>>>>> Also, AI review has flagged several possible issues, all appear to be
>>>>>> serious:
>>>>>> https://sashiko.dev/#/patchset/20260714081510.16895-1-jiahao.kernel@gmail.com
>>>>>
>>>>> For AI review comments on this patch:
>>>>> I suspect this scenario might only exist in theory. For zswap LRU to be
>>>>> empty while zswap_total_pages() > thr holds true, it would require a
>>>>> prolonged state where there are always more than thr zswap entries on
>>>>> the zswap LRU whenever zswap_total_pages() > thr is evaluated, yet the
>>>>> zswap LRU happens to be empty during shrink_memcg(root_memcg).
>>>>>
>>>>> If we want to fix this, perhaps we could do something like this?
>>>>>
>>>>> Yosry, Nhat, what are your thoughts on this?
>>>>
>>>> Do we need to do this? The last paragraph in your changelog explains
>>>> why this can't happen because zswap_total_pages() should be 0 in this
>>>> case. Did I miss something?
>>>
>>> The loop would require the following sequence to repeat indefinitely:
>>>
>>> 1、zswap_total_pages() > thr evaluates to true.
>>> 2、During shrink_memcg(root_memcg), the zswap LRU is concurrently drained
>>> to empty.
>>> 3、Before zswap_total_pages() > thr is evaluated again, the zswap LRU is
>>> heavily refilled such that zswap_total_pages() > thr holds true once more.
>>>
>>> For our case to manifest, it would require zswap_total_pages() and the
>>> zswap LRU state to repeatedly hit this exact window with perfect
>>> alignment over a **prolonged period**. Therefore, I suspect this
>>> scenario might only exist in theory.
>>
>> Yeah seems very artificial indeed.
>>
>> If we want to be extra careful here, maybe we can put a cond_resched()
>> there or replace the continue with goto resched or sth?
>
> I think it's also possible with the current code with memcg enabled.
> It's still possible that the shrinker puts usage under the acceptance
> threshold and then a new zswap stores puts it back above the
> threshold, and so on.
>
> Perhaps as Nhat said, just goto resched instead of continue if we're
> really worried, but I think even that is not really necessary.
>
Yes, but to be strictly rigorous, let's add a goto resched; anyway.
Thanks,
Hao
> I think we generally want to clean up and simplify the shrinking loop
> in zswap_shrinker(), but I don't have any great ideas.
^ permalink raw reply
* [PATCH v22 14/14] crypto: qce - Communicate the base physical address to the dmaengine
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
In order to communicate to the BAM DMA engine which address should be
used as a scratchpad for dummy writes related to BAM pipe locking,
fill out the dedicated BAM peripheral config structure and pass it to
the DMA driver using dmaengine_slave_config().
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/core.c | 6 ++++--
drivers/crypto/qce/dma.c | 13 +++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 4031b4516d6519fc5024bbbcc439500a7b3314b2..50fdc8d808d0fe1d9d430ec601ab2ccf2ddf6e91 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -246,6 +246,9 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret)
return ret;
+ qce->base_phys = res->start;
+ qce->dma_size = resource_size(res);
+
ret = devm_qce_dma_request(qce);
if (ret)
return ret;
@@ -263,10 +266,9 @@ static int qce_crypto_probe(struct platform_device *pdev)
qce->async_req_enqueue = qce_async_request_enqueue;
qce->async_req_done = qce_async_request_done;
- qce->dma_size = resource_size(res);
qce->base_dma = dma_map_resource(dev, res->start, qce->dma_size,
DMA_BIDIRECTIONAL, 0);
- qce->base_phys = res->start;
+
ret = dma_mapping_error(dev, qce->base_dma);
if (ret)
return ret;
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index 88d253d9147cfc9ed275653f7fd248538378194a..b45a65a2ebb511d6619a49f35a89730a551421c6 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -11,6 +11,7 @@
#include "core.h"
#include "dma.h"
+#include "regs-v5.h"
#define QCE_IGNORE_BUF_SZ (2 * QCE_BAM_BURST_SIZE)
#define QCE_BAM_CMD_SGL_SIZE 128
@@ -116,8 +117,16 @@ static void qce_dma_terminate(void *data)
int devm_qce_dma_request(struct qce_device *qce)
{
+ struct bam_config peripheral_cfg = {
+ .lock_scratchpad_addr = qce->base_phys + REG_VERSION,
+ };
+ struct dma_slave_config cfg = {
+ .direction = DMA_MEM_TO_DEV,
+ .peripheral_config = &peripheral_cfg,
+ };
struct qce_dma_data *dma = &qce->dma;
struct device *dev = qce->dev;
+ int ret;
dma->result_buf = devm_kmalloc(dev, QCE_RESULT_BUF_SZ + QCE_IGNORE_BUF_SZ, GFP_KERNEL);
if (!dma->result_buf)
@@ -133,6 +142,10 @@ int devm_qce_dma_request(struct qce_device *qce)
return dev_err_probe(dev, PTR_ERR(dma->rxchan),
"Failed to get RX DMA channel\n");
+ ret = dmaengine_slave_config(dma->rxchan, &cfg);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to configure the RX DMA channel\n");
+
dma->bam_txn = devm_kzalloc(dev, sizeof(*dma->bam_txn), GFP_KERNEL);
if (!dma->bam_txn)
return -ENOMEM;
--
2.47.3
^ permalink raw reply related
* [PATCH v22 13/14] crypto: qce - Add BAM DMA support for crypto register I/O
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Switch to using BAM DMA for register I/O in addition to passing data. To
that end: provide the necessary infrastructure in the driver, modify the
ordering of operations as required and replace all direct register writes
with wrappers queueing DMA command descriptors.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/aead.c | 10 ++--
drivers/crypto/qce/common.c | 20 ++++---
drivers/crypto/qce/dma.c | 122 ++++++++++++++++++++++++++++++++++++++++--
drivers/crypto/qce/dma.h | 5 ++
drivers/crypto/qce/sha.c | 10 ++--
drivers/crypto/qce/skcipher.c | 10 ++--
6 files changed, 146 insertions(+), 31 deletions(-)
diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index 1461a08e6c58b00e60aa35515f3392c096726f6a..544a3cf8709248a5f3eb2b669e30b09183d3a69d 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -463,17 +463,17 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
src_nents = dst_nents - 1;
}
- ret = qce_dma_prep_sgs(&qce->dma, rctx->src_sg, src_nents, rctx->dst_sg, dst_nents,
- qce_aead_done, async_req);
+ ret = qce_start(async_req, tmpl->crypto_alg_type);
if (ret)
goto error_unmap_src;
- qce_dma_issue_pending(&qce->dma);
-
- ret = qce_start(async_req, tmpl->crypto_alg_type);
+ ret = qce_dma_prep_sgs(&qce->dma, rctx->src_sg, src_nents, rctx->dst_sg, dst_nents,
+ qce_aead_done, async_req);
if (ret)
goto error_terminate;
+ qce_dma_issue_pending(&qce->dma);
+
return 0;
error_terminate:
diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
index 54a78a57f63028f01870a3edeb8e390f523bb190..37bb6f03244d317a887aeb0aa10cefe327b4ce05 100644
--- a/drivers/crypto/qce/common.c
+++ b/drivers/crypto/qce/common.c
@@ -25,7 +25,7 @@ static inline u32 qce_read(struct qce_device *qce, u32 offset)
static inline void qce_write(struct qce_device *qce, u32 offset, u32 val)
{
- writel(val, qce->base + offset);
+ qce_write_dma(qce, offset, val);
}
static inline void qce_write_array(struct qce_device *qce, u32 offset,
@@ -82,6 +82,8 @@ static void qce_setup_config(struct qce_device *qce)
{
u32 config;
+ qce_clear_bam_transaction(qce);
+
/* get big endianness */
config = qce_config_reg(qce, 0);
@@ -90,12 +92,14 @@ static void qce_setup_config(struct qce_device *qce)
qce_write(qce, REG_CONFIG, config);
}
-static inline void qce_crypto_go(struct qce_device *qce, bool result_dump)
+static inline int qce_crypto_go(struct qce_device *qce, bool result_dump)
{
if (result_dump)
qce_write(qce, REG_GOPROC, BIT(GO_SHIFT) | BIT(RESULTS_DUMP_SHIFT));
else
qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
+
+ return qce_submit_cmd_desc(qce);
}
#if defined(CONFIG_CRYPTO_DEV_QCE_SHA) || defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
@@ -223,9 +227,7 @@ static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
config = qce_config_reg(qce, 1);
qce_write(qce, REG_CONFIG, config);
- qce_crypto_go(qce, true);
-
- return 0;
+ return qce_crypto_go(qce, true);
}
#endif
@@ -386,9 +388,7 @@ static int qce_setup_regs_skcipher(struct crypto_async_request *async_req)
config = qce_config_reg(qce, 1);
qce_write(qce, REG_CONFIG, config);
- qce_crypto_go(qce, true);
-
- return 0;
+ return qce_crypto_go(qce, true);
}
#endif
@@ -535,9 +535,7 @@ static int qce_setup_regs_aead(struct crypto_async_request *async_req)
qce_write(qce, REG_CONFIG, config);
/* Start the process */
- qce_crypto_go(qce, !IS_CCM(flags));
-
- return 0;
+ return qce_crypto_go(qce, !IS_CCM(flags));
}
#endif
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index 26347e9fc078adede712722107e74958538accdf..88d253d9147cfc9ed275653f7fd248538378194a 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -4,6 +4,8 @@
*/
#include <linux/device.h>
+#include <linux/dma/qcom_bam_dma.h>
+#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
#include <crypto/scatterwalk.h>
@@ -11,6 +13,98 @@
#include "dma.h"
#define QCE_IGNORE_BUF_SZ (2 * QCE_BAM_BURST_SIZE)
+#define QCE_BAM_CMD_SGL_SIZE 128
+#define QCE_BAM_CMD_ELEMENT_SIZE 128
+
+struct qce_desc_info {
+ struct dma_async_tx_descriptor *dma_desc;
+ enum dma_data_direction dir;
+};
+
+struct qce_bam_transaction {
+ struct bam_cmd_element bam_ce[QCE_BAM_CMD_ELEMENT_SIZE];
+ struct scatterlist wr_sgl[QCE_BAM_CMD_SGL_SIZE];
+ struct qce_desc_info *desc;
+ u32 bam_ce_idx;
+ u32 pre_bam_ce_idx;
+ u32 wr_sgl_cnt;
+};
+
+void qce_clear_bam_transaction(struct qce_device *qce)
+{
+ struct qce_bam_transaction *bam_txn = qce->dma.bam_txn;
+
+ bam_txn->bam_ce_idx = 0;
+ bam_txn->wr_sgl_cnt = 0;
+ bam_txn->pre_bam_ce_idx = 0;
+}
+
+int qce_submit_cmd_desc(struct qce_device *qce)
+{
+ struct qce_desc_info *qce_desc = qce->dma.bam_txn->desc;
+ struct qce_bam_transaction *bam_txn = qce->dma.bam_txn;
+ struct dma_async_tx_descriptor *dma_desc;
+ struct dma_chan *chan = qce->dma.rxchan;
+ unsigned long attrs = DMA_PREP_CMD;
+ dma_cookie_t cookie;
+ unsigned int mapped;
+ int ret;
+
+ mapped = dma_map_sg(qce->dev, bam_txn->wr_sgl, bam_txn->wr_sgl_cnt, DMA_TO_DEVICE);
+ if (!mapped)
+ return -ENOMEM;
+
+ dma_desc = dmaengine_prep_slave_sg(chan, bam_txn->wr_sgl, mapped, DMA_MEM_TO_DEV, attrs);
+ if (!dma_desc) {
+ ret = -ENOMEM;
+ goto err_unmap_sg;
+ }
+
+ qce_desc->dma_desc = dma_desc;
+ cookie = dmaengine_submit(qce_desc->dma_desc);
+
+ ret = dma_submit_error(cookie);
+ if (ret)
+ goto err_free_desc;
+
+ return 0;
+
+err_free_desc:
+ dmaengine_desc_free(dma_desc);
+err_unmap_sg:
+ dma_unmap_sg(qce->dev, bam_txn->wr_sgl, bam_txn->wr_sgl_cnt, DMA_TO_DEVICE);
+ return ret;
+}
+
+static void qce_prep_dma_cmd_desc(struct qce_device *qce, struct qce_dma_data *dma,
+ unsigned int addr, void *buf)
+{
+ struct qce_bam_transaction *bam_txn = dma->bam_txn;
+ struct bam_cmd_element *bam_ce_buf;
+ int bam_ce_size, cnt, idx;
+
+ idx = bam_txn->bam_ce_idx;
+ bam_ce_buf = &bam_txn->bam_ce[idx];
+ bam_prep_ce_le32(bam_ce_buf, addr, BAM_WRITE_COMMAND, *((__le32 *)buf));
+
+ bam_ce_buf = &bam_txn->bam_ce[bam_txn->pre_bam_ce_idx];
+ bam_txn->bam_ce_idx++;
+ bam_ce_size = (bam_txn->bam_ce_idx - bam_txn->pre_bam_ce_idx) * sizeof(*bam_ce_buf);
+
+ cnt = bam_txn->wr_sgl_cnt;
+
+ sg_set_buf(&bam_txn->wr_sgl[cnt], bam_ce_buf, bam_ce_size);
+
+ ++bam_txn->wr_sgl_cnt;
+ bam_txn->pre_bam_ce_idx = bam_txn->bam_ce_idx;
+}
+
+void qce_write_dma(struct qce_device *qce, unsigned int offset, u32 val)
+{
+ unsigned int reg_addr = ((unsigned int)(qce->base_phys) + offset);
+
+ qce_prep_dma_cmd_desc(qce, &qce->dma, reg_addr, &val);
+}
static void qce_dma_terminate(void *data)
{
@@ -39,6 +133,16 @@ int devm_qce_dma_request(struct qce_device *qce)
return dev_err_probe(dev, PTR_ERR(dma->rxchan),
"Failed to get RX DMA channel\n");
+ dma->bam_txn = devm_kzalloc(dev, sizeof(*dma->bam_txn), GFP_KERNEL);
+ if (!dma->bam_txn)
+ return -ENOMEM;
+
+ dma->bam_txn->desc = devm_kzalloc(dev, sizeof(*dma->bam_txn->desc), GFP_KERNEL);
+ if (!dma->bam_txn->desc)
+ return -ENOMEM;
+
+ sg_init_table(dma->bam_txn->wr_sgl, QCE_BAM_CMD_SGL_SIZE);
+
return devm_add_action_or_reset(dev, qce_dma_terminate, dma);
}
@@ -98,28 +202,36 @@ int qce_dma_prep_sgs(struct qce_dma_data *dma, struct scatterlist *rx_sg,
{
struct dma_chan *rxchan = dma->rxchan;
struct dma_chan *txchan = dma->txchan;
- unsigned long flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
+ unsigned long txflags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
+ unsigned long rxflags = txflags | DMA_PREP_FENCE;
int ret;
- ret = qce_dma_prep_sg(rxchan, rx_sg, rx_nents, flags, DMA_MEM_TO_DEV,
+ ret = qce_dma_prep_sg(rxchan, rx_sg, rx_nents, rxflags, DMA_MEM_TO_DEV,
NULL, NULL);
if (ret)
return ret;
- return qce_dma_prep_sg(txchan, tx_sg, tx_nents, flags, DMA_DEV_TO_MEM,
+ return qce_dma_prep_sg(txchan, tx_sg, tx_nents, txflags, DMA_DEV_TO_MEM,
cb, cb_param);
}
void qce_dma_issue_pending(struct qce_dma_data *dma)
{
- dma_async_issue_pending(dma->rxchan);
dma_async_issue_pending(dma->txchan);
+ dma_async_issue_pending(dma->rxchan);
}
int qce_dma_terminate_all(struct qce_dma_data *dma)
{
+ struct qce_device *qce = container_of(dma, struct qce_device, dma);
+ struct qce_bam_transaction *bam_txn = dma->bam_txn;
int ret;
ret = dmaengine_terminate_all(dma->rxchan);
- return ret ?: dmaengine_terminate_all(dma->txchan);
+ if (ret)
+ return ret;
+
+ dma_unmap_sg(qce->dev, bam_txn->wr_sgl, bam_txn->wr_sgl_cnt, DMA_TO_DEVICE);
+
+ return dmaengine_terminate_all(dma->txchan);
}
diff --git a/drivers/crypto/qce/dma.h b/drivers/crypto/qce/dma.h
index 483789d9fa98e79d1283de8297bf2fc2a773f3a7..f05dfa9e6b25bd60e32f45079a8bc7e6a4cf81f9 100644
--- a/drivers/crypto/qce/dma.h
+++ b/drivers/crypto/qce/dma.h
@@ -8,6 +8,7 @@
#include <linux/dmaengine.h>
+struct qce_bam_transaction;
struct qce_device;
/* maximum data transfer block size between BAM and CE */
@@ -32,6 +33,7 @@ struct qce_dma_data {
struct dma_chan *txchan;
struct dma_chan *rxchan;
struct qce_result_dump *result_buf;
+ struct qce_bam_transaction *bam_txn;
};
int devm_qce_dma_request(struct qce_device *qce);
@@ -43,5 +45,8 @@ int qce_dma_terminate_all(struct qce_dma_data *dma);
struct scatterlist *
qce_sgtable_add(struct sg_table *sgt, struct scatterlist *sg_add,
unsigned int max_len);
+void qce_write_dma(struct qce_device *qce, unsigned int offset, u32 val);
+int qce_submit_cmd_desc(struct qce_device *qce);
+void qce_clear_bam_transaction(struct qce_device *qce);
#endif /* _DMA_H_ */
diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index 5476d4d30fae7eb72bbcbcdd7d8be7a76f6732c2..5cfd769a59a791a79da42e2a5b0554ad974f7631 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -109,17 +109,17 @@ static int qce_ahash_async_req_handle(struct crypto_async_request *async_req)
goto error_unmap_src;
}
- ret = qce_dma_prep_sgs(&qce->dma, req->src, rctx->src_nents,
- &rctx->result_sg, 1, qce_ahash_done, async_req);
+ ret = qce_start(async_req, tmpl->crypto_alg_type);
if (ret)
goto error_unmap_dst;
- qce_dma_issue_pending(&qce->dma);
-
- ret = qce_start(async_req, tmpl->crypto_alg_type);
+ ret = qce_dma_prep_sgs(&qce->dma, req->src, rctx->src_nents,
+ &rctx->result_sg, 1, qce_ahash_done, async_req);
if (ret)
goto error_terminate;
+ qce_dma_issue_pending(&qce->dma);
+
return 0;
error_terminate:
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index a9b59e68df4b6837805d45391f5a5fe43fd47709..b4ef3748fbb4dde542b0307f32d4c871b7c33ac2 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -142,18 +142,18 @@ qce_skcipher_async_req_handle(struct crypto_async_request *async_req)
src_nents = dst_nents - 1;
}
+ ret = qce_start(async_req, tmpl->crypto_alg_type);
+ if (ret)
+ goto error_unmap_src;
+
ret = qce_dma_prep_sgs(&qce->dma, rctx->src_sg, src_nents,
rctx->dst_sg, dst_nents,
qce_skcipher_done, async_req);
if (ret)
- goto error_unmap_src;
+ goto error_terminate;
qce_dma_issue_pending(&qce->dma);
- ret = qce_start(async_req, tmpl->crypto_alg_type);
- if (ret)
- goto error_terminate;
-
return 0;
error_terminate:
--
2.47.3
^ permalink raw reply related
* [PATCH v22 12/14] crypto: qce - Map crypto memory for DMA
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
As the first step in converting the driver to using DMA for register
I/O, let's map the crypto memory range.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/core.c | 23 ++++++++++++++++++++++-
drivers/crypto/qce/core.h | 6 ++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index aa4a0b17749081f1ad653424bc265ee81e348e15..4031b4516d6519fc5024bbbcc439500a7b3314b2 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -191,10 +191,19 @@ static void qce_cancel_work(void *data)
cancel_work_sync(work);
}
+static void qce_crypto_unmap_dma(void *data)
+{
+ struct qce_device *qce = data;
+
+ dma_unmap_resource(qce->dev, qce->base_dma, qce->dma_size,
+ DMA_BIDIRECTIONAL, 0);
+}
+
static int qce_crypto_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct qce_device *qce;
+ struct resource *res;
int ret;
qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL);
@@ -204,7 +213,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
qce->dev = dev;
platform_set_drvdata(pdev, qce);
- qce->base = devm_platform_ioremap_resource(pdev, 0);
+ qce->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
if (IS_ERR(qce->base))
return PTR_ERR(qce->base);
@@ -254,6 +263,18 @@ static int qce_crypto_probe(struct platform_device *pdev)
qce->async_req_enqueue = qce_async_request_enqueue;
qce->async_req_done = qce_async_request_done;
+ qce->dma_size = resource_size(res);
+ qce->base_dma = dma_map_resource(dev, res->start, qce->dma_size,
+ DMA_BIDIRECTIONAL, 0);
+ qce->base_phys = res->start;
+ ret = dma_mapping_error(dev, qce->base_dma);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(qce->dev, qce_crypto_unmap_dma, qce);
+ if (ret)
+ return ret;
+
return devm_qce_register_algs(qce);
}
diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index f092ce2d3b04a936a37805c20ac5ba78d8fdd2df..a80e12eac6c87e5321cce16c56a4bf5003474ef0 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -27,6 +27,9 @@
* @dma: pointer to dma data
* @burst_size: the crypto burst size
* @pipe_pair_id: which pipe pair id the device using
+ * @base_dma: base DMA address
+ * @base_phys: base physical address
+ * @dma_size: size of memory mapped for DMA
* @async_req_enqueue: invoked by every algorithm to enqueue a request
* @async_req_done: invoked by every algorithm to finish its request
*/
@@ -43,6 +46,9 @@ struct qce_device {
struct qce_dma_data dma;
int burst_size;
unsigned int pipe_pair_id;
+ dma_addr_t base_dma;
+ phys_addr_t base_phys;
+ size_t dma_size;
int (*async_req_enqueue)(struct qce_device *qce,
struct crypto_async_request *req);
void (*async_req_done)(struct qce_device *qce, int ret);
--
2.47.3
^ permalink raw reply related
* [PATCH v22 11/14] crypto: qce - Use existing devres APIs in devm_qce_dma_request()
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski,
Konrad Dybcio
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Switch to devm_kmalloc() and devm_dma_alloc_chan() in
devm_qce_dma_request(). This allows us to drop two labels and shrink the
function.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/dma.c | 37 +++++++++++--------------------------
1 file changed, 11 insertions(+), 26 deletions(-)
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index d60efb5c26d88f8b0259b1dccc8724d0f75571c6..26347e9fc078adede712722107e74958538accdf 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -12,49 +12,34 @@
#define QCE_IGNORE_BUF_SZ (2 * QCE_BAM_BURST_SIZE)
-static void qce_dma_release(void *data)
+static void qce_dma_terminate(void *data)
{
struct qce_dma_data *dma = data;
dmaengine_terminate_sync(dma->txchan);
dmaengine_terminate_sync(dma->rxchan);
- dma_release_channel(dma->txchan);
- dma_release_channel(dma->rxchan);
- kfree(dma->result_buf);
}
int devm_qce_dma_request(struct qce_device *qce)
{
struct qce_dma_data *dma = &qce->dma;
struct device *dev = qce->dev;
- int ret;
- dma->txchan = dma_request_chan(dev, "tx");
+ dma->result_buf = devm_kmalloc(dev, QCE_RESULT_BUF_SZ + QCE_IGNORE_BUF_SZ, GFP_KERNEL);
+ if (!dma->result_buf)
+ return -ENOMEM;
+
+ dma->txchan = devm_dma_request_chan(dev, "tx");
if (IS_ERR(dma->txchan))
return dev_err_probe(dev, PTR_ERR(dma->txchan),
"Failed to get TX DMA channel\n");
- dma->rxchan = dma_request_chan(dev, "rx");
- if (IS_ERR(dma->rxchan)) {
- ret = dev_err_probe(dev, PTR_ERR(dma->rxchan),
- "Failed to get RX DMA channel\n");
- goto error_rx;
- }
-
- dma->result_buf = kmalloc(QCE_RESULT_BUF_SZ + QCE_IGNORE_BUF_SZ,
- GFP_KERNEL);
- if (!dma->result_buf) {
- ret = -ENOMEM;
- goto error_nomem;
- }
-
- return devm_add_action_or_reset(dev, qce_dma_release, dma);
+ dma->rxchan = devm_dma_request_chan(dev, "rx");
+ if (IS_ERR(dma->rxchan))
+ return dev_err_probe(dev, PTR_ERR(dma->rxchan),
+ "Failed to get RX DMA channel\n");
-error_nomem:
- dma_release_channel(dma->rxchan);
-error_rx:
- dma_release_channel(dma->txchan);
- return ret;
+ return devm_add_action_or_reset(dev, qce_dma_terminate, dma);
}
struct scatterlist *
--
2.47.3
^ permalink raw reply related
* [PATCH v22 10/14] crypto: qce - Simplify arguments of devm_qce_dma_request()
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
This function can extract all the information it needs from struct
qce_device alone so simplify its arguments. This is done in preparation
for adding support for register I/O over DMA which will require
accessing even more fields from struct qce_device.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/core.c | 2 +-
drivers/crypto/qce/dma.c | 5 ++++-
drivers/crypto/qce/dma.h | 4 +++-
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index dd860435d2c47a608c82cc2686583a44ff96c889..aa4a0b17749081f1ad653424bc265ee81e348e15 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -237,7 +237,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = devm_qce_dma_request(qce->dev, &qce->dma);
+ ret = devm_qce_dma_request(qce);
if (ret)
return ret;
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index d1daa229361aa74da5d3d7bfe1bc8ab189761e38..d60efb5c26d88f8b0259b1dccc8724d0f75571c6 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -7,6 +7,7 @@
#include <linux/dmaengine.h>
#include <crypto/scatterwalk.h>
+#include "core.h"
#include "dma.h"
#define QCE_IGNORE_BUF_SZ (2 * QCE_BAM_BURST_SIZE)
@@ -22,8 +23,10 @@ static void qce_dma_release(void *data)
kfree(dma->result_buf);
}
-int devm_qce_dma_request(struct device *dev, struct qce_dma_data *dma)
+int devm_qce_dma_request(struct qce_device *qce)
{
+ struct qce_dma_data *dma = &qce->dma;
+ struct device *dev = qce->dev;
int ret;
dma->txchan = dma_request_chan(dev, "tx");
diff --git a/drivers/crypto/qce/dma.h b/drivers/crypto/qce/dma.h
index fc337c435cd14917bdfb99febcf9119275afdeba..483789d9fa98e79d1283de8297bf2fc2a773f3a7 100644
--- a/drivers/crypto/qce/dma.h
+++ b/drivers/crypto/qce/dma.h
@@ -8,6 +8,8 @@
#include <linux/dmaengine.h>
+struct qce_device;
+
/* maximum data transfer block size between BAM and CE */
#define QCE_BAM_BURST_SIZE 64
@@ -32,7 +34,7 @@ struct qce_dma_data {
struct qce_result_dump *result_buf;
};
-int devm_qce_dma_request(struct device *dev, struct qce_dma_data *dma);
+int devm_qce_dma_request(struct qce_device *qce);
int qce_dma_prep_sgs(struct qce_dma_data *dma, struct scatterlist *sg_in,
int in_ents, struct scatterlist *sg_out, int out_ents,
dma_async_tx_callback cb, void *cb_param);
--
2.47.3
^ permalink raw reply related
* [PATCH v22 09/14] crypto: qce - Remove unused ignore_buf
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
It's unclear what the purpose of this field is. It has been here since
the initial commit but without any explanation. The driver works fine
without it. We still keep allocating more space in the result buffer, we
just don't need to store its address. While at it: move the
QCE_IGNORE_BUF_SZ definition into dma.c as it's not used outside of this
compilation unit.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/dma.c | 4 ++--
drivers/crypto/qce/dma.h | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index 7ec9d72fd690fb17e03ade7efe3cc522fb47e1ac..d1daa229361aa74da5d3d7bfe1bc8ab189761e38 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -9,6 +9,8 @@
#include "dma.h"
+#define QCE_IGNORE_BUF_SZ (2 * QCE_BAM_BURST_SIZE)
+
static void qce_dma_release(void *data)
{
struct qce_dma_data *dma = data;
@@ -43,8 +45,6 @@ int devm_qce_dma_request(struct device *dev, struct qce_dma_data *dma)
goto error_nomem;
}
- dma->ignore_buf = dma->result_buf + QCE_RESULT_BUF_SZ;
-
return devm_add_action_or_reset(dev, qce_dma_release, dma);
error_nomem:
diff --git a/drivers/crypto/qce/dma.h b/drivers/crypto/qce/dma.h
index 31629185000e12242fa07c2cc08b95fcbd5d4b8c..fc337c435cd14917bdfb99febcf9119275afdeba 100644
--- a/drivers/crypto/qce/dma.h
+++ b/drivers/crypto/qce/dma.h
@@ -23,7 +23,6 @@ struct qce_result_dump {
u32 status2;
};
-#define QCE_IGNORE_BUF_SZ (2 * QCE_BAM_BURST_SIZE)
#define QCE_RESULT_BUF_SZ \
ALIGN(sizeof(struct qce_result_dump), QCE_BAM_BURST_SIZE)
@@ -31,7 +30,6 @@ struct qce_dma_data {
struct dma_chan *txchan;
struct dma_chan *rxchan;
struct qce_result_dump *result_buf;
- void *ignore_buf;
};
int devm_qce_dma_request(struct device *dev, struct qce_dma_data *dma);
--
2.47.3
^ permalink raw reply related
* [PATCH v22 08/14] crypto: qce - Include algapi.h in the core.h header
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The header defines a struct embedding struct crypto_queue whose size
needs to be known and which is defined in crypto/algapi.h. Move the
inclusion from core.c to core.h.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/core.c | 1 -
drivers/crypto/qce/core.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index b52a26ffff5ee733adcf4e8cf8bef75018dfa63e..dd860435d2c47a608c82cc2686583a44ff96c889 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -12,7 +12,6 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/types.h>
-#include <crypto/algapi.h>
#include <crypto/internal/hash.h>
#include "core.h"
diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index eb6fa7a8b64a81daf9ad5304a3ae4e5e597a70b8..f092ce2d3b04a936a37805c20ac5ba78d8fdd2df 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -8,6 +8,7 @@
#include <linux/mutex.h>
#include <linux/workqueue.h>
+#include <crypto/algapi.h>
#include "dma.h"
--
2.47.3
^ permalink raw reply related
* [PATCH v22 07/14] crypto: qce - Cancel work on device detach
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
The workqueue is setup in probe() but never cancelled on error or in
remove(). Set up a devres action to clean it up. We need to move the
initialization earlier as we don't want to cancel the work before any
outstanding DMA transfer is terminated. Make sure we do terminate all
transfers in qce_dma_release() devres action.
Fixes: eb7986e5e14d ("crypto: qce - convert tasklet to workqueue")
Closes: https://sashiko.dev/#/patchset/20260427-qcom-qce-cmd-descr-v16-0-945fd1cafbbc%40oss.qualcomm.com?part=7
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/crypto/qce/core.c | 13 ++++++++++++-
drivers/crypto/qce/dma.c | 2 ++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index ac74f69914d6175b39ccde43f16269570fbcf715..b52a26ffff5ee733adcf4e8cf8bef75018dfa63e 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -185,6 +185,13 @@ static int qce_check_version(struct qce_device *qce)
return 0;
}
+static void qce_cancel_work(void *data)
+{
+ struct work_struct *work = data;
+
+ cancel_work_sync(work);
+}
+
static int qce_crypto_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -226,6 +233,11 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret)
return ret;
+ INIT_WORK(&qce->done_work, qce_req_done_work);
+ ret = devm_add_action_or_reset(dev, qce_cancel_work, &qce->done_work);
+ if (ret)
+ return ret;
+
ret = devm_qce_dma_request(qce->dev, &qce->dma);
if (ret)
return ret;
@@ -238,7 +250,6 @@ static int qce_crypto_probe(struct platform_device *pdev)
if (ret)
return ret;
- INIT_WORK(&qce->done_work, qce_req_done_work);
crypto_init_queue(&qce->queue, QCE_QUEUE_LENGTH);
qce->async_req_enqueue = qce_async_request_enqueue;
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index 68cafd4741ad3d91906d39e817fc7873b028d498..7ec9d72fd690fb17e03ade7efe3cc522fb47e1ac 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -13,6 +13,8 @@ static void qce_dma_release(void *data)
{
struct qce_dma_data *dma = data;
+ dmaengine_terminate_sync(dma->txchan);
+ dmaengine_terminate_sync(dma->rxchan);
dma_release_channel(dma->txchan);
dma_release_channel(dma->rxchan);
kfree(dma->result_buf);
--
2.47.3
^ permalink raw reply related
* [PATCH v22 06/14] dmaengine: qcom: bam_dma: add support for BAM locking
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
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.
The dummy writes must be issued into a scratchpad register of the client
so provide a mechanism to communicate the right address via descriptor
metadata.
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/dma/qcom/bam_dma.c | 169 +++++++++++++++++++++++++++++++++++++--
include/linux/dma/qcom_bam_dma.h | 13 +++
2 files changed, 176 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index f3e713a5259c2c7c24cfdcec094814eb1202971a..edaa694eb9df06043c0a4294561c8ad594103e3f 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -28,11 +28,13 @@
#include <linux/clk.h>
#include <linux/device.h>
#include <linux/dma-mapping.h>
+#include <linux/dma/qcom_bam_dma.h>
#include <linux/dmaengine.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
+#include <linux/lockdep.h>
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_dma.h>
@@ -60,6 +62,8 @@ struct bam_desc_hw {
#define DESC_FLAG_EOB BIT(13)
#define DESC_FLAG_NWD BIT(12)
#define DESC_FLAG_CMD BIT(11)
+#define DESC_FLAG_LOCK BIT(10)
+#define DESC_FLAG_UNLOCK BIT(9)
struct bam_async_desc {
struct virt_dma_desc vd;
@@ -72,6 +76,11 @@ struct bam_async_desc {
struct bam_desc_hw *curr_desc;
+ /* BAM locking infrastructure */
+ bool is_lock_desc;
+ struct scatterlist lock_sg;
+ struct bam_cmd_element lock_ce;
+
/* list node for the desc in the bam_chan list of descriptors */
struct list_head desc_node;
enum dma_transfer_direction dir;
@@ -425,6 +434,11 @@ struct bam_chan {
struct list_head desc_list;
struct list_head node;
+
+ /* BAM locking infrastructure */
+ phys_addr_t lock_scratchpad_addr;
+ enum dma_transfer_direction direction;
+ bool bam_locked;
};
static inline struct bam_chan *to_bam_chan(struct dma_chan *common)
@@ -638,8 +652,10 @@ static void bam_free_chan(struct dma_chan *chan)
goto err;
}
- scoped_guard(spinlock_irqsave, &bchan->vc.lock)
+ scoped_guard(spinlock_irqsave, &bchan->vc.lock) {
bam_reset_channel(bchan);
+ bchan->bam_locked = false;
+ }
dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE, bchan->fifo_virt,
bchan->fifo_phys);
@@ -676,13 +692,24 @@ static void bam_free_chan(struct dma_chan *chan)
static int bam_slave_config(struct dma_chan *chan,
struct dma_slave_config *cfg)
{
+ struct bam_config *peripheral_cfg = cfg->peripheral_config;
struct bam_chan *bchan = to_bam_chan(chan);
+ const struct bam_device_data *bdata = bchan->bdev->dev_data;
guard(spinlock_irqsave)(&bchan->vc.lock);
memcpy(&bchan->slave, cfg, sizeof(*cfg));
bchan->reconfigure = 1;
+ /*
+ * This is required to setup the pipe locking and must be done even
+ * before the first call to bam_start_dma().
+ */
+ if (bdata->pipe_lock_supported && peripheral_cfg) {
+ bchan->lock_scratchpad_addr = peripheral_cfg->lock_scratchpad_addr;
+ bchan->direction = cfg->direction;
+ }
+
return 0;
}
@@ -802,6 +829,7 @@ static int bam_dma_terminate_all(struct dma_chan *chan)
}
vchan_get_all_descriptors(&bchan->vc, &head);
+ bchan->bam_locked = false;
}
vchan_dma_desc_free_list(&bchan->vc, &head);
@@ -859,6 +887,15 @@ static int bam_resume(struct dma_chan *chan)
return 0;
}
+static void bam_dma_free_lock_desc(struct virt_dma_desc *vd)
+{
+ struct bam_async_desc *async_desc = container_of(vd, struct bam_async_desc, vd);
+ struct dma_chan *chan = vd->tx.chan;
+
+ dma_unmap_sg(chan->slave, &async_desc->lock_sg, 1, DMA_TO_DEVICE);
+ kfree(async_desc);
+}
+
/**
* process_channel_irqs - processes the channel interrupts
* @bdev: bam controller
@@ -870,6 +907,7 @@ static u32 process_channel_irqs(struct bam_device *bdev)
{
u32 i, srcs, pipe_stts, offset, avail;
struct bam_async_desc *async_desc, *tmp;
+ struct bam_desc_hw *hdesc;
srcs = readl_relaxed(bam_addr(bdev, 0, BAM_IRQ_SRCS_EE));
@@ -919,13 +957,19 @@ static u32 process_channel_irqs(struct bam_device *bdev)
* push back to front of desc_issued so that
* it gets restarted by the work queue.
*/
+
+ list_del(&async_desc->desc_node);
if (!async_desc->num_desc) {
- vchan_cookie_complete(&async_desc->vd);
+ hdesc = async_desc->desc;
+
+ if (async_desc->is_lock_desc)
+ bam_dma_free_lock_desc(&async_desc->vd);
+ else
+ vchan_cookie_complete(&async_desc->vd);
} else {
list_add(&async_desc->vd.node,
&bchan->vc.desc_issued);
}
- list_del(&async_desc->desc_node);
}
}
@@ -1046,13 +1090,101 @@ static void bam_apply_new_config(struct bam_chan *bchan,
bchan->reconfigure = 0;
}
+static struct bam_async_desc *
+bam_make_lock_desc(struct bam_chan *bchan, unsigned long flag)
+{
+ struct bam_async_desc *async_desc;
+ struct bam_desc_hw *desc;
+ struct virt_dma_desc *vd;
+ struct virt_dma_chan *vc;
+ unsigned int mapped;
+
+ async_desc = kzalloc_flex(*async_desc, desc, 1, GFP_NOWAIT);
+ if (!async_desc) {
+ dev_err(bchan->bdev->dev, "failed to allocate the BAM lock descriptor\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ sg_init_table(&async_desc->lock_sg, 1);
+
+ async_desc->num_desc = 1;
+ async_desc->curr_desc = async_desc->desc;
+ async_desc->dir = DMA_MEM_TO_DEV;
+ async_desc->is_lock_desc = true;
+
+ desc = async_desc->desc;
+
+ bam_prep_ce_le32(&async_desc->lock_ce, bchan->lock_scratchpad_addr, BAM_WRITE_COMMAND, 0);
+ sg_set_buf(&async_desc->lock_sg, &async_desc->lock_ce, sizeof(async_desc->lock_ce));
+
+ mapped = dma_map_sg(bchan->bdev->dev, &async_desc->lock_sg, 1, DMA_TO_DEVICE);
+ if (!mapped) {
+ kfree(async_desc);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ desc->flags |= cpu_to_le16(DESC_FLAG_CMD | flag);
+ desc->addr = cpu_to_le32(sg_dma_address(&async_desc->lock_sg));
+ desc->size = cpu_to_le16(sizeof(struct bam_cmd_element));
+
+ vc = &bchan->vc;
+ vd = &async_desc->vd;
+
+ dma_async_tx_descriptor_init(&vd->tx, &vc->chan);
+ vd->tx.flags = DMA_PREP_CMD;
+ vd->tx_result.result = DMA_TRANS_NOERROR;
+ vd->tx_result.residue = 0;
+
+ return async_desc;
+}
+
+static int bam_setup_pipe_lock(struct bam_chan *bchan)
+{
+ const struct bam_device_data *bdata = bchan->bdev->dev_data;
+ struct bam_async_desc *lock_desc, *unlock_desc;
+
+ lockdep_assert_held(&bchan->vc.lock);
+
+ if (!bdata->pipe_lock_supported || !bchan->lock_scratchpad_addr ||
+ bchan->direction != DMA_MEM_TO_DEV)
+ return 0;
+
+ /*
+ * Allocate both the LOCK and the UNLOCK descriptors up-front so the
+ * operation is all-or-nothing: if either allocation fails we free both
+ * and run the sequence unlocked rather than leave the pipe locked with
+ * no matching UNLOCK.
+ *
+ * Both are queued in-band around the currently issued work: the LOCK is
+ * prepended so it enters the FIFO first, the UNLOCK is appended so it is
+ * the last descriptor of the sequence. They are loaded together with the
+ * payload in a single operation so the engine executes LOCK, the work
+ * and UNLOCK as one ordered batch.
+ */
+ lock_desc = bam_make_lock_desc(bchan, DESC_FLAG_LOCK);
+ if (IS_ERR(lock_desc))
+ return PTR_ERR(lock_desc);
+
+ unlock_desc = bam_make_lock_desc(bchan, DESC_FLAG_UNLOCK);
+ if (IS_ERR(unlock_desc)) {
+ bam_dma_free_lock_desc(&lock_desc->vd);
+ return PTR_ERR(unlock_desc);
+ }
+
+ list_add(&lock_desc->vd.node, &bchan->vc.desc_issued);
+ list_add_tail(&unlock_desc->vd.node, &bchan->vc.desc_issued);
+ bchan->bam_locked = true;
+
+ return 0;
+}
+
/**
* bam_start_dma - start next transaction
* @bchan: bam dma channel
*/
static void bam_start_dma(struct bam_chan *bchan)
{
- struct virt_dma_desc *vd = vchan_next_desc(&bchan->vc);
+ struct virt_dma_desc *vd;
struct bam_device *bdev = bchan->bdev;
struct bam_async_desc *async_desc = NULL;
struct bam_desc_hw *desc;
@@ -1064,6 +1196,7 @@ static void bam_start_dma(struct bam_chan *bchan)
lockdep_assert_held(&bchan->vc.lock);
+ vd = vchan_next_desc(&bchan->vc);
if (!vd)
return;
@@ -1072,6 +1205,22 @@ static void bam_start_dma(struct bam_chan *bchan)
return;
while (vd && !IS_BUSY(bchan)) {
+ /*
+ * Open a LOCK/UNLOCK bracket around each fresh sequence.
+ * Sentinels inserted by bam_setup_pipe_lock() are skipped: they
+ * already have bam_locked set and must not trigger a second pair.
+ */
+ if (!bchan->bam_locked) {
+ ret = bam_setup_pipe_lock(bchan);
+ if (ret) {
+ dev_err_ratelimited(bdev->dev,
+ "failed to setup the pipe lock, deferring transfer: %d\n");
+ break;
+ }
+ if (bchan->bam_locked)
+ vd = vchan_next_desc(&bchan->vc);
+ }
+
list_del(&vd->node);
async_desc = container_of(vd, struct bam_async_desc, vd);
@@ -1133,6 +1282,10 @@ static void bam_start_dma(struct bam_chan *bchan)
bchan->tail += async_desc->xfer_len;
bchan->tail %= MAX_DESCRIPTORS;
list_add_tail(&async_desc->desc_node, &bchan->desc_list);
+
+ if (async_desc->is_lock_desc &&
+ (le16_to_cpu(async_desc->desc->flags) & DESC_FLAG_UNLOCK))
+ bchan->bam_locked = false;
}
/* ensure descriptor writes and dma start not reordered */
@@ -1191,8 +1344,11 @@ static void bam_issue_pending(struct dma_chan *chan)
*/
static void bam_dma_free_desc(struct virt_dma_desc *vd)
{
- struct bam_async_desc *async_desc = container_of(vd,
- struct bam_async_desc, vd);
+ struct bam_async_desc *async_desc = container_of(vd, struct bam_async_desc, vd);
+ struct dma_chan *chan = vd->tx.chan;
+
+ if (async_desc->is_lock_desc)
+ dma_unmap_sg(chan->slave, &async_desc->lock_sg, 1, DMA_TO_DEVICE);
kfree(async_desc);
}
@@ -1384,6 +1540,7 @@ static int bam_dma_probe(struct platform_device *pdev)
bdev->common.device_terminate_all = bam_dma_terminate_all;
bdev->common.device_issue_pending = bam_issue_pending;
bdev->common.device_tx_status = bam_tx_status;
+ bdev->common.desc_metadata_modes = DESC_METADATA_CLIENT;
bdev->common.dev = bdev->dev;
ret = dma_async_device_register(&bdev->common);
diff --git a/include/linux/dma/qcom_bam_dma.h b/include/linux/dma/qcom_bam_dma.h
index 68fc0e643b1b97fe4520d5878daa322b81f4f559..30c0323851ed5201bd23f5382c700bef66ba789c 100644
--- a/include/linux/dma/qcom_bam_dma.h
+++ b/include/linux/dma/qcom_bam_dma.h
@@ -6,6 +6,8 @@
#ifndef _QCOM_BAM_DMA_H
#define _QCOM_BAM_DMA_H
+#include <linux/dmaengine.h>
+
#include <asm/byteorder.h>
/*
@@ -34,6 +36,17 @@ enum bam_command_type {
BAM_READ_COMMAND,
};
+/**
+ * struct bam_config - BAM DMA peripheral config.
+ *
+ * @lock_scratchpad_addr: Physical address to use for dummy write operations
+ * when queuing command descriptors with LOCK/UNLOCK
+ * bits set.
+ */
+struct bam_config {
+ phys_addr_t lock_scratchpad_addr;
+};
+
/*
* prep_bam_ce_le32 - Wrapper function to prepare a single BAM command
* element with the data already in le32 format.
--
2.47.3
^ permalink raw reply related
* [PATCH v22 05/14] dmaengine: qcom: bam_dma: Add pipe_lock_supported flag support
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski,
Dmitry Baryshkov
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Extend the device match data with a flag indicating whether the IP
supports the BAM lock/unlock feature. Set it to true on BAM IP versions
1.4.0 and above.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/dma/qcom/bam_dma.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 8ce0fe085c5fea6cc614edd692b5cfd264b94d5a..f3e713a5259c2c7c24cfdcec094814eb1202971a 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -115,6 +115,7 @@ struct reg_offset_data {
struct bam_device_data {
const struct reg_offset_data *reg_info;
+ bool pipe_lock_supported;
};
static const struct reg_offset_data bam_v1_3_reg_info[] = {
@@ -181,6 +182,7 @@ static const struct reg_offset_data bam_v1_4_reg_info[] = {
static const struct bam_device_data bam_v1_4_data = {
.reg_info = bam_v1_4_reg_info,
+ .pipe_lock_supported = true,
};
static const struct reg_offset_data bam_v1_7_reg_info[] = {
@@ -214,6 +216,7 @@ static const struct reg_offset_data bam_v1_7_reg_info[] = {
static const struct bam_device_data bam_v1_7_data = {
.reg_info = bam_v1_7_reg_info,
+ .pipe_lock_supported = true,
};
static const struct reg_offset_data bam_v2_0_reg_info[] = {
@@ -247,6 +250,7 @@ static const struct reg_offset_data bam_v2_0_reg_info[] = {
static const struct bam_device_data bam_v2_0_data = {
.reg_info = bam_v2_0_reg_info,
+ .pipe_lock_supported = true,
};
/* BAM CTRL */
--
2.47.3
^ permalink raw reply related
* [PATCH v22 04/14] dmaengine: qcom: bam_dma: Extend the driver's device match data
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
In preparation for supporting the pipe locking feature flag, extend the
amount of information we can carry in device match data: create a
separate structure and make the register information one of its fields.
This way, in subsequent patches, it will be just a matter of adding a
new field to the device data.
Reviewed-by: Dmitry Baryshkov <lumag@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/dma/qcom/bam_dma.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index ea3df28e777f99c0532761b6aee6807ab23ab4ca..8ce0fe085c5fea6cc614edd692b5cfd264b94d5a 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -113,6 +113,10 @@ struct reg_offset_data {
unsigned int pipe_mult, evnt_mult, ee_mult;
};
+struct bam_device_data {
+ const struct reg_offset_data *reg_info;
+};
+
static const struct reg_offset_data bam_v1_3_reg_info[] = {
[BAM_CTRL] = { 0x0F80, 0x00, 0x00, 0x00 },
[BAM_REVISION] = { 0x0F84, 0x00, 0x00, 0x00 },
@@ -142,6 +146,10 @@ static const struct reg_offset_data bam_v1_3_reg_info[] = {
[BAM_P_FIFO_SIZES] = { 0x1020, 0x00, 0x40, 0x00 },
};
+static const struct bam_device_data bam_v1_3_data = {
+ .reg_info = bam_v1_3_reg_info,
+};
+
static const struct reg_offset_data bam_v1_4_reg_info[] = {
[BAM_CTRL] = { 0x0000, 0x00, 0x00, 0x00 },
[BAM_REVISION] = { 0x0004, 0x00, 0x00, 0x00 },
@@ -171,6 +179,10 @@ static const struct reg_offset_data bam_v1_4_reg_info[] = {
[BAM_P_FIFO_SIZES] = { 0x1820, 0x00, 0x1000, 0x00 },
};
+static const struct bam_device_data bam_v1_4_data = {
+ .reg_info = bam_v1_4_reg_info,
+};
+
static const struct reg_offset_data bam_v1_7_reg_info[] = {
[BAM_CTRL] = { 0x00000, 0x00, 0x00, 0x00 },
[BAM_REVISION] = { 0x01000, 0x00, 0x00, 0x00 },
@@ -200,6 +212,10 @@ static const struct reg_offset_data bam_v1_7_reg_info[] = {
[BAM_P_FIFO_SIZES] = { 0x13820, 0x00, 0x1000, 0x00 },
};
+static const struct bam_device_data bam_v1_7_data = {
+ .reg_info = bam_v1_7_reg_info,
+};
+
static const struct reg_offset_data bam_v2_0_reg_info[] = {
[BAM_CTRL] = { 0x0000, 0x00, 0x00, 0x00 },
[BAM_REVISION] = { 0x1000, 0x00, 0x00, 0x00 },
@@ -229,6 +245,10 @@ static const struct reg_offset_data bam_v2_0_reg_info[] = {
[BAM_P_FIFO_SIZES] = { 0xC820, 0x00, 0x1000, 0x00 },
};
+static const struct bam_device_data bam_v2_0_data = {
+ .reg_info = bam_v2_0_reg_info,
+};
+
/* BAM CTRL */
#define BAM_SW_RST BIT(0)
#define BAM_EN BIT(1)
@@ -422,7 +442,7 @@ struct bam_device {
bool powered_remotely;
u32 active_channels;
- const struct reg_offset_data *layout;
+ const struct bam_device_data *dev_data;
struct clk *bamclk;
int irq;
@@ -440,7 +460,7 @@ struct bam_device {
static inline void __iomem *bam_addr(struct bam_device *bdev, u32 pipe,
enum bam_reg reg)
{
- const struct reg_offset_data r = bdev->layout[reg];
+ const struct reg_offset_data r = bdev->dev_data->reg_info[reg];
return bdev->regs + r.base_offset +
r.pipe_mult * pipe +
@@ -1234,10 +1254,10 @@ static void bam_channel_init(struct bam_device *bdev, struct bam_chan *bchan,
}
static const struct of_device_id bam_of_match[] = {
- { .compatible = "qcom,bam-v1.3.0", .data = &bam_v1_3_reg_info },
- { .compatible = "qcom,bam-v1.4.0", .data = &bam_v1_4_reg_info },
- { .compatible = "qcom,bam-v1.7.0", .data = &bam_v1_7_reg_info },
- { .compatible = "qcom,bam-v2.0.0", .data = &bam_v2_0_reg_info },
+ { .compatible = "qcom,bam-v1.3.0", .data = &bam_v1_3_data },
+ { .compatible = "qcom,bam-v1.4.0", .data = &bam_v1_4_data },
+ { .compatible = "qcom,bam-v1.7.0", .data = &bam_v1_7_data },
+ { .compatible = "qcom,bam-v2.0.0", .data = &bam_v2_0_data },
{}
};
@@ -1261,7 +1281,7 @@ static int bam_dma_probe(struct platform_device *pdev)
return -ENODEV;
}
- bdev->layout = match->data;
+ bdev->dev_data = match->data;
bdev->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(bdev->regs))
--
2.47.3
^ permalink raw reply related
* [PATCH v22 03/14] dmaengine: qcom: bam_dma: convert tasklet to a BH workqueue
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski,
Dmitry Baryshkov
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
BH workqueues are a modern mechanism, aiming to replace legacy tasklets.
Let's convert the BAM DMA driver to using the high-priority variant of
the BH workqueue.
[Vinod: suggested using the BG workqueue instead of the regular one
running in process context]
Suggested-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/dma/qcom/bam_dma.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index fc155e0d1870cbb7e099a2c4280f9f8fbdf6cf15..ea3df28e777f99c0532761b6aee6807ab23ab4ca 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -42,6 +42,7 @@
#include <linux/pm_runtime.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
+#include <linux/workqueue.h>
#include "../dmaengine.h"
#include "../virt-dma.h"
@@ -426,8 +427,8 @@ struct bam_device {
struct clk *bamclk;
int irq;
- /* dma start transaction tasklet */
- struct tasklet_struct task;
+ /* dma start transaction workqueue */
+ struct work_struct work;
};
/**
@@ -892,7 +893,7 @@ static u32 process_channel_irqs(struct bam_device *bdev)
/*
* if complete, process cookie. Otherwise
* push back to front of desc_issued so that
- * it gets restarted by the tasklet
+ * it gets restarted by the work queue.
*/
if (!async_desc->num_desc) {
vchan_cookie_complete(&async_desc->vd);
@@ -922,9 +923,9 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
srcs |= process_channel_irqs(bdev);
- /* kick off tasklet to start next dma transfer */
+ /* kick off the work queue to start next dma transfer */
if (srcs & P_IRQ)
- tasklet_schedule(&bdev->task);
+ queue_work(system_bh_highpri_wq, &bdev->work);
ret = pm_runtime_get_sync(bdev->dev);
if (ret < 0)
@@ -1120,14 +1121,14 @@ static void bam_start_dma(struct bam_chan *bchan)
}
/**
- * dma_tasklet - DMA IRQ tasklet
- * @t: tasklet argument (bam controller structure)
+ * bam_dma_work() - DMA interrupt work queue callback
+ * @work: work queue struct embedded in the BAM controller device struct
*
* Sets up next DMA operation and then processes all completed transactions
*/
-static void dma_tasklet(struct tasklet_struct *t)
+static void bam_dma_work(struct work_struct *work)
{
- struct bam_device *bdev = from_tasklet(bdev, t, task);
+ struct bam_device *bdev = from_work(bdev, work, work);
struct bam_chan *bchan;
unsigned int i;
@@ -1140,14 +1141,13 @@ static void dma_tasklet(struct tasklet_struct *t)
if (!list_empty(&bchan->vc.desc_issued) && !IS_BUSY(bchan))
bam_start_dma(bchan);
}
-
}
/**
* bam_issue_pending - starts pending transactions
* @chan: dma channel
*
- * Calls tasklet directly which in turn starts any pending transactions
+ * Calls work queue directly which in turn starts any pending transactions
*/
static void bam_issue_pending(struct dma_chan *chan)
{
@@ -1316,14 +1316,14 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_disable_clk;
- tasklet_setup(&bdev->task, dma_tasklet);
+ INIT_WORK(&bdev->work, bam_dma_work);
bdev->channels = devm_kcalloc(bdev->dev, bdev->num_channels,
sizeof(*bdev->channels), GFP_KERNEL);
if (!bdev->channels) {
ret = -ENOMEM;
- goto err_tasklet_kill;
+ goto err_workqueue_cancel;
}
/* allocate and initialize channels */
@@ -1389,8 +1389,8 @@ static int bam_dma_probe(struct platform_device *pdev)
err_bam_channel_exit:
for (i = 0; i < bdev->num_channels; i++)
tasklet_kill(&bdev->channels[i].vc.task);
-err_tasklet_kill:
- tasklet_kill(&bdev->task);
+err_workqueue_cancel:
+ cancel_work_sync(&bdev->work);
err_disable_clk:
clk_disable_unprepare(bdev->bamclk);
@@ -1424,7 +1424,7 @@ static void bam_dma_remove(struct platform_device *pdev)
bdev->channels[i].fifo_phys);
}
- tasklet_kill(&bdev->task);
+ cancel_work_sync(&bdev->work);
clk_disable_unprepare(bdev->bamclk);
}
--
2.47.3
^ permalink raw reply related
* [PATCH v22 02/14] dmaengine: qcom: bam_dma: free interrupt before the clock in error path
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
The BAM interrupt is requested with a devres helper and so on error it's
freed after probe() returns. We disable the clock before freeing or
masking it so it may still fire and we may end up reading BAM registers
with clock disabled.
Stop using devres for interrupts as we free it in remove() manually
anyway. Add an appropriate label and free the interrupt before disabling
the clock in error path and in remove().
Fixes: e7c0fe2a5c84 ("dmaengine: add Qualcomm BAM dma driver")
Closes: https://sashiko.dev/#/patchset/20260427-qcom-qce-cmd-descr-v16-0-945fd1cafbbc%40oss.qualcomm.com?part=2
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/dma/qcom/bam_dma.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 1bb26af0405f3a16f97e0d4b86c945c252d97f57..fc155e0d1870cbb7e099a2c4280f9f8fbdf6cf15 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -1332,8 +1332,7 @@ static int bam_dma_probe(struct platform_device *pdev)
for (i = 0; i < bdev->num_channels; i++)
bam_channel_init(bdev, &bdev->channels[i], i);
- ret = devm_request_irq(bdev->dev, bdev->irq, bam_dma_irq,
- IRQF_TRIGGER_HIGH, "bam_dma", bdev);
+ ret = request_irq(bdev->irq, bam_dma_irq, IRQF_TRIGGER_HIGH, "bam_dma", bdev);
if (ret)
goto err_bam_channel_exit;
@@ -1366,7 +1365,7 @@ static int bam_dma_probe(struct platform_device *pdev)
ret = dma_async_device_register(&bdev->common);
if (ret) {
dev_err(bdev->dev, "failed to register dma async device\n");
- goto err_bam_channel_exit;
+ goto err_free_irq;
}
ret = of_dma_controller_register(pdev->dev.of_node, bam_dma_xlate,
@@ -1385,6 +1384,8 @@ static int bam_dma_probe(struct platform_device *pdev)
err_unregister_dma:
dma_async_device_unregister(&bdev->common);
+err_free_irq:
+ free_irq(bdev->irq, bdev);
err_bam_channel_exit:
for (i = 0; i < bdev->num_channels; i++)
tasklet_kill(&bdev->channels[i].vc.task);
@@ -1401,6 +1402,8 @@ static void bam_dma_remove(struct platform_device *pdev)
struct bam_device *bdev = platform_get_drvdata(pdev);
u32 i;
+ free_irq(bdev->irq, bdev);
+
pm_runtime_force_suspend(&pdev->dev);
of_dma_controller_free(pdev->dev.of_node);
@@ -1409,8 +1412,6 @@ static void bam_dma_remove(struct platform_device *pdev)
/* mask all interrupts for this execution environment */
writel_relaxed(0, bam_addr(bdev, 0, BAM_IRQ_SRCS_MSK_EE));
- devm_free_irq(bdev->dev, bdev->irq, bdev);
-
for (i = 0; i < bdev->num_channels; i++) {
bam_dma_terminate_all(&bdev->channels[i].vc.chan);
tasklet_kill(&bdev->channels[i].vc.task);
--
2.47.3
^ permalink raw reply related
* [PATCH v22 01/14] dmaengine: constify struct dma_descriptor_metadata_ops
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski,
Radhey Shyam Pandey
In-Reply-To: <20260717-qcom-qce-cmd-descr-v22-0-ff0d18b27246@oss.qualcomm.com>
There's no reason for the instances of this struct to be modifiable.
Constify the pointer in struct dma_async_tx_descriptor and all drivers
currently using it.
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/dma/ti/k3-udma.c | 2 +-
drivers/dma/xilinx/xilinx_dma.c | 2 +-
include/linux/dmaengine.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index 1cf158eb7bdb541c4e7f4f79f65ab70be4311fad..fb21e0df5ab7b20e4e16777b5ff7f61d2ae67b2b 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -3408,7 +3408,7 @@ static int udma_set_metadata_len(struct dma_async_tx_descriptor *desc,
return 0;
}
-static struct dma_descriptor_metadata_ops metadata_ops = {
+static const struct dma_descriptor_metadata_ops metadata_ops = {
.attach = udma_attach_metadata,
.get_ptr = udma_get_metadata_ptr,
.set_len = udma_set_metadata_len,
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 404235c1735384635597e88edc25c67c7d250647..165b11a7c776abc6a8d66d631e19da669644577d 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -653,7 +653,7 @@ static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
return seg->hw.app;
}
-static struct dma_descriptor_metadata_ops xilinx_dma_metadata_ops = {
+static const struct dma_descriptor_metadata_ops xilinx_dma_metadata_ops = {
.get_ptr = xilinx_dma_get_metadata_ptr,
};
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index b3d251c9734e95e1b75cf6763d4d2c3a1c6a9910..5244edb90e7e7510bf4460b6a74ee2a7f91c1ccc 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -623,7 +623,7 @@ struct dma_async_tx_descriptor {
void *callback_param;
struct dmaengine_unmap_data *unmap;
enum dma_desc_metadata_mode desc_metadata_mode;
- struct dma_descriptor_metadata_ops *metadata_ops;
+ const struct dma_descriptor_metadata_ops *metadata_ops;
#ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
struct dma_async_tx_descriptor *next;
struct dma_async_tx_descriptor *parent;
--
2.47.3
^ permalink raw reply related
* [PATCH v22 00/14] crypto/dmaengine: qce: introduce BAM locking and use DMA for register I/O
From: Bartosz Golaszewski @ 2026-07-17 8:36 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Stephan Gerhold, Bjorn Andersson,
Mukesh Kumar Savaliya, Peter Ujfalusi, Michal Simek, Frank Li,
Andy Gross, Neil Armstrong, Vignesh Raghavendra
Cc: dmaengine, linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski,
Radhey Shyam Pandey, Dmitry Baryshkov, Konrad Dybcio
Another round of issues pointed out by Stephan and Sashiko. Stephan: if
you consider this iteration satisfactory, I would appreciate any
Tested-by and Reviewed/Acked-by you could provide in order to help move
this forward.
Merging strategy: there are build-time dependencies between the crypto
and DMA patches so the best approach is for Vinod to create an immutable
branch with the DMA part pulled in by the crypto tree.
Currently the QCE crypto driver accesses the crypto engine registers
directly via CPU. Trust Zone may perform crypto operations simultaneously
resulting in a race condition. To remedy that, let's introduce support
for BAM locking/unlocking to the driver. The BAM driver will now wrap
any existing issued descriptor chains with additional descriptors
performing the locking when the client starts the transaction
(dmaengine_issue_pending()). The client wanting to profit from locking
needs to switch to performing register I/O over DMA and communicate the
address to which to perform the dummy writes via a call to
dmaengine_desc_attach_metadata().
In the specific case of the BAM DMA this translates to sending command
descriptors performing dummy writes with the relevant flags set. The BAM
will then lock all other pipes not related to the current pipe group, and
keep handling the current pipe only until it sees the the unlock bit.
In order for the locking to work correctly, we also need to switch to
using DMA for all register I/O.
On top of this, the series contains some additional tweaks and
refactoring.
The goal of this is not to improve the performance but to prepare the
driver for supporting decryption into secure buffers in the future.
Tested with tcrypt.ko, kcapi and cryptsetup.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v22:
- Switch back to passing the QCE scratchpad register address to the BAM
DMA driver using the slave config structure but this time provide a
dedicated peripheral config structure
- Rename the scratchpad_addr field to lock_scratchpad_addr
- Remove leftover local variables
- Use the BAM struct device for DMA mapping instead of the QCE device
- Use cpu_to_le32() where needed
- Don't silently queue data if setting up the locks failed, defer
transfer and emit an error message
- Remove redundant is_lock_desc check in bam_start_dma()
- Link to v21: https://patch.msgid.link/20260713-qcom-qce-cmd-descr-v21-0-bc2583e18475@oss.qualcomm.com
Changes in v21:
- Fix a potential race with new descriptors submitted while hardware is
processing a locked sequence by clearing the bam_locked state right
after queueing the UNLOCK descriptor
- Link to v20: https://patch.msgid.link/20260629-qcom-qce-cmd-descr-v20-0-56f67da84c05@oss.qualcomm.com
Changes in v20:
- Don't use DMA cookies for LOCK/UNLOCK descriptors as this leads to
dmaengine state corruption
- Handle re-scheduling of a DMA transaction on full FIFO
- Fix DMA descriptor leak in qce_submit_cmd_desc()
- Link to v19: https://patch.msgid.link/20260526-qcom-qce-cmd-descr-v19-0-08472fdcbf4a@oss.qualcomm.com
Changes in v19:
- Fix more potential issues in remove path (sashiko)
- Remove unneeded return value check for vchan_tx_prep() as it can never
fail
- Link to v18: https://patch.msgid.link/20260522-qcom-qce-cmd-descr-v18-0-99103926bafc@oss.qualcomm.com
Changes in v18:
- Free the BAM interrupt before disabling the clock in remove() path too
- convert the size assigned to command descriptors to little endian
- don't pass DMA mapping attributes to dma_map_sg() in bam_dma when
setting up command descriptors
- Cancel the QCE workqueue *after* any outstanding DMA transfer
completes
- When mapping the scatterlist for command descriptors: use the actual
number of mapped segments for dmaengine_prep_slave_sg()
- Drop the leftover read_buf field from struct qce_device
- Unmap command descriptors only after terminating the RX transfer
- Pass the actual size of the metadata struct to
dmaengine_desc_attach_metadata(), this is not really required for our
use-case but let's do this for correctness and make sashiko happy
- Drop double assignment of bam_ce_idx in qce_clear_bam_transaction()
- Remove unused QCE_MAX_REG_READ
- Link to v17: https://patch.msgid.link/20260519-qcom-qce-cmd-descr-v17-0-53a595414b79@oss.qualcomm.com
Changes in v17:
- New patch: free the interrupt before disabling the clock in error path
in probe()
- New patch: cancel the QCE work on device detach
- Hold the channel lock when attaching the metadata
- Reorder the operations in devm_qce_dma_request() to avoid freeing
memory that may still be used by the DMA channel
- Register algorithms as the last step in QCE's probe() to avoid making
the resources available to the system before the DMA is fully set up
- Fix error paths in algo request handlers
- Don't pass dmaengine attributes to map_sg_attrs() as it expects
dma-mapping attribute flags
- Fix a dma mapping leak for command descriptors
- Rebase on top of v7.1-rc4
- Link to v16: https://patch.msgid.link/20260427-qcom-qce-cmd-descr-v16-0-945fd1cafbbc@oss.qualcomm.com
Changes in v16:
- Fix a reported race between dma_map_sg() called with spinlock taken
and the corresponding dma_unmap_sg() called without it by moving the
descriptor locking data into the descriptor struct
- Also queue the TX data descriptors before the command descriptors to
match what downstream is doing
- Tweak commit messages
- Rebase on top of v7.1-rc1
- Link to v15: https://patch.msgid.link/20260402-qcom-qce-cmd-descr-v15-0-98b5361f7ed7@oss.qualcomm.com
Changes in v15:
- Extend the descriptor metadata struct to also carry the channel's
transfer direction and stop using dmaengine_slave_config() for that
- Link to v14: https://patch.msgid.link/20260323-qcom-qce-cmd-descr-v14-0-f323af411274@oss.qualcomm.com
Changes in v14:
- Don't return an error to a client which wants to use locking on BAM
that doesn't support it
- Add a comment describing the DMA descriptor metadata structure
- Fix memory leaks
- Remove leftovers from previous iterations
- Propagate errors from dma_cookie_assign() when setting up lock
descriptors
- Link to v13: https://patch.msgid.link/20260317-qcom-qce-cmd-descr-v13-0-0968eb4f8c40@oss.qualcomm.com
Changes in v13:
- As part of the DMA changes in the QCE driver: reverse the order of
queueing the descriptors in the QCE driver: queue command descriptors
with all the register writes first, followed by all the data descriptors,
this is in line with the recommandations from the BAM HPG
- Set the NWD (notify-when-done) bit (DMA_PREP_FENCE in dmaengine
parlance) on the data descriptors to ensure that the UNLOCK descriptor
will not be processed until after they have been processed by the
engine. While technically the NWD bit is only needed on the final data
descriptor, it's hard to tell which one *will* be the last from the
driver's point-of-view and both the downstream driver as well as
the Qualcomm TZ against which we want to synchronize sets NWD on every
data descriptor,
- Revert to creating the LOCK/UNLOCK command descriptor pair in one
place now that the NWD bit is in place,
- Link to v12: https://patch.msgid.link/20260310-qcom-qce-cmd-descr-v12-0-398f37f26ef0@oss.qualcomm.com
Changes in v12:
- Wait until the transaction is done before queueing the UNLOCK command
descriptor
- Use descriptor metadata for communicating the scratchpad address to
the BAM driver
- To that end: reverse the order of the series (first BAM, then QCE) to
maintain bisectability
- Unmap buffers used for dummy writes after the transaction
- Link to v11: https://patch.msgid.link/20260302-qcom-qce-cmd-descr-v11-0-4bf1f5db4802@oss.qualcomm.com
Changes in v11:
- Use new approach, not requiring the client to be involved in locking.
- Add a patch constifying dma_descriptor_metadata_ops
- Rebase on top of v7.0-rc1
- Link to v10: https://lore.kernel.org/r/20251219-qcom-qce-cmd-descr-v10-0-ff7e4bf7dad4@oss.qualcomm.com
Changes in v10:
- Move DESC_FLAG_(UN)LOCK BIT definitions from patch 2 to 3
- Add a patch constifying the dma engine metadata as the first in the
series
- Use the VERSION register for dummy lock/unlock writes
- Link to v9: https://lore.kernel.org/r/20251128-qcom-qce-cmd-descr-v9-0-9a5f72b89722@linaro.org
Changes in v9:
- Drop the global, generic LOCK/UNLOCK flags and instead use DMA
descriptor metadata ops to pass BAM-specific information from the QCE
to the DMA engine
- Link to v8: https://lore.kernel.org/r/20251106-qcom-qce-cmd-descr-v8-0-ecddca23ca26@linaro.org
Changes in v8:
- Rework the command descriptor logic and drop a lot of unneeded code
- Use the physical address for BAM command descriptor access, not the
mapped DMA address
- Fix the problems with iommu faults on newer platforms
- Generalize the LOCK/UNLOCK flags in dmaengine and reword the docs and
commit messages
- Make the BAM locking logic stricter in the DMA engine driver
- Add some additional minor QCE driver refactoring changes to the series
- Lots of small reworks and tweaks to rebase on current mainline and fix
previous issues
- Link to v7: https://lore.kernel.org/all/20250311-qce-cmd-descr-v7-0-db613f5d9c9f@linaro.org/
Changes in v7:
- remove unused code: writing to multiple registers was not used in v6,
neither were the functions for reading registers over BAM DMA-
- remove
- don't read the SW_VERSION register needlessly in the BAM driver,
instead: encode the information on whether the IP supports BAM locking
in device match data
- shrink code where possible with logic modifications (for instance:
change the implementation of qce_write() instead of replacing it
everywhere with a new symbol)
- remove duplicated error messages
- rework commit messages
- a lot of shuffling code around for easier review and a more
streamlined series
- Link to v6: https://lore.kernel.org/all/20250115103004.3350561-1-quic_mdalam@quicinc.com/
Changes in v6:
- change "BAM" to "DMA"
- Ensured this series is compilable with the current Linux-next tip of
the tree (TOT).
Changes in v5:
- Added DMA_PREP_LOCK and DMA_PREP_UNLOCK flag support in separate patch
- Removed DMA_PREP_LOCK & DMA_PREP_UNLOCK flag
- Added FIELD_GET and GENMASK macro to extract major and minor version
Changes in v4:
- Added feature description and test hardware
with test command
- Fixed patch version numbering
- Dropped dt-binding patch
- Dropped device tree changes
- Added BAM_SW_VERSION register read
- Handled the error path for the api dma_map_resource()
in probe
- updated the commit messages for batter redability
- Squash the change where qce_bam_acquire_lock() and
qce_bam_release_lock() api got introduce to the change where
the lock/unlock flag get introced
- changed cover letter subject heading to
"dmaengine: qcom: bam_dma: add cmd descriptor support"
- Added the very initial post for BAM lock/unlock patch link
as v1 to track this feature
Changes in v3:
- https://lore.kernel.org/lkml/183d4f5e-e00a-8ef6-a589-f5704bc83d4a@quicinc.com/
- Addressed all the comments from v2
- Added the dt-binding
- Fix alignment issue
- Removed type casting from qce_write_reg_dma()
and qce_read_reg_dma()
- Removed qce_bam_txn = dma->qce_bam_txn; line from
qce_alloc_bam_txn() api and directly returning
dma->qce_bam_txn
Changes in v2:
- https://lore.kernel.org/lkml/20231214114239.2635325-1-quic_mdalam@quicinc.com/
- Initial set of patches for cmd descriptor support
- Add client driver to use BAM lock/unlock feature
- Added register read/write via BAM in QCE Crypto driver
to use BAM lock/unlock feature
---
Bartosz Golaszewski (14):
dmaengine: constify struct dma_descriptor_metadata_ops
dmaengine: qcom: bam_dma: free interrupt before the clock in error path
dmaengine: qcom: bam_dma: convert tasklet to a BH workqueue
dmaengine: qcom: bam_dma: Extend the driver's device match data
dmaengine: qcom: bam_dma: Add pipe_lock_supported flag support
dmaengine: qcom: bam_dma: add support for BAM locking
crypto: qce - Cancel work on device detach
crypto: qce - Include algapi.h in the core.h header
crypto: qce - Remove unused ignore_buf
crypto: qce - Simplify arguments of devm_qce_dma_request()
crypto: qce - Use existing devres APIs in devm_qce_dma_request()
crypto: qce - Map crypto memory for DMA
crypto: qce - Add BAM DMA support for crypto register I/O
crypto: qce - Communicate the base physical address to the dmaengine
drivers/crypto/qce/aead.c | 10 +-
drivers/crypto/qce/common.c | 20 ++--
drivers/crypto/qce/core.c | 41 ++++++-
drivers/crypto/qce/core.h | 7 ++
drivers/crypto/qce/dma.c | 175 ++++++++++++++++++++++-----
drivers/crypto/qce/dma.h | 11 +-
drivers/crypto/qce/sha.c | 10 +-
drivers/crypto/qce/skcipher.c | 10 +-
drivers/dma/qcom/bam_dma.c | 250 +++++++++++++++++++++++++++++++++------
drivers/dma/ti/k3-udma.c | 2 +-
drivers/dma/xilinx/xilinx_dma.c | 2 +-
include/linux/dma/qcom_bam_dma.h | 13 ++
include/linux/dmaengine.h | 2 +-
13 files changed, 453 insertions(+), 100 deletions(-)
---
base-commit: 9c2d57db8bee1d5603f98c1a5a510c2114fb0d26
change-id: 20251103-qcom-qce-cmd-descr-c5e9b11fe609
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 2/2] mm/zswap: Support batch writeback in shrink_memcg()
From: Hao Jia @ 2026-07-17 8:29 UTC (permalink / raw)
To: Nhat Pham
Cc: Yosry Ahmed, akpm, tj, hannes, shakeel.butt, mhocko, mkoutny,
chengming.zhou, muchun.song, roman.gushchin, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <CAKEwX=Mdi5au5aXWdx3C7d1N4_3o2uiJh2sZ=sf6kpKAdtD1zQ@mail.gmail.com>
On 2026/7/17 02:04, Nhat Pham wrote:
> On Wed, Jul 15, 2026 at 7:54 PM Hao Jia <jiahao.kernel@gmail.com> wrote:
>>
>>
>>
>> On 2026/7/16 00:14, Yosry Ahmed wrote:
>>>>>> Test Setup:
>>>>>> Total memory: 32 GB.
>>>>>> zswap settings: max_pool_percent=1, accept_threshold_percent=50,
>>>>>> shrinker_enabled=N.
>>>>>> Allocate 512MB of anonymous pages and fill them with random data (to avoid
>>>>>> compression), then use cgroup memory.reclaim to force a large amount of
>>>>>> anonymous pages into zswap. At an interval of 2ms, allocate a 4K anonymous
>>>>>> page where the first 4 bytes are random numbers and the rest are zeros, and
>>>>>> then trigger a reclamation of this 4K anonymous page through cgroup
>>>>>> memory.reclaim. When the pool threshold is reached, shrink_memcg() will
>>>>>> be triggered.
>>>>>>
>>>>>> The test data after running for 120s is as follows:
>>>>>> Baseline Patched
>>>>>> shrink_worker wakeups 5363 85
>>>>>> shrink_memcg calls 11,345,012 188,264
>>>>>> written_back 40214 40275
>>>>>>
>>>>>> Conclusion:
>>>>>> Under the same workload and run duration, the patched kernel shows a
>>>>>> significant reduction in both shrink_worker wakeups and shrink_memcg calls.
>>>>>
>>>>> Please also include data from the case where zswap store failures are
>>>>> observed and pages go to disk, and compare before and after this
>>>>> patch. I think that part is also really important.
>>>>
>>>> I retested and added some collected information. Perhaps
>>>> `pool_limit_hit` and `pswpout` can explain that batch shrinking of zswap
>>>> can reduce the number of pages that fail to be stored due to the pool
>>>> limit, allowing zswap to skip zswap and go directly to disk.
>>>
>>> Oh I meant the other test case with high memory pressure where we saw
>>> a lot more writeback with this patch. Do you have similar data from
>>> that test case?
>>>
>>
>> Below are the results from stress-ng (high memory pressure case), which
>> lead to a similar conclusion:
>>
>> Baseline Patched
>> shrink_worker wakeups 5,640 987
>> shrink_memcg calls 8,481,500 2,504,818
>> written_back pages 260 768,576
>> zswap_store calls 2,742,756 2,301,414
>> store succeeded (ret=1) 934,640 1,308,686
>> store rejected (ret=0) 1,808,116 992,728 <-
>> store reject rate ~65% ~43%
>> pool_limit_hit delta 1,181,310 101,593 <-
>> pswpout 1,808,376 1,761,304
>> pswpin 4,288,497 3,902,658
>>
>>
>> pswpout = store rejected + written_back pages
>> Note that pswpout comprises two parts: the store rejected count (where
>> we skip zswap and go directly to disk) and the number of pages written
>> back by shrink_memcg().
>>
>> Therefore, we can directly leverage store rejected to evaluate the
>> scenario where zswap is skipped and pages bypass it to disk. This
>> provides a much clearer picture than looking at pswpout alone.
>>
>> Do we need to include both sets of test results along with the
>> comparison data in the final commit message?
>
> I mean, it helps your case no? :)
Yes, I will add both sets of test results to the commit message in the
next version.
>
> But nicely done. My understanding is, this batching of the global
> shrinker reduces overall disk swap fallback, correct?
>
> Other than Yosry's comment, the rest LGTM FWIW :) Thanks for fixing this, Hao!
Thanks for the review, Nhat!
Thanks,
Hao
^ permalink raw reply
* Re: [PATCH] crypto: fix bracket
From: Herbert Xu @ 2026-07-17 8:15 UTC (permalink / raw)
To: Manuel Ebner
Cc: David S. Miller, Jonathan Corbet, Shuah Khan, Stephan Mueller,
linux-crypto, linux-doc, linux-kernel
In-Reply-To: <20260709143837.405758-2-manuelebner@mailbox.org>
On Thu, Jul 09, 2026 at 04:38:37PM +0200, Manuel Ebner wrote:
> Remove needless ')' from code block.
>
> Fixes: 3b72c814a8e8 ("crypto: doc - convert crypto API documentation to Sphinx")
> Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
> ---
> Documentation/crypto/architecture.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v21 06/14] dmaengine: qcom: bam_dma: add support for BAM locking
From: Bartosz Golaszewski @ 2026-07-17 8:08 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Vinod Koul, Jonathan Corbet, Thara Gopinath, Herbert Xu,
David S. Miller, Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Bjorn Andersson, Peter Ujfalusi,
Michal Simek, Frank Li, Andy Gross, Neil Armstrong, dmaengine,
linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, brgl, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <alYGGu7_3G6mJAzQ@linaro.org>
On Tue, 14 Jul 2026 11:49:14 +0200, Stephan Gerhold
<stephan.gerhold@linaro.org> said:
>
>> +
>> /**
>> * bam_prep_slave_sg - Prep slave sg transaction
>> *
>> [...]
>> @@ -802,6 +851,7 @@ static int bam_dma_terminate_all(struct dma_chan *chan)
>> }
>>
>> vchan_get_all_descriptors(&bchan->vc, &head);
>> + bchan->bam_locked = false;
>
> I wonder about the implications of this. If the LOCK descriptor was
> already processed, will we cause a deadlock if we never submit the
> UNLOCK descriptor? Or I guess bam_reset_channel() might reset the lock
> as well?
>
There's nothing in the HPG that would imply it but I instrumented a test where
I excercised this situation and QCE was not locked up in any way. If there
were more execution environments doing the same, I'm not sure but this series
was tested by the NAND team and they didn't notice anything.
Bart
^ permalink raw reply
* RE: [External Mail] Re: [PATCH v4 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports
From: Wu. JackBB (GSM) @ 2026-07-17 7:26 UTC (permalink / raw)
To: Simon Horman
Cc: loic.poulain@oss.qualcomm.com, ryazanov.s.a@gmail.com,
johannes@sipsolutions.net, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, wen-zhi.huang@mediatek.com,
shi-wei.yeh@mediatek.com, Minano.tseng@mediatek.com,
matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com,
corbet@lwn.net, skhan@linuxfoundation.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20260716093429.231132-1-horms@kernel.org>
Hi Simon,
On ... sashiko wrote:
> > +static int mtk_port_wwan_write(...)
> > + if (ret < 0)
> > + return ret;
> > +
> > + consume_skb(skb);
> > + return 0;
>
> [Severity: High]
> If mtk_port_common_write() successfully sends the first few fragments
> but fails on subsequent ones ... does this code silently drop the
> remaining data?
This is a design limitation of the WWAN port API:
wwan_port_op_tx returns 0 for success or negative for error —
there is no mechanism to report partial writes back to the WWAN
core. The already-submitted fragments cannot be recalled from
the DMA engine. For the AT/MBIM control ports, messages are
small (typically under 1KB, within a single MTU). The
multi-fragment path is rarely exercised for control plane
traffic.
> > +static int mtk_port_wwan_write_blocking(...)
> > + if (ret < 0)
> > + return ret;
> > +
> > + consume_skb(skb);
> > + return 0;
>
> [Severity: High]
> Can the same silent drop of partially written SKBs happen here?
Same reasoning as above. The WWAN port API does not support
partial write reporting, and control plane messages are
typically within a single MTU.
> > +static int mtk_port_wwan_enable(...)
> > + port->w_priv.w_port = wwan_create_port(...);
> > + if (IS_ERR(port->w_priv.w_port)) {
>
> [Severity: High]
> Can a race condition occur here if wwan_create_port() fails?
mtk_port_wwan_enable() is called from the FSM thread during
the handshake sequence, before the port starts receiving data.
The CLDMA RX queue for this port has not been opened at this
point — RX data only arrives after the modem completes its
handshake. The RX path cannot observe the error pointer.
During teardown, mtk_port_wwan_disable() holds w_lock when
clearing w_port, and mtk_port_wwan_recv() checks w_port under
the same lock, so the RX path is properly synchronized.
Thanks.
Jack Wu
^ permalink raw reply
* RE: [External Mail] Re: [PATCH v4 5/7] net: wwan: t9xx: Add FSM thread
From: Wu. JackBB (GSM) @ 2026-07-17 7:24 UTC (permalink / raw)
To: Simon Horman
Cc: loic.poulain@oss.qualcomm.com, ryazanov.s.a@gmail.com,
johannes@sipsolutions.net, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, wen-zhi.huang@mediatek.com,
shi-wei.yeh@mediatek.com, Minano.tseng@mediatek.com,
matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com,
corbet@lwn.net, skhan@linuxfoundation.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20260716093422.231112-1-horms@kernel.org>
Hi Simon,
> > +static int mtk_fsm_parse_hs2_msg(...)
>
> [Severity: High]
> Assumes entire SKB payload is linearly accessible without checking
> boundaries using pskb_may_pull() before direct dereferencing.
The RX SKBs are allocated internally by the CLDMA driver using
__dev_alloc_skb with linear buffers. The control message data
always resides in the contiguous linear data region. These SKBs
are not received from the network stack and are always linear,
so pskb_may_pull is not needed.
> [Severity: High]
> The rtft_entry->data is passed to action callbacks before data_len
> is validated against buffer bounds.
The bounds check at the loop start ensures the rtft_entry struct
header is within bounds. The action functions only read fixed-size
fields (e.g., a single __le32 for packet padding mode). The modem
firmware is a trusted source with head_pattern/tail_pattern
integrity checks.
> > +static int mtk_fsm_send_hs3_msg(...)
> > + skb = __dev_alloc_skb(...)
>
> [Severity: High]
> Uninitialized kernel heap memory leak. Buffer is not zeroed.
In mtk_fsm_append_rtft_entries, each runtime_feature_entry has
feature_id, support_info, and data_len explicitly set. The
reserved[2] field is not zeroed, but this data is sent to the
local modem over PCIe DMA as part of the HS3 handshake — it is
not exposed to external networks. The modem firmware ignores the
reserved field.
> > +static int mtk_fsm_sap_ctrl_msg_handler(...)
> > + skb_pull(skb, ...)
>
> [Severity: High]
> Missing pskb_may_pull() call.
Same as above. The RX SKBs are internal linear buffers from the
CLDMA driver. skb->len is checked before accessing skb->data.
> [Severity: High]
> Dangling pointer and SKB leak. If mtk_fsm_evt_submit() fails,
> SKB is freed but hs_info->rt_data not nulled.
We will add hs_info->rt_data = NULL after dev_kfree_skb(skb) in
the evt_submit failure path to avoid leaving a dangling pointer.
The same fix will be applied to mtk_fsm_md_ctrl_msg_handler
which has the identical pattern.
> > +static int mtk_port_ch_disable(...)
> > + wait_event_interruptible_timeout(...)
>
> [Severity: High]
> Infinite busy loop on signal pending.
This is intentional. Channel disable must complete or timeout —
aborting mid-operation would leave the channel in an inconsistent
state. The loop does not busy-spin: each iteration sleeps in
wait_event_interruptible_timeout until the condition is met or
the timeout expires. The timeout guarantees bounded execution.
> > +static int mtk_fsm_evt_submit(...)
> > + spin_unlock_irqrestore(...)
>
> [Severity: High]
> Race condition during teardown. fsm->fsm_handler could become
> NULL before wake_up_process().
mtk_fsm_exit() first submits a FSM_EVT_DEV_RM event. The FSM
kthread processes this and sets EVT_TF_GATECLOSED under
evtq_lock. Only after the kthread finishes does mtk_fsm_exit()
call kthread_stop() and set fsm_handler to NULL. If
mtk_fsm_evt_submit() passes the EVT_TF_GATECLOSED check inside
the spinlock, fsm_handler is still valid at the point of
wake_up_process().
> > +static int mtk_port_disable(...)
> > + kcalloc(...)
>
> [Severity: High]
> Silent allocation failure.
The kcalloc allocates a small temporary array of pointers
(typically a few hundred bytes). Allocation failure at this size
is extremely unlikely. Additionally, mtk_port_disable is called
during FSM_STATE_OFF transition, when the modem hardware has
already been power-cycled via PLDR. The hardware channels are
no longer active regardless of the software port state.
> > +static int mtk_fsm_notifier_register(...)
>
> [Severity: Medium]
> Race condition on notifier lists.
The notifier registration and unregistration are only called
during driver probe and remove paths, which are serialized by
the driver model. The FSM kthread does not start processing
events until probe completes, so there is no concurrent access
to the notifier lists.
> > +static void mtk_fsm_evt_cleanup(...)
>
> [Severity: Medium]
> Incomplete event queue cleanup. Cleanup misses freeing SKB stored
> in shared fsm->hs_info[].rt_data if HS2 message was pending.
We Will add rt_data cleanup in the teardown path in v5.
Thanks.
Jack Wu
^ permalink raw reply
* Re: [PATCH v4 net-next 3/7] selftests/ptp: Add testptp support for attributes ioctls
From: Arthur Kiyanovski @ 2026-07-17 7:20 UTC (permalink / raw)
To: saeed bishara
Cc: Arthur Kiyanovski, David Miller, Jakub Kicinski, netdev,
Richard Cochran, Eric Dumazet, Paolo Abeni, David Woodhouse,
Thomas Gleixner, Miroslav Lichvar, Andrew Lunn, Wen Gu, Xuan Zhuo,
David Woodhouse, Yonatan Sarna, Zorik Machulsky,
Alexander Matushevsky, Saeed Bshara, Matt Wilson, Anthony Liguori,
Nafea Bshara, Evgeny Schmeilin, Netanel Belgazal, Ali Saidi,
Benjamin Herrenschmidt, Noam Dagan, David Arinzon,
Evgeny Ostrovsky, Ofir Tabachnik, Amit Bernstein, linux-kselftest,
linux-doc, shuah, Jonathan Corbet, Shuah Khan, Simon Horman,
vadim.fedorenko
In-Reply-To: <CAHfVqdWDaLEKYMw-ZfkYvKLoSJ-_HOm3X9hUUhEf7ReBbOi__w@mail.gmail.com>
On 2026-07-16 16:00:49+03:00, saeed bishara wrote:
> > " -t val shift the ptp clock time by 'val' seconds\n"
> > " -T val set the ptp clock time to 'val' seconds\n"
> > " -x val get an extended ptp clock time with the desired number of samples (up to %d)\n"
> > + " -a val get extended timestamps with attributes (error_bound,\n"
>
> timestamps -> ptp clock to be consistent
> also, have you considered making the "-a" additional flag to -x and -A
> instead of an exclusive option?
>
> > + printf("sample #%2d: unknown clock %d %s: %lld.%09u\n",
> > + sample_num, clockid, when, sec, nsec);
>
> In the case of an unknown clock, will the additional parameters
> (when/sec/..) be useful?
>
> > break;
> > }
> > }
> > @@ -188,6 +193,7 @@ int main(int argc, char *argv[])
> > struct ptp_sys_offset *sysoff;
> > struct ptp_sys_offset_extended *soe;
> > struct ptp_sys_offset_precise *xts;
> > + struct ptp_sys_offset_attrs *attrs_data;
> >
> > char *progname;
> > unsigned int i;
> > @@ -208,7 +214,9 @@ int main(int argc, char *argv[])
> > int list_pins = 0;
> > int pct_offset = 0;
> > int getextended = 0;
> > + int getextendedattrs = 0;
> > int getcross = 0;
> > + int getcrossattrs = 0;
> > int n_samples = 0;
> > int pin_index = -1, pin_func;
> > int pps = -1;
> > @@ -226,7 +234,8 @@ int main(int argc, char *argv[])
> >
> > progname = strrchr(argv[0], '/');
> > progname = progname ? 1+progname : argv[0];
> > - while (EOF != (c = getopt(argc, argv, "cd:e:E:f:F:ghH:i:k:lL:n:o:p:P:rsSt:T:w:x:Xy:z"))) {
> > + while (EOF != (c = getopt(argc, argv,
> > + "a:Acd:e:E:f:F:ghH:i:k:lL:n:o:p:P:rsSt:T:w:x:Xy:z"))) {
> > switch (c) {
> > case 'c':
> > capabilities = 1;
> > @@ -311,9 +320,22 @@ int main(int argc, char *argv[])
> > return -1;
> > }
> > break;
> > + case 'a':
> > + getextendedattrs = atoi(optarg);
> > + if (getextendedattrs < 1 ||
> > + getextendedattrs > PTP_MAX_SAMPLES) {
> > + fprintf(stderr,
> > + "number of extended attrs timestamp samples must be between 1 and %d; was asked for %d\n",
> > + PTP_MAX_SAMPLES, getextendedattrs);
> > + return -1;
> > + }
> > + break;
> > case 'X':
> > getcross = 1;
> > break;
> > + case 'A':
> > + getcrossattrs = 1;
> > + break;
> > case 'y':
> > if (!strcasecmp(optarg, "realtime"))
> > ext_clockid = CLOCK_REALTIME;
> > @@ -367,6 +389,8 @@ int main(int argc, char *argv[])
> > " %d programmable pins\n"
> > " %d cross timestamping\n"
> > " %d adjust_phase\n"
> > + " %d extended_attrs\n"
> > + " %d precise_attrs\n"
> > " %d maximum phase adjustment (ns)\n",
> > caps.max_adj,
> > caps.n_alarm,
> > @@ -376,6 +400,8 @@ int main(int argc, char *argv[])
> > caps.n_pins,
> > caps.cross_timestamping,
> > caps.adjust_phase,
> > + caps.extended_attrs,
> > + caps.precise_attrs,
> > caps.max_phase_adj);
> > }
> > }
> > @@ -648,6 +674,49 @@ int main(int argc, char *argv[])
> > free(soe);
> > }
> >
> > + if (getextendedattrs) {
> > + attrs_data = calloc(1, sizeof(*attrs_data) +
> > + getextendedattrs * sizeof(struct ptp_timestamp));
> > + if (!attrs_data) {
> > + perror("calloc");
> > + return -1;
> > + }
> > +
> > + attrs_data->request.num_samples = getextendedattrs;
> > + attrs_data->request.clock_id = ext_clockid;
> > +
> > + if (ioctl(fd, PTP_SYS_OFFSET_EXTENDED_ATTRS, attrs_data)) {
> > + perror("PTP_SYS_OFFSET_EXTENDED_ATTRS");
> > + } else {
> > + printf("extended attrs timestamp request returned %d samples\n",
> > + getextendedattrs);
> > +
> > + for (i = 0; i < getextendedattrs; i++) {
> > + struct ptp_timestamp *ts = &attrs_data->timestamps[i];
> > +
> > + printf(" sample #%u:\n", i);
> > + printf(" sys before: %lld ns\n",
> > + (long long)ts->pre_systime.sys_time);
> > + printf(" phc time: %lld.%09u\n",
> > + ts->devtime.device_time.sec,
> > + ts->devtime.device_time.nsec);
> > + if (ts->devtime.attrs.valid & PTP_ATTRS_VALID_ERROR_BOUND)
> > + printf(" error_bound: %u ns\n",
> > + ts->devtime.attrs.error_bound);
>
> in case device doesn't report error bound, I think it's better to
> print explicit message
All addressed in v5: -a is now a modifier on -x/-X (dropped -A, use
-X -a) as you suggested; usage text updated; the unknown-clock line is
trimmed to just the clock id; and attributes the device doesn't report
now print "not reported".
^ permalink raw reply
* RE: [External Mail] Re: [PATCH v4 4/7] net: wwan: t9xx: Add control port
From: Wu. JackBB (GSM) @ 2026-07-17 7:20 UTC (permalink / raw)
To: Simon Horman
Cc: loic.poulain@oss.qualcomm.com, ryazanov.s.a@gmail.com,
johannes@sipsolutions.net, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, wen-zhi.huang@mediatek.com,
shi-wei.yeh@mediatek.com, Minano.tseng@mediatek.com,
matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com,
corbet@lwn.net, skhan@linuxfoundation.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20260716093414.231093-1-horms@kernel.org>
Hi Simon,
> > +static void mtk_port_free_or_backup(...)
>
> [Severity: High]
> Does this directly bypass the kref API?
Will fix in v5. The kref count is 1 at this point (the initial
allocation reference only), so the behavior is correct — but
kref_put is the proper API. Will change to
kref_put(&port->kref, mtk_port_release). Same fix applied to
mtk_port_stale_list_grp_cleanup below.
> > +static struct mtk_port *mtk_port_search_by_id(...)
>
> [Severity: High]
> Is it safe to perform radix tree lookups without an RCU read-side
> critical section?
mtk_port_search_by_id is called from the CLDMA workqueue context
(rx_dispatch) and during probe/FSM startup. Port creation and
destruction are driven by FSM state transitions on the FSM
kthread. Ports are created before the CLDMA RX queue is opened,
and the RX queue is stopped before ports are destroyed. No
concurrent modification occurs during radix tree lookups.
The tree was initialized with INIT_RADIX_TREE (not RCU-tagged),
and all modifications are serialized by FSM state ordering.
> > +static struct mtk_port *mtk_port_search_by_name(...)
>
> [Severity: High]
> Does this radix tree iteration require rcu_read_lock() protection?
mtk_port_search_by_name is only called from
mtk_port_internal_open, which is exclusively invoked by the FSM
kthread during mtk_fsm_ctrl_ch_start. Port creation and
destruction are also driven by FSM state transitions on the same
kthread. Since the FSM serializes these operations, no concurrent
port removal can occur while iterating.
> > +static void mtk_port_stale_list_grp_cleanup(...)
>
> [Severity: High]
> This appears to be another instance of bypassing the kref API by
> calling mtk_port_release() directly instead of kref_put().
We will fix in v5:
Use kref_put(&port->kref, mtk_port_release) instead of
calling mtk_port_release() directly. The clear_bit before
it ensures no double list_del, and kref count is 1 so the
behavior is unchanged — but kref_put is the proper API.
> [Severity: Medium]
> Does this leak the s_list->dev_id? The list entries are freed here during
> module unload, but there doesn't appear to be a corresponding call to
> ida_free() to return the dev_id to the allocator.
We will fix in v5,Add ida_free before kfree(s_list) to properly release the
dev_id:
if (s_list->dev_id >= 0)
ida_free(&ccci_dev_ids, s_list->dev_id);
kfree(s_list);
> > +static int mtk_port_trb_init(...)
>
> [Severity: High]
> Does this assignment need to take a reference to the port with
> kref_get()?
The TRB lifecycle is protected by kref on the TRB itself
(kref_get before submit, kref_put in both the caller and the
completion callback). The port is not freed while TRBs reference
it — port destruction goes through kref_put(&port->kref,
mtk_port_release), and TRB_CMD_DISABLE triggers
mtk_ctrl_ch_flush, which forces all pending TRBs to complete
before the port teardown proceeds.
> > +static int mtk_port_send_data(...)
>
> [Severity: Medium]
> Can concurrent writers corrupt the tx_seq variable?
tx_seq is a per-port 8-bit sequence number with a single writer
per port: external ports are serialized by the WWAN core
ops_lock mutex around tx/tx_blocking callbacks, and internal
ports are written exclusively by the FSM kthread. Concurrent
transmissions on the same port do not occur.
> > +static int mtk_port_rx_dispatch_frag_skb(...)
>
> [Severity: High]
> Can this lead to use-after-free and double-free on the skb?
No. The recv callback (mtk_port_internal_recv) does not free
the skb on error — it returns -ENXIO and leaves skb ownership
with the caller. When recv returns an error, it is safe to
access the skb to restore frag_list. The caller
(mtk_port_rx_dispatch) then frees the skb once in the
drop_data path.
> > +static int mtk_port_rx_dispatch(...)
>
> [Severity: High]
> This is where the second free of the skb occurs when propagating
> the error from the recv callback.
As above, the recv callback does not free the skb on error.
The drop_data path in mtk_port_rx_dispatch is the only place
the skb is freed — no double free occurs.
> > +static void mtk_port_add_header(...)
>
> [Severity: Medium]
> This is the other side of the tx_seq data race.
Same as above — tx_seq is serialized by the WWAN core ops_lock
for external ports and the single-threaded FSM kthread for
internal ports.
> > +static int mtk_port_status_update(...)
>
> [Severity: High]
> Is there a missing bounds check on msg->port_cnt here?
The message is validated by head_pattern, tail_pattern, and
version checks before the loop. The port enumeration protocol
between host and modem guarantees the buffer contains exactly
port_cnt entries. The modem firmware is a trusted source —
this is a control plane interface, not an untrusted network
protocol.
> > +static int mtk_port_ch_enable(...)
>
> [Severity: High]
> Could this create an infinite loop in kernel space?
This is intentional. Channel enable/disable are control plane
operations that must complete or timeout — aborting mid-operation
due to a signal would leave the channel in an inconsistent state.
The loop does not busy-spin: each iteration sleeps in
wait_event_interruptible_timeout until the condition is met or
the timeout (MTK_DFLT_TRB_TIMEOUT) expires. The timeout
guarantees bounded execution.
> > +static int mtk_port_ch_disable(...)
>
> [Severity: High]
> This appears to have the same infinite loop risk.
Same reasoning as mtk_port_ch_enable. The timeout guarantees
bounded execution per iteration.
> > +static int mtk_port_internal_recv(...)
>
> [Severity: High]
> This is where the skb is freed on the error path before returning
> -ENXIO.
mtk_port_internal_recv does not free the skb on the error path.
The drop_data label simply returns -ENXIO without calling
dev_kfree_skb_any(). The skb ownership remains with the caller.
> > +static int mtk_port_common_open(...)
>
> [Severity: High]
> Is this test_bit() and set_bit() sequence intended to provide
> mutual exclusion?
All callers of mtk_port_common_open acquire port_mngr_grp_mtx
via mtk_port_get_locked before calling this function, which
serializes concurrent open attempts on the same port. The
test_bit/set_bit sequence is protected by the mutex.
Thanks.
Jack Wu
^ permalink raw reply
* Re: [PATCH v4 net-next 2/7] selftests/ptp: Extract print_system_timestamp helper in testptp
From: Arthur Kiyanovski @ 2026-07-17 7:19 UTC (permalink / raw)
To: saeed bishara
Cc: Arthur Kiyanovski, David Miller, Jakub Kicinski, netdev,
Richard Cochran, Eric Dumazet, Paolo Abeni, David Woodhouse,
Thomas Gleixner, Miroslav Lichvar, Andrew Lunn, Wen Gu, Xuan Zhuo,
David Woodhouse, Yonatan Sarna, Zorik Machulsky,
Alexander Matushevsky, Saeed Bshara, Matt Wilson, Anthony Liguori,
Nafea Bshara, Evgeny Schmeilin, Netanel Belgazal, Ali Saidi,
Benjamin Herrenschmidt, Noam Dagan, David Arinzon,
Evgeny Ostrovsky, Ofir Tabachnik, Amit Bernstein, linux-kselftest,
linux-doc, shuah, Jonathan Corbet, Shuah Khan, Simon Horman,
vadim.fedorenko
In-Reply-To: <CAHfVqdWNk0QmvZBXi21eUKcU34DpXXZBNyJVUep+Tx1n52ihAg@mail.gmail.com>
On 2026-07-16 14:34:57+03:00, saeed bishara wrote:
> > +static void print_system_timestamp(int sample_num, __kernel_clockid_t clockid,
> > + long long sec, unsigned int nsec,
> > + const char *when)
> > +{
> > + switch (clockid) {
> > + case CLOCK_REALTIME:
> > + printf("sample #%2d: real time %s: %lld.%09u\n",
> > + sample_num, when, sec, nsec);
> > + break;
> > + case CLOCK_MONOTONIC:
> > + printf("sample #%2d: monotonic time %s: %lld.%09u\n",
> > + sample_num, when, sec, nsec);
> > + break;
> > + case CLOCK_MONOTONIC_RAW:
> > + printf("sample #%2d: monotonic-raw time %s: %lld.%09u\n",
> > + sample_num, when, sec, nsec);
> > + break;
>
> These three printfs are also kind of duplicated, I think mapping the
> clockid to name first would save a few lines.
Done in v5 - map the clockid to a name and print once.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox