Re: [PATCH 6.12.y] spi: spi-fsl-dspi: Avoid setup_accel logic for DMA transfers

On Tue, Aug 11, 2026 at 02:55:56PM -0400, Sasha Levin wrote:
> > Either way the behaviour is the same. On vf610 in DMA mode the accel path drops
> > the tail of odd length transfers and byte swaps under SPI_LSB_FIRST, and this
> > commit removes both. That is what makes it worth having in 6.12.y, whatever the
> > original intent was.
> 
> cac7e5054115 ("spi: spi-fsl-dspi: Avoid setup_accel logic for DMA transfers")
> applies cleanly to 6.12 with no dependencies, so this is not a mechanical
> question - it is whether it qualifies. It has no Fixes: tag and no stable tag,
> and James reads it as a refactor.
> 
> Larisa, Mark, Vladimir - was this a fix? If so, a Fixes: tag would let me take
> it here and on the older trees, where it applies just as cleanly.
> 
> -- 
> Thanks,
> Sasha

It wasn't understood as a correctness change until now, but yes, it is a fix.

Fixes: a957499bd437 ("spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode")
Acked-by: Vladimir Oltean <[email protected]>

Explanation:
As part of the original introduction of dspi_setup_accel() in commit
6c1c26ecd9a3 ("spi: spi-fsl-dspi: Accelerate transfers using larger word
size if possible"), it was well understood that this is not applicable
to DMA transfers.

The reason is that the correct clustering of 8 bit frames into 16 bit PUSHR
transfers ultimately depends on the ability to modify the SPI_CTAR_FMSZ
(frame size) on the go. In the case of a 3 byte SPI transfer using
8-on-16 acceleration, the logic of this clustering is to first transfer
the first 2 bytes using a 16-bit PUSHR transfer (with SPI_CTAR_FMSZ=15),
then to update SPI_CTAR_FMSZ=7 in order to be able to push the last byte
using a single 8-bit PUSHR write.

The difference between FIFO mode and DMA mode is that in DMA mode, there
is no software hook to update SPI_CTAR_FMSZ in between PUSHR FIFO
updates. The DMA engine handles them.

This was well understood and was the basis of this code path, which
explicitly excluded DMA from dspi_setup_accel() with its dynamic frame
size updating:

		/*
		 * Static CTAR setup for modes that don't dynamically adjust it
		 * via dspi_setup_accel (aka for DMA)
		 */
		regmap_write(dspi->regmap, SPI_CTAR(0),
			     dspi->cur_chip->ctar_val |
			     SPI_FRAME_BITS(transfer->bits_per_word));

However, this truth was forgotten soon after, because as soon as a bug
report came in - the trigger behind commit a957499bd437 ("spi:
spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode") - it became
broken.

Namely, the separate code path for static SPI_CTAR_FMSZ settings for DMA
mode got deleted, and dspi_dma_xfer() started calling dspi_setup_accel().
This had two effects:
- dspi_setup_accel() correctly updates dspi->oper_word_size, necessary
  in common code: intended, fixes the bug reported by Michael Walle
- dspi_setup_accel() enables 8-on-16 acceleration for DMA mode now,
  which will transfer 1 byte too few if the buffer size is odd (it
  incorrectly assumes that the caller can dynamically alter
  SPI_CTAR_FMSZ and then send the trailing word separately):
  unintended, causes the bug reported by Mehmet Fide

The breakage probably went largely unnoticed because Michael Walle's
peripheral only used even-sized buffers (a flash, IIRC), and the silicon
on which I regularly test the DSPI driver doesn't use DMA.

The commit under question here - cac7e5054115 ("spi: spi-fsl-dspi: Avoid
setup_accel logic for DMA transfers") - fixes the unintended side effect
while maintaining the intention of previous bug fix a957499bd437 ("spi:
spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode"). By having
the "goto no_accel", we bypass the 8-on-16 acceleration on DMA, while
still assigning dspi->oper_word_size - which was the reason for calling
dspi_setup_accel() in the first place.

Note that 8-on-16 acceleration is not intrinsically broken for DMA mode
(it can yield a DMA buffer more densely packed with PUSHR data), it just
needs more work to skip it for odd-sized transfers. However, that work
may or may not be justified from a performance standpoint, so the
approach taken here is reasonable.


Regarding the SPI_LSB_FIRST issue - from the description it seems to be
a completely distinct problem not intrinsically limited to DMA mode
(should also be visible in XSPI mode), so disabling dspi_setup_accel()
on Vyber and Coldfire only partially addresses it.

I don't have a use case for SPI_LSB_FIRST peripherals, so I don't
personally mind another "goto no_accel" follow-up patch rather than
fixing the underlying byte packing mechanism, BUT this should be done
by the issue reporter with a proper explanation in the commit message
now that the issue is more clearly understood, rather than just be
happy that backporting commit cac7e5054115 sidesteps the problem on his
platform.

I am currently on vacation, and I am unable to do much testing on actual
hardware. I also haven't completely evaluated the SPI_LSB_FIRST behaviour
with 8-on-16 acceleration, it just *seems* plausible that there is an issue.