[alsa-devel] [PATCH] ASoC: omap-pcm: Lower the dma coherent mask to 32bits
The underlying API dma_coerce_mask_and_coherent() checks the requested bits mask against the size of dma_addr_t which is 32bits on OMAP. Because of the previously used 64bits mask audio was not probing after commit c9bd5e6 (and 4dcfa6007). 32bits for the DMA_BIT_MASK looks to be the correct one to use.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- Hi Mark,
This is the fix for OMAP audio for 3.13-rc2+. The dma coherent changes went in between -rc1 and -rc2 which broke things because omap-pcm was using 64bits for dma coherent mask.
Regards, Peter
sound/soc/omap/omap-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index b8fa986..01d59d0 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -202,7 +202,7 @@ static int omap_pcm_new(struct snd_soc_pcm_runtime *rtd) struct snd_pcm *pcm = rtd->pcm; int ret;
- ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(64)); + ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32)); if (ret) return ret;
On Thu, Dec 05, 2013 at 10:06:42AM +0200, Peter Ujfalusi wrote:
The underlying API dma_coerce_mask_and_coherent() checks the requested bits mask against the size of dma_addr_t which is 32bits on OMAP. Because of the previously used 64bits mask audio was not probing after commit c9bd5e6 (and 4dcfa6007). 32bits for the DMA_BIT_MASK looks to be the correct one to use.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Hi Mark,
This is the fix for OMAP audio for 3.13-rc2+. The dma coherent changes went in between -rc1 and -rc2 which broke things because omap-pcm was using 64bits for dma coherent mask.
Can you please try to understand _why_ it broke and post an explanation. This breakage wasn't expected and shouldn't have happened.
On 12/05/2013 11:56 AM, Russell King - ARM Linux wrote:
On Thu, Dec 05, 2013 at 10:06:42AM +0200, Peter Ujfalusi wrote:
The underlying API dma_coerce_mask_and_coherent() checks the requested bits mask against the size of dma_addr_t which is 32bits on OMAP. Because of the previously used 64bits mask audio was not probing after commit c9bd5e6 (and 4dcfa6007). 32bits for the DMA_BIT_MASK looks to be the correct one to use.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Hi Mark,
This is the fix for OMAP audio for 3.13-rc2+. The dma coherent changes went in between -rc1 and -rc2 which broke things because omap-pcm was using 64bits for dma coherent mask.
Can you please try to understand _why_ it broke and post an explanation. This breakage wasn't expected and shouldn't have happened.
I'm not that familiar with this part of the code (mm, dma-mapping) but so far this is what I found:
ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(64)); is successful, no failure to set the mask to 64.
later on when requesting the dma channel however: arm_dma_alloc() -> get_coherent_dma_mask() fails.
As far I can see we have different checks in case of dma_coerce_mask_and_coherent() and arm_dma_alloc():
[1] dma_coerce_mask_and_coherent() -> dma_supported()
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) > arm_dma_pfn_limit) /* !!! */ return 0;
[2] arm_dma_alloc() -> get_coherent_dma_mask()
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) > min(max_pfn, arm_dma_pfn_limit)) /* !!! */ return 0;
On omap4: max_pfn: 0xc0000, arm_dma_pfn_limit: 0xfffff
Not sure which check is the correct one but I think in both cases we should have the same check in this way we can catch the issue at dma_coerce_mask_and_coherent() time and try to figure out what to do. In case of omap-pcm we request for 64 bit because of future SoCs, but I think it would be fine to try first 64 and in case it is not supported fall back to 32.
On Thu, Dec 05, 2013 at 05:33:46PM +0200, Peter Ujfalusi wrote:
I'm not that familiar with this part of the code (mm, dma-mapping) but so far this is what I found:
ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(64)); is successful, no failure to set the mask to 64.
later on when requesting the dma channel however: arm_dma_alloc() -> get_coherent_dma_mask() fails.
As far I can see we have different checks in case of dma_coerce_mask_and_coherent() and arm_dma_alloc():
[1] dma_coerce_mask_and_coherent() -> dma_supported()
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) > arm_dma_pfn_limit) /* !!! */ return 0;
[2] arm_dma_alloc() -> get_coherent_dma_mask()
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) > min(max_pfn, arm_dma_pfn_limit)) /* !!! */ return 0;
On omap4: max_pfn: 0xc0000, arm_dma_pfn_limit: 0xfffff
Obviously, we should not be saying that the mask is okay, and then failing with that same mask.
I've thinking about what the right fix here is - it's been quite a while since I devised those tests and the knowledge I had back then has been swapped out...
The point of these tests are to deal with masks which are larger than dma_addr_t allows, and to only deny if we have sufficient system memory that we may overflow dma_addr_t.
So...
sizeof(u64) != sizeof(dma_addr_t)
detects when we have non-64 bit dma_addr_t.
mask > (dma_addr_t)~0
detects if the mask is larger than 4GiB.
dma_to_pfn(dev, ~0)
gives us the very last PFN which the bus associated with dev is able to address. Hmm - so I got the test wrong on both twice - they should both be:
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) < min(max_pfn, arm_dma_pfn_limit)) return 0;
since we want to fail if we have _more_ memory than the bus will allow. Can you try the above in both locations please?
At Thu, 5 Dec 2013 19:28:53 +0000, Russell King - ARM Linux wrote:
On Thu, Dec 05, 2013 at 05:33:46PM +0200, Peter Ujfalusi wrote:
I'm not that familiar with this part of the code (mm, dma-mapping) but so far this is what I found:
ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(64)); is successful, no failure to set the mask to 64.
later on when requesting the dma channel however: arm_dma_alloc() -> get_coherent_dma_mask() fails.
As far I can see we have different checks in case of dma_coerce_mask_and_coherent() and arm_dma_alloc():
[1] dma_coerce_mask_and_coherent() -> dma_supported()
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) > arm_dma_pfn_limit) /* !!! */ return 0;
[2] arm_dma_alloc() -> get_coherent_dma_mask()
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) > min(max_pfn, arm_dma_pfn_limit)) /* !!! */ return 0;
On omap4: max_pfn: 0xc0000, arm_dma_pfn_limit: 0xfffff
Obviously, we should not be saying that the mask is okay, and then failing with that same mask.
I've thinking about what the right fix here is - it's been quite a while since I devised those tests and the knowledge I had back then has been swapped out...
The point of these tests are to deal with masks which are larger than dma_addr_t allows, and to only deny if we have sufficient system memory that we may overflow dma_addr_t.
So...
sizeof(u64) != sizeof(dma_addr_t)
detects when we have non-64 bit dma_addr_t.
mask > (dma_addr_t)~0
detects if the mask is larger than 4GiB.
dma_to_pfn(dev, ~0)
gives us the very last PFN which the bus associated with dev is able to address. Hmm - so I got the test wrong on both twice - they should both be:
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) < min(max_pfn, arm_dma_pfn_limit)) return 0;
since we want to fail if we have _more_ memory than the bus will allow. Can you try the above in both locations please?
Does the check of dma_to_pfn(dev, ~0) is really necessary? In get_coherent_dma_mask(), it checks further
if (dma_to_pfn(dev, mask) < max_dma_pfn) {
and since mask > ~0, this check should suffice, I think.
And this made me wonder why can we simply use dma_supported() for the check in get_coherent_dma_mask(). If the check in get_coherent_mask() must be different, it'd be helpful if you comment on it there, too.
thanks,
Takashi
On Thu, Dec 05, 2013 at 09:11:50PM +0100, Takashi Iwai wrote:
Does the check of dma_to_pfn(dev, ~0) is really necessary?
Of course.
In get_coherent_dma_mask(), it checks further
if (dma_to_pfn(dev, mask) < max_dma_pfn) {and since mask > ~0, this check should suffice, I think.
dma_to_pfn() takes a dma_addr_t as the second argument. At this point, we've ascertained that 'mask' is larger than dma_addr_t, so the above will truncate the mask.
The whole point of this is to protect the following code which does exactly that from this truncation.
At Thu, 5 Dec 2013 21:07:07 +0000, Russell King - ARM Linux wrote:
On Thu, Dec 05, 2013 at 09:11:50PM +0100, Takashi Iwai wrote:
Does the check of dma_to_pfn(dev, ~0) is really necessary?
Of course.
In get_coherent_dma_mask(), it checks further
if (dma_to_pfn(dev, mask) < max_dma_pfn) {and since mask > ~0, this check should suffice, I think.
dma_to_pfn() takes a dma_addr_t as the second argument. At this point, we've ascertained that 'mask' is larger than dma_addr_t, so the above will truncate the mask.
Ah, I missed that point...
The whole point of this is to protect the following code which does exactly that from this truncation.
OK, I understand that part. Thanks for clarification!
But, it's still unclear why only get_coherent_dma_mask() takes max_pfn into account for the check of dma_to_pfn(dev, mask).
In dma_supported(),
unsigned long limit; limit = dma_to_pfn(dev, mask); if (limit < arm_dma_pfn_limit) return 0;
while in get_coherent_dma_mask(),
unsigned long max_dma_pfn; max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); if (dma_to_pfn(dev, mask) < max_dma_pfn) return 0;
So, the current code looks to me that the results from dma_set_coherent_mask() and the actual allocation may conflict.
If adding warnings is the purpose of the open code, we can create a function like __dma_supported(struct device *dev, u64 mask, bool warning) and call it from the both places.
Takashi
On Fri, Dec 06, 2013 at 09:12:22AM +0100, Takashi Iwai wrote:
But, it's still unclear why only get_coherent_dma_mask() takes max_pfn into account for the check of dma_to_pfn(dev, mask).
In dma_supported(),
unsigned long limit; limit = dma_to_pfn(dev, mask); if (limit < arm_dma_pfn_limit) return 0;
while in get_coherent_dma_mask(),
unsigned long max_dma_pfn; max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); if (dma_to_pfn(dev, mask) < max_dma_pfn) return 0;
So, the current code looks to me that the results from dma_set_coherent_mask() and the actual allocation may conflict.
I did ask Peter to replace *both* with the same thing.
At Fri, 6 Dec 2013 12:25:43 +0000, Russell King - ARM Linux wrote:
On Fri, Dec 06, 2013 at 09:12:22AM +0100, Takashi Iwai wrote:
But, it's still unclear why only get_coherent_dma_mask() takes max_pfn into account for the check of dma_to_pfn(dev, mask).
In dma_supported(),
unsigned long limit; limit = dma_to_pfn(dev, mask); if (limit < arm_dma_pfn_limit) return 0;
while in get_coherent_dma_mask(),
unsigned long max_dma_pfn; max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); if (dma_to_pfn(dev, mask) < max_dma_pfn) return 0;
So, the current code looks to me that the results from dma_set_coherent_mask() and the actual allocation may conflict.
I did ask Peter to replace *both* with the same thing.
Well, I'm looking at different points. You requested Peter to test both functions to take:
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) < min(max_pfn, arm_dma_pfn_limit)) return 0;
But, after that line, dma_supported() has another check:
if (dma_to_pfn(dev, mask) < arm_dma_pfn_limit) return 0;
and get_coherent_dma_mask() has a different check:
if (dma_to_pfn(dev, mask) < min(max_pfn, arm_dma_pfn_limit)) return 0;
(the expressions here are expanded for easier comparison)
This is what I'm wondering.
Takashi
On 12/06/2013 03:04 PM, Takashi Iwai wrote:
At Fri, 6 Dec 2013 12:25:43 +0000, Russell King - ARM Linux wrote:
On Fri, Dec 06, 2013 at 09:12:22AM +0100, Takashi Iwai wrote:
But, it's still unclear why only get_coherent_dma_mask() takes max_pfn into account for the check of dma_to_pfn(dev, mask).
In dma_supported(),
unsigned long limit; limit = dma_to_pfn(dev, mask); if (limit < arm_dma_pfn_limit) return 0;
while in get_coherent_dma_mask(),
unsigned long max_dma_pfn; max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); if (dma_to_pfn(dev, mask) < max_dma_pfn) return 0;
So, the current code looks to me that the results from dma_set_coherent_mask() and the actual allocation may conflict.
I did ask Peter to replace *both* with the same thing.
Well, I'm looking at different points. You requested Peter to test both functions to take:
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) < min(max_pfn, arm_dma_pfn_limit)) return 0;
But, after that line, dma_supported() has another check:
if (dma_to_pfn(dev, mask) < arm_dma_pfn_limit) return 0;
and get_coherent_dma_mask() has a different check:
if (dma_to_pfn(dev, mask) < min(max_pfn, arm_dma_pfn_limit)) return 0;
(the expressions here are expanded for easier comparison)
This is what I'm wondering.
The tests in get_coherent_dma_mask() and dma_supported() should be identical. Fixing the dma_supported() checks and calling dma_supported() from get_coherent_dma_mask() instead of doing the same test coded locally there should be the preferred solution.
I think dma_supported() should be like this:
int dma_supported(struct device *dev, u64 mask) { unsigned long max_dma_pfn;
/* * If the mask allows for more memory than we can address, * and we actually have that much memory, then we must * indicate that DMA to this device is not supported. */ max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) < max_dma_pfn) return 0;
/* * Translate the device's DMA mask to a PFN limit. This * PFN number includes the page which we can DMA to. */ if (dma_to_pfn(dev, mask) < max_dma_pfn) return 0;
return 1; }
On Fri, Dec 06, 2013 at 09:12:22AM +0100, Takashi Iwai wrote:
But, it's still unclear why only get_coherent_dma_mask() takes max_pfn into account for the check of dma_to_pfn(dev, mask).
In dma_supported(),
unsigned long limit; limit = dma_to_pfn(dev, mask); if (limit < arm_dma_pfn_limit) return 0;
while in get_coherent_dma_mask(),
unsigned long max_dma_pfn; max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); if (dma_to_pfn(dev, mask) < max_dma_pfn) return 0;
So, the current code looks to me that the results from dma_set_coherent_mask() and the actual allocation may conflict.
Yes, that's also wrong. Both should be the same, and both should take max_pfn into account.
The reason why max_pfn needs to be taken into account is that is the top of memory - arm_dma_pfn_limit is hard-coded to 4GiB if there's no DMA zone. With some buses (eg, ONAP) this results in a failure as dma_to_pfn(dev, 32-bit) is less than 4GiB.
If adding warnings is the purpose of the open code, we can create a function like __dma_supported(struct device *dev, u64 mask, bool warning) and call it from the both places.
Yes, that's a good idea. Revised version of the patch. Peter, can you retest please, thanks.
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f6b6bfa88ecf..86c564e52ea7 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -158,6 +158,44 @@ struct dma_map_ops arm_coherent_dma_ops = { }; EXPORT_SYMBOL(arm_coherent_dma_ops);
+static int __dma_supported(struct device *dev, u64 mask, bool warn) +{ + unsigned long max_dma_pfn; + + /* + * If the mask allows for more memory than we can address, + * and we actually have that much memory, then we must + * indicate that DMA to this device is not supported. + */ + if (sizeof(mask) != sizeof(dma_addr_t) && + mask > (dma_addr_t)~0 && + dma_to_pfn(dev, ~0) < max_pfn) { + if (warn) { + dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n", + mask); + dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n"); + } + return 0; + } + + max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); + + /* + * Translate the device's DMA mask to a PFN limit. This + * PFN number includes the page which we can DMA to. + */ + if (dma_to_pfn(dev, mask) < max_dma_pfn) { + if (warn) + dev_warn(dev, "Coherent DMA mask %#llx (pfn %#lx-%#lx) covers a smaller range of system memory than the DMA zone pfn 0x0-%#lx\n", + mask, + dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) + 1, + max_dma_pfn + 1); + return 0; + } + + return 1; +} + static u64 get_coherent_dma_mask(struct device *dev) { u64 mask = (u64)DMA_BIT_MASK(32); @@ -176,34 +214,8 @@ static u64 get_coherent_dma_mask(struct device *dev) return 0; }
- max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); - - /* - * If the mask allows for more memory than we can address, - * and we actually have that much memory, then fail the - * allocation. - */ - if (sizeof(mask) != sizeof(dma_addr_t) && - mask > (dma_addr_t)~0 && - dma_to_pfn(dev, ~0) > max_dma_pfn) { - dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n", - mask); - dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n"); - return 0; - } - - /* - * Now check that the mask, when translated to a PFN, - * fits within the allowable addresses which we can - * allocate. - */ - if (dma_to_pfn(dev, mask) < max_dma_pfn) { - dev_warn(dev, "Coherent DMA mask %#llx (pfn %#lx-%#lx) covers a smaller range of system memory than the DMA zone pfn 0x0-%#lx\n", - mask, - dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) + 1, - arm_dma_pfn_limit + 1); + if (!__dma_supported(dev, mask, true)) return 0; - } }
return mask; @@ -1032,28 +1044,7 @@ void arm_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, */ int dma_supported(struct device *dev, u64 mask) { - unsigned long limit; - - /* - * If the mask allows for more memory than we can address, - * and we actually have that much memory, then we must - * indicate that DMA to this device is not supported. - */ - if (sizeof(mask) != sizeof(dma_addr_t) && - mask > (dma_addr_t)~0 && - dma_to_pfn(dev, ~0) > arm_dma_pfn_limit) - return 0; - - /* - * Translate the device's DMA mask to a PFN limit. This - * PFN number includes the page which we can DMA to. - */ - limit = dma_to_pfn(dev, mask); - - if (limit < arm_dma_pfn_limit) - return 0; - - return 1; + return __dma_supported(dev, mask, false); } EXPORT_SYMBOL(dma_supported);
Hi Russell,
On 12/09/2013 07:03 PM, Russell King - ARM Linux wrote:
Yes, that's a good idea. Revised version of the patch. Peter, can you retest please, thanks.
With this patch PandaES and BeagleXM works fine on top of today's mainline.
Thank you Russell and Takashi!
Tested-by: Peter Ujfalusi peter.ujfalusi@ti.com
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f6b6bfa88ecf..86c564e52ea7 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -158,6 +158,44 @@ struct dma_map_ops arm_coherent_dma_ops = { }; EXPORT_SYMBOL(arm_coherent_dma_ops);
+static int __dma_supported(struct device *dev, u64 mask, bool warn) +{
- unsigned long max_dma_pfn;
- /*
* If the mask allows for more memory than we can address,* and we actually have that much memory, then we must* indicate that DMA to this device is not supported.*/- if (sizeof(mask) != sizeof(dma_addr_t) &&
mask > (dma_addr_t)~0 &&dma_to_pfn(dev, ~0) < max_pfn) {if (warn) {dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n",mask);dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n");}return 0;- }
- max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);
- /*
* Translate the device's DMA mask to a PFN limit. This* PFN number includes the page which we can DMA to.*/- if (dma_to_pfn(dev, mask) < max_dma_pfn) {
if (warn)dev_warn(dev, "Coherent DMA mask %#llx (pfn %#lx-%#lx) covers a smaller range of system memory than the DMA zone pfn 0x0-%#lx\n",mask,dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) + 1,max_dma_pfn + 1);return 0;- }
- return 1;
+}
static u64 get_coherent_dma_mask(struct device *dev) { u64 mask = (u64)DMA_BIT_MASK(32); @@ -176,34 +214,8 @@ static u64 get_coherent_dma_mask(struct device *dev) return 0; }
max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);/** If the mask allows for more memory than we can address,* and we actually have that much memory, then fail the* allocation.*/if (sizeof(mask) != sizeof(dma_addr_t) &&mask > (dma_addr_t)~0 &&dma_to_pfn(dev, ~0) > max_dma_pfn) {dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n",mask);dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n");return 0;}/** Now check that the mask, when translated to a PFN,* fits within the allowable addresses which we can* allocate.*/if (dma_to_pfn(dev, mask) < max_dma_pfn) {dev_warn(dev, "Coherent DMA mask %#llx (pfn %#lx-%#lx) covers a smaller range of system memory than the DMA zone pfn 0x0-%#lx\n",mask,dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) + 1,arm_dma_pfn_limit + 1);
if (!__dma_supported(dev, mask, true)) return 0;
}}
return mask;
@@ -1032,28 +1044,7 @@ void arm_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, */ int dma_supported(struct device *dev, u64 mask) {
- unsigned long limit;
- /*
* If the mask allows for more memory than we can address,* and we actually have that much memory, then we must* indicate that DMA to this device is not supported.*/- if (sizeof(mask) != sizeof(dma_addr_t) &&
mask > (dma_addr_t)~0 &&dma_to_pfn(dev, ~0) > arm_dma_pfn_limit)return 0;- /*
* Translate the device's DMA mask to a PFN limit. This* PFN number includes the page which we can DMA to.*/- limit = dma_to_pfn(dev, mask);
- if (limit < arm_dma_pfn_limit)
return 0;- return 1;
- return __dma_supported(dev, mask, false);
} EXPORT_SYMBOL(dma_supported);
One note for the patch:
On 12/10/2013 11:37 AM, Peter Ujfalusi wrote:
Hi Russell,
On 12/09/2013 07:03 PM, Russell King - ARM Linux wrote:
Yes, that's a good idea. Revised version of the patch. Peter, can you retest please, thanks.
With this patch PandaES and BeagleXM works fine on top of today's mainline.
Thank you Russell and Takashi!
Tested-by: Peter Ujfalusi peter.ujfalusi@ti.com
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f6b6bfa88ecf..86c564e52ea7 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -158,6 +158,44 @@ struct dma_map_ops arm_coherent_dma_ops = { }; EXPORT_SYMBOL(arm_coherent_dma_ops);
+static int __dma_supported(struct device *dev, u64 mask, bool warn) +{
- unsigned long max_dma_pfn;
- /*
* If the mask allows for more memory than we can address,* and we actually have that much memory, then we must* indicate that DMA to this device is not supported.*/- if (sizeof(mask) != sizeof(dma_addr_t) &&
mask > (dma_addr_t)~0 &&dma_to_pfn(dev, ~0) < max_pfn) {if (warn) {dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n",mask);dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n");}return 0;- }
- max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);
- /*
* Translate the device's DMA mask to a PFN limit. This* PFN number includes the page which we can DMA to.*/- if (dma_to_pfn(dev, mask) < max_dma_pfn) {
if (warn)dev_warn(dev, "Coherent DMA mask %#llx (pfn %#lx-%#lx) covers a smaller range of system memory than the DMA zone pfn 0x0-%#lx\n",mask,dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) + 1,max_dma_pfn + 1);return 0;- }
- return 1;
+}
static u64 get_coherent_dma_mask(struct device *dev) { u64 mask = (u64)DMA_BIT_MASK(32);
CC arch/arm/mach-omap2/control.o arch/arm/mm/dma-mapping.c: In function ‘get_coherent_dma_mask’: arch/arm/mm/dma-mapping.c:204:17: warning: unused variable ‘max_dma_pfn’ [-Wunused-variable] unsigned long max_dma_pfn; ^
@@ -176,34 +214,8 @@ static u64 get_coherent_dma_mask(struct device *dev) return 0; }
max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);/** If the mask allows for more memory than we can address,* and we actually have that much memory, then fail the* allocation.*/if (sizeof(mask) != sizeof(dma_addr_t) &&mask > (dma_addr_t)~0 &&dma_to_pfn(dev, ~0) > max_dma_pfn) {dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n",mask);dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n");return 0;}/** Now check that the mask, when translated to a PFN,* fits within the allowable addresses which we can* allocate.*/if (dma_to_pfn(dev, mask) < max_dma_pfn) {dev_warn(dev, "Coherent DMA mask %#llx (pfn %#lx-%#lx) covers a smaller range of system memory than the DMA zone pfn 0x0-%#lx\n",mask,dma_to_pfn(dev, 0), dma_to_pfn(dev, mask) + 1,arm_dma_pfn_limit + 1);
if (!__dma_supported(dev, mask, true)) return 0;
}}
return mask;
@@ -1032,28 +1044,7 @@ void arm_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, */ int dma_supported(struct device *dev, u64 mask) {
- unsigned long limit;
- /*
* If the mask allows for more memory than we can address,* and we actually have that much memory, then we must* indicate that DMA to this device is not supported.*/- if (sizeof(mask) != sizeof(dma_addr_t) &&
mask > (dma_addr_t)~0 &&dma_to_pfn(dev, ~0) > arm_dma_pfn_limit)return 0;- /*
* Translate the device's DMA mask to a PFN limit. This* PFN number includes the page which we can DMA to.*/- limit = dma_to_pfn(dev, mask);
- if (limit < arm_dma_pfn_limit)
return 0;- return 1;
- return __dma_supported(dev, mask, false);
} EXPORT_SYMBOL(dma_supported);
On 12/10/2013 11:37 AM, Peter Ujfalusi wrote:
Hi Russell,
On 12/09/2013 07:03 PM, Russell King - ARM Linux wrote:
Yes, that's a good idea. Revised version of the patch. Peter, can you retest please, thanks.
With this patch PandaES and BeagleXM works fine on top of today's mainline.
Thank you Russell and Takashi!
Tested-by: Peter Ujfalusi peter.ujfalusi@ti.com
Russell,
are you planning to send this for 3.13? Without the patch all OMAP audio fails to probe in 3.13 kernel.
On Fri, Dec 13, 2013 at 01:46:46PM +0200, Peter Ujfalusi wrote:
On 12/10/2013 11:37 AM, Peter Ujfalusi wrote:
Hi Russell,
On 12/09/2013 07:03 PM, Russell King - ARM Linux wrote:
Yes, that's a good idea. Revised version of the patch. Peter, can you retest please, thanks.
With this patch PandaES and BeagleXM works fine on top of today's mainline.
Thank you Russell and Takashi!
Tested-by: Peter Ujfalusi peter.ujfalusi@ti.com
Russell,
are you planning to send this for 3.13? Without the patch all OMAP audio fails to probe in 3.13 kernel.
It's queued up in -fixes, but there's a few other fixes in progress which I want to get together before I finally push it to Linus.
On 12/13/2013 01:49 PM, Russell King - ARM Linux wrote:
On Fri, Dec 13, 2013 at 01:46:46PM +0200, Peter Ujfalusi wrote:
On 12/10/2013 11:37 AM, Peter Ujfalusi wrote:
Hi Russell,
On 12/09/2013 07:03 PM, Russell King - ARM Linux wrote:
Yes, that's a good idea. Revised version of the patch. Peter, can you retest please, thanks.
With this patch PandaES and BeagleXM works fine on top of today's mainline.
Thank you Russell and Takashi!
Tested-by: Peter Ujfalusi peter.ujfalusi@ti.com
Russell,
are you planning to send this for 3.13? Without the patch all OMAP audio fails to probe in 3.13 kernel.
It's queued up in -fixes, but there's a few other fixes in progress which I want to get together before I finally push it to Linus.
Sure, the important thing is that it is going.
Thank you, Péter
On 12/05/2013 09:28 PM, Russell King - ARM Linux wrote:
gives us the very last PFN which the bus associated with dev is able to address. Hmm - so I got the test wrong on both twice - they should both be:
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) < min(max_pfn, arm_dma_pfn_limit)) return 0;
since we want to fail if we have _more_ memory than the bus will allow. Can you try the above in both locations please?
Thanks, works fine. I will be out for a long weekend so you can add my:
Tested-by: Peter Ujfalusi peter.ujfalusi@ti.com
to the final patch.
On Fri, Dec 06, 2013 at 11:31:52AM +0200, Peter Ujfalusi wrote:
On 12/05/2013 09:28 PM, Russell King - ARM Linux wrote:
gives us the very last PFN which the bus associated with dev is able to address. Hmm - so I got the test wrong on both twice - they should both be:
if (sizeof(mask) != sizeof(dma_addr_t) && mask > (dma_addr_t)~0 && dma_to_pfn(dev, ~0) < min(max_pfn, arm_dma_pfn_limit)) return 0;
since we want to fail if we have _more_ memory than the bus will allow. Can you try the above in both locations please?
Thanks, works fine. I will be out for a long weekend so you can add my:
Tested-by: Peter Ujfalusi peter.ujfalusi@ti.com
Thanks Peter.
participants (3)
-
Peter Ujfalusi -
Russell King - ARM Linux -
Takashi Iwai