[PATCH] spi: Increase imx51 ecspi burst length based on transfer length

[Marc Kleine-Budde]

On 18.06.2024 13:58:07, Marc Kleine-Budde wrote:
> Hi Stefan,
> 
> On 28.06.2023 15:20:39, Stefan Moring wrote:
> > In our application we send ~80kB at 10MHz. The total transfer time
> > went from ~80ms to 67ms, so that would be a reduction of 15%.
> > I tested it on an IMX8MM platform.
> 
> I'm currently debugging a problem with spi-imx, HW CS and SPI_CS_WORD on
> torvalds/master. The breakage goes back this patch.
> 
> I'm wondering what is your setup you have optimized with this patch?
> - Are you using HW or GPIO CS?
> - What are bits_per_word?
> - What's the length of the spi_transfer?
> 
> I'm asking because with a 8, 16 or 32 bit-per-word setting, the driver
> should use dynamic_burst on the imx8mm, which will overwrite the burst
> length in spi_imx_push().

This patch, even with all the fixes on top of it (torvalds/master)
breaks the HW CS + SPI_CS_WORD support which was added in 6e95b23a5b2d
("spi: imx: Implement support for CS_WORD").

I think this also breaks the support for bits-per-word != multiple of 8
bits. For these transfers the in-memory wordsizes are powers of two
bytes (e.g. 20 bit samples use 32 bits) [1] and via the burst length
configuration only the bits-per-word number of bits are shifted out.

[1] https://elixir.bootlin.com/linux/v6.9/source/include/linux/spi/spi.h#L144

| 	if (spi_imx->target_mode && is_imx53_ecspi(spi_imx))
| 		ctrl |= (spi_imx->target_burst * 8 - 1)
| 			<< MX51_ECSPI_CTRL_BL_OFFSET;
| 	else {
| 		if (spi_imx->usedma) {
| 			ctrl |= (spi_imx->bits_per_word - 1)
| 				<< MX51_ECSPI_CTRL_BL_OFFSET;
| 		} else {
| 			if (spi_imx->count >= MX51_ECSPI_CTRL_MAX_BURST)
| 				ctrl |= (MX51_ECSPI_CTRL_MAX_BURST * BITS_PER_BYTE - 1)
| 						<< MX51_ECSPI_CTRL_BL_OFFSET;
| 			else
| 				ctrl |= (spi_imx->count / DIV_ROUND_UP(spi_imx->bits_per_word,
| 						BITS_PER_BYTE) * spi_imx->bits_per_word - 1)
| 						<< MX51_ECSPI_CTRL_BL_OFFSET;
| 		}
| 	}

Consider a message with bits-per-word = 9 consisting of 4 words. It uses
4 word x 2 bytes/word = 8 bytes of memory. This boils down to a burst
length of 36 bits. Which means the spi-imx sends the first 36 bits from
the 64 bits of memory, this is clearly wrong.

This patch (15a6af94a277 ("spi: Increase imx51 ecspi burst length based
on transfer length")) is wrong and the 4 fixes on top of it don't
finally fix it. I can send a series of 5 reverts, or a manually revert
the burst length calculation to the original value in one patch.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20240618/5cf18373/attachment.sig>