Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jean-Michel Hautbois" <jeanmichel.hautbois@yoseli.org>
Cc: dmaengine@vger.kernel.org, vkoul@kernel.org, imx@lists.linux.dev,
	Frank.Li@kernel.org
Subject: Re: [PATCH v3 2/5] dmaengine: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA
Date: Thu, 25 Jun 2026 09:20:38 +0000	[thread overview]
Message-ID: <20260625092038.ADEF71F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260625-b4-edma-dmaengine-v3-2-44be00ace37d@yoseli.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch correctly bypasses the XOR byte-lane adjustment for 8-bit registers on MCF eDMA via the FSL_EDMA_DRV_MCF flag, but fails to apply the same bypass for 16-bit registers written via edma_writew(). This will corrupt TCD 16-bit register writes on ColdFire platforms.
--

commit e23a155133dba0ca56d6eea575c63bb35a8ef9ab
Author: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

dmaengine: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA

This commit adds a new flag FSL_EDMA_DRV_MCF for the ColdFire eDMA driver to
bypass the XOR byte-lane adjustment for 8-bit registers, ensuring writes
target the correct documented addresses.

> diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
> index abc8f78055156..64b5375272917 100644
> --- a/drivers/dma/fsl-edma-common.h
> +++ b/drivers/dma/fsl-edma-common.h

[ ... ]

> @@ -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);

[Severity: High]
This isn't a bug introduced by this patch, but should edma_writew() be updated
to bypass the address adjustment as well?

Looking at edma_writew() right below this function, it performs 16-bit writes
and unconditionally applies the ^ 0x2 XOR offset adjustment for big-endian
controllers:

drivers/dma/fsl-edma-common.h:edma_writew() {
	/* swap the reg offset for these in big-endian mode */
	if (edma->big_endian)
		iowrite16be(val, (void __iomem *)((unsigned long)addr ^ 0x2));
    ...
}

Without checking FSL_EDMA_DRV_MCF here, wouldn't writing 16-bit TCD registers
on native ColdFire platforms still apply the ^ 0x2 offset and incorrectly swap
registers like soff and attr?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-b4-edma-dmaengine-v3-0-44be00ace37d@yoseli.org?part=2

  reply	other threads:[~2026-06-25  9:20 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-25  8:59 [PATCH v3 0/5] dmaengine: mcf-edma: fix 64-channel handling and modernize IRQ setup Jean-Michel Hautbois
2026-06-25  8:59 ` [PATCH v3 1/5] dmaengine: fsl-edma: Move error handler out of header file Jean-Michel Hautbois
2026-06-25  9:18   ` sashiko-bot
2026-06-25 15:33   ` Frank Li
2026-06-25  8:59 ` [PATCH v3 2/5] dmaengine: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA Jean-Michel Hautbois
2026-06-25  9:20   ` sashiko-bot [this message]
2026-06-25 15:16   ` Frank Li
2026-06-25  8:59 ` [PATCH v3 3/5] dmaengine: mcf-edma: Fix interrupt handler for 64 DMA channels Jean-Michel Hautbois
2026-06-25 15:30   ` Frank Li
2026-06-25  8:59 ` [PATCH v3 4/5] dmaengine: mcf-edma: Fix error handler for all " Jean-Michel Hautbois
2026-06-25  9:17   ` sashiko-bot
2026-06-25 15:23   ` Frank Li
2026-06-25  8:59 ` [PATCH v3 5/5] dmaengine: mcf-edma: Use devm for per-channel IRQ registration Jean-Michel Hautbois
2026-06-25  9:21   ` sashiko-bot
2026-06-25 15:26   ` Frank Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260625092038.ADEF71F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=jeanmichel.hautbois@yoseli.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox