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

["Jon Medhurst (Tixy)" <tixy@linaro.org>]

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