Re: [PATCH v3 26/34] dmaengine: mpc512x: route callbacks via channel BH

[Frank Li]

On Mon, Aug 10, 2026 at 11:09:27AM -0700, Allen Pais wrote:
> Schedule per-channel BH work for completions and process callbacks there,
> leaving the controller tasklet for IRQ/error handling.
>
> Signed-off-by: Allen Pais <allen.lkml@xxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

>  drivers/dma/mpc512x_dma.c | 74 +++++++++++++++++++++++++--------------
>  1 file changed, 47 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 0adc8e01057e..19e0b2f1b2aa 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -372,45 +372,47 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>
> -/* process completed descriptors */
> -static void mpc_dma_process_completed(struct mpc_dma *mdma)
> +static void mpc_dma_process_chan_completed(struct mpc_dma_chan *mchan)
>  {
>  	dma_cookie_t last_cookie = 0;
> -	struct mpc_dma_chan *mchan;
>  	struct mpc_dma_desc *mdesc;
>  	struct dma_async_tx_descriptor *desc;
>  	unsigned long flags;
>  	LIST_HEAD(list);
> -	int i;
>
> -	for (i = 0; i < mdma->dma.chancnt; i++) {
> -		mchan = &mdma->channels[i];
> +	/* Get all completed descriptors */
> +	spin_lock_irqsave(&mchan->lock, flags);
> +	if (!list_empty(&mchan->completed))
> +		list_splice_tail_init(&mchan->completed, &list);
> +	spin_unlock_irqrestore(&mchan->lock, flags);
>
> -		/* Get all completed descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		if (!list_empty(&mchan->completed))
> -			list_splice_tail_init(&mchan->completed, &list);
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> +	if (list_empty(&list))
> +		return;
>
> -		if (list_empty(&list))
> -			continue;
> +	/* Execute callbacks and run dependencies */
> +	list_for_each_entry(mdesc, &list, node) {
> +		desc = &mdesc->desc;
>
> -		/* Execute callbacks and run dependencies */
> -		list_for_each_entry(mdesc, &list, node) {
> -			desc = &mdesc->desc;
> +		dmaengine_desc_get_callback_invoke(desc, NULL);
>
> -			dmaengine_desc_get_callback_invoke(desc, NULL);
> +		last_cookie = desc->cookie;
> +		dma_run_dependencies(desc);
> +	}
>
> -			last_cookie = desc->cookie;
> -			dma_run_dependencies(desc);
> -		}
> +	/* Free descriptors */
> +	spin_lock_irqsave(&mchan->lock, flags);
> +	list_splice_tail_init(&list, &mchan->free);
> +	mchan->chan.completed_cookie = last_cookie;
> +	spin_unlock_irqrestore(&mchan->lock, flags);
> +}
>
> -		/* Free descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		list_splice_tail_init(&list, &mchan->free);
> -		mchan->chan.completed_cookie = last_cookie;
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> -	}
> +/* process completed descriptors */
> +static void mpc_dma_process_completed(struct mpc_dma *mdma)
> +{
> +	int i;
> +
> +	for (i = 0; i < mdma->dma.chancnt; i++)
> +		mpc_dma_process_chan_completed(&mdma->channels[i]);
>  }
>
>  /* DMA Tasklet */
> @@ -419,6 +421,7 @@ static void mpc_dma_tasklet(struct tasklet_struct *t)
>  	struct mpc_dma *mdma = from_tasklet(mdma, t, tasklet);
>  	unsigned long flags;
>  	uint es;
> +	int i;
>
>  	spin_lock_irqsave(&mdma->error_status_lock, flags);
>  	es = mdma->error_status;
> @@ -453,7 +456,21 @@ static void mpc_dma_tasklet(struct tasklet_struct *t)
>  			dev_err(mdma->dma.dev, "- Destination Bus Error\n");
>  	}
>
> -	mpc_dma_process_completed(mdma);
> +	for (i = 0; i < mdma->dma.chancnt; i++) {
> +		struct mpc_dma_chan *mchan = &mdma->channels[i];
> +
> +		spin_lock_irqsave(&mchan->lock, flags);
> +		if (!list_empty(&mchan->completed))
> +			dmaengine_schedule_bh(&mchan->chan);
> +		spin_unlock_irqrestore(&mchan->lock, flags);
> +	}
> +}
> +
> +static void mpc_dma_chan_bh(struct dma_chan *chan)
> +{
> +	struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
> +
> +	mpc_dma_process_chan_completed(mchan);
>  }
>
>  /* Submit descriptor to hardware */
> @@ -551,6 +568,8 @@ static void mpc_dma_free_chan_resources(struct dma_chan *chan)
>  	unsigned long flags;
>  	LIST_HEAD(descs);
>
> +	dmaengine_kill_bh(&mchan->chan);
> +
>  	spin_lock_irqsave(&mchan->lock, flags);
>
>  	/* Channel must be idle */
> @@ -1007,6 +1026,7 @@ static int mpc_dma_probe(struct platform_device *op)
>  		INIT_LIST_HEAD(&mchan->completed);
>
>  		spin_lock_init(&mchan->lock);
> +		dmaengine_init_bh(&mchan->chan, mpc_dma_chan_bh);
>  		list_add_tail(&mchan->chan.device_node, &dma->channels);
>  	}
>
> --
> 2.43.0
>