public inbox for linux-m68k@lists.linux-m68k.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support
@ 2025-11-26  8:36 Jean-Michel Hautbois via B4 Relay
  2025-11-26  8:36 ` [PATCH v2 1/5] dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA Jean-Michel Hautbois via B4 Relay
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Jean-Michel Hautbois via B4 Relay @ 2025-11-26  8:36 UTC (permalink / raw)
  To: Frank Li, Vinod Koul, Angelo Dureghello
  Cc: Greg Ungerer, imx, dmaengine, linux-m68k, linux-kernel,
	Jean-Michel Hautbois

This series addresses several bugs in the fsl-edma and mcf-edma drivers
affecting MCF54418 ColdFire processors.

Patch 1 adds the FSL_EDMA_DRV_MCF flag to fix byte-lane addressing for
MCF54418.

Patch 2 adds per-channel IRQ naming for easier debugging.

Patches 3-5 fix the interrupt and error handlers for all 64 DMA
channels:
- Patch 3 fixes the interrupt handler to process all 64 channels
- Patch 4 moves the error handler out of the header file for clarity
- Patch 5 fixes the error handler for all 64 channels with proper types

Tested on a custom MCF54418-based platform with slave DMA transfers.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
Changes in v2:
- Check devm_kasprintf() return value
- Keep request_irq on one line in naming patch
- Remove non needed memory barrier
- Remove the interleave patch for now
- Link to v1: https://lore.kernel.org/r/20251124-dma-coldfire-v1-0-dc8f93185464@yoseli.org

---
Jean-Michel Hautbois (5):
      dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA
      dma: mcf-edma: Add per-channel IRQ naming for debugging
      dma: mcf-edma: Fix interrupt handler for 64 DMA channels
      dma: fsl-edma: Move error handler out of header file
      dma: mcf-edma: Fix error handler for all 64 DMA channels

 drivers/dma/fsl-edma-common.c |  5 +++
 drivers/dma/fsl-edma-common.h | 11 +++----
 drivers/dma/mcf-edma-main.c   | 72 +++++++++++++++++++++++++------------------
 3 files changed, 52 insertions(+), 36 deletions(-)
---
base-commit: ac3fd01e4c1efce8f2c054cdeb2ddd2fc0fb150d
change-id: 20251123-dma-coldfire-5f36aee143b3

Best regards,
--  
Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/5] dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA
  2025-11-26  8:36 [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Jean-Michel Hautbois via B4 Relay
@ 2025-11-26  8:36 ` Jean-Michel Hautbois via B4 Relay
  2025-11-26  8:36 ` [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging Jean-Michel Hautbois via B4 Relay
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jean-Michel Hautbois via B4 Relay @ 2025-11-26  8:36 UTC (permalink / raw)
  To: Frank Li, Vinod Koul, Angelo Dureghello
  Cc: Greg Ungerer, imx, dmaengine, linux-m68k, linux-kernel,
	Jean-Michel Hautbois

From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Add FSL_EDMA_DRV_MCF driver flag to identify MCF ColdFire eDMA
controllers which have a native M68K register layout.

The edma_writeb() function applies an XOR ^ 0x3 byte-lane adjustment for
big-endian eDMA controllers where byte registers within a 32-bit word
need address correction.

However, the MCF54418 eDMA 8-bit registers (SERQ, CERQ, SEEI, CEEI,
CINT, CERR, SSRT, CDNE) are located at sequential byte addresses
(0x4018-0x401F) as documented in the MCF54418 Reference Manual Table
19-2. No byte-lane adjustment is needed, as applying the XOR causes
writes to target incorrect registers (writing to CERR at 0x401D would
actually access SSRT at 0x401E).

Set this flag in the MCF eDMA driver to bypass the XOR adjustment and
access registers at their documented addresses.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 drivers/dma/fsl-edma-common.h | 5 ++++-
 drivers/dma/mcf-edma-main.c   | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 205a96489094..4c86f2f39c1d 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -225,6 +225,8 @@ struct fsl_edma_desc {
 #define FSL_EDMA_DRV_TCD64		BIT(15)
 /* All channel ERR IRQ share one IRQ line */
 #define FSL_EDMA_DRV_ERRIRQ_SHARE       BIT(16)
+/* MCF eDMA: Different register layout, no XOR for byte access */
+#define FSL_EDMA_DRV_MCF                BIT(17)
 
 
 #define FSL_EDMA_DRV_EDMA3	(FSL_EDMA_DRV_SPLIT_REG |	\
@@ -419,7 +421,8 @@ static inline void edma_writeb(struct fsl_edma_engine *edma,
 			       u8 val, void __iomem *addr)
 {
 	/* swap the reg offset for these in big-endian mode */
-	if (edma->big_endian)
+	/* MCF eDMA has different register layout, no XOR needed */
+	if (edma->big_endian && !(edma->drvdata->flags & FSL_EDMA_DRV_MCF))
 		iowrite8(val, (void __iomem *)((unsigned long)addr ^ 0x3));
 	else
 		iowrite8(val, addr);
diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index 9e1c6400c77b..f95114829d80 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -145,7 +145,7 @@ static void mcf_edma_irq_free(struct platform_device *pdev,
 }
 
 static struct fsl_edma_drvdata mcf_data = {
-	.flags = FSL_EDMA_DRV_EDMA64,
+	.flags = FSL_EDMA_DRV_EDMA64 | FSL_EDMA_DRV_MCF,
 	.setup_irq = mcf_edma_irq_init,
 };
 

-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging
  2025-11-26  8:36 [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Jean-Michel Hautbois via B4 Relay
  2025-11-26  8:36 ` [PATCH v2 1/5] dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA Jean-Michel Hautbois via B4 Relay
@ 2025-11-26  8:36 ` Jean-Michel Hautbois via B4 Relay
  2025-11-26 16:12   ` Frank Li
  2025-11-26  8:36 ` [PATCH v2 3/5] dma: mcf-edma: Fix interrupt handler for 64 DMA channels Jean-Michel Hautbois via B4 Relay
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jean-Michel Hautbois via B4 Relay @ 2025-11-26  8:36 UTC (permalink / raw)
  To: Frank Li, Vinod Koul, Angelo Dureghello
  Cc: Greg Ungerer, imx, dmaengine, linux-m68k, linux-kernel,
	Jean-Michel Hautbois

From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Add dynamic per-channel IRQ naming to make DMA interrupt identification
easier in /proc/interrupts and debugging tools.

Instead of all channels showing "eDMA", they now show:
- "eDMA-0" through "eDMA-15" for channels 0-15
- "eDMA-16" through "eDMA-55" for channels 16-55
- "eDMA-tx-56" for the shared channel 56-63 interrupt
- "eDMA-err" for the error interrupt

This aids debugging DMA issues by making it clear which channel's
interrupt is being serviced.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 drivers/dma/mcf-edma-main.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index f95114829d80..6a7d88895501 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -81,8 +81,14 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
 	if (!res)
 		return -1;
 
-	for (ret = 0, i = res->start; i <= res->end; ++i)
-		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
+	for (ret = 0, i = res->start; i <= res->end; ++i) {
+		char *irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						"eDMA-%d", (int)(i - res->start));
+		if (!irq_name)
+			return -ENOMEM;
+
+		ret |= request_irq(i, mcf_edma_tx_handler, 0, irq_name, mcf_edma);
+	}
 	if (ret)
 		return ret;
 
@@ -91,23 +97,27 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
 	if (!res)
 		return -1;
 
-	for (ret = 0, i = res->start; i <= res->end; ++i)
-		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
+	for (ret = 0, i = res->start; i <= res->end; ++i) {
+		char *irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						"eDMA-%d", (int)(16 + i - res->start));
+		if (!irq_name)
+			return -ENOMEM;
+
+		ret |= request_irq(i, mcf_edma_tx_handler, 0, irq_name, mcf_edma);
+	}
 	if (ret)
 		return ret;
 
 	ret = platform_get_irq_byname(pdev, "edma-tx-56-63");
 	if (ret != -ENXIO) {
-		ret = request_irq(ret, mcf_edma_tx_handler,
-				  0, "eDMA", mcf_edma);
+		ret = request_irq(ret, mcf_edma_tx_handler, 0, "eDMA-tx-56", mcf_edma);
 		if (ret)
 			return ret;
 	}
 
 	ret = platform_get_irq_byname(pdev, "edma-err");
 	if (ret != -ENXIO) {
-		ret = request_irq(ret, mcf_edma_err_handler,
-				  0, "eDMA", mcf_edma);
+		ret = request_irq(ret, mcf_edma_err_handler, 0, "eDMA-err", mcf_edma);
 		if (ret)
 			return ret;
 	}

-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/5] dma: mcf-edma: Fix interrupt handler for 64 DMA channels
  2025-11-26  8:36 [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Jean-Michel Hautbois via B4 Relay
  2025-11-26  8:36 ` [PATCH v2 1/5] dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA Jean-Michel Hautbois via B4 Relay
  2025-11-26  8:36 ` [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging Jean-Michel Hautbois via B4 Relay
@ 2025-11-26  8:36 ` Jean-Michel Hautbois via B4 Relay
  2025-11-26  8:36 ` [PATCH v2 4/5] dma: fsl-edma: Move error handler out of header file Jean-Michel Hautbois via B4 Relay
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jean-Michel Hautbois via B4 Relay @ 2025-11-26  8:36 UTC (permalink / raw)
  To: Frank Li, Vinod Koul, Angelo Dureghello
  Cc: Greg Ungerer, imx, dmaengine, linux-m68k, linux-kernel,
	Jean-Michel Hautbois

From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Fix the DMA completion interrupt handler to properly handle all 64
channels on MCF54418 ColdFire processors.

The previous code used BIT(ch) to test interrupt status bits, which
causes undefined behavior on 32-bit architectures when ch >= 32 because
unsigned long is 32 bits and the shift would exceed the type width.

Replace with bitmap_from_u64() and for_each_set_bit() which correctly
handle 64-bit values on 32-bit systems by using a proper bitmap
representation.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 drivers/dma/mcf-edma-main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index 6a7d88895501..6353303df957 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -18,7 +18,8 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
 {
 	struct fsl_edma_engine *mcf_edma = dev_id;
 	struct edma_regs *regs = &mcf_edma->regs;
-	unsigned int ch;
+	unsigned long ch;
+	DECLARE_BITMAP(status_mask, 64);
 	u64 intmap;
 
 	intmap = ioread32(regs->inth);
@@ -27,11 +28,11 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
 	if (!intmap)
 		return IRQ_NONE;
 
-	for (ch = 0; ch < mcf_edma->n_chans; ch++) {
-		if (intmap & BIT(ch)) {
-			iowrite8(EDMA_MASK_CH(ch), regs->cint);
-			fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
-		}
+	bitmap_from_u64(status_mask, intmap);
+
+	for_each_set_bit(ch, status_mask, mcf_edma->n_chans) {
+		iowrite8(EDMA_MASK_CH(ch), regs->cint);
+		fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
 	}
 
 	return IRQ_HANDLED;

-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/5] dma: fsl-edma: Move error handler out of header file
  2025-11-26  8:36 [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Jean-Michel Hautbois via B4 Relay
                   ` (2 preceding siblings ...)
  2025-11-26  8:36 ` [PATCH v2 3/5] dma: mcf-edma: Fix interrupt handler for 64 DMA channels Jean-Michel Hautbois via B4 Relay
@ 2025-11-26  8:36 ` Jean-Michel Hautbois via B4 Relay
  2025-11-26  8:36 ` [PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels Jean-Michel Hautbois via B4 Relay
  2025-12-16 15:27 ` [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Vinod Koul
  5 siblings, 0 replies; 13+ messages in thread
From: Jean-Michel Hautbois via B4 Relay @ 2025-11-26  8:36 UTC (permalink / raw)
  To: Frank Li, Vinod Koul, Angelo Dureghello
  Cc: Greg Ungerer, imx, dmaengine, linux-m68k, linux-kernel,
	Jean-Michel Hautbois

From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Move fsl_edma_err_chan_handler from an inline function in the header
to a proper function in fsl-edma-common.c. This prepares for MCF
ColdFire eDMA support where the error handler needs to be called from
the MCF-specific error interrupt handler.

No functional change for existing users.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 drivers/dma/fsl-edma-common.c | 5 +++++
 drivers/dma/fsl-edma-common.h | 6 +-----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index 4976d7dde080..49d5dca216b6 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -44,6 +44,11 @@
 #define EDMA64_ERRH		0x28
 #define EDMA64_ERRL		0x2c
 
+void fsl_edma_err_chan_handler(struct fsl_edma_chan *fsl_chan)
+{
+	fsl_chan->status = DMA_ERROR;
+}
+
 void fsl_edma_tx_chan_handler(struct fsl_edma_chan *fsl_chan)
 {
 	spin_lock(&fsl_chan->vchan.lock);
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 4c86f2f39c1d..64b537527291 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -478,11 +478,7 @@ static inline struct fsl_edma_desc *to_fsl_edma_desc(struct virt_dma_desc *vd)
 	return container_of(vd, struct fsl_edma_desc, vdesc);
 }
 
-static inline void fsl_edma_err_chan_handler(struct fsl_edma_chan *fsl_chan)
-{
-	fsl_chan->status = DMA_ERROR;
-}
-
+void fsl_edma_err_chan_handler(struct fsl_edma_chan *fsl_chan);
 void fsl_edma_tx_chan_handler(struct fsl_edma_chan *fsl_chan);
 void fsl_edma_disable_request(struct fsl_edma_chan *fsl_chan);
 void fsl_edma_chan_mux(struct fsl_edma_chan *fsl_chan,

-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels
  2025-11-26  8:36 [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Jean-Michel Hautbois via B4 Relay
                   ` (3 preceding siblings ...)
  2025-11-26  8:36 ` [PATCH v2 4/5] dma: fsl-edma: Move error handler out of header file Jean-Michel Hautbois via B4 Relay
@ 2025-11-26  8:36 ` Jean-Michel Hautbois via B4 Relay
  2025-11-26 16:14   ` Frank Li
  2025-12-16 15:27 ` [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Vinod Koul
  5 siblings, 1 reply; 13+ messages in thread
From: Jean-Michel Hautbois via B4 Relay @ 2025-11-26  8:36 UTC (permalink / raw)
  To: Frank Li, Vinod Koul, Angelo Dureghello
  Cc: Greg Ungerer, imx, dmaengine, linux-m68k, linux-kernel,
	Jean-Michel Hautbois

From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Fix the DMA error interrupt handler to properly handle errors on all
64 channels. The previous implementation had several issues:

1. Returned IRQ_NONE if low channels had no errors, even if high
   channels did
2. Used direct status assignment instead of fsl_edma_err_chan_handler()
   for high channels

Split the error handling into two separate loops for the low (0-31)
and high (32-63) channel groups, using for_each_set_bit() for cleaner
iteration. Both groups now consistently use fsl_edma_err_chan_handler()
for proper error status reporting.

Fixes: e7a3ff92eaf1 ("dmaengine: fsl-edma: add ColdFire mcf5441x edma support")
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 drivers/dma/mcf-edma-main.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index 6353303df957..5bd04faa639c 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -12,6 +12,7 @@
 #include "fsl-edma-common.h"
 
 #define EDMA_CHANNELS		64
+#define EDMA_CHANS_PER_REG	(EDMA_CHANNELS / 2)
 #define EDMA_MASK_CH(x)		((x) & GENMASK(5, 0))
 
 static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
@@ -42,33 +43,33 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
 {
 	struct fsl_edma_engine *mcf_edma = dev_id;
 	struct edma_regs *regs = &mcf_edma->regs;
-	unsigned int err, ch;
+	unsigned int ch;
+	unsigned long err;
+	bool handled = false;
 
+	/* Check low 32 channels (0-31) */
 	err = ioread32(regs->errl);
-	if (!err)
-		return IRQ_NONE;
-
-	for (ch = 0; ch < (EDMA_CHANNELS / 2); ch++) {
-		if (err & BIT(ch)) {
+	if (err) {
+		handled = true;
+		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
 			fsl_edma_disable_request(&mcf_edma->chans[ch]);
 			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
 			fsl_edma_err_chan_handler(&mcf_edma->chans[ch]);
 		}
 	}
 
+	/* Check high 32 channels (32-63) */
 	err = ioread32(regs->errh);
-	if (!err)
-		return IRQ_NONE;
-
-	for (ch = (EDMA_CHANNELS / 2); ch < EDMA_CHANNELS; ch++) {
-		if (err & (BIT(ch - (EDMA_CHANNELS / 2)))) {
-			fsl_edma_disable_request(&mcf_edma->chans[ch]);
-			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
-			mcf_edma->chans[ch].status = DMA_ERROR;
+	if (err) {
+		handled = true;
+		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
+			fsl_edma_disable_request(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
+			iowrite8(EDMA_CERR_CERR(ch + EDMA_CHANS_PER_REG), regs->cerr);
+			fsl_edma_err_chan_handler(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
 		}
 	}
 
-	return IRQ_HANDLED;
+	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
 static int mcf_edma_irq_init(struct platform_device *pdev,

-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging
  2025-11-26  8:36 ` [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging Jean-Michel Hautbois via B4 Relay
@ 2025-11-26 16:12   ` Frank Li
  2025-12-16 15:26     ` Vinod Koul
  0 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2025-11-26 16:12 UTC (permalink / raw)
  To: jeanmichel.hautbois
  Cc: Vinod Koul, Angelo Dureghello, Greg Ungerer, imx, dmaengine,
	linux-m68k, linux-kernel

On Wed, Nov 26, 2025 at 09:36:03AM +0100, Jean-Michel Hautbois via B4 Relay wrote:
> From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
>
> Add dynamic per-channel IRQ naming to make DMA interrupt identification
> easier in /proc/interrupts and debugging tools.
>
> Instead of all channels showing "eDMA", they now show:
> - "eDMA-0" through "eDMA-15" for channels 0-15
> - "eDMA-16" through "eDMA-55" for channels 16-55
> - "eDMA-tx-56" for the shared channel 56-63 interrupt
> - "eDMA-err" for the error interrupt
>
> This aids debugging DMA issues by making it clear which channel's
> interrupt is being serviced.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> ---
>  drivers/dma/mcf-edma-main.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> index f95114829d80..6a7d88895501 100644
> --- a/drivers/dma/mcf-edma-main.c
> +++ b/drivers/dma/mcf-edma-main.c
> @@ -81,8 +81,14 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
>  	if (!res)
>  		return -1;
>
> -	for (ret = 0, i = res->start; i <= res->end; ++i)
> -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> +	for (ret = 0, i = res->start; i <= res->end; ++i) {
> +		char *irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> +						"eDMA-%d", (int)(i - res->start));
> +		if (!irq_name)
> +			return -ENOMEM;
> +
> +		ret |= request_irq(i, mcf_edma_tx_handler, 0, irq_name, mcf_edma);
> +	}
>  	if (ret)
>  		return ret;

The existing code have problem, it should use devm_request_irq(). if one
irq request failure, return here,  requested irq will not free.

You'd better add patch before this one to change to use devm_request_irq()

Frank

>
> @@ -91,23 +97,27 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
>  	if (!res)
>  		return -1;
>
> -	for (ret = 0, i = res->start; i <= res->end; ++i)
> -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> +	for (ret = 0, i = res->start; i <= res->end; ++i) {
> +		char *irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> +						"eDMA-%d", (int)(16 + i - res->start));
> +		if (!irq_name)
> +			return -ENOMEM;
> +
> +		ret |= request_irq(i, mcf_edma_tx_handler, 0, irq_name, mcf_edma);
> +	}
>  	if (ret)
>  		return ret;
>
>  	ret = platform_get_irq_byname(pdev, "edma-tx-56-63");
>  	if (ret != -ENXIO) {
> -		ret = request_irq(ret, mcf_edma_tx_handler,
> -				  0, "eDMA", mcf_edma);
> +		ret = request_irq(ret, mcf_edma_tx_handler, 0, "eDMA-tx-56", mcf_edma);
>  		if (ret)
>  			return ret;
>  	}
>
>  	ret = platform_get_irq_byname(pdev, "edma-err");
>  	if (ret != -ENXIO) {
> -		ret = request_irq(ret, mcf_edma_err_handler,
> -				  0, "eDMA", mcf_edma);
> +		ret = request_irq(ret, mcf_edma_err_handler, 0, "eDMA-err", mcf_edma);
>  		if (ret)
>  			return ret;
>  	}
>
> --
> 2.39.5
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels
  2025-11-26  8:36 ` [PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels Jean-Michel Hautbois via B4 Relay
@ 2025-11-26 16:14   ` Frank Li
  0 siblings, 0 replies; 13+ messages in thread
From: Frank Li @ 2025-11-26 16:14 UTC (permalink / raw)
  To: jeanmichel.hautbois
  Cc: Vinod Koul, Angelo Dureghello, Greg Ungerer, imx, dmaengine,
	linux-m68k, linux-kernel

On Wed, Nov 26, 2025 at 09:36:06AM +0100, Jean-Michel Hautbois via B4 Relay wrote:
> From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
>
> Fix the DMA error interrupt handler to properly handle errors on all
> 64 channels. The previous implementation had several issues:
>
> 1. Returned IRQ_NONE if low channels had no errors, even if high
>    channels did
> 2. Used direct status assignment instead of fsl_edma_err_chan_handler()
>    for high channels
>
> Split the error handling into two separate loops for the low (0-31)
> and high (32-63) channel groups, using for_each_set_bit() for cleaner
> iteration. Both groups now consistently use fsl_edma_err_chan_handler()
> for proper error status reporting.
>
> Fixes: e7a3ff92eaf1 ("dmaengine: fsl-edma: add ColdFire mcf5441x edma support")
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> ---

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

>  drivers/dma/mcf-edma-main.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> index 6353303df957..5bd04faa639c 100644
> --- a/drivers/dma/mcf-edma-main.c
> +++ b/drivers/dma/mcf-edma-main.c
> @@ -12,6 +12,7 @@
>  #include "fsl-edma-common.h"
>
>  #define EDMA_CHANNELS		64
> +#define EDMA_CHANS_PER_REG	(EDMA_CHANNELS / 2)
>  #define EDMA_MASK_CH(x)		((x) & GENMASK(5, 0))
>
>  static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> @@ -42,33 +43,33 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
>  {
>  	struct fsl_edma_engine *mcf_edma = dev_id;
>  	struct edma_regs *regs = &mcf_edma->regs;
> -	unsigned int err, ch;
> +	unsigned int ch;
> +	unsigned long err;
> +	bool handled = false;
>
> +	/* Check low 32 channels (0-31) */
>  	err = ioread32(regs->errl);
> -	if (!err)
> -		return IRQ_NONE;
> -
> -	for (ch = 0; ch < (EDMA_CHANNELS / 2); ch++) {
> -		if (err & BIT(ch)) {
> +	if (err) {
> +		handled = true;
> +		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
>  			fsl_edma_disable_request(&mcf_edma->chans[ch]);
>  			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
>  			fsl_edma_err_chan_handler(&mcf_edma->chans[ch]);
>  		}
>  	}
>
> +	/* Check high 32 channels (32-63) */
>  	err = ioread32(regs->errh);
> -	if (!err)
> -		return IRQ_NONE;
> -
> -	for (ch = (EDMA_CHANNELS / 2); ch < EDMA_CHANNELS; ch++) {
> -		if (err & (BIT(ch - (EDMA_CHANNELS / 2)))) {
> -			fsl_edma_disable_request(&mcf_edma->chans[ch]);
> -			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
> -			mcf_edma->chans[ch].status = DMA_ERROR;
> +	if (err) {
> +		handled = true;
> +		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
> +			fsl_edma_disable_request(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
> +			iowrite8(EDMA_CERR_CERR(ch + EDMA_CHANS_PER_REG), regs->cerr);
> +			fsl_edma_err_chan_handler(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
>  		}
>  	}
>
> -	return IRQ_HANDLED;
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>  }
>
>  static int mcf_edma_irq_init(struct platform_device *pdev,
>
> --
> 2.39.5
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging
  2025-11-26 16:12   ` Frank Li
@ 2025-12-16 15:26     ` Vinod Koul
  2025-12-16 15:38       ` Frank Li
  0 siblings, 1 reply; 13+ messages in thread
From: Vinod Koul @ 2025-12-16 15:26 UTC (permalink / raw)
  To: Frank Li
  Cc: jeanmichel.hautbois, Angelo Dureghello, Greg Ungerer, imx,
	dmaengine, linux-m68k, linux-kernel

On 26-11-25, 11:12, Frank Li wrote:
> On Wed, Nov 26, 2025 at 09:36:03AM +0100, Jean-Michel Hautbois via B4 Relay wrote:
> > From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> >
> > Add dynamic per-channel IRQ naming to make DMA interrupt identification
> > easier in /proc/interrupts and debugging tools.
> >
> > Instead of all channels showing "eDMA", they now show:
> > - "eDMA-0" through "eDMA-15" for channels 0-15
> > - "eDMA-16" through "eDMA-55" for channels 16-55
> > - "eDMA-tx-56" for the shared channel 56-63 interrupt
> > - "eDMA-err" for the error interrupt
> >
> > This aids debugging DMA issues by making it clear which channel's
> > interrupt is being serviced.
> >
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > ---
> >  drivers/dma/mcf-edma-main.c | 26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> > index f95114829d80..6a7d88895501 100644
> > --- a/drivers/dma/mcf-edma-main.c
> > +++ b/drivers/dma/mcf-edma-main.c
> > @@ -81,8 +81,14 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
> >  	if (!res)
> >  		return -1;
> >
> > -	for (ret = 0, i = res->start; i <= res->end; ++i)
> > -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> > +	for (ret = 0, i = res->start; i <= res->end; ++i) {
> > +		char *irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> > +						"eDMA-%d", (int)(i - res->start));
> > +		if (!irq_name)
> > +			return -ENOMEM;
> > +
> > +		ret |= request_irq(i, mcf_edma_tx_handler, 0, irq_name, mcf_edma);
> > +	}
> >  	if (ret)
> >  		return ret;
> 
> The existing code have problem, it should use devm_request_irq(). if one
> irq request failure, return here,  requested irq will not free.

Why is that an error!

> 
> You'd better add patch before this one to change to use devm_request_irq()

Not really, devm_ is a not always good option.

-- 
~Vinod

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support
  2025-11-26  8:36 [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Jean-Michel Hautbois via B4 Relay
                   ` (4 preceding siblings ...)
  2025-11-26  8:36 ` [PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels Jean-Michel Hautbois via B4 Relay
@ 2025-12-16 15:27 ` Vinod Koul
  5 siblings, 0 replies; 13+ messages in thread
From: Vinod Koul @ 2025-12-16 15:27 UTC (permalink / raw)
  To: jeanmichel.hautbois
  Cc: Frank Li, Angelo Dureghello, Greg Ungerer, imx, dmaengine,
	linux-m68k, linux-kernel

On 26-11-25, 09:36, Jean-Michel Hautbois via B4 Relay wrote:
> This series addresses several bugs in the fsl-edma and mcf-edma drivers
> affecting MCF54418 ColdFire processors.
> 
> Patch 1 adds the FSL_EDMA_DRV_MCF flag to fix byte-lane addressing for
> MCF54418.
> 
> Patch 2 adds per-channel IRQ naming for easier debugging.
> 
> Patches 3-5 fix the interrupt and error handlers for all 64 DMA
> channels:
> - Patch 3 fixes the interrupt handler to process all 64 channels
> - Patch 4 moves the error handler out of the header file for clarity
> - Patch 5 fixes the error handler for all 64 channels with proper types
> 
> Tested on a custom MCF54418-based platform with slave DMA transfers.

The subsystem is dmaengine, please fix it in the patchseries

> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> ---
> Changes in v2:
> - Check devm_kasprintf() return value
> - Keep request_irq on one line in naming patch
> - Remove non needed memory barrier
> - Remove the interleave patch for now
> - Link to v1: https://lore.kernel.org/r/20251124-dma-coldfire-v1-0-dc8f93185464@yoseli.org
> 
> ---
> Jean-Michel Hautbois (5):
>       dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA
>       dma: mcf-edma: Add per-channel IRQ naming for debugging
>       dma: mcf-edma: Fix interrupt handler for 64 DMA channels
>       dma: fsl-edma: Move error handler out of header file
>       dma: mcf-edma: Fix error handler for all 64 DMA channels
> 
>  drivers/dma/fsl-edma-common.c |  5 +++
>  drivers/dma/fsl-edma-common.h | 11 +++----
>  drivers/dma/mcf-edma-main.c   | 72 +++++++++++++++++++++++++------------------
>  3 files changed, 52 insertions(+), 36 deletions(-)
> ---
> base-commit: ac3fd01e4c1efce8f2c054cdeb2ddd2fc0fb150d
> change-id: 20251123-dma-coldfire-5f36aee143b3
> 
> Best regards,
> --  
> Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> 

-- 
~Vinod

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging
  2025-12-16 15:26     ` Vinod Koul
@ 2025-12-16 15:38       ` Frank Li
  2025-12-17  6:34         ` Jean-Michel Hautbois
  0 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2025-12-16 15:38 UTC (permalink / raw)
  To: Vinod Koul
  Cc: jeanmichel.hautbois, Angelo Dureghello, Greg Ungerer, imx,
	dmaengine, linux-m68k, linux-kernel

On Tue, Dec 16, 2025 at 08:56:01PM +0530, Vinod Koul wrote:
> On 26-11-25, 11:12, Frank Li wrote:
> > On Wed, Nov 26, 2025 at 09:36:03AM +0100, Jean-Michel Hautbois via B4 Relay wrote:
> > > From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > >
> > > Add dynamic per-channel IRQ naming to make DMA interrupt identification
> > > easier in /proc/interrupts and debugging tools.
> > >
> > > Instead of all channels showing "eDMA", they now show:
> > > - "eDMA-0" through "eDMA-15" for channels 0-15
> > > - "eDMA-16" through "eDMA-55" for channels 16-55
> > > - "eDMA-tx-56" for the shared channel 56-63 interrupt
> > > - "eDMA-err" for the error interrupt
> > >
> > > This aids debugging DMA issues by making it clear which channel's
> > > interrupt is being serviced.
> > >
> > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > > ---
> > >  drivers/dma/mcf-edma-main.c | 26 ++++++++++++++++++--------
> > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> > > index f95114829d80..6a7d88895501 100644
> > > --- a/drivers/dma/mcf-edma-main.c
> > > +++ b/drivers/dma/mcf-edma-main.c
> > > @@ -81,8 +81,14 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
> > >  	if (!res)
> > >  		return -1;
> > >
> > > -	for (ret = 0, i = res->start; i <= res->end; ++i)
> > > -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> > > +	for (ret = 0, i = res->start; i <= res->end; ++i) {
> > > +		char *irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> > > +						"eDMA-%d", (int)(i - res->start));
> > > +		if (!irq_name)
> > > +			return -ENOMEM;
> > > +
> > > +		ret |= request_irq(i, mcf_edma_tx_handler, 0, irq_name, mcf_edma);
> > > +	}
> > >  	if (ret)
> > >  		return ret;
> >
> > The existing code have problem, it should use devm_request_irq(). if one
> > irq request failure, return here,  requested irq will not free.
>
> Why is that an error!

        ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
        if (ret)
                return ret;

if last one of request_irq() failure, mcf_edma_irq_init() return failure.
probe will return failure also without free irq.

So previous irq by request_irq() is never free at this case.

Frank

>
> >
> > You'd better add patch before this one to change to use devm_request_irq()
>
> Not really, devm_ is a not always good option.
>
> --
> ~Vinod

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging
  2025-12-16 15:38       ` Frank Li
@ 2025-12-17  6:34         ` Jean-Michel Hautbois
  2025-12-17 16:21           ` Frank Li
  0 siblings, 1 reply; 13+ messages in thread
From: Jean-Michel Hautbois @ 2025-12-17  6:34 UTC (permalink / raw)
  To: Vinod Koul, Frank Li, Greg Ungerer
  Cc: imx, dmaengine, linux-m68k, linux-kernel

Hi Frank,

Le mardi 16 décembre 2025, 16:38:38 heure normale d’Europe centrale Frank Li a 
écrit :
> On Tue, Dec 16, 2025 at 08:56:01PM +0530, Vinod Koul wrote:
> > On 26-11-25, 11:12, Frank Li wrote:
> > > On Wed, Nov 26, 2025 at 09:36:03AM +0100, Jean-Michel Hautbois via B4 
Relay wrote:
> > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > > > 
> > > > Add dynamic per-channel IRQ naming to make DMA interrupt
> > > > identification
> > > > easier in /proc/interrupts and debugging tools.
> > > > 
> > > > Instead of all channels showing "eDMA", they now show:
> > > > - "eDMA-0" through "eDMA-15" for channels 0-15
> > > > - "eDMA-16" through "eDMA-55" for channels 16-55
> > > > - "eDMA-tx-56" for the shared channel 56-63 interrupt
> > > > - "eDMA-err" for the error interrupt
> > > > 
> > > > This aids debugging DMA issues by making it clear which channel's
> > > > interrupt is being serviced.
> > > > 
> > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > > > ---
> > > > 
> > > >  drivers/dma/mcf-edma-main.c | 26 ++++++++++++++++++--------
> > > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> > > > index f95114829d80..6a7d88895501 100644
> > > > --- a/drivers/dma/mcf-edma-main.c
> > > > +++ b/drivers/dma/mcf-edma-main.c
> > > > @@ -81,8 +81,14 @@ static int mcf_edma_irq_init(struct platform_device
> > > > *pdev,> > > 
> > > >  	if (!res)
> > > >  	
> > > >  		return -1;
> > > > 
> > > > -	for (ret = 0, i = res->start; i <= res->end; ++i)
> > > > -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", 
mcf_edma);
> > > > +	for (ret = 0, i = res->start; i <= res->end; ++i) {
> > > > +		char *irq_name = devm_kasprintf(&pdev->dev, 
GFP_KERNEL,
> > > > +						"eDMA-%d", 
(int)(i - res->start));
> > > > +		if (!irq_name)
> > > > +			return -ENOMEM;
> > > > +
> > > > +		ret |= request_irq(i, mcf_edma_tx_handler, 0, 
irq_name, mcf_edma);
> > > > +	}
> > > > 
> > > >  	if (ret)
> > > >  	
> > > >  		return ret;
> > > 
> > > The existing code have problem, it should use devm_request_irq(). if one
> > > irq request failure, return here,  requested irq will not free.
> > 
> > Why is that an error!
> 
>         ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
>         if (ret)
>                 return ret;
> 
> if last one of request_irq() failure, mcf_edma_irq_init() return failure.
> probe will return failure also without free irq.
> 
> So previous irq by request_irq() is never free at this case.

From kernel/irq/manage.c I see in a nutshell:
	request_threaded_irq() {
		action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
		retval = __setup_irq(irq, desc, action);
		if (retval) {
			kfree(action->secondary);
			kfree(action);
		}
	}

I don't see an issue with the existing code then ?
Am I wrong ?

Thanks,
JM

> 
> Frank
> 
> > > You'd better add patch before this one to change to use
> > > devm_request_irq()
> > 
> > Not really, devm_ is a not always good option.
> > 
> > --
> > ~Vinod





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging
  2025-12-17  6:34         ` Jean-Michel Hautbois
@ 2025-12-17 16:21           ` Frank Li
  0 siblings, 0 replies; 13+ messages in thread
From: Frank Li @ 2025-12-17 16:21 UTC (permalink / raw)
  To: Jean-Michel Hautbois
  Cc: Vinod Koul, Greg Ungerer, imx, dmaengine, linux-m68k,
	linux-kernel

On Wed, Dec 17, 2025 at 07:34:07AM +0100, Jean-Michel Hautbois wrote:
> Hi Frank,
>
> Le mardi 16 décembre 2025, 16:38:38 heure normale d’Europe centrale Frank Li a
> écrit :
> > On Tue, Dec 16, 2025 at 08:56:01PM +0530, Vinod Koul wrote:
> > > On 26-11-25, 11:12, Frank Li wrote:
> > > > On Wed, Nov 26, 2025 at 09:36:03AM +0100, Jean-Michel Hautbois via B4
> Relay wrote:
> > > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > > > >
> > > > > Add dynamic per-channel IRQ naming to make DMA interrupt
> > > > > identification
> > > > > easier in /proc/interrupts and debugging tools.
> > > > >
> > > > > Instead of all channels showing "eDMA", they now show:
> > > > > - "eDMA-0" through "eDMA-15" for channels 0-15
> > > > > - "eDMA-16" through "eDMA-55" for channels 16-55
> > > > > - "eDMA-tx-56" for the shared channel 56-63 interrupt
> > > > > - "eDMA-err" for the error interrupt
> > > > >
> > > > > This aids debugging DMA issues by making it clear which channel's
> > > > > interrupt is being serviced.
> > > > >
> > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > > > > ---
> > > > >
> > > > >  drivers/dma/mcf-edma-main.c | 26 ++++++++++++++++++--------
> > > > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> > > > > index f95114829d80..6a7d88895501 100644
> > > > > --- a/drivers/dma/mcf-edma-main.c
> > > > > +++ b/drivers/dma/mcf-edma-main.c
> > > > > @@ -81,8 +81,14 @@ static int mcf_edma_irq_init(struct platform_device
> > > > > *pdev,> > >
> > > > >  	if (!res)
> > > > >
> > > > >  		return -1;
> > > > >
> > > > > -	for (ret = 0, i = res->start; i <= res->end; ++i)
> > > > > -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA",
> mcf_edma);
> > > > > +	for (ret = 0, i = res->start; i <= res->end; ++i) {
> > > > > +		char *irq_name = devm_kasprintf(&pdev->dev,
> GFP_KERNEL,
> > > > > +						"eDMA-%d",
> (int)(i - res->start));
> > > > > +		if (!irq_name)
> > > > > +			return -ENOMEM;
> > > > > +
> > > > > +		ret |= request_irq(i, mcf_edma_tx_handler, 0,
> irq_name, mcf_edma);
> > > > > +	}
> > > > >
> > > > >  	if (ret)
> > > > >
> > > > >  		return ret;
> > > >
> > > > The existing code have problem, it should use devm_request_irq(). if one
> > > > irq request failure, return here,  requested irq will not free.
> > >
> > > Why is that an error!
> >
> >         ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
> >         if (ret)
> >                 return ret;
> >
> > if last one of request_irq() failure, mcf_edma_irq_init() return failure.
> > probe will return failure also without free irq.
> >
> > So previous irq by request_irq() is never free at this case.
>
> From kernel/irq/manage.c I see in a nutshell:
> 	request_threaded_irq() {
> 		action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
> 		retval = __setup_irq(irq, desc, action);
> 		if (retval) {
> 			kfree(action->secondary);
> 			kfree(action);
> 		}
> 	}

This should only free current failure irq.

ret |= request_irq(0, ..) success
ret |= request_irq(1, ..) fail

so return ret (failure).

irq1's resource free.

How about irq0's resource?

Frank

>
> I don't see an issue with the existing code then ?
> Am I wrong ?
>
> Thanks,
> JM
>
> >
> > Frank
> >
> > > > You'd better add patch before this one to change to use
> > > > devm_request_irq()
> > >
> > > Not really, devm_ is a not always good option.
> > >
> > > --
> > > ~Vinod
>
>
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-12-17 16:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-26  8:36 [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Jean-Michel Hautbois via B4 Relay
2025-11-26  8:36 ` [PATCH v2 1/5] dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA Jean-Michel Hautbois via B4 Relay
2025-11-26  8:36 ` [PATCH v2 2/5] dma: mcf-edma: Add per-channel IRQ naming for debugging Jean-Michel Hautbois via B4 Relay
2025-11-26 16:12   ` Frank Li
2025-12-16 15:26     ` Vinod Koul
2025-12-16 15:38       ` Frank Li
2025-12-17  6:34         ` Jean-Michel Hautbois
2025-12-17 16:21           ` Frank Li
2025-11-26  8:36 ` [PATCH v2 3/5] dma: mcf-edma: Fix interrupt handler for 64 DMA channels Jean-Michel Hautbois via B4 Relay
2025-11-26  8:36 ` [PATCH v2 4/5] dma: fsl-edma: Move error handler out of header file Jean-Michel Hautbois via B4 Relay
2025-11-26  8:36 ` [PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels Jean-Michel Hautbois via B4 Relay
2025-11-26 16:14   ` Frank Li
2025-12-16 15:27 ` [PATCH v2 0/5] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox