Re: [PATCH v6 8/9] dmaengine: fsl-edma: support per-channel DT child nodes

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

On Thu, Nov 12, 2026 at 03:53:11PM +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()
> - 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 dev_err_probe() so -EPROBE_DEFER is not logged as an error, and reject

This common sense, can remove

> 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.h |   1 +
>  drivers/dma/fsl-edma-main.c   | 199 ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 194 insertions(+), 6 deletions(-)
>
> 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 818b9b986f13..88d2b9d6b6ab 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>
> @@ -400,12 +404,21 @@ static int fsl_edma3_irq_init(struct platform_device *pdev, struct fsl_edma_engi
>  		if (fsl_edma->chan_masked & BIT_ULL(i))
>  			continue;
>
> -		/* 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 */
> @@ -703,6 +720,168 @@ 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);
> +	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;
> +	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_scoped(np, child) {
> +		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);
> +			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)
> +			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);
> +			return irq < 0 ? irq : -EINVAL;

logic have some complex,

	if (irq <= 0)
		return dev_err_probe(&pdev->dev, irq ? irq: -EINVAL,
				     "...." )

basic irq < 0 (include -EPROBE_DEFER), return irq
      irq == 0, return -EINVAL
      irq > 0, okay

> +		}
> +
> +		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;
> +	}

print error only when bit is 0 at both fsl_edma->chan_masked and
child_mask 0

if (~(fsl_edma->chan_masked | child_mask))
	return dev_err_probe(....);

> +
> +	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;
> +	int ret;
> +
> +	ret = of_address_to_resource(np, 0, &parent_res);
> +	if (ret)
> +		return ret;
> +
> +	for_each_available_child_of_node_scoped(np, child) {
> +		struct fsl_edma_chan *fsl_chan;
> +		struct dma_chan *chan;
> +		struct device *chdev;
> +		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];
> +		chan = &fsl_chan->vchan.chan;
> +		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);
> +			return ret;

return dev_err_probe(...)

> +		}
> +
> +		if (!device_iommu_mapped(chdev)) {
> +			dev_err_probe(&pdev->dev, -ENODEV,
> +				      "channel %d is not behind an IOMMU\n",
> +				      ch_id);
> +			return -ENODEV;
> +		}
> +
> +		chan->chan_dev->chan_dma_dev = true;

I think you move fetch irq logic into this funcitons, over half of both
code is similar.

Frank

> +	}
> +
> +	return 0;
> +}
> +
>  static int fsl_edma_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> @@ -835,6 +1014,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 +1080,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);
>
> --
> 2.34.1
>