On 13/03/18 13:17, Christoph Hellwig wrote:
On Tue, Mar 13, 2018 at 12:11:49PM +0000, Robin Murphy wrote:Taking a step back, though, provided the original rationale about dma_declare_coherent_memory() is still valid, I wonder if we should simply permit the USB code to call dma_{alloc,free}_from_dev_coherent() directly here instead of being "good" and indirecting through the top-level DMA API (which is the part which leads to problems). Given that it's a specific DMA bounce buffer implementation within a core API, not just any old driver code, I personally would consider that reasonable.Looking back I don't really understand why we even indirect the "classic" per-device dma_declare_coherent_memory use case through the DMA API.
It certainly makes sense for devices which can exist in both shared-memory and device-local-memory configurations, so the driver doesn't have to care about the details (particularly on mobile SoCs where the 'local' memory might just be a chunk of system RAM reserved by the bootloader, and it's just a matter of different use-cases on identical hardware).
It seems like a pretty different use case to me. In the USB case we also have the following additional twist in that it doesn't even need the mapping to be coherent.
I'm pretty sure it does (in the sense that it needs to ensure the arch code makes the mapping non-cacheable), otherwise I can't see how the bouncing could work properly. I think the last bit of the comment above hcd_alloc_coherent() is a bit misleading.
So maybe for now the quick fix is to move the sleep check as suggested earlier in this thread, but in the long run we probably need to do some major rework of how dma_declare_coherent_memory and friends work.
Maybe; I do think the specific hcd_alloc_coherent() case could still be fixed within the scope of the existing code, but it's not quite as clean and straightforward as I first thought, and the practical impact of tweaking the WARN should be effectively zero despite the theoretical edge cases it opens up. Do you want me to write it up as a proper patch?
Robin. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
