Re: [PATCH] staging: android: ion: Set the length of the DMA sg entries in buffer

From	"Jon Medhurst (Tixy)" <tixy@linaro.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH] staging: android: ion: Set the length of the DMA sg entries in buffer
Date	2016-01-22 11:10 +0100
Message-ID	<qTGPf-2B3-1@gated-at.bofh.it> (permalink)
References	<qTm4a-4Jh-13@gated-at.bofh.it> <qTrnb-5X-7@gated-at.bofh.it> <qTtS2-1R8-19@gated-at.bofh.it> <qTyf1-4I6-15@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Thu, 2016-01-21 at 16:58 -0800, Laura Abbott wrote:
> On 01/21/2016 12:19 PM, Jon Medhurst (Tixy) wrote:
[...]
> > If sg_dma_len() is correct or acceptable then it seems to me that the
> > ION code should set that length. Especially as the comment in the code
> > implies it's faking a call to map_sg and grepping the kernel tree for
> > real implementations of that functionality seems to show the dma_address
> > getting set.
> >
> > As you can probably tell, I feel I may be on shaky ground. This is
> > because I don't fully understanding the code and suspecting both the ION
> > and GPU code is rather dodgy (and possibly the bits in between :-)
> >
> 
> I blame the Ion code completely. I remember hitting a similar problem
> with other out of tree drivers. The solution then was to have drivers
> switch to using sg->length instead of sg_dma_len given the state of that
> tree. For the Mali driver, if it is ever going to be backed by an IOMMU
> you will need to use sg_dma_len so I think at least that part of your
> code is correct.
> 
> Thinking about it some, I'm okay with the patch going in. I thought
> there was some reason why the out of tree code from before didn't just
> do this hack but I can't remember it. It may have been an out of tree
> use case. This does go well with Ion's behavior of pretending to do
> DMA mapping. More out of tree users can plead their case if it breaks.

Well, the $subject patch is copying the value of 'length' into
'dma_length', and if dma_length was previously uninitialised then I
don't see that it can really cause additional problems for ION users. (I
hate the way I've been using 'if' a lot so I'm going to spend some time
educating myself.)

> Acked-by: Laura Abbott <labbott@redhat.com>

Thanks

-- 
Tixy

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH] staging: android: ion: Set the length of the DMA sg entries  in buffer "Jon Medhurst (Tixy)" <tixy@linaro.org> - 2016-01-21 13:00 +0100
  Re: [PATCH] staging: android: ion: Set the length of the DMA sg  entries in buffer Laura Abbott <labbott@redhat.com> - 2016-01-21 18:40 +0100
    Re: [PATCH] staging: android: ion: Set the length of the DMA sg  entries in buffer "Jon Medhurst (Tixy)" <tixy@linaro.org> - 2016-01-21 21:20 +0100
      Re: [PATCH] staging: android: ion: Set the length of the DMA sg  entries in buffer Laura Abbott <labbott@redhat.com> - 2016-01-22 02:00 +0100
        Re: [PATCH] staging: android: ion: Set the length of the DMA sg  entries in buffer "Jon Medhurst (Tixy)" <tixy@linaro.org> - 2016-01-22 11:10 +0100

csiph-web