Re: [PATCH v5 6/7] dmaengine: fsl-edma: support per-channel DT child nodes

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Nov 11, 2026 at 09:47:34PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add support for parsing per-channel child nodes from devicetree.
> Each child node can carry its own interrupts and iommus properties.
> Child power-domains are not handled here; power domains remain attached
> at the parent level.
>
> The implementation is split into three functions:
>
> - fsl_edma_child_to_chan_id(): derives the channel index from the
>   child MMIO address relative to the parent base.  Validate that the
>   child resource is contained in the controller resource and aligned to
>   the channel register spacing, and use div_u64_rem() to compute
>   the quotient and alignment remainder together.
>
> - fsl_edma_configure_chan_from_dt(): called before setup_irq() to
>   populate per-channel txirq from child interrupt properties.
>   fsl_edma3_irq_init() is updated to skip platform_get_irq() for
>   channels that already have an IRQ assigned from their child node.
>
> - fsl_edma_configure_chan_iommu(): called after
>   dmaenginem_async_device_register() to configure per-channel IOMMU
>   by calling of_dma_configure() on each dma_chan_dev and setting
>   chan_dma_dev so dmaengine_get_dma_device() returns the per-channel
>   device.  Propagate the controller dma_parms to the per-channel device
>   so the DMA segment size limit is preserved.

Needn't detail for each funciton.

The implementation is split into three functions:
  - fsl_edma_child_to_chan_id()
  - fsl_edma_configure_chan_from_dt()
  - fsl_edma_configure_chan_iommu()

>
> Child nodes beyond the configured dma-channels are ignored, which allows
> boards to reduce dma-channels without disabling every unused child node.
> If child nodes are present, every unmasked channel must have a child node
> with a valid interrupt; otherwise probe fails with an error asking for
> dma-channel-mask.  In child-node mode fsl_edma3_irq_init() does not fall
> back to the parent's flat interrupt array.
>
> The shared error IRQ remains a parent-node interrupt.  A child-node-only
> DTS without that entry simply has no shared error IRQ.
>
> Use BIT_ULL() for the 64-bit chan_masked bitmap, and use dev_err_probe()
> so -EPROBE_DEFER is not logged as an error.  Reject a channel whose iommus
> property did not result in an IOMMU mapping.
>
> Existing DTS without child nodes continues to work unchanged.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/dma/fsl-edma-common.c |  10 +-
>  drivers/dma/fsl-edma-common.h |   1 +
>  drivers/dma/fsl-edma-main.c   | 229 +++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 222 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index 513b0f4e5be2..62cd60179bc6 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -310,10 +310,10 @@ int fsl_edma_resume(struct dma_chan *chan)
>
>  static void fsl_edma_unprep_slave_dma(struct fsl_edma_chan *fsl_chan)
>  {
> +	struct device *dev = dmaengine_get_dma_device(&fsl_chan->vchan.chan);
> +
>  	if (fsl_chan->dma_dir != DMA_NONE)
> -		dma_unmap_resource(fsl_chan->vchan.chan.device->dev,
> -				   fsl_chan->dma_dev_addr,
> -				   fsl_chan->dma_dev_size,
> +		dma_unmap_resource(dev, fsl_chan->dma_dev_addr, fsl_chan->dma_dev_size,
>  				   fsl_chan->dma_dir, 0);

Use dmaengine_get_dma_device() needs sperate patches.

>  	fsl_chan->dma_dir = DMA_NONE;
>  }
> @@ -321,7 +321,7 @@ static void fsl_edma_unprep_slave_dma(struct fsl_edma_chan *fsl_chan)
>  static bool fsl_edma_prep_slave_dma(struct fsl_edma_chan *fsl_chan,
>  				    enum dma_transfer_direction dir)
>  {
> -	struct device *dev = fsl_chan->vchan.chan.device->dev;
> +	struct device *dev = dmaengine_get_dma_device(&fsl_chan->vchan.chan);
>  	enum dma_data_direction dma_dir;
>  	phys_addr_t addr = 0;
>  	u32 size = 0;
> @@ -1090,7 +1090,7 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
>  	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK)
>  		clk_prepare_enable(fsl_chan->clk);
>
> -	fsl_chan->tcd_pool = dma_pool_create("tcd_pool", chan->device->dev,
> +	fsl_chan->tcd_pool = dma_pool_create("tcd_pool", dmaengine_get_dma_device(chan),
>  				fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_TCD64 ?
>  				sizeof(struct fsl_edma_hw_tcd64) : sizeof(struct fsl_edma_hw_tcd),
>  				32, 0);
> diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
> index cbb077e98f55..8843dd5661fb 100644
> --- a/drivers/dma/fsl-edma-common.h
> +++ b/drivers/dma/fsl-edma-common.h
> @@ -271,6 +271,7 @@ struct fsl_edma_engine {
>  	int			txirq_16_31;
>  	int			errirq;
>  	bool			big_endian;
> +	bool			per_chan_irqs;
>  	struct edma_regs	regs;
>  	u64			chan_masked;
>  	struct fsl_edma_chan	chans[] __counted_by(n_chans);
> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 7f82dcb7831d..e9a7592ef076 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -12,11 +12,15 @@
>
>  #include <dt-bindings/dma/fsl-edma.h>
>  #include <linux/bitfield.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
>  #include <linux/clk.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
>  #include <linux/of_dma.h>
> +#include <linux/of_irq.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pm_domain.h>
> @@ -123,7 +127,7 @@ static irqreturn_t fsl_edma3_err_handler_shared(int irq, void *dev_id)
>  	unsigned int ch;
>
>  	for (ch = 0; ch < fsl_edma->n_chans; ch++) {
> -		if (fsl_edma->chan_masked & BIT(ch))
> +		if (fsl_edma->chan_masked & BIT_ULL(ch))
>  			continue;
>
>  		fsl_edma3_err_check(&fsl_edma->chans[ch]);
> @@ -397,15 +401,24 @@ static int fsl_edma3_irq_init(struct platform_device *pdev, struct fsl_edma_engi
>
>  		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[i];
>
> -		if (fsl_edma->chan_masked & BIT(i))
> +		if (fsl_edma->chan_masked & BIT_ULL(i))
>  			continue;


This one need sperate patch to fix it.

>
> -		/* request channel irq */
> -		fsl_chan->txirq = platform_get_irq(pdev, i);
> -		if (fsl_chan->txirq < 0)
> -			return  -EINVAL;
> +		if (fsl_chan->txirq <= 0) {
> +			/*
> +			 * In child-node mode, missing child nodes are rejected
> +			 * by fsl_edma_configure_chan_from_dt(); do not fall
> +			 * back to the parent's flat interrupt array.
> +			 */
> +			if (fsl_edma->per_chan_irqs)
> +				continue;
>
> -		fsl_chan->irq_handler = fsl_edma3_tx_handler;
> +			fsl_chan->txirq = platform_get_irq(pdev, i);
> +			if (fsl_chan->txirq < 0)
> +				return fsl_chan->txirq;
> +
> +			fsl_chan->irq_handler = fsl_edma3_tx_handler;
> +		}
>
>  		if (!(fsl_edma->drvdata->flags & FSL_EDMA_DRV_ERRIRQ_SHARE)) {
>  			fsl_chan->errirq = fsl_chan->txirq;
> @@ -415,7 +428,11 @@ static int fsl_edma3_irq_init(struct platform_device *pdev, struct fsl_edma_engi
>
>  	/* All channel err use one irq number */
>  	if (fsl_edma->drvdata->flags & FSL_EDMA_DRV_ERRIRQ_SHARE) {
> -		/* last one is error irq */
> +		/*
> +		 * The shared error IRQ remains a parent-node interrupt at
> +		 * index n_chans.  A child-node-only DTS without that entry
> +		 * simply has no shared error IRQ.
> +		 */
>  		fsl_edma->errirq = platform_get_irq_optional(pdev, fsl_edma->n_chans);
>  		if (fsl_edma->errirq < 0)
>  			return 0; /* dts miss err irq, treat as no err irq case */
> @@ -641,7 +658,7 @@ static void fsl_edma3_detach_pd(struct fsl_edma_engine *fsl_edma)
>  	int i;
>
>  	for (i = 0; i < fsl_edma->n_chans; i++) {
> -		if (fsl_edma->chan_masked & BIT(i))
> +		if (fsl_edma->chan_masked & BIT_ULL(i))
>  			continue;
>  		fsl_chan = &fsl_edma->chans[i];
>  		if (fsl_chan->pd_dev_link)
> @@ -669,7 +686,7 @@ static int fsl_edma3_attach_pd(struct platform_device *pdev, struct fsl_edma_eng
>  	dev = &pdev->dev;
>
>  	for (i = 0; i < fsl_edma->n_chans; i++) {
> -		if (fsl_edma->chan_masked & BIT(i))
> +		if (fsl_edma->chan_masked & BIT_ULL(i))
>  			continue;
>
>  		fsl_chan = &fsl_edma->chans[i];
> @@ -703,6 +720,184 @@ static int fsl_edma3_attach_pd(struct platform_device *pdev, struct fsl_edma_eng
>  	return -EINVAL;
>  }
>
> +static int fsl_edma_child_to_chan_id(struct fsl_edma_engine *fsl_edma,
> +				     struct device_node *child,
> +				     struct resource *parent_res)
> +{
> +	const struct fsl_edma_drvdata *drvdata = fsl_edma->drvdata;
> +	struct resource child_res;
> +	u64 offset, ch_id;
> +	u32 rem;
> +	int ret;
> +
> +	ret = of_address_to_resource(child, 0, &child_res);
> +	if (ret)
> +		return ret;
> +
> +	if (!resource_contains(parent_res, &child_res))
> +		return -EINVAL;
> +
> +	if (child_res.start < parent_res->start + drvdata->chreg_off)
> +		return -EINVAL;
> +
> +	offset = child_res.start - parent_res->start - drvdata->chreg_off;
> +	ch_id = div_u64_rem(offset, drvdata->chreg_space_sz, &rem);

It is okay for now. If each channel have sperate node, we can consider
dmaengine consumer direct to point to channel node when channel is fixed.

> +	if (rem)
> +		return -EINVAL;
> +	if (ch_id > INT_MAX)
> +		return -EINVAL;
> +
> +	return ch_id;
> +}
> +
> +static int fsl_edma_configure_chan_from_dt(struct platform_device *pdev,
> +					   struct fsl_edma_engine *fsl_edma)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct resource parent_res;
> +	struct device_node *child;
> +	u64 child_mask = 0;
> +	bool has_children = false;
> +	int i, ret;
> +
> +	ret = of_address_to_resource(np, 0, &parent_res);
> +	if (ret)
> +		return ret;
> +
> +	for_each_available_child_of_node(np, child) {

for_each_available_child_of_node_scoped()

> +		struct fsl_edma_chan *fsl_chan;
> +		int ch_id, irq;
> +
> +		has_children = true;
> +
> +		ch_id = fsl_edma_child_to_chan_id(fsl_edma, child,
> +						  &parent_res);
> +		if (ch_id < 0) {
> +			dev_err(&pdev->dev, "invalid channel in %pOFn\n", child);
> +			of_node_put(child);
> +			return -EINVAL;
> +		}
> +
> +		if (ch_id >= fsl_edma->n_chans)
> +			continue;
> +
> +		if (fsl_edma->chan_masked & BIT_ULL(ch_id))
> +			continue;
> +
> +		child_mask |= BIT_ULL(ch_id);
> +
> +		fsl_chan = &fsl_edma->chans[ch_id];
> +
> +		irq = of_irq_get(child, 0);
> +		if (irq == -EPROBE_DEFER) {
> +			of_node_put(child);
> +			return -EPROBE_DEFER;
> +		}
> +		if (irq <= 0) {
> +			dev_err(&pdev->dev,
> +				"channel %d child node has no valid interrupt; add interrupts or disable it with dma-channel-mask\n",
> +				ch_id);
> +			of_node_put(child);
> +			return irq < 0 ? irq : -EINVAL;
> +		}
> +
> +		fsl_chan->txirq = irq;
> +		fsl_chan->irq_handler = fsl_edma3_tx_handler;
> +	}
> +
> +	if (!has_children)
> +		return 0;
> +
> +	fsl_edma->per_chan_irqs = true;
> +
> +	for (i = 0; i < fsl_edma->n_chans; i++) {
> +		if (fsl_edma->chan_masked & BIT_ULL(i))
> +			continue;
> +		if (child_mask & BIT_ULL(i))
> +			continue;
> +
> +		dev_err(&pdev->dev,
> +			"channel %d has no dma-channel child node; add one or disable it with dma-channel-mask\n",
> +			i);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsl_edma_configure_chan_iommu(struct platform_device *pdev,
> +					struct fsl_edma_engine *fsl_edma)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct resource parent_res;
> +	struct device_node *child;
> +	struct dma_chan *chan;
> +	int ret;
> +
> +	ret = of_address_to_resource(np, 0, &parent_res);
> +	if (ret)
> +		return ret;
> +
> +	for_each_available_child_of_node(np, child) {
> +		struct fsl_edma_chan *fsl_chan;
> +		int ch_id;
> +
> +		if (!of_property_present(child, "iommus"))
> +			continue;
> +
> +		ch_id = fsl_edma_child_to_chan_id(fsl_edma, child,
> +						  &parent_res);
> +		if (ch_id < 0)
> +			continue;
> +
> +		if (ch_id >= fsl_edma->n_chans)
> +			continue;
> +
> +		if (fsl_edma->chan_masked & BIT_ULL(ch_id))
> +			continue;
> +
> +		fsl_chan = &fsl_edma->chans[ch_id];
> +
> +		list_for_each_entry(chan, &fsl_edma->dma_dev.channels,
> +				    device_node) {
> +			struct device *chdev;
> +
> +			if (to_fsl_edma_chan(chan) != fsl_chan)
> +				continue;

Can we avoid scan channel twice?

Frank
> +
> +			chdev = dmaengine_chan_dev(chan);
> +			chdev->dma_parms = pdev->dev.dma_parms;
> +			if (fsl_edma->drvdata->flags & FSL_EDMA_DRV_TCD64)
> +				dma_coerce_mask_and_coherent(chdev,
> +					DMA_BIT_MASK(64));
> +			else
> +				dma_coerce_mask_and_coherent(chdev,
> +					DMA_BIT_MASK(32));
> +			ret = of_dma_configure(chdev, child, true);
> +			if (ret) {
> +				dev_err_probe(&pdev->dev, ret,
> +					      "IOMMU config failed for channel %d\n",
> +					      ch_id);
> +				of_node_put(child);
> +				return ret;
> +			}
> +
> +			if (!device_iommu_mapped(chdev)) {
> +				dev_err_probe(&pdev->dev, -ENODEV,
> +					      "channel %d is not behind an IOMMU\n",
> +					      ch_id);
> +				of_node_put(child);
> +				return -ENODEV;
> +			}
> +
> +			chan->chan_dev->chan_dma_dev = true;
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int fsl_edma_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> @@ -797,7 +992,7 @@ static int fsl_edma_probe(struct platform_device *pdev)
>  		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[i];
>  		int len;
>
> -		if (fsl_edma->chan_masked & BIT(i))
> +		if (fsl_edma->chan_masked & BIT_ULL(i))
>  			continue;
>
>  		snprintf(fsl_chan->chan_name, sizeof(fsl_chan->chan_name), "%s-CH%02d",
> @@ -835,6 +1030,10 @@ static int fsl_edma_probe(struct platform_device *pdev)
>  			clk_disable_unprepare(fsl_chan->clk);
>  	}
>
> +	ret = fsl_edma_configure_chan_from_dt(pdev, fsl_edma);
> +	if (ret)
> +		return ret;
> +
>  	ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
>  	if (ret)
>  		return ret;
> @@ -897,6 +1096,10 @@ static int fsl_edma_probe(struct platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, ret,
>  				     "Can't register Freescale eDMA engine.\n");
>
> +	ret = fsl_edma_configure_chan_iommu(pdev, fsl_edma);
> +	if (ret)
> +		return ret;
> +
>  	ret = devm_of_dma_controller_register(&pdev->dev, np,
>  			drvdata->dmamuxs ? fsl_edma_xlate : fsl_edma3_xlate,
>  			fsl_edma);
> @@ -927,7 +1130,7 @@ static int fsl_edma_suspend_late(struct device *dev)
>
>  	for (i = 0; i < fsl_edma->n_chans; i++) {
>  		fsl_chan = &fsl_edma->chans[i];
> -		if (fsl_edma->chan_masked & BIT(i))
> +		if (fsl_edma->chan_masked & BIT_ULL(i))
>  			continue;
>  		spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
>  		/* Make sure chan is idle or will force disable. */
> @@ -953,7 +1156,7 @@ static int fsl_edma_resume_early(struct device *dev)
>
>  	for (i = 0; i < fsl_edma->n_chans; i++) {
>  		fsl_chan = &fsl_edma->chans[i];
> -		if (fsl_edma->chan_masked & BIT(i))
> +		if (fsl_edma->chan_masked & BIT_ULL(i))
>  			continue;
>  		fsl_chan->pm_state = RUNNING;
>  		edma_write_tcdreg(fsl_chan, 0, csr);
>
> --
> 2.34.1
>