public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Jon Medhurst (Tixy)" <tixy@linaro.org>
To: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Riley Andrews" <riandrews@android.com>
Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
	Zeng Tao <prime.zeng@huawei.com>,
	Laura Abbott <labbott@redhat.com>,
	Robin Murphy <robin.murphy@arm.com>
Subject: [PATCH] staging: ion: ion_cma_heap: Don't directly use dma_common_get_sgtable
Date: Fri, 17 Jul 2015 12:01:29 +0100	[thread overview]
Message-ID: <1437130889.3221.53.camel@linaro.org> (raw)

Use dma_get_sgtable rather than dma_common_get_sgtable so a device's
dma_ops aren't bypassed. This is essential in situations where a device
uses an IOMMU and the physical memory is not contiguous (as the common
function assumes).

Signed-off-by: Jon Medhurst <tixy@linaro.org>
---

This also begs the question as to what happens if the memory region _is_
contiguous but is in highmem or an ioremapped region. Should a device
always provide dma_ops for that case? Because I believe the current
implementation of dma_common_get_sgtable won't work for those as it uses
virt_to_page.

I see that this point has been raised before [1] by Zeng Tao, and I
myself have been given a different fix to apply to a Linaro kernel tree.
However, both solutions looked wrong to me as they treat a dma_addr_t as
a physical address, so should at least be using dma_to_phys.

So, should we fix dma_common_get_sgtable or mandate that the device
has dma_ops? The latter seems to be implied by the commit message which
introduced the function:

        This patch provides a generic implementation based on
        virt_to_page() call. Architectures which require more
        sophisticated translation might provide their own get_sgtable()
        methods.

Note, I don't have a system where any of this code is used to test
things, and have never looked at this area before yesterday, so I may
have misunderstood what’s going on in the code.

[1] https://lkml.org/lkml/2014/12/1/584

 drivers/staging/android/ion/ion_cma_heap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
index f4211f1..86b91fd 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -73,8 +73,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
 	if (!info->table)
 		goto free_mem;
 
-	if (dma_common_get_sgtable
-	    (dev, info->table, info->cpu_addr, info->handle, len))
+	if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle, len))
 		goto free_table;
 	/* keep this for memory release */
 	buffer->priv_virt = info;
-- 
2.1.4



             reply	other threads:[~2015-07-17 11:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-17 11:01 Jon Medhurst (Tixy) [this message]
2015-07-17 15:21 ` [PATCH] staging: ion: ion_cma_heap: Don't directly use dma_common_get_sgtable Robin Murphy
2015-07-17 16:29   ` Jon Medhurst (Tixy)
2015-07-17 16:50   ` Laura Abbott
2015-07-20 18:30     ` Robin Murphy

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=1437130889.3221.53.camel@linaro.org \
    --to=tixy@linaro.org \
    --cc=arve@android.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=labbott@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=prime.zeng@huawei.com \
    --cc=riandrews@android.com \
    --cc=robin.murphy@arm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox