[22/22] mtd: rawnand: Use dma_alloc_noncoherent() for dma buffer

Use dma_alloc_noncoherent() instead of directly allocating buffer
from kmalloc with GFP_DMA. DMA API will try to allocate buffer
depending on devices addressing limitation.

[ 42.hyeyoo@gmail.com: Use dma_alloc_noncoherent() instead of
  __get_free_page() and update changelog.

  As it does not allocate high order buffers, allocate buffer
  when needed and free after DMA. ]

Signed-off-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: christian.koenig@amd.com
Cc: linux-mtd@lists.infradead.org

---
 drivers/mtd/nand/raw/marvell_nand.c | 55 ++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 21 deletions(-)

On Sat, Feb 19, 2022 at 08:52:21AM +0800, Baoquan He wrote:
> Use dma_alloc_noncoherent() instead of directly allocating buffer
> from kmalloc with GFP_DMA. DMA API will try to allocate buffer
> depending on devices addressing limitation.

I think it would be better to still allocate the buffer at allocation
time and then just transfer ownership using dma_sync_single* in the I/O
path to avoid the GFP_ATOMIC allocation.

On Sat, Feb 19, 2022 at 08:19:00AM +0100, Christoph Hellwig wrote:
> On Sat, Feb 19, 2022 at 08:52:21AM +0800, Baoquan He wrote:
> > Use dma_alloc_noncoherent() instead of directly allocating buffer
> > from kmalloc with GFP_DMA. DMA API will try to allocate buffer
> > depending on devices addressing limitation.
> 
> I think it would be better to still allocate the buffer at allocation
> time and then just transfer ownership using dma_sync_single* in the I/O
> path to avoid the GFP_ATOMIC allocation.

This driver allocates the buffer at initialization step and maps the buffer
for DMA_TO_DEVICE and DMA_FROM_DEVICE when processing IO.

But after making this driver to use dma_alloc_noncoherent(), remapping
dma_alloc_noncoherent()-ed buffer is strange So I just made it to allocate
the buffer in IO path.

At this point I thought we need an API that allocates based on
address bit mask (like dma_alloc_noncoherent()), which does not maps buffer
into dma address. __get_free_pages/kmalloc(GFP_DMA) has been so confusing..

Hmm.. for this specific case, What about allocating two buffers
for DMA_TO_DEVICE and DMA_FROM_DEVICE at initialization time?

Thanks,
Hyeonggon

On Sat, Feb 19, 2022 at 11:18:24AM +0000, Hyeonggon Yoo wrote:
> > I think it would be better to still allocate the buffer at allocation
> > time and then just transfer ownership using dma_sync_single* in the I/O
> > path to avoid the GFP_ATOMIC allocation.
> 
> This driver allocates the buffer at initialization step and maps the buffer
> for DMA_TO_DEVICE and DMA_FROM_DEVICE when processing IO.
> 
> But after making this driver to use dma_alloc_noncoherent(), remapping
> dma_alloc_noncoherent()-ed buffer is strange So I just made it to allocate
> the buffer in IO path.

You should not remap it.  Just use dma_sync_single* to transfer ownership.

> Hmm.. for this specific case, What about allocating two buffers
> for DMA_TO_DEVICE and DMA_FROM_DEVICE at initialization time?

That will work, but I don't see the benefit as you'd still need to call
dma_sync_single* before and after each data transfer.

From: Christoph Hellwig
> Sent: 22 February 2022 08:47
...
> > Hmm.. for this specific case, What about allocating two buffers
> > for DMA_TO_DEVICE and DMA_FROM_DEVICE at initialization time?
> 
> That will work, but I don't see the benefit as you'd still need to call
> dma_sync_single* before and after each data transfer.

For systems with an iommu that should save all the iommu setup
for every transfer.
I'd also guess that it saves worrying about the error path
when the dma_map fails (eg because the iommu has no space).

OTOH the driver would be 'hogging' iommu space, so maybe
allocate during open() (or equivalent).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

On Tue, Feb 22, 2022 at 09:06:48AM +0000, David Laight wrote:
> From: Christoph Hellwig
> > Sent: 22 February 2022 08:47
> ...
> > > Hmm.. for this specific case, What about allocating two buffers
> > > for DMA_TO_DEVICE and DMA_FROM_DEVICE at initialization time?
> > 
> > That will work, but I don't see the benefit as you'd still need to call
> > dma_sync_single* before and after each data transfer.
> 
> For systems with an iommu that should save all the iommu setup
> for every transfer.

So does allocating a single buffer as in the patch we are replying to.