Hi Hans, On Sat, May 05, 2012 at 12:55:01PM +0200, Hans Verkuil wrote: > On Sat May 5 2012 12:25:45 Sakari Ailus wrote: > > From: Rémi Denis-Courmont <remi@xxxxxxxxxx> > > > > V4L2 uses the enum type in IOCTL arguments in IOCTLs that were defined until > > the use of enum was considered less than ideal. Recently Rémi Denis-Courmont > > brought up the issue by proposing a patch to convert the enums to unsigned: > > > > <URL:http://www.spinics.net/lists/linux-media/msg46167.html> > > > > This sparked a long discussion where another solution to the issue was > > proposed: two sets of IOCTL structures, one with __u32 and the other with > > enums, and conversion code between the two: > > > > <URL:http://www.spinics.net/lists/linux-media/msg47168.html> > > > > Both approaches implement a complete solution that resolves the problem. The > > first one is simple but requires assuming enums and __u32 are the same in > > size (so we won't break the ABI) while the second one is more complex and > > less clean but does not require making that assumption. > > > > The issue boils down to whether enums are fundamentally different from __u32 > > or not, and can the former be substituted by the latter. During the > > discussion it was concluded that the __u32 has the same size as enums on all > > archs Linux is supported: it has not been shown that replacing those enums > > in IOCTL arguments would break neither source or binary compatibility. If no > > such reason is found, just replacing the enums with __u32s is the way to go. > > > > This is what this patch does. This patch is slightly different from Remi's > > first RFC (link above): it uses __u32 instead of unsigned and also changes > > the arguments of VIDIOC_G_PRIORITY and VIDIOC_S_PRIORITY. > > > > Signed-off-by: Rémi Denis-Courmont <remi@xxxxxxxxxx> > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx> > > --- > > include/linux/videodev2.h | 46 ++++++++++++++++++++++---------------------- > > 1 files changed, 23 insertions(+), 23 deletions(-) > > > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > index 5a09ac3..585e4b4 100644 > > --- a/include/linux/videodev2.h > > +++ b/include/linux/videodev2.h > > @@ -292,10 +292,10 @@ struct v4l2_pix_format { > > __u32 width; > > __u32 height; > > __u32 pixelformat; > > - enum v4l2_field field; > > + __u32 field; > > One suggestion: add a comment like this: > > __u32 field; /* refer to enum v4l2_field */ > > This keeps the link between the u32 and the enum values. > > Note that the DocBook documentation also has to be updated. > > Looks good to me otherwise. Thanks for the comments. I'll make the above changes. Cheers, -- Sakari Ailus e-mail: sakari.ailus@xxxxxx jabber/XMPP/Gmail: sailus@xxxxxxxxxxxxxx -- 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
