[PATCH 0/4] dmaengine: ti: cppi41: Fix suspend/resume issues

[PATCH 0/4] dmaengine: ti: cppi41: Fix suspend/resume issues

[Markus Schneider-Pargmann (TI)]

Hi,

there are some issues with cppi41 regarding suspend/resume. This series
fixes suspend/resume when the device is currently runtime suspended. It
also fixes an incomplete restore of QMGR_MEMCTRL during resume. To make
the restore on resume more reliable the last patch unifies the register
writes for init and resume.

I tested this series with am335x-evm

Best,
Markus

Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
Markus Schneider-Pargmann (TI) (4):
      dmaengine: ti: cppi41: Use td_queue to restore DMA_TDFDQ
      dmaengine: ti: cppi41: Fix register access in suspend_late
      dmaengine: ti: cppi41: Restore QMGR_MEMCTRL in resume
      dmaengine: ti: cppi41: Share register setup with resume

 drivers/dma/ti/cppi41.c | 74 ++++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 31 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261030-v7-3-topic-am335lpm-cppi41-b180a2d62987

Best regards,
--  
Markus Schneider-Pargmann (TI) <msp@baylibre.com>

[PATCH 1/4] dmaengine: ti: cppi41: Use td_queue to restore DMA_TDFDQ

[Markus Schneider-Pargmann (TI)]

In init_cppi41() td_queue.submit is used to setup DMA_TDFDQ. In
suspend/resume the same value is read, stored and restored.

To get rid of the suspend register access, use td_queue.submit in resume
as well to restore the value.

Cc: stable@vger.kernel.org
Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 drivers/dma/ti/cppi41.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index 88756dccd62c..458d20b12462 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -146,9 +146,6 @@ struct cppi41_dd {
 	struct list_head pending;	/* Pending queued transfers */
 	spinlock_t lock;		/* Lock for pending list */
 
-	/* context for suspend/resume */
-	unsigned int dma_tdfdq;
-
 	bool is_suspended;
 };
 
@@ -1174,7 +1171,6 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
 {
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
 
-	cdd->dma_tdfdq = cppi_readl(cdd->ctrl_mem + DMA_TDFDQ);
 	disable_sched(cdd);
 
 	return 0;
@@ -1195,7 +1191,7 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 
 	init_sched(cdd);
 
-	cppi_writel(cdd->dma_tdfdq, cdd->ctrl_mem + DMA_TDFDQ);
+	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
 	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
 	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
 	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);

-- 
2.55.0

[PATCH 2/4] dmaengine: ti: cppi41: Fix register access in suspend_late

[Markus Schneider-Pargmann (TI)]

cppi41_suspend() and cppi41_resume() access registers. However the
device may be runtime suspended in which case it is not allowed to
access the registers.

On suspend do not run disable_sched() when it is runtime suspended. This
avoids accessing the registers and crashing the kernel during suspend.

Resume when runtime suspended is delayed until cppi41_runtime_resume()
is called using the variable ctx_lost. This keeps any access while the
device is runtime suspended.

Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 drivers/dma/ti/cppi41.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index 458d20b12462..f5a824418fe7 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -146,6 +146,9 @@ struct cppi41_dd {
 	struct list_head pending;	/* Pending queued transfers */
 	spinlock_t lock;		/* Lock for pending list */
 
+	/* Register context may have been lost during system suspend */
+	bool ctx_lost;
+
 	bool is_suspended;
 };
 
@@ -1171,7 +1174,10 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
 {
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
 
-	disable_sched(cdd);
+	if (!pm_runtime_status_suspended(dev))
+		disable_sched(cdd);
+
+	cdd->ctx_lost = true;
 
 	return 0;
 }
@@ -1182,6 +1188,9 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 	struct cppi41_channel *c;
 	int i;
 
+	if (pm_runtime_status_suspended(dev))
+		return 0;
+
 	for (i = 0; i < DESCS_AREAS; i++)
 		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
 
@@ -1196,6 +1205,8 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
 	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
 
+	cdd->ctx_lost = false;
+
 	return 0;
 }
 
@@ -1217,6 +1228,9 @@ static int __maybe_unused cppi41_runtime_resume(struct device *dev)
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
 	unsigned long flags;
 
+	if (cdd->ctx_lost)
+		cppi41_resume(dev);
+
 	spin_lock_irqsave(&cdd->lock, flags);
 	cdd->is_suspended = false;
 	cppi41_run_queue(cdd);

-- 
2.55.0

[PATCH 3/4] dmaengine: ti: cppi41: Restore QMGR_MEMCTRL in resume

[Markus Schneider-Pargmann (TI)]

Currently only QMGR_MEMBASE is restored in resume, QMGR_MEMCTRL is left
at its default values.

Add a restore for QMGR_MEMCTRL. This introduces a helper function
cppi41_memctrl() to not duplicate this calcuation in init and resume.

Fixes: f97b98d6d02e ("dma: cppi41: add support for suspend and resume")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 drivers/dma/ti/cppi41.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index f5a824418fe7..e2a089066c6d 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -842,13 +842,22 @@ static void deinit_cppi41(struct device *dev, struct cppi41_dd *cdd)
 			cdd->scratch_phys);
 }
 
+static u32 cppi41_memctrl(int area)
+{
+	u32 reg;
+
+	reg = (area * ALLOC_DECS_NUM) << QMGR_MEMCTRL_IDX_SH;
+	reg |= (ilog2(sizeof(struct cppi41_desc)) - 5) << QMGR_MEMCTRL_DESC_SH;
+	reg |= ilog2(ALLOC_DECS_NUM) - 5;
+
+	return reg;
+}
+
 static int init_descs(struct device *dev, struct cppi41_dd *cdd)
 {
 	unsigned int desc_size;
 	unsigned int mem_decs;
 	int i;
-	u32 reg;
-	u32 idx;
 
 	BUILD_BUG_ON(sizeof(struct cppi41_desc) &
 			(sizeof(struct cppi41_desc) - 1));
@@ -858,13 +867,7 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
 	desc_size = sizeof(struct cppi41_desc);
 	mem_decs = ALLOC_DECS_NUM * desc_size;
 
-	idx = 0;
 	for (i = 0; i < DESCS_AREAS; i++) {
-
-		reg = idx << QMGR_MEMCTRL_IDX_SH;
-		reg |= (ilog2(desc_size) - 5) << QMGR_MEMCTRL_DESC_SH;
-		reg |= ilog2(ALLOC_DECS_NUM) - 5;
-
 		BUILD_BUG_ON(DESCS_AREAS != 1);
 		cdd->cd = dma_alloc_coherent(dev, mem_decs,
 				&cdd->descs_phys, GFP_KERNEL);
@@ -872,9 +875,7 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
 			return -ENOMEM;
 
 		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
-		cppi_writel(reg, cdd->qmgr_mem + QMGR_MEMCTRL(i));
-
-		idx += ALLOC_DECS_NUM;
+		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
 	}
 	return 0;
 }
@@ -1191,8 +1192,10 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 	if (pm_runtime_status_suspended(dev))
 		return 0;
 
-	for (i = 0; i < DESCS_AREAS; i++)
+	for (i = 0; i < DESCS_AREAS; i++) {
 		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
+		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
+	}
 
 	list_for_each_entry(c, &cdd->ddev.channels, chan.device_node)
 		if (!c->is_tx)

-- 
2.55.0

[PATCH 4/4] dmaengine: ti: cppi41: Share register setup with resume

[Markus Schneider-Pargmann (TI)]

Both probe and resume basically do the same thing and setup the
registers. But resume has its own copy of the register writes. This will
always lead to differences when changes are made.

Create a separate function that is used by both probe and resume.
Note that the ordering is slightly different but I didn't see or found
any issues with the new order while testing.

Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 drivers/dma/ti/cppi41.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index e2a089066c6d..b62a26c2659d 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -873,9 +873,6 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
 				&cdd->descs_phys, GFP_KERNEL);
 		if (!cdd->cd)
 			return -ENOMEM;
-
-		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
-		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
 	}
 	return 0;
 }
@@ -903,6 +900,24 @@ static void init_sched(struct cppi41_dd *cdd)
 	cppi_writel(reg, cdd->sched_mem + DMA_SCHED_CTRL);
 }
 
+/* Program the registers, also used to restore the context on resume */
+static void cppi41_init_hw(struct cppi41_dd *cdd)
+{
+	int i;
+
+	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
+	cppi_writel(TOTAL_DESCS_NUM, cdd->qmgr_mem + QMGR_LRAM_SIZE);
+	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
+
+	for (i = 0; i < DESCS_AREAS; i++) {
+		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
+		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
+	}
+
+	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
+	init_sched(cdd);
+}
+
 static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
 {
 	int ret;
@@ -913,16 +928,11 @@ static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
 	if (!cdd->qmgr_scratch)
 		return -ENOMEM;
 
-	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
-	cppi_writel(TOTAL_DESCS_NUM, cdd->qmgr_mem + QMGR_LRAM_SIZE);
-	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
-
 	ret = init_descs(dev, cdd);
 	if (ret)
 		goto err_td;
 
-	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
-	init_sched(cdd);
+	cppi41_init_hw(cdd);
 
 	return 0;
 err_td:
@@ -1187,26 +1197,15 @@ static int __maybe_unused cppi41_resume(struct device *dev)
 {
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
 	struct cppi41_channel *c;
-	int i;
 
 	if (pm_runtime_status_suspended(dev))
 		return 0;
 
-	for (i = 0; i < DESCS_AREAS; i++) {
-		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
-		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
-	}
-
 	list_for_each_entry(c, &cdd->ddev.channels, chan.device_node)
 		if (!c->is_tx)
 			cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
 
-	init_sched(cdd);
-
-	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
-	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
-	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
-	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
+	cppi41_init_hw(cdd);
 
 	cdd->ctx_lost = false;
 

-- 
2.55.0

Re: [PATCH 1/4] dmaengine: ti: cppi41: Use td_queue to restore DMA_TDFDQ

[Adivi, Sai Sree Kartheek]

On 04/11/26 01:07, Markus Schneider-Pargmann (TI) wrote:
> In init_cppi41() td_queue.submit is used to setup DMA_TDFDQ. In
> suspend/resume the same value is read, stored and restored.
>
> To get rid of the suspend register access, use td_queue.submit in resume
> as well to restore the value.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
Reviewed-by: Sai Sree Kartheek Adivi <s-adivi@ti.com>
> ---
>   drivers/dma/ti/cppi41.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index 88756dccd62c..458d20b12462 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -146,9 +146,6 @@ struct cppi41_dd {
>   	struct list_head pending;	/* Pending queued transfers */
>   	spinlock_t lock;		/* Lock for pending list */
>   
> -	/* context for suspend/resume */
> -	unsigned int dma_tdfdq;
> -
>   	bool is_suspended;
>   };
>   
> @@ -1174,7 +1171,6 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
>   {
>   	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>   
> -	cdd->dma_tdfdq = cppi_readl(cdd->ctrl_mem + DMA_TDFDQ);
>   	disable_sched(cdd);
>   
>   	return 0;
> @@ -1195,7 +1191,7 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>   
>   	init_sched(cdd);
>   
> -	cppi_writel(cdd->dma_tdfdq, cdd->ctrl_mem + DMA_TDFDQ);
> +	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
>   	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
>   	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
>   	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
>

Re: [PATCH 2/4] dmaengine: ti: cppi41: Fix register access in suspend_late

[Adivi, Sai Sree Kartheek]

On 04/11/26 01:07, Markus Schneider-Pargmann (TI) wrote:
> cppi41_suspend() and cppi41_resume() access registers. However the
> device may be runtime suspended in which case it is not allowed to
> access the registers.
>
> On suspend do not run disable_sched() when it is runtime suspended. This
> avoids accessing the registers and crashing the kernel during suspend.
>
> Resume when runtime suspended is delayed until cppi41_runtime_resume()
> is called using the variable ctx_lost. This keeps any access while the
> device is runtime suspended.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
Reviewed-by: Sai Sree Kartheek Adivi <s-adivi@ti.com>
> ---
>   drivers/dma/ti/cppi41.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index 458d20b12462..f5a824418fe7 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -146,6 +146,9 @@ struct cppi41_dd {
>   	struct list_head pending;	/* Pending queued transfers */
>   	spinlock_t lock;		/* Lock for pending list */
>   
> +	/* Register context may have been lost during system suspend */
> +	bool ctx_lost;
> +
>   	bool is_suspended;
>   };
>   
> @@ -1171,7 +1174,10 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
>   {
>   	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>   
> -	disable_sched(cdd);
> +	if (!pm_runtime_status_suspended(dev))
> +		disable_sched(cdd);
> +
> +	cdd->ctx_lost = true;
>   
>   	return 0;
>   }
> @@ -1182,6 +1188,9 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>   	struct cppi41_channel *c;
>   	int i;
>   
> +	if (pm_runtime_status_suspended(dev))
> +		return 0;
> +
>   	for (i = 0; i < DESCS_AREAS; i++)
>   		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
>   
> @@ -1196,6 +1205,8 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>   	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
>   	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
>   
> +	cdd->ctx_lost = false;
> +
>   	return 0;
>   }
>   
> @@ -1217,6 +1228,9 @@ static int __maybe_unused cppi41_runtime_resume(struct device *dev)
>   	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>   	unsigned long flags;
>   
> +	if (cdd->ctx_lost)
> +		cppi41_resume(dev);
> +
>   	spin_lock_irqsave(&cdd->lock, flags);
>   	cdd->is_suspended = false;
>   	cppi41_run_queue(cdd);
>

Re: [PATCH 3/4] dmaengine: ti: cppi41: Restore QMGR_MEMCTRL in resume

[Adivi, Sai Sree Kartheek]

On 04/11/26 01:07, Markus Schneider-Pargmann (TI) wrote:
> Currently only QMGR_MEMBASE is restored in resume, QMGR_MEMCTRL is left
> at its default values.
>
> Add a restore for QMGR_MEMCTRL. This introduces a helper function
> cppi41_memctrl() to not duplicate this calcuation in init and resume.
>
> Fixes: f97b98d6d02e ("dma: cppi41: add support for suspend and resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>

Reviewed-by: Sai Sree Kartheek Adivi <s-adivi@ti.com>


> ---
>   drivers/dma/ti/cppi41.c | 27 +++++++++++++++------------
>   1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index f5a824418fe7..e2a089066c6d 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -842,13 +842,22 @@ static void deinit_cppi41(struct device *dev, struct cppi41_dd *cdd)
>   			cdd->scratch_phys);
>   }
>   
> +static u32 cppi41_memctrl(int area)
> +{
> +	u32 reg;
> +
> +	reg = (area * ALLOC_DECS_NUM) << QMGR_MEMCTRL_IDX_SH;
> +	reg |= (ilog2(sizeof(struct cppi41_desc)) - 5) << QMGR_MEMCTRL_DESC_SH;
> +	reg |= ilog2(ALLOC_DECS_NUM) - 5;
> +
> +	return reg;
> +}
> +
>   static int init_descs(struct device *dev, struct cppi41_dd *cdd)
>   {
>   	unsigned int desc_size;
>   	unsigned int mem_decs;
>   	int i;
> -	u32 reg;
> -	u32 idx;
>   
>   	BUILD_BUG_ON(sizeof(struct cppi41_desc) &
>   			(sizeof(struct cppi41_desc) - 1));
> @@ -858,13 +867,7 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
>   	desc_size = sizeof(struct cppi41_desc);
>   	mem_decs = ALLOC_DECS_NUM * desc_size;
>   
> -	idx = 0;
>   	for (i = 0; i < DESCS_AREAS; i++) {
> -
> -		reg = idx << QMGR_MEMCTRL_IDX_SH;
> -		reg |= (ilog2(desc_size) - 5) << QMGR_MEMCTRL_DESC_SH;
> -		reg |= ilog2(ALLOC_DECS_NUM) - 5;
> -
>   		BUILD_BUG_ON(DESCS_AREAS != 1);
>   		cdd->cd = dma_alloc_coherent(dev, mem_decs,
>   				&cdd->descs_phys, GFP_KERNEL);
> @@ -872,9 +875,7 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
>   			return -ENOMEM;
>   
>   		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> -		cppi_writel(reg, cdd->qmgr_mem + QMGR_MEMCTRL(i));
> -
> -		idx += ALLOC_DECS_NUM;
> +		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
>   	}
>   	return 0;
>   }
> @@ -1191,8 +1192,10 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>   	if (pm_runtime_status_suspended(dev))
>   		return 0;
>   
> -	for (i = 0; i < DESCS_AREAS; i++)
> +	for (i = 0; i < DESCS_AREAS; i++) {
>   		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> +		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
> +	}
>   
>   	list_for_each_entry(c, &cdd->ddev.channels, chan.device_node)
>   		if (!c->is_tx)
>

Re: [PATCH 4/4] dmaengine: ti: cppi41: Share register setup with resume

[Adivi, Sai Sree Kartheek]

On 04/11/26 01:07, Markus Schneider-Pargmann (TI) wrote:
> Both probe and resume basically do the same thing and setup the
> registers. But resume has its own copy of the register writes. This will
> always lead to differences when changes are made.
>
> Create a separate function that is used by both probe and resume.
> Note that the ordering is slightly different but I didn't see or found
> any issues with the new order while testing.
>
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>

Reviewed-by: Sai Sree Kartheek Adivi <s-adivi@ti.com>


> ---
>   drivers/dma/ti/cppi41.c | 41 ++++++++++++++++++++---------------------
>   1 file changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index e2a089066c6d..b62a26c2659d 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -873,9 +873,6 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
>   				&cdd->descs_phys, GFP_KERNEL);
>   		if (!cdd->cd)
>   			return -ENOMEM;
> -
> -		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> -		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
>   	}
>   	return 0;
>   }
> @@ -903,6 +900,24 @@ static void init_sched(struct cppi41_dd *cdd)
>   	cppi_writel(reg, cdd->sched_mem + DMA_SCHED_CTRL);
>   }
>   
> +/* Program the registers, also used to restore the context on resume */
> +static void cppi41_init_hw(struct cppi41_dd *cdd)
> +{
> +	int i;
> +
> +	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
> +	cppi_writel(TOTAL_DESCS_NUM, cdd->qmgr_mem + QMGR_LRAM_SIZE);
> +	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
> +
> +	for (i = 0; i < DESCS_AREAS; i++) {
> +		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> +		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
> +	}
> +
> +	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
> +	init_sched(cdd);
> +}
> +
>   static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
>   {
>   	int ret;
> @@ -913,16 +928,11 @@ static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
>   	if (!cdd->qmgr_scratch)
>   		return -ENOMEM;
>   
> -	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
> -	cppi_writel(TOTAL_DESCS_NUM, cdd->qmgr_mem + QMGR_LRAM_SIZE);
> -	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
> -
>   	ret = init_descs(dev, cdd);
>   	if (ret)
>   		goto err_td;
>   
> -	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
> -	init_sched(cdd);
> +	cppi41_init_hw(cdd);
>   
>   	return 0;
>   err_td:
> @@ -1187,26 +1197,15 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>   {
>   	struct cppi41_dd *cdd = dev_get_drvdata(dev);
>   	struct cppi41_channel *c;
> -	int i;
>   
>   	if (pm_runtime_status_suspended(dev))
>   		return 0;
>   
> -	for (i = 0; i < DESCS_AREAS; i++) {
> -		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> -		cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
> -	}
> -
>   	list_for_each_entry(c, &cdd->ddev.channels, chan.device_node)
>   		if (!c->is_tx)
>   			cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
>   
> -	init_sched(cdd);
> -
> -	cppi_writel(cdd->td_queue.submit, cdd->ctrl_mem + DMA_TDFDQ);
> -	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
> -	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
> -	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
> +	cppi41_init_hw(cdd);
>   
>   	cdd->ctx_lost = false;
>   
>

Re: [PATCH 0/4] dmaengine: ti: cppi41: Fix suspend/resume issues

[Judith Mendez]

Hi Markus,

On 11/3/26 1:37 PM, Markus Schneider-Pargmann (TI) wrote:
> Hi,
> 
> there are some issues with cppi41 regarding suspend/resume. This series
> fixes suspend/resume when the device is currently runtime suspended. It
> also fixes an incomplete restore of QMGR_MEMCTRL during resume. To make
> the restore on resume more reliable the last patch unifies the register
> writes for init and resume.
> 
> I tested this series with am335x-evm
> 

Thanks for the patches.

I was able to test on multiple am335 boards, so for the series:

Tested-by: Judith Mendez <jm@ti.com>

Re: [PATCH 1/4] dmaengine: ti: cppi41: Use td_queue to restore DMA_TDFDQ

[Frank Li]

On Tue, Nov 03, 2026 at 08:37:28PM +0100, Markus Schneider-Pargmann (TI) wrote:
> [You don't often get email from msp@baylibre.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> In init_cppi41() td_queue.submit is used to setup DMA_TDFDQ. In
> suspend/resume the same value is read, stored and restored.
>
> To get rid of the suspend register access, use td_queue.submit in resume
> as well to restore the value.
>
> Cc: stable@vger.kernel.org

Do you fix some problem? why only have cc stable without fixes tag?

Frank

Re: [PATCH 2/4] dmaengine: ti: cppi41: Fix register access in suspend_late

[Frank Li]

On Tue, Nov 03, 2026 at 08:37:29PM +0100, Markus Schneider-Pargmann (TI) wrote:
> [You don't often get email from msp@baylibre.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> cppi41_suspend() and cppi41_resume() access registers. However the
> device may be runtime suspended in which case it is not allowed to
> access the registers.
>
> On suspend do not run disable_sched() when it is runtime suspended. This
> avoids accessing the registers and crashing the kernel during suspend.
>
> Resume when runtime suspended is delayed until cppi41_runtime_resume()
> is called using the variable ctx_lost. This keeps any access while the
> device is runtime suspended.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/ti/cppi41.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index 458d20b12462..f5a824418fe7 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -146,6 +146,9 @@ struct cppi41_dd {
>         struct list_head pending;       /* Pending queued transfers */
>         spinlock_t lock;                /* Lock for pending list */
>
> +       /* Register context may have been lost during system suspend */
> +       bool ctx_lost;
> +
>         bool is_suspended;
>  };
>
> @@ -1171,7 +1174,10 @@ static int __maybe_unused cppi41_suspend(struct device *dev)
>  {
>         struct cppi41_dd *cdd = dev_get_drvdata(dev);
>
> -       disable_sched(cdd);
> +       if (!pm_runtime_status_suspended(dev))
> +               disable_sched(cdd);
> +
> +       cdd->ctx_lost = true;
>
>         return 0;
>  }
> @@ -1182,6 +1188,9 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>         struct cppi41_channel *c;
>         int i;
>
> +       if (pm_runtime_status_suspended(dev))
> +               return 0;
> +
>         for (i = 0; i < DESCS_AREAS; i++)
>                 cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
>
> @@ -1196,6 +1205,8 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>         cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
>         cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
>
> +       cdd->ctx_lost = false;
> +
>         return 0;
>  }
>
> @@ -1217,6 +1228,9 @@ static int __maybe_unused cppi41_runtime_resume(struct device *dev)
>         struct cppi41_dd *cdd = dev_get_drvdata(dev);
>         unsigned long flags;
>
> +       if (cdd->ctx_lost)
> +               cppi41_resume(dev);
> +
>         spin_lock_irqsave(&cdd->lock, flags);
>         cdd->is_suspended = false;
>         cppi41_run_queue(cdd);
>
> --
> 2.55.0
>

Re: [PATCH 3/4] dmaengine: ti: cppi41: Restore QMGR_MEMCTRL in resume

[Frank Li]

On Tue, Nov 03, 2026 at 08:37:30PM +0100, Markus Schneider-Pargmann (TI) wrote:
> [You don't often get email from msp@baylibre.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Currently only QMGR_MEMBASE is restored in resume, QMGR_MEMCTRL is left
> at its default values.
>
> Add a restore for QMGR_MEMCTRL. This introduces a helper function
> cppi41_memctrl() to not duplicate this calcuation in init and resume.
>
> Fixes: f97b98d6d02e ("dma: cppi41: add support for suspend and resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/ti/cppi41.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index f5a824418fe7..e2a089066c6d 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -842,13 +842,22 @@ static void deinit_cppi41(struct device *dev, struct cppi41_dd *cdd)
>                         cdd->scratch_phys);
>  }
>
> +static u32 cppi41_memctrl(int area)
> +{
> +       u32 reg;
> +
> +       reg = (area * ALLOC_DECS_NUM) << QMGR_MEMCTRL_IDX_SH;
> +       reg |= (ilog2(sizeof(struct cppi41_desc)) - 5) << QMGR_MEMCTRL_DESC_SH;
> +       reg |= ilog2(ALLOC_DECS_NUM) - 5;
> +
> +       return reg;
> +}
> +
>  static int init_descs(struct device *dev, struct cppi41_dd *cdd)
>  {
>         unsigned int desc_size;
>         unsigned int mem_decs;
>         int i;
> -       u32 reg;
> -       u32 idx;
>
>         BUILD_BUG_ON(sizeof(struct cppi41_desc) &
>                         (sizeof(struct cppi41_desc) - 1));
> @@ -858,13 +867,7 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
>         desc_size = sizeof(struct cppi41_desc);
>         mem_decs = ALLOC_DECS_NUM * desc_size;
>
> -       idx = 0;
>         for (i = 0; i < DESCS_AREAS; i++) {
> -
> -               reg = idx << QMGR_MEMCTRL_IDX_SH;
> -               reg |= (ilog2(desc_size) - 5) << QMGR_MEMCTRL_DESC_SH;
> -               reg |= ilog2(ALLOC_DECS_NUM) - 5;
> -
>                 BUILD_BUG_ON(DESCS_AREAS != 1);
>                 cdd->cd = dma_alloc_coherent(dev, mem_decs,
>                                 &cdd->descs_phys, GFP_KERNEL);
> @@ -872,9 +875,7 @@ static int init_descs(struct device *dev, struct cppi41_dd *cdd)
>                         return -ENOMEM;
>
>                 cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> -               cppi_writel(reg, cdd->qmgr_mem + QMGR_MEMCTRL(i));
> -
> -               idx += ALLOC_DECS_NUM;
> +               cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
>         }
>         return 0;
>  }
> @@ -1191,8 +1192,10 @@ static int __maybe_unused cppi41_resume(struct device *dev)
>         if (pm_runtime_status_suspended(dev))
>                 return 0;
>
> -       for (i = 0; i < DESCS_AREAS; i++)
> +       for (i = 0; i < DESCS_AREAS; i++) {
>                 cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
> +               cppi_writel(cppi41_memctrl(i), cdd->qmgr_mem + QMGR_MEMCTRL(i));
> +       }
>
>         list_for_each_entry(c, &cdd->ddev.channels, chan.device_node)
>                 if (!c->is_tx)
>
> --
> 2.55.0
>

Re: [PATCH 4/4] dmaengine: ti: cppi41: Share register setup with resume

[Frank Li]

On Tue, Nov 03, 2026 at 08:37:31PM +0100, Markus Schneider-Pargmann (TI) wrote:
> [You don't often get email from msp@baylibre.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Both probe and resume basically do the same thing and setup the
> registers. But resume has its own copy of the register writes. This will
> always lead to differences when changes are made.
>
> Create a separate function that is used by both probe and resume.
> Note that the ordering is slightly different but I didn't see or found
> any issues with the new order while testing.

But no any issues found during testing.

Frank