Use dma_alloc_noncoherent() instead to get the DMA buffer.
[ [email protected]: Only keep label free.
Remove unnecessary alignment checks. it's guaranteed by DMA API.
Just use GFP_KERNEL as it's called in sleepable context.
Specify its dma capability using dma_set_mask_and_coherent() ]
Signed-off-by: Baoquan He <[email protected]>
Signed-off-by: Hyeonggon Yoo <[email protected]>
Cc: Pierre Ossman <[email protected]>
Cc: Ulf Hansson <[email protected]>
Cc: [email protected]
---
drivers/mmc/host/wbsd.c | 45 +++++++++--------------------------------
1 file changed, 9 insertions(+), 36 deletions(-)
diff --git a/drivers/mmc/host/wbsd.c b/drivers/mmc/host/wbsd.c
index 67ecd342fe5f..50b0197583c7 100644
--- a/drivers/mmc/host/wbsd.c
+++ b/drivers/mmc/host/wbsd.c
@@ -1366,55 +1366,28 @@ static void wbsd_request_dma(struct wbsd_host *host, int dma)
if (request_dma(dma, DRIVER_NAME))
goto err;
+ dma_set_mask_and_coherent(mmc_dev(host->mmc), DMA_BIT_MASK(24));
+
/*
* We need to allocate a special buffer in
* order for ISA to be able to DMA to it.
*/
- host->dma_buffer = kmalloc(WBSD_DMA_SIZE,
- GFP_NOIO | GFP_DMA | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
+ host->dma_buffer = dma_alloc_noncoherent(mmc_dev(host->mmc),
+ WBSD_DMA_SIZE, &host->dma_addr,
+ DMA_BIDIRECTIONAL,
+ GFP_KERNEL);
if (!host->dma_buffer)
goto free;
- /*
- * Translate the address to a physical address.
- */
- host->dma_addr = dma_map_single(mmc_dev(host->mmc), host->dma_buffer,
- WBSD_DMA_SIZE, DMA_BIDIRECTIONAL);
- if (dma_mapping_error(mmc_dev(host->mmc), host->dma_addr))
- goto kfree;
-
- /*
- * ISA DMA must be aligned on a 64k basis.
- */
- if ((host->dma_addr & 0xffff) != 0)
- goto unmap;
- /*
- * ISA cannot access memory above 16 MB.
- */
- else if (host->dma_addr >= 0x1000000)
- goto unmap;
-
host->dma = dma;
return;
-unmap:
- /*
- * If we've gotten here then there is some kind of alignment bug
- */
- BUG_ON(1);
-
- dma_unmap_single(mmc_dev(host->mmc), host->dma_addr,
- WBSD_DMA_SIZE, DMA_BIDIRECTIONAL);
- host->dma_addr = 0;
-
-kfree:
- kfree(host->dma_buffer);
- host->dma_buffer = NULL;
-
free:
+ dma_free_noncoherent(mmc_dev(host->mmc), WBSD_DMA_SIZE, host->dma_buffer,
+ host->dma_addr, DMA_BIDIRECTIONAL);
+ host->dma_buffer = NULL;
free_dma(dma);
-
err:
pr_warn(DRIVER_NAME ": Unable to allocate DMA %d - falling back on FIFO\n",
dma);
--
2.17.2
On 02/19/22 at 08:17am, Christoph Hellwig wrote:
> On Sat, Feb 19, 2022 at 08:52:20AM +0800, Baoquan He wrote:
> > if (request_dma(dma, DRIVER_NAME))
> > goto err;
> >
> > + dma_set_mask_and_coherent(mmc_dev(host->mmc), DMA_BIT_MASK(24));
>
> This also sets the streaming mask, but the driver doesn't seem to make
> use of that. Please document it in the commit log.
Thanks for reviewing. I will change it to dma_set_mask(), and describe
this change in patch log.
>
> Also setting smaller than 32 bit masks can fail, so this should have
> error handling.
OK, will check and add error handling.
On Tue, Feb 22, 2022 at 05:14:16PM +0800, Baoquan He wrote:
> > No, if you change it, it should be dma_set_coherent_mask only as it is
> > not using streaming mappings. I suspect dma_set_mask_and_coherent is
> > the right thing if the driver ever wants to use streaming mapping,
> > it would just need to be documented in the commit message.
>
> It will serve dma_alloc_noncoherent() calling later, should be streaming
> mapping?
No, that also looks at the coherent mask. Which is a bit misnamed these
days, it really should be the alloc mask.
On 02/22/22 at 02:11pm, Christoph Hellwig wrote:
> On Tue, Feb 22, 2022 at 05:14:16PM +0800, Baoquan He wrote:
> > > No, if you change it, it should be dma_set_coherent_mask only as it is
> > > not using streaming mappings. I suspect dma_set_mask_and_coherent is
> > > the right thing if the driver ever wants to use streaming mapping,
> > > it would just need to be documented in the commit message.
> >
> > It will serve dma_alloc_noncoherent() calling later, should be streaming
> > mapping?
>
> No, that also looks at the coherent mask. Which is a bit misnamed these
> days, it really should be the alloc mask.
I noticed the misnamed code and have made two draft patches, please help
check if it's necessary.
In the old dma mapping, coherent mapping uses dma_alloc_coherent() to
allocate DMA buffer and mapping; while streaming mapping can only get
memory from slab or buddy allocator, then map with dma_map_single().
In that situation, dma_alloc_direct() checks a direct mapping for
coherent DMA, dma_map_direct() checks a direct mapping for streaming
DMA.
However, several new APIs have been added for streaming mapping, e.g
dma_alloc_pages(). These new APIs take care of DMA buffer allocating
and mapping which are similar with dma_alloc_coherent(). So we should
rename both of them to reflect their real intention to avoid confusion.
dma_alloc_direct() ==> dma_coherent_direct()
dma_map_direct() ==> dma_streaming_direct()
Signed-off-by: Baoquan He <[email protected]>
---
kernel/dma/mapping.c | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index e66847aeac67..2835b08e96c6 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -127,13 +127,13 @@ static bool dma_go_direct(struct device *dev, dma_addr_t mask,
* This allows IOMMU drivers to set a bypass mode if the DMA mask is large
* enough.
*/
-static inline bool dma_alloc_direct(struct device *dev,
+static inline bool dma_coherent_direct(struct device *dev,
const struct dma_map_ops *ops)
{
return dma_go_direct(dev, dev->coherent_dma_mask, ops);
}
-static inline bool dma_map_direct(struct device *dev,
+static inline bool dma_streaming_direct(struct device *dev,
const struct dma_map_ops *ops)
{
return dma_go_direct(dev, *dev->dma_mask, ops);
@@ -151,7 +151,7 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
if (WARN_ON_ONCE(!dev->dma_mask))
return DMA_MAPPING_ERROR;
- if (dma_map_direct(dev, ops) ||
+ if (dma_streaming_direct(dev, ops) ||
arch_dma_map_page_direct(dev, page_to_phys(page) + offset + size))
addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
else
@@ -168,7 +168,7 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_map_direct(dev, ops) ||
+ if (dma_streaming_direct(dev, ops) ||
arch_dma_unmap_page_direct(dev, addr + size))
dma_direct_unmap_page(dev, addr, size, dir, attrs);
else if (ops->unmap_page)
@@ -188,7 +188,7 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
if (WARN_ON_ONCE(!dev->dma_mask))
return 0;
- if (dma_map_direct(dev, ops) ||
+ if (dma_streaming_direct(dev, ops) ||
arch_dma_map_sg_direct(dev, sg, nents))
ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
else
@@ -277,7 +277,7 @@ void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
BUG_ON(!valid_dma_direction(dir));
debug_dma_unmap_sg(dev, sg, nents, dir);
- if (dma_map_direct(dev, ops) ||
+ if (dma_streaming_direct(dev, ops) ||
arch_dma_unmap_sg_direct(dev, sg, nents))
dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
else if (ops->unmap_sg)
@@ -296,7 +296,7 @@ dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
if (WARN_ON_ONCE(!dev->dma_mask))
return DMA_MAPPING_ERROR;
- if (dma_map_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs);
else if (ops->map_resource)
addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
@@ -312,7 +312,7 @@ void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (!dma_map_direct(dev, ops) && ops->unmap_resource)
+ if (!dma_streaming_direct(dev, ops) && ops->unmap_resource)
ops->unmap_resource(dev, addr, size, dir, attrs);
debug_dma_unmap_resource(dev, addr, size, dir);
}
@@ -324,7 +324,7 @@ void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, size_t size,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_map_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
dma_direct_sync_single_for_cpu(dev, addr, size, dir);
else if (ops->sync_single_for_cpu)
ops->sync_single_for_cpu(dev, addr, size, dir);
@@ -338,7 +338,7 @@ void dma_sync_single_for_device(struct device *dev, dma_addr_t addr,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_map_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
dma_direct_sync_single_for_device(dev, addr, size, dir);
else if (ops->sync_single_for_device)
ops->sync_single_for_device(dev, addr, size, dir);
@@ -352,7 +352,7 @@ void dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_map_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
dma_direct_sync_sg_for_cpu(dev, sg, nelems, dir);
else if (ops->sync_sg_for_cpu)
ops->sync_sg_for_cpu(dev, sg, nelems, dir);
@@ -366,7 +366,7 @@ void dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_map_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
dma_direct_sync_sg_for_device(dev, sg, nelems, dir);
else if (ops->sync_sg_for_device)
ops->sync_sg_for_device(dev, sg, nelems, dir);
@@ -391,7 +391,7 @@ int dma_get_sgtable_attrs(struct device *dev, struct sg_table *sgt,
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_alloc_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
return dma_direct_get_sgtable(dev, sgt, cpu_addr, dma_addr,
size, attrs);
if (!ops->get_sgtable)
@@ -430,7 +430,7 @@ bool dma_can_mmap(struct device *dev)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_alloc_direct(dev, ops))
+ if (dma_coherent_direct(dev, ops))
return dma_direct_can_mmap(dev);
return ops->mmap != NULL;
}
@@ -455,7 +455,7 @@ int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_alloc_direct(dev, ops))
+ if (dma_coherent_direct(dev, ops))
return dma_direct_mmap(dev, vma, cpu_addr, dma_addr, size,
attrs);
if (!ops->mmap)
@@ -468,7 +468,7 @@ u64 dma_get_required_mask(struct device *dev)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_alloc_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
return dma_direct_get_required_mask(dev);
if (ops->get_required_mask)
return ops->get_required_mask(dev);
@@ -499,7 +499,7 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
/* let the implementation decide on the zone to allocate from: */
flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
- if (dma_alloc_direct(dev, ops))
+ if (dma_coherent_direct(dev, ops))
cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs);
else if (ops->alloc)
cpu_addr = ops->alloc(dev, size, dma_handle, flag, attrs);
@@ -531,7 +531,7 @@ void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
return;
debug_dma_free_coherent(dev, size, cpu_addr, dma_handle);
- if (dma_alloc_direct(dev, ops))
+ if (dma_coherent_direct(dev, ops))
dma_direct_free(dev, size, cpu_addr, dma_handle, attrs);
else if (ops->free)
ops->free(dev, size, cpu_addr, dma_handle, attrs);
@@ -550,7 +550,7 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size,
gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
size = PAGE_ALIGN(size);
- if (dma_alloc_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp);
if (!ops->alloc_pages)
return NULL;
@@ -574,7 +574,7 @@ static void __dma_free_pages(struct device *dev, size_t size, struct page *page,
const struct dma_map_ops *ops = get_dma_ops(dev);
size = PAGE_ALIGN(size);
- if (dma_alloc_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
dma_direct_free_pages(dev, size, page, dma_handle, dir);
else if (ops->free_pages)
ops->free_pages(dev, size, page, dma_handle, dir);
@@ -769,7 +769,7 @@ size_t dma_max_mapping_size(struct device *dev)
const struct dma_map_ops *ops = get_dma_ops(dev);
size_t size = SIZE_MAX;
- if (dma_map_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
size = dma_direct_max_mapping_size(dev);
else if (ops && ops->max_mapping_size)
size = ops->max_mapping_size(dev);
@@ -782,7 +782,7 @@ bool dma_need_sync(struct device *dev, dma_addr_t dma_addr)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_map_direct(dev, ops))
+ if (dma_streaming_direct(dev, ops))
return dma_direct_need_sync(dev, dma_addr);
return ops->sync_single_for_cpu || ops->sync_single_for_device;
}
--
2.31.1