On 2018-07-27 7:13 PM, Russell King - ARM Linux wrote:
On Fri, Jul 27, 2018 at 12:36:00PM +0100, Robin Murphy wrote:That is intentional. Consider a 32-bit device on a bus with an upstream DMA range of 40 bits (which could easily happen with e.g. PCI). If the firmware code gives that device 40-bit DMA masks and the driver doesn't change them, then DMA is not going to work correctly. Therefore the firmware code cannot safely make {coherent_}dma_mask larger than the default value passed in, and if the default is 0 then that should be fixed in whatever code created the device, not worked around later.I think you're missing the point.
No, I'm just failing to make it clearly enough. Sorry about that.
When OF creates platform devices, they are created without any DMA masks what so ever. It is only during device probe that dma_configure() gets called, which then calls of_dma_configure(), where the DMA mask for any DT declared device is setup.
Indeed, because the DMA mask initialisation which was originally part of OF platform device creation got factored out into of_dma_configure(), then of_dma_configure() got repurposed into a non-platform-device-specific helper, then DMA configuration got generalised more and moved from device creation to probe time, and it now transpires that what looked like an unnecessary vestigial leftover was in fact some OF-platform-specific code all along. Which, having now made that realisitaion, I've put back where it originally was and conceptually should be.
What this means is that your change has broken all existing DT devices that are trying to use DMA, because they now end up with a zero coherent DMA mask and/or streaming DMA mask.
All existing DT devices that are trying to use DMA *without first calling dma_set_mask() etc. as the API expects* - I could point you at lines 166-171 of DMA-API-HOWTO.txt, but given that the last person to touch half this stuff was you, that feels almost impolite. It appears all the drivers on the machines I boot-tested are well-behaved and do do that. In a few hours we'll have a useful datapoint from the kernelci.org boot logs about how many aren't and don't. Happy accidents 'n'all...
Your original idea behind the patch may be a sound one, but since the implementation has caused a regression, the implementation is obviously broken, and that needs fixing or reworking in some way.
Already done: http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/a5516219b10218a87abb3352c82248ce3088e94a
Robin. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
