On Wed, May 21, 2014 at 10:48 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Wednesday 21 May 2014 10:26:01 Rob Herring wrote: >> 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. > > We should compare against the size returned by of_dma_get_range(). If > the mask requested by the driver is larger than the mask of the bus > it's attached on, dma_set_mask should fail. > > We can always allow 64-bit masks if the actual bus capability is enough > to cover all the installed RAM. That is a relatively common case. Agreed. However, if we check dma-ranges, it may be large enough for "all of RAM", but less than a full 64-bit. There is also the edge case that you cannot set the size to 2^64, but only 2^64 - 1. That means dma_set_mask(2^64 - 1) will always fail. > I'm not entirely sure how to handle drivers trying to set a 32-bit mask > when the bus has less than 32-bits, such as for the shmobile rcar EHCI. > Maybe we should succeed but cap the mask instead. Yes, adjusting the mask is what I have been suggesting for this example and my example here. 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
