[PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists

[PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The driver lists (ld_free, ld_queue) are used in
rz_dmac_free_chan_resources(), rz_dmac_terminate_all(),
rz_dmac_issue_pending(), and rz_dmac_irq_handler_thread(), all under
the virtual channel lock. Take the same lock in rz_dmac_prep_slave_sg()
and rz_dmac_prep_dma_memcpy() as well to avoid concurrency issues, since
these functions also check whether the lists are empty and update or
remove list entries.

Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- none

Changes in v6:
- none

Changes in v5:
- none, this patch is new

 drivers/dma/sh/rz-dmac.c | 57 ++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 3dde4b006bcc..36f5fc80a17a 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #include <linux/interrupt.h>
@@ -453,6 +454,7 @@ static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
 		if (!desc)
 			break;
 
+		/* No need to lock. This is called only for the 1st client. */
 		list_add_tail(&desc->node, &channel->ld_free);
 		channel->descs_allocated++;
 	}
@@ -508,18 +510,21 @@ rz_dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 	dev_dbg(dmac->dev, "%s channel: %d src=0x%pad dst=0x%pad len=%zu\n",
 		__func__, channel->index, &src, &dest, len);
 
-	if (list_empty(&channel->ld_free))
-		return NULL;
+	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
+		if (list_empty(&channel->ld_free))
+			return NULL;
+
+		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
 
-	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
+		desc->type = RZ_DMAC_DESC_MEMCPY;
+		desc->src = src;
+		desc->dest = dest;
+		desc->len = len;
+		desc->direction = DMA_MEM_TO_MEM;
 
-	desc->type = RZ_DMAC_DESC_MEMCPY;
-	desc->src = src;
-	desc->dest = dest;
-	desc->len = len;
-	desc->direction = DMA_MEM_TO_MEM;
+		list_move_tail(channel->ld_free.next, &channel->ld_queue);
+	}
 
-	list_move_tail(channel->ld_free.next, &channel->ld_queue);
 	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
 }
 
@@ -535,27 +540,29 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	int dma_length = 0;
 	int i = 0;
 
-	if (list_empty(&channel->ld_free))
-		return NULL;
+	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
+		if (list_empty(&channel->ld_free))
+			return NULL;
 
-	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
+		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
 
-	for_each_sg(sgl, sg, sg_len, i) {
-		dma_length += sg_dma_len(sg);
-	}
+		for_each_sg(sgl, sg, sg_len, i)
+			dma_length += sg_dma_len(sg);
 
-	desc->type = RZ_DMAC_DESC_SLAVE_SG;
-	desc->sg = sgl;
-	desc->sgcount = sg_len;
-	desc->len = dma_length;
-	desc->direction = direction;
+		desc->type = RZ_DMAC_DESC_SLAVE_SG;
+		desc->sg = sgl;
+		desc->sgcount = sg_len;
+		desc->len = dma_length;
+		desc->direction = direction;
 
-	if (direction == DMA_DEV_TO_MEM)
-		desc->src = channel->src_per_address;
-	else
-		desc->dest = channel->dst_per_address;
+		if (direction == DMA_DEV_TO_MEM)
+			desc->src = channel->src_per_address;
+		else
+			desc->dest = channel->dst_per_address;
+
+		list_move_tail(channel->ld_free.next, &channel->ld_queue);
+	}
 
-	list_move_tail(channel->ld_free.next, &channel->ld_queue);
 	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
 }
 
-- 
2.43.0

[PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Both rz_dmac_disable_hw() and rz_dmac_irq_handle_channel() update the
CHCTRL register. To avoid concurrency issues when configuring
functionalities exposed by this registers, take the virtual channel lock.
All other CHCTRL updates were already protected by the same lock.

Previously, rz_dmac_disable_hw() disabled and re-enabled local IRQs, before
accessing CHCTRL registers but this does not ensure race-free access.
Remove the local IRQ disable/enable code as well.

Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
Cc: stable@vger.kernel.org
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- collected tags

Changes in v6:
- update patch title and description
- in rz_dmac_irq_handle_channel() lock only around the
  updates for the error path and continued using the vc lock
  as this is the error path and the channel will anyway be
  stopped; this avoids updating the code with another lock
  as it was suggested in the review process of v5 and the code
  remain simpler for a fix, w/o any impact on performance

Changes in v5:
- none, this patch is new

 drivers/dma/sh/rz-dmac.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 36f5fc80a17a..c0f1e77996bd 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -304,13 +304,10 @@ static void rz_dmac_disable_hw(struct rz_dmac_chan *channel)
 {
 	struct dma_chan *chan = &channel->vc.chan;
 	struct rz_dmac *dmac = to_rz_dmac(chan->device);
-	unsigned long flags;
 
 	dev_dbg(dmac->dev, "%s channel %d\n", __func__, channel->index);
 
-	local_irq_save(flags);
 	rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
-	local_irq_restore(flags);
 }
 
 static void rz_dmac_set_dmars_register(struct rz_dmac *dmac, int nr, u32 dmars)
@@ -574,8 +571,8 @@ static int rz_dmac_terminate_all(struct dma_chan *chan)
 	unsigned int i;
 	LIST_HEAD(head);
 
-	rz_dmac_disable_hw(channel);
 	spin_lock_irqsave(&channel->vc.lock, flags);
+	rz_dmac_disable_hw(channel);
 	for (i = 0; i < DMAC_NR_LMDESC; i++)
 		lmdesc[i].header = 0;
 
@@ -706,7 +703,9 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
 	if (chstat & CHSTAT_ER) {
 		dev_err(dmac->dev, "DMAC err CHSTAT_%d = %08X\n",
 			channel->index, chstat);
-		rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
+
+		scoped_guard(spinlock_irqsave, &channel->vc.lock)
+			rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
 		goto done;
 	}
 
-- 
2.43.0

Re: [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists

[Frank Li]

On Tue, Jan 20, 2026 at 03:33:23PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The driver lists (ld_free, ld_queue) are used in
> rz_dmac_free_chan_resources(), rz_dmac_terminate_all(),
> rz_dmac_issue_pending(), and rz_dmac_irq_handler_thread(), all under
> the virtual channel lock. Take the same lock in rz_dmac_prep_slave_sg()
> and rz_dmac_prep_dma_memcpy() as well to avoid concurrency issues, since
> these functions also check whether the lists are empty and update or
> remove list entries.
>
> Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - none
>
> Changes in v6:
> - none
>
> Changes in v5:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 57 ++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 3dde4b006bcc..36f5fc80a17a 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  #include <linux/interrupt.h>
> @@ -453,6 +454,7 @@ static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
>  		if (!desc)
>  			break;
>
> +		/* No need to lock. This is called only for the 1st client. */
>  		list_add_tail(&desc->node, &channel->ld_free);
>  		channel->descs_allocated++;
>  	}
> @@ -508,18 +510,21 @@ rz_dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>  	dev_dbg(dmac->dev, "%s channel: %d src=0x%pad dst=0x%pad len=%zu\n",
>  		__func__, channel->index, &src, &dest, len);
>
> -	if (list_empty(&channel->ld_free))
> -		return NULL;
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		if (list_empty(&channel->ld_free))
> +			return NULL;
> +
> +		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
>
> -	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
> +		desc->type = RZ_DMAC_DESC_MEMCPY;
> +		desc->src = src;
> +		desc->dest = dest;
> +		desc->len = len;
> +		desc->direction = DMA_MEM_TO_MEM;
>
> -	desc->type = RZ_DMAC_DESC_MEMCPY;
> -	desc->src = src;
> -	desc->dest = dest;
> -	desc->len = len;
> -	desc->direction = DMA_MEM_TO_MEM;
> +		list_move_tail(channel->ld_free.next, &channel->ld_queue);
> +	}
>
> -	list_move_tail(channel->ld_free.next, &channel->ld_queue);
>  	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
>  }
>
> @@ -535,27 +540,29 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  	int dma_length = 0;
>  	int i = 0;
>
> -	if (list_empty(&channel->ld_free))
> -		return NULL;
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		if (list_empty(&channel->ld_free))
> +			return NULL;
>
> -	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
> +		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
>
> -	for_each_sg(sgl, sg, sg_len, i) {
> -		dma_length += sg_dma_len(sg);
> -	}
> +		for_each_sg(sgl, sg, sg_len, i)
> +			dma_length += sg_dma_len(sg);
>
> -	desc->type = RZ_DMAC_DESC_SLAVE_SG;
> -	desc->sg = sgl;
> -	desc->sgcount = sg_len;
> -	desc->len = dma_length;
> -	desc->direction = direction;
> +		desc->type = RZ_DMAC_DESC_SLAVE_SG;
> +		desc->sg = sgl;
> +		desc->sgcount = sg_len;
> +		desc->len = dma_length;
> +		desc->direction = direction;
>
> -	if (direction == DMA_DEV_TO_MEM)
> -		desc->src = channel->src_per_address;
> -	else
> -		desc->dest = channel->dst_per_address;
> +		if (direction == DMA_DEV_TO_MEM)
> +			desc->src = channel->src_per_address;
> +		else
> +			desc->dest = channel->dst_per_address;
> +
> +		list_move_tail(channel->ld_free.next, &channel->ld_queue);
> +	}
>
> -	list_move_tail(channel->ld_free.next, &channel->ld_queue);
>  	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
>  }
>
> --
> 2.43.0
>

Re: [PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock

[Frank Li]

On Tue, Jan 20, 2026 at 03:33:24PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Both rz_dmac_disable_hw() and rz_dmac_irq_handle_channel() update the
> CHCTRL register. To avoid concurrency issues when configuring
> functionalities exposed by this registers, take the virtual channel lock.
> All other CHCTRL updates were already protected by the same lock.
>
> Previously, rz_dmac_disable_hw() disabled and re-enabled local IRQs, before
> accessing CHCTRL registers but this does not ensure race-free access.
> Remove the local IRQ disable/enable code as well.
>
> Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
> Cc: stable@vger.kernel.org
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - collected tags
>
> Changes in v6:
> - update patch title and description
> - in rz_dmac_irq_handle_channel() lock only around the
>   updates for the error path and continued using the vc lock
>   as this is the error path and the channel will anyway be
>   stopped; this avoids updating the code with another lock
>   as it was suggested in the review process of v5 and the code
>   remain simpler for a fix, w/o any impact on performance
>
> Changes in v5:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 36f5fc80a17a..c0f1e77996bd 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -304,13 +304,10 @@ static void rz_dmac_disable_hw(struct rz_dmac_chan *channel)
>  {
>  	struct dma_chan *chan = &channel->vc.chan;
>  	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> -	unsigned long flags;
>
>  	dev_dbg(dmac->dev, "%s channel %d\n", __func__, channel->index);
>
> -	local_irq_save(flags);
>  	rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
> -	local_irq_restore(flags);
>  }
>
>  static void rz_dmac_set_dmars_register(struct rz_dmac *dmac, int nr, u32 dmars)
> @@ -574,8 +571,8 @@ static int rz_dmac_terminate_all(struct dma_chan *chan)
>  	unsigned int i;
>  	LIST_HEAD(head);
>
> -	rz_dmac_disable_hw(channel);
>  	spin_lock_irqsave(&channel->vc.lock, flags);
> +	rz_dmac_disable_hw(channel);
>  	for (i = 0; i < DMAC_NR_LMDESC; i++)
>  		lmdesc[i].header = 0;
>
> @@ -706,7 +703,9 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>  	if (chstat & CHSTAT_ER) {
>  		dev_err(dmac->dev, "DMAC err CHSTAT_%d = %08X\n",
>  			channel->index, chstat);
> -		rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
> +
> +		scoped_guard(spinlock_irqsave, &channel->vc.lock)
> +			rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
>  		goto done;
>  	}
>
> --
> 2.43.0
>