On Tue, May 20, 2014 at 3:01 AM, Arnd Bergmann <arnd@xxxxxxxx> 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@xxxxxxx>wrote: >> > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
