Re: [PATCH 01/15] [media] Move mediabus format definition to a more standard place
Hi, On Tue, Nov 04, 2014 at 12:09:59PM +0100, Hans Verkuil wrote: Well, I gave two alternatives :-) Both are fine as far as I am concerned, but it would be nice to hear what others think. In fact I think both are good options. :-) I'd perhaps lean towards the latter, for it has the benefit of pushing to use the new definitions and the old ones can be deprecated (and eventually removed in year 2030 or so ;)). Either way, preprocessor macros should be used instead of an enum since that way it's possible to figure out at that phase whether something is defined or not. There is for enums, too, but it results in a compilation error... -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/15] [media] Move mediabus format definition to a more standard place
On Wed, 5 Nov 2014 17:00:25 +0200 Sakari Ailus sakari.ai...@iki.fi wrote: Hi, On Tue, Nov 04, 2014 at 12:09:59PM +0100, Hans Verkuil wrote: Well, I gave two alternatives :-) Both are fine as far as I am concerned, but it would be nice to hear what others think. In fact I think both are good options. :-) I'd perhaps lean towards the latter, for it has the benefit of pushing to use the new definitions and the old ones can be deprecated (and eventually removed in year 2030 or so ;)). Either way, preprocessor macros should be used instead of an enum since that way it's possible to figure out at that phase whether something is defined or not. There is for enums, too, but it results in a compilation error... I don't get that last part :-). -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/15] [media] Move mediabus format definition to a more standard place
On 11/04/14 11:20, Hans Verkuil wrote: Hi Boris, On 11/04/14 10:54, Boris Brezillon wrote: Rename mediabus formats and move the enum into a separate header file so that it can be used by DRM/KMS subsystem without any reference to the V4L2 subsystem. Old V4L2_MBUS_FMT_ definitions are now referencing MEDIA_BUS_FMT_ value. I missed earlier that v4l2-mediabus.h contained a struct as well, so it can't be deprecated and neither can a #warning be added. The best approach, I think, is to use a macro in media-bus-format.h that will either define just the MEDIA_BUS value when compiled in the kernel, or define both MEDIA_BUS and V4L2_MBUS values when compiled for userspace. E.g. something like this: #ifdef __KERNEL__ #define MEDIA_BUS_FMT_ENTRY(name, val) MEDIA_BUS_FMT_ # name = val #else /* Keep V4L2_MBUS_FMT for backwards compatibility */ #define MEDIA_BUS_FMT_ENTRY(name, val) \ MEDIA_BUS_FMT_ # name = val, \ V4L2_MBUS_FMT_ # name = val #endif And v4l2-mediabus.h needs this as well: #ifndef __KERNEL__ /* For backwards compatibility */ #define v4l2_mbus_pixelcode media_bus_format #endif Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/15] [media] Move mediabus format definition to a more standard place
Hi Hans, On Tue, 04 Nov 2014 11:20:40 +0100 Hans Verkuil hansv...@cisco.com wrote: Hi Boris, On 11/04/14 10:54, Boris Brezillon wrote: Rename mediabus formats and move the enum into a separate header file so that it can be used by DRM/KMS subsystem without any reference to the V4L2 subsystem. Old V4L2_MBUS_FMT_ definitions are now referencing MEDIA_BUS_FMT_ value. I missed earlier that v4l2-mediabus.h contained a struct as well, so it can't be deprecated and neither can a #warning be added. The best approach, I think, is to use a macro in media-bus-format.h that will either define just the MEDIA_BUS value when compiled in the kernel, or define both MEDIA_BUS and V4L2_MBUS values when compiled for userspace. E.g. something like this: #ifdef __KERNEL__ #define MEDIA_BUS_FMT_ENTRY(name, val) MEDIA_BUS_FMT_ # name = val #else /* Keep V4L2_MBUS_FMT for backwards compatibility */ #define MEDIA_BUS_FMT_ENTRY(name, val) \ MEDIA_BUS_FMT_ # name = val, \ V4L2_MBUS_FMT_ # name = val #endif Okay, but this means we keep adding V4L2_MBUS_FMT_ definitions even for new formats (which definitely doesn't encourage people to move on). Moreover, we add a V4L2 prefix in what was supposed to be a subsystem neutral header. Anyway, these are just nitpicks, and if you prefer this approach I'll rework my series :-). An alternative approach is to have v4l2-mediabus.h include media-bus-format.h, put #ifndef __KERNEL__ around the enum v4l2_mbus_pixelcode and add a big comment there that applications should use the defines from media-bus-format.h and that this enum is frozen (i.e. new values are only added to media-bus-format.h). But I think I like the macro idea best. As you wish, my only intent is to use those bus format definitions in a DRM driver :-). Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/15] [media] Move mediabus format definition to a more standard place
Well, I gave two alternatives :-) Both are fine as far as I am concerned, but it would be nice to hear what others think. Regards, Hans On 11/04/14 11:45, Boris Brezillon wrote: Hi Hans, On Tue, 04 Nov 2014 11:20:40 +0100 Hans Verkuil hansv...@cisco.com wrote: Hi Boris, On 11/04/14 10:54, Boris Brezillon wrote: Rename mediabus formats and move the enum into a separate header file so that it can be used by DRM/KMS subsystem without any reference to the V4L2 subsystem. Old V4L2_MBUS_FMT_ definitions are now referencing MEDIA_BUS_FMT_ value. I missed earlier that v4l2-mediabus.h contained a struct as well, so it can't be deprecated and neither can a #warning be added. The best approach, I think, is to use a macro in media-bus-format.h that will either define just the MEDIA_BUS value when compiled in the kernel, or define both MEDIA_BUS and V4L2_MBUS values when compiled for userspace. E.g. something like this: #ifdef __KERNEL__ #define MEDIA_BUS_FMT_ENTRY(name, val) MEDIA_BUS_FMT_ # name = val #else /* Keep V4L2_MBUS_FMT for backwards compatibility */ #define MEDIA_BUS_FMT_ENTRY(name, val) \ MEDIA_BUS_FMT_ # name = val, \ V4L2_MBUS_FMT_ # name = val #endif Okay, but this means we keep adding V4L2_MBUS_FMT_ definitions even for new formats (which definitely doesn't encourage people to move on). Moreover, we add a V4L2 prefix in what was supposed to be a subsystem neutral header. Anyway, these are just nitpicks, and if you prefer this approach I'll rework my series :-). An alternative approach is to have v4l2-mediabus.h include media-bus-format.h, put #ifndef __KERNEL__ around the enum v4l2_mbus_pixelcode and add a big comment there that applications should use the defines from media-bus-format.h and that this enum is frozen (i.e. new values are only added to media-bus-format.h). But I think I like the macro idea best. As you wish, my only intent is to use those bus format definitions in a DRM driver :-). Thanks, Boris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
