Hi Hans,
Thank your for the review.
Please refer to the comments below.
On 08/22/2012 12:27 PM, Hans Verkuil wrote:
> On Tue August 14 2012 17:34:31 Tomasz Stanislawski wrote:
>> From: Sumit Semwal <sumit.semwal@xxxxxx>
>>
>> Adds DMABUF memory type to v4l framework. Also adds the related file
>> descriptor in v4l2_plane and v4l2_buffer.
>>
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@xxxxxxxxxxx>
>> [original work in the PoC for buffer sharing]
>> Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxx>
>> Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
>> Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>> ---
>> drivers/media/video/v4l2-compat-ioctl32.c | 18 ++++++++++++++++++
>> drivers/media/video/v4l2-ioctl.c | 1 +
>> include/linux/videodev2.h | 7 +++++++
>> 3 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
>> index 9ebd5c5..a2e0549 100644
>> --- a/drivers/media/video/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
>> @@ -304,6 +304,7 @@ struct v4l2_plane32 {
>> union {
>> __u32 mem_offset;
>> compat_long_t userptr;
>> + __u32 fd;
>
> Shouldn't this be int?
>
Notice that this field should be consistent with fd field used in
'struct v4l2_exportbuffer'. Therefore I prefer to use fixed-size types.
One could use __s32 here but notice that file descriptors are defined
as small, nonnegative integers according to POSIX spec. The type __u32
suits well for this purpose. The negative values returned by open
syscall are used only to indicate failures.
On the other hand, using __s32 may help to avoid compiler warning while
building userspace apps due to 'signed-vs-unsigned comparisons'.
However, I do not have any strong opinion about 'int vs __u32' issue :).
Do you think that using __s32 for both QUERYBUF and EXPBUF is a good
compromise?
>> } m;
>> __u32 data_offset;
>> __u32 reserved[11];
>> @@ -325,6 +326,7 @@ struct v4l2_buffer32 {
>> __u32 offset;
>> compat_long_t userptr;
>> compat_caddr_t planes;
>> + __u32 fd;
>
> Ditto.
>
>> } m;
>> __u32 length;
>> __u32 reserved2;
> Regards,
>
> Hans
>
Regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
