LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Ladislav Michl <ladis@linux-mips.org>,
	<linux-kernel@vger.kernel.org>, <linux-mtd@lists.infradead.org>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
	Roger Quadros <rogerq@ti.com>,
	Aaro Koskinen <aaro.koskinen@iki.fi>,
	Tony Lindgren <tony@atomide.com>,
	"H. Nikolaus Schaller" <hns@goldelico.com>,
	Andreas Kemnade <andreas@kemnade.info>
Subject: Re: [PATCH v2] mtd: onenand: omap2: Disable DMA for HIGHMEM buffers
Date: Mon, 16 Apr 2018 14:34:54 +0300	[thread overview]
Message-ID: <8b073dce-2a9e-5068-6499-a36915f5ed90@ti.com> (raw)
In-Reply-To: <20180416065256.GA24455@lenoch>



On 2018-04-16 09:52, Ladislav Michl wrote:
> dma_map_single doesn't get the proper DMA address for vmalloced area,

Which is not a big surprise as vmalloc will allocate contiguous virtual
memory (which might corresponds to non-contiguous physical memory). Even
if you somehow get the physical address of the start of the vmalloced
buffer, you don't really know how long that chunk is and where the
buffer continues in physical memory.

Creating sg_list of the vmalloced buffer should be possible also by
walking the virt memory and get the pages with vmalloc_to_page().
I don't think there is a generic vmalloc_to_sg(), one can be implemented.

> so disable DMA in this case.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> Reported-by: "H. Nikolaus Schaller" <hns@goldelico.com>
> Tested-by: "H. Nikolaus Schaller" <hns@goldelico.com>
> ---
>  Changes:
>  -v2: Added Tested-by tag, based on v4.17-rc1 (no change in patch itself)
> 
>  drivers/mtd/nand/onenand/omap2.c | 105 +++++++++++--------------------
>  1 file changed, 38 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/mtd/nand/onenand/omap2.c b/drivers/mtd/nand/onenand/omap2.c
> index 9c159f0dd9a6..321137158ff3 100644
> --- a/drivers/mtd/nand/onenand/omap2.c
> +++ b/drivers/mtd/nand/onenand/omap2.c
> @@ -375,56 +375,42 @@ static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area,
>  {
>  	struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd);
>  	struct onenand_chip *this = mtd->priv;
> -	dma_addr_t dma_src, dma_dst;
> -	int bram_offset;
> +	struct device *dev = &c->pdev->dev;
>  	void *buf = (void *)buffer;
> +	dma_addr_t dma_src, dma_dst;
> +	int bram_offset, err;
>  	size_t xtra;
> -	int ret;
>  
>  	bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset;
> -	if (bram_offset & 3 || (size_t)buf & 3 || count < 384)
> -		goto out_copy;
> -
> -	/* panic_write() may be in an interrupt context */
> -	if (in_interrupt() || oops_in_progress)
> +	/*
> +	 * If the buffer address is not DMA-able, len is not long enough to make
> +	 * DMA transfers profitable or panic_write() may be in an interrupt
> +	 * context fallback to PIO mode.
> +	 */
> +	if (!virt_addr_valid(buf) || bram_offset & 3 || (size_t)buf & 3 ||
> +	    count < 384 || in_interrupt() || oops_in_progress )
>  		goto out_copy;
>  
> -	if (buf >= high_memory) {
> -		struct page *p1;
> -
> -		if (((size_t)buf & PAGE_MASK) !=
> -		    ((size_t)(buf + count - 1) & PAGE_MASK))
> -			goto out_copy;
> -		p1 = vmalloc_to_page(buf);
> -		if (!p1)
> -			goto out_copy;
> -		buf = page_address(p1) + ((size_t)buf & ~PAGE_MASK);
> -	}
> -
>  	xtra = count & 3;
>  	if (xtra) {
>  		count -= xtra;
>  		memcpy(buf + count, this->base + bram_offset + count, xtra);
>  	}
>  
> +	dma_dst = dma_map_single(dev, buf, count, DMA_FROM_DEVICE);
>  	dma_src = c->phys_base + bram_offset;
> -	dma_dst = dma_map_single(&c->pdev->dev, buf, count, DMA_FROM_DEVICE);
> -	if (dma_mapping_error(&c->pdev->dev, dma_dst)) {
> -		dev_err(&c->pdev->dev,
> -			"Couldn't DMA map a %d byte buffer\n",
> -			count);
> -		goto out_copy;
> -	}
>  
> -	ret = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
> -	dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE);
> -
> -	if (ret) {
> -		dev_err(&c->pdev->dev, "timeout waiting for DMA\n");
> +	if (dma_mapping_error(dev, dma_dst)) {
> +		dev_err(dev, "Couldn't DMA map a %d byte buffer\n", count);
>  		goto out_copy;
>  	}
>  
> -	return 0;
> +	err = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
> +	dma_unmap_single(dev, dma_dst, count, DMA_FROM_DEVICE);
> +	if (!err)
> +		return 0;
> +
> +	dev_err(dev, "timeout waiting for DMA\n");
>  
>  out_copy:
>  	memcpy(buf, this->base + bram_offset, count);
> @@ -437,49 +423,34 @@ static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area,
>  {
>  	struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd);
>  	struct onenand_chip *this = mtd->priv;
> -	dma_addr_t dma_src, dma_dst;
> -	int bram_offset;
> +	struct device *dev = &c->pdev->dev;
>  	void *buf = (void *)buffer;
> -	int ret;
> +	dma_addr_t dma_src, dma_dst;
> +	int bram_offset, err;
>  
>  	bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset;
> -	if (bram_offset & 3 || (size_t)buf & 3 || count < 384)
> -		goto out_copy;
> -
> -	/* panic_write() may be in an interrupt context */
> -	if (in_interrupt() || oops_in_progress)
> +	/*
> +	 * If the buffer address is not DMA-able, len is not long enough to make
> +	 * DMA transfers profitable or panic_write() may be in an interrupt
> +	 * context fallback to PIO mode.
> +	 */
> +	if (!virt_addr_valid(buf) || bram_offset & 3 || (size_t)buf & 3 ||
> +	    count < 384 || in_interrupt() || oops_in_progress )
>  		goto out_copy;
>  
> -	if (buf >= high_memory) {
> -		struct page *p1;
> -
> -		if (((size_t)buf & PAGE_MASK) !=
> -		    ((size_t)(buf + count - 1) & PAGE_MASK))
> -			goto out_copy;
> -		p1 = vmalloc_to_page(buf);
> -		if (!p1)
> -			goto out_copy;
> -		buf = page_address(p1) + ((size_t)buf & ~PAGE_MASK);
> -	}
> -
> -	dma_src = dma_map_single(&c->pdev->dev, buf, count, DMA_TO_DEVICE);
> +	dma_src = dma_map_single(dev, buf, count, DMA_TO_DEVICE);
>  	dma_dst = c->phys_base + bram_offset;
> -	if (dma_mapping_error(&c->pdev->dev, dma_src)) {
> -		dev_err(&c->pdev->dev,
> -			"Couldn't DMA map a %d byte buffer\n",
> -			count);
> -		return -1;
> -	}
> -
> -	ret = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
> -	dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE);
> -
> -	if (ret) {
> -		dev_err(&c->pdev->dev, "timeout waiting for DMA\n");
> +	if (dma_mapping_error(dev, dma_src)) {
> +		dev_err(dev, "Couldn't DMA map a %d byte buffer\n", count);
>  		goto out_copy;
>  	}
>  
> -	return 0;
> +	err = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
> +	dma_unmap_page(dev, dma_src, count, DMA_TO_DEVICE);
> +	if (!err)
> +		return 0;
> +
> +	dev_err(dev, "timeout waiting for DMA\n");
>  
>  out_copy:
>  	memcpy(this->base + bram_offset, buf, count);
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

  reply	other threads:[~2018-04-16 11:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-16  6:52 Ladislav Michl
2018-04-16 11:34 ` Peter Ujfalusi [this message]
2018-04-16 13:32   ` Ladislav Michl
2018-04-20 20:01 ` Boris Brezillon
2018-05-02  8:06   ` Ladislav Michl
2018-05-02  9:16     ` Boris Brezillon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b073dce-2a9e-5068-6499-a36915f5ed90@ti.com \
    --to=peter.ujfalusi@ti.com \
    --cc=aaro.koskinen@iki.fi \
    --cc=andreas@kemnade.info \
    --cc=boris.brezillon@free-electrons.com \
    --cc=hns@goldelico.com \
    --cc=ladis@linux-mips.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=rogerq@ti.com \
    --cc=tony@atomide.com \
    --subject='Re: [PATCH v2] mtd: onenand: omap2: Disable DMA for HIGHMEM buffers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).