Hi Laurent,
Thank your your review.

On 10/11/2012 11:49 PM, Laurent Pinchart wrote:
> Hi Tomasz,
> 
> Thanks for the patch.
> 
> On Wednesday 10 October 2012 16:46:40 Tomasz Stanislawski wrote:
>> This patch adds taking reference to the device for MMAP buffers.
>>
>> Such buffers, may be exported using DMABUF mechanism. If the driver that
>> created a queue is unloaded then the queue is released, the device might be
>> released too.  However, buffers cannot be released if they are referenced by
>> DMABUF descriptor(s). The device pointer kept in a buffer must be valid for
>> the whole buffer's lifetime. Therefore MMAP buffers should take a reference
>> to the device to avoid risk of dangling pointers.
>>
>> Signed-off-by: Tomasz Stanislawski <t.stanisl...@samsung.com>
>> Acked-by: Hans Verkuil <hans.verk...@cisco.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> 
> But two small comments below.
> 
>> ---
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index b138b5c..2d661fd
>> 100644
>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> @@ -148,6 +148,7 @@ static void vb2_dc_put(void *buf_priv)
>>              kfree(buf->sgt_base);
>>      }
>>      dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
>> +    put_device(buf->dev);
>>      kfree(buf);
>>  }
>>
>> @@ -168,6 +169,9 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
>> size) return ERR_PTR(-ENOMEM);
>>      }
>>
>> +    /* prevent the device from release while the buffer is exported */
> 
> s/prevent/Prevent/ ?
> 

s/release/being released/ ?

>> +    get_device(dev);
>> +
>>      buf->dev = dev;
> 
> What about just
> 
>       buf->dev = get_device(dev);
> 

Right, sorry I missed that from your previous review :).

Regards,
Tomasz Stanislawski

>>      buf->size = size;

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to