Hi Trent, Le 27/12/2017 à 21:15, Trent Piepho a écrit : > On Wed, 2017-12-27 at 10:36 +0000, Mark Brown wrote: >> On Tue, Dec 26, 2017 at 06:45:28PM +0000, Trent Piepho wrote: >> >>> Or, since this only fixes instances of DMA-unsafe buffers used in >>> access to SPI NOR flash chips, and since there are other SPI master >>> interface users, those chip specific fixes in some/all spi master >>> drivers are still needed to fix transfers not originated via spi-nor? >> >> SPI client drivers are *supposed* to use DMA safe memory already. How >> often that happens in cases where it matters is a separate question, we >> definitely have users with smaller transfers that don't do the right >> thing but they're normally done using PIO anyway. > > I wonder what the end goal is here? > > A random collection of spi master drivers will accept DMA-unsafe > buffers in some way. In some cases a framework like spi-nor provides > the fixup to spi-nor master drivers (none so far) and in other cases > (atmel-quadspi), the spi-nor master driver has its own fixes. > At the spi-nor side, atmel-quadspi is also an example showing how the bounce buffer feature should be used by other spi-flash drivers. Actually, the m25p80 driver taken aside, no spi-flash driver is currently able to perform DMA safe transfers. Some patches were proposed but all rejected because they were doing wrong things: calling dma_map_single() even if the buffer is not guaranteed to be contiguous in physical memory or not being aware of the data cache aliasing issue on some architectures. So this series offers a common helper solution for all drivers in spi-nor. I don't want each spi-flash driver to implement its own custom solution. > Generic spi masters like spi-atmel, spi-ti-qspi, and spi-davinci will > have their fixes for certain cases. > If UBIFS was the only reason for those drivers to implement their own fixes then those fixes would no longer be needed with this series. However if other spi-clients also provide the SPI sub-system with DMA-unsafe buffers then maybe those fixes are still needed. I think Mark knows better than anyone else about the SPI sub-system. Besides, another reason to allocate the bounce buffer from spi-nor is that we can choose a suited size as a trade-off between performance and memory footprint. > Perhaps spi flash drivers like m25p80 will have fixes too? > patch 1 of the series enables the bounce buffer for both read and write operations in the m25p80 driver, in order to be compliant with buffer constraints expressed in the kernel-doc of 'struct spi_transfer'. > Some spi clients, like spidev, will have internal bounce buffers, > rather than userspace addresses or commands in stack variables, so that > they follow the rules about DMA safe buffers. > > What exactly is caught as DMA unsafe and what is not will of course > vary greatly from driver to driver. Some drivers will catch highmem > memory while other drivers will only detect vmalloc memory. Some will > only catch an unsafe buffer if a specific SoC known to the driver to > have an aliasing cache is enabled. Some will check buffers that arrive > via the spi_flash_read interface but not via generic spi transfers, > while others will check all spi transfer buffers. > That's why I've asked for pieces of advice on how to implement a relevant [spi_nor_]is_dma_safe() function and Vignesh has provided good suggestion! > Obviously, I don't think this path will lead to a desirable end. Maybe Here you seem to only take the m25p80 and SPI sub-system cases into account. However, at the spi-nor side, we currently have to other solution to let spi-nor flash drivers implement DMA transfers safely. Best regards, Cyrille > the basic assumption, that clients should provide DMA safe buffers, > should be revisited. Experience has shown that it's too much to ask > for and spi clients will never get it right. It would be better to try > to fix this at some common point between the clients and masters so it > can be done once and for all. > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
