On Wed, May 21, 2014 at 9:43 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Wednesday 21 May 2014 14:56:36 Catalin Marinas wrote: >> On Tue, May 20, 2014 at 08:37:10PM +0100, Arnd Bergmann wrote: >> > On Tuesday 20 May 2014 15:12:49 Catalin Marinas wrote: >> > > On Tue, May 20, 2014 at 12:08:58PM +0100, Arnd Bergmann wrote: >> > > > On Tuesday 20 May 2014 12:02:46 Catalin Marinas wrote: >> > > > > On Mon, May 19, 2014 at 05:55:56PM +0100, Arnd Bergmann wrote: >> > > > > > On Monday 19 May 2014 16:56:08 Catalin Marinas 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: >> > > > > > We probably want to default to 32-bit for arm32 in the absence of dma-ranges. >> > > > > > For arm64, I'd prefer if we could always mandate dma-ranges to be present >> > > > > > for each bus, just like we mandate ranges to be present. >> > > > > > I hope it's not too late for that. >> > > > > > >> > > > > > dma_set_mask should definitely look at the dma-ranges properties, and the >> > > > > > helper that Santosh just introduced should give us all the information >> > > > > > we need. We just need to decide on the correct behavior. >> > > > > >> > > > > Last time I looked at Santosh's patches I thought the dma-ranges is per >> > > > > device rather than per bus. We could make it per bus only and let the >> > > > > device call dma_set_mask() explicitly if it wants to restrict it >> > > > > further. >> > > > >> > > > Can you check again? I've read the code again yesterday to check this, >> > > > and I concluded it was correctly doing this per bus. >> > > >> > > You are right, I missed the fact that of_dma_get_range() checks the >> > > dma-ranges property in the node's parent. >> > > >> > > So what we need is setting the default dma mask based on the size >> > > information in dma-ranges to something like this: >> > > >> > > mask = rounddown_pow_of_two(size) - 1; >> > > dma_set_mask(dev, mask); /* or dma_set_mask_and_coherent() */ >> > >> > I don't think we should be calling dma_set_mask here, since that would >> > just go and parse the same property again once we fix the implementation. >> > >> > Since this is a low-level helper, we can probably just assign the dma mask >> > directly. >> >> I was thinking of calling it in of_dma_configure() as that's where we >> already set the default dma_mask and coherent_dma_mask. Default to >> 32-bit if no dma-ranges are present. > > Right. Actually it should also be capped to 32-bit, to allow compatibility > with drivers that don't call dma_set_mask and can't do 64-bit DMA. This > is the normal behavior for PCI drivers. They need to set a 64-bit mask > and check the result. What are you checking against to cause a failure and what do you do on failure? I'm guessing that PCI masks are compared to the mask of parent bridges and PCI devices just set the mask to 32-bit if 64-bit fails. That doesn't work if your mask needs to be somewhere between 32 and 64-bit due to some bus constraints. Perhaps that's not something we need to worry about until we see hardware with that condition. 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