On Wednesday 21 May 2014 10:26:01 Rob Herring wrote:
On Wed, May 21, 2014 at 9:43 AM, Arnd Bergmann arnd@arndb.de 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