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 |
Attachment:
signature.asc
Description: PGP signature
