On Tue, May 20, 2014 at 3:01 AM, Arnd Bergmann arnd@arndb.de wrote:
On Monday 19 May 2014 16:42:18 Rob Herring wrote:
On Mon, May 19, 2014 at 10:56 AM, Catalin Marinas catalin.marinas@arm.comwrote:
On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote:
On Monday 19 May 2014 10:03:40 Catalin Marinas wrote:
On Mon, May 19, 2014 at 09:32:43AM +0100, Arnd Bergmann wrote:
We definitely have to fix this very quickly, before people start building real arm64 systems and shipping them.
We should not merge any hacks that attempt to work around the problem, but try to come to a conclusion how to handle them properly. My hope was that we could just always set the dma mask to whatever the DT says it should be to keep the burden from device drivers, unless they want to restrict it further (e.g. when the specific peripheral hardware has a bug that prevents us from using high addresses, even though the SoC in theory supports it everywhere).
I agree.
Rob Herring argued that we should always mimic PCI and call
dma_set_mask()
in drivers but default to a 32-bit mask otherwise, independent of whether the hardware can do more or less than that, IIRC.
Can we not default to something built up from dma-ranges? Or 32-bit if dma-ranges property is missing?
My reasoning was the information may not come from DT. For AHCI, the 32 vs. 64 bit capability is in a capability register and the DMA mask is set based on that. This information only exists with-in the driver.
I think some sort of capability merging is needed here to merge a bus mask with a device mask. The current api doesn't seem to support this as adjusting the set mask is expected to fail if the requested mask is not supported. Here is how I see it working. Devices can have a default mask (32-bit typically). The default is simply the common case. Really the mask should only be set for DMA capable devices, but many drivers fail to set the mask. Devices can then enlarge or shrink what they support based on knowing the IP block features or getting capabilities from the IP block such as the AHCI case. This mask then needs to be adjusted (only shrinking) if the parent bus or platform has restrictions which is the part that should come from dma-ranges.
Where do you think the shrinking should be done then? I'd like to see that as part of the initial bus probe that Santosh just implemented to set the offset. I think it would be reasonable to apply whatever the bus can do there, but limit it to 32-bit.
Before probe, we can only set a default unless we add a bus_mask or dma_mask from a parent device (i.e. a bus device). After driver probe would be too late as DMA allocations could occur during probe. So that basically leaves it to driver calls to dma_set_mask_and_coherent/dma_set_mask. We could do something like this for .set_dma_mask:
bus_mask = of_get_dma_ranges_mask(); if (bus_mask) mask &= bus_mask;
*dev->dma_mask = mask;
There is an issue in how dma_set_coherent_mask works though. By default it can only check if the mask is valid, but cannot adjust the mask. As long as drivers use dma_set_mask or dma_set_mask_and_coherent, the above should work.
This also adds an OF dependency into the DMA mapping code which I don't like too much. The alternative is storing the bus mask somewhere in struct device.
Rob