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. 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. Arnd -- 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
