zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.96k stars 6.68k forks source link

Drivers: SPI: `nxp,imx-lpspi` DMA usage incompatible with most SPI drivers #81125

Open matt-wood-ct opened 2 weeks ago

matt-wood-ct commented 2 weeks ago

Is your enhancement proposal related to a problem? Please describe. Yes, when using LPSPI with Zephyr SPI device drivers you cannot enable DMA as it does not manage coherency internally, the driver has the expectation the usage will manage cache coherency so in practice this means that virtually all mainstream SPI device drivers can/will misbehave in unusual and unpredictable manners.

I noticed this on a project which utilised and IMXRT1040 + a NRF52 series IC connected using zephyr,bt-hci-spi which is a very high traffic driver so thus is best implemented using DMA.
I also ran into the issues when using the worldsemi,ws2812-spi and jedec,spi-nor drivers.
Those are just the in-tree drivers I use in this project, but I would suspect this issue applies universally given the cause.

Describe the solution you'd like I would like the nxp,imx-lpspi driver to implement a cache coherency scheme similar to what the existing STM32H series driver does which also had DCache issue where by if caching is enabled and the memory provided to the transact function is cacheable then it uses a small internal buffer for transactions to enforce coherency in all cases while enabling applications to provide large nocache buffers if their application requires it (see 0e72d63a0177744a49b6a0be71e1c9c012fa121c and #71048)

Describe alternatives you've considered It would be possible to do the opposite, say that all SPI device drivers need to account for coherency and make their buffers no cache, but this will likely cause significantly more work & problems.

Additional context For my active project I have taking the nxp,imx-lpspi out of tree and added a bodge where in the transact function I flush the buffer locations then in the DMA callback I invalidate the cache over the RX buffer, this functionally works but is technically not thread safe (when the buffers are allocated from stack or 32byte aligned & rounded up it is safe).

matt-wood-ct commented 2 weeks ago

To clarify what I mean when I say "unusual and unpredictable manners". Generally this manifested as random junk data coming out of the SPI and the driver also receiving back bad data which does not match bus captures, all of which is inconsistent build to build as it depends on what uses adjacent memory areas. Many of instances showed 0xAA coming out of the SPI, which is not surprising as the unused memory is stuffed with 0xAA as part of the high water mark and memory protection schemes.

Raymond0225 commented 2 weeks ago

I believe that this DMA coherency issue exists on all DMA related peripheral devices such like uart, i2c i2s spi, it is better we come up with a total solution for it.

matt-wood-ct commented 2 weeks ago

I believe that this DMA coherency issue exists on all DMA related peripheral devices such like uart, i2c i2s spi, it is better we come up with a total solution for it.

Agree but in a practical sense there is precedence to implement the fix at peripheral driver level and this should be done unless someone is actively working on a zephyr wide DMA memory management system.