[PATCHv2 1/9] media: Fix DVB representation at media controller API

[PATCHv2 1/9] media: Fix DVB representation at media controller API

[Mauro Carvalho Chehab]

The DVB devices are identified via a (major, minor) tuple,
and not by a random id. Fix it, before we start using it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index e00459185d20..de333cc8261b 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -97,7 +97,10 @@ struct media_entity {
 			u32 device;
 			u32 subdevice;
 		} alsa;
-		int dvb;
+		struct {
+			u32 major;
+			u32 minor;
+		} dvb;
 
 		/* Sub-device specifications */
 		/* Nothing needed yet */
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index d847c760e8f0..7902e800f019 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -27,7 +27,7 @@
 #include <linux/types.h>
 #include <linux/version.h>
 
-#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 0)
+#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 1)
 
 struct media_device_info {
 	char driver[16];
@@ -88,7 +88,10 @@ struct media_entity_desc {
 			__u32 device;
 			__u32 subdevice;
 		} alsa;
-		int dvb;
+		struct {
+			__u32 major;
+			__u32 minor;
+		} dvb;
 
 		/* Sub-device specifications */
 		/* Nothing needed yet */
-- 
2.1.0

Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> The DVB devices are identified via a (major, minor) tuple,
> and not by a random id. Fix it, before we start using it.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index e00459185d20..de333cc8261b 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -97,7 +97,10 @@ struct media_entity {
>  			u32 device;
>  			u32 subdevice;
>  		} alsa;
> -		int dvb;
> +		struct {
> +			u32 major;
> +			u32 minor;
> +		} dvb;
> 
>  		/* Sub-device specifications */
>  		/* Nothing needed yet */
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index d847c760e8f0..7902e800f019 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -27,7 +27,7 @@
>  #include <linux/types.h>
>  #include <linux/version.h>
> 
> -#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 0)
> +#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 1)
> 
>  struct media_device_info {
>  	char driver[16];
> @@ -88,7 +88,10 @@ struct media_entity_desc {
>  			__u32 device;
>  			__u32 subdevice;
>  		} alsa;
> -		int dvb;
> +		struct {
> +			__u32 major;
> +			__u32 minor;
> +		} dvb;

Won't this break compilation of existing userspace code ? As DVB is not 
properly supported in MC at the moment we could consider that only mediactl 
will be affected, so it shouldn't be a big issue.

> 
>  		/* Sub-device specifications */
>  		/* Nothing needed yet */

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API

[Mauro Carvalho Chehab]

Em Sun, 11 Jan 2015 15:50:04 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> > The DVB devices are identified via a (major, minor) tuple,
> > and not by a random id. Fix it, before we start using it.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > 
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index e00459185d20..de333cc8261b 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -97,7 +97,10 @@ struct media_entity {
> >  			u32 device;
> >  			u32 subdevice;
> >  		} alsa;
> > -		int dvb;
> > +		struct {
> > +			u32 major;
> > +			u32 minor;
> > +		} dvb;
> > 
> >  		/* Sub-device specifications */
> >  		/* Nothing needed yet */
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index d847c760e8f0..7902e800f019 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -27,7 +27,7 @@
> >  #include <linux/types.h>
> >  #include <linux/version.h>
> > 
> > -#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 0)
> > +#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 1)
> > 
> >  struct media_device_info {
> >  	char driver[16];
> > @@ -88,7 +88,10 @@ struct media_entity_desc {
> >  			__u32 device;
> >  			__u32 subdevice;
> >  		} alsa;
> > -		int dvb;
> > +		struct {
> > +			__u32 major;
> > +			__u32 minor;
> > +		} dvb;
> 
> Won't this break compilation of existing userspace code ? As DVB is not 
> properly supported in MC at the moment we could consider that only mediactl 
> will be affected, so it shouldn't be a big issue.

Well, media-ctl uses a local copy of the videodev2.h header, so it won't
break.

I'm not aware of any other application using MC for DVB.

Yet, imagining that such application exists, then, IMHO, it is better
to break compilation for it, as probably such application was written for
some OOT driver that might be using its own version of the media
controller implementation.

Regards,
Mauro

Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API

[Laurent Pinchart]

Hi Mauro,

On Sunday 11 January 2015 11:58:24 Mauro Carvalho Chehab wrote:
> Em Sun, 11 Jan 2015 15:50:04 +0200 Laurent Pinchart escreveu:
> > On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> >> The DVB devices are identified via a (major, minor) tuple,
> >> and not by a random id. Fix it, before we start using it.
> >> 
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> >> 
> >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> >> index e00459185d20..de333cc8261b 100644
> >> --- a/include/media/media-entity.h
> >> +++ b/include/media/media-entity.h
> >> @@ -97,7 +97,10 @@ struct media_entity {
> >>  			u32 device;
> >>  			u32 subdevice;
> >>  		} alsa;
> >> -		int dvb;
> >> +		struct {
> >> +			u32 major;
> >> +			u32 minor;
> >> +		} dvb;
> >> 
> >>  		/* Sub-device specifications */
> >>  		/* Nothing needed yet */
> >> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> >> index d847c760e8f0..7902e800f019 100644
> >> --- a/include/uapi/linux/media.h
> >> +++ b/include/uapi/linux/media.h
> >> @@ -27,7 +27,7 @@
> >>  #include <linux/types.h>
> >>  #include <linux/version.h>
> >> 
> >> -#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 0)
> >> +#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 1)
> >> 
> >>  struct media_device_info {
> >>  	char driver[16];
> >> @@ -88,7 +88,10 @@ struct media_entity_desc {
> >>  			__u32 device;
> >>  			__u32 subdevice;
> >>  		} alsa;
> >> -		int dvb;
> >> +		struct {
> >> +			__u32 major;
> >> +			__u32 minor;
> >> +		} dvb;
> > 
> > Won't this break compilation of existing userspace code ? As DVB is not
> > properly supported in MC at the moment we could consider that only
> > mediactl will be affected, so it shouldn't be a big issue.
> 
> Well, media-ctl uses a local copy of the videodev2.h header, so it won't
> break.

It's media.h, but you're correct here.

> I'm not aware of any other application using MC for DVB.
> 
> Yet, imagining that such application exists, then, IMHO, it is better
> to break compilation for it, as probably such application was written for
> some OOT driver that might be using its own version of the media
> controller implementation.

OK. I'll remember that argument the next time I want to break a kernel API 
though ;-)

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API

[Mauro Carvalho Chehab]

Em Sun, 11 Jan 2015 16:05:32 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> escreveu:

> Hi Mauro,
> 
> On Sunday 11 January 2015 11:58:24 Mauro Carvalho Chehab wrote:
> > Em Sun, 11 Jan 2015 15:50:04 +0200 Laurent Pinchart escreveu:
> > > On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> > >> The DVB devices are identified via a (major, minor) tuple,
> > >> and not by a random id. Fix it, before we start using it.
> > >> 
> > >> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > >> 
> > >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > >> index e00459185d20..de333cc8261b 100644
> > >> --- a/include/media/media-entity.h
> > >> +++ b/include/media/media-entity.h
> > >> @@ -97,7 +97,10 @@ struct media_entity {
> > >>  			u32 device;
> > >>  			u32 subdevice;
> > >>  		} alsa;
> > >> -		int dvb;
> > >> +		struct {
> > >> +			u32 major;
> > >> +			u32 minor;
> > >> +		} dvb;
> > >> 
> > >>  		/* Sub-device specifications */
> > >>  		/* Nothing needed yet */
> > >> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > >> index d847c760e8f0..7902e800f019 100644
> > >> --- a/include/uapi/linux/media.h
> > >> +++ b/include/uapi/linux/media.h
> > >> @@ -27,7 +27,7 @@
> > >>  #include <linux/types.h>
> > >>  #include <linux/version.h>
> > >> 
> > >> -#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 0)
> > >> +#define MEDIA_API_VERSION	KERNEL_VERSION(0, 1, 1)
> > >> 
> > >>  struct media_device_info {
> > >>  	char driver[16];
> > >> @@ -88,7 +88,10 @@ struct media_entity_desc {
> > >>  			__u32 device;
> > >>  			__u32 subdevice;
> > >>  		} alsa;
> > >> -		int dvb;
> > >> +		struct {
> > >> +			__u32 major;
> > >> +			__u32 minor;
> > >> +		} dvb;
> > > 
> > > Won't this break compilation of existing userspace code ? As DVB is not
> > > properly supported in MC at the moment we could consider that only
> > > mediactl will be affected, so it shouldn't be a big issue.
> > 
> > Well, media-ctl uses a local copy of the videodev2.h header, so it won't
> > break.
> 
> It's media.h, but you're correct here.

Ah, yes, that's what I meant ;)

Btw, I have also the patches adding support for DVB at v4l-utils:
	http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/log/?h=dvb-media-ctl

> 
> > I'm not aware of any other application using MC for DVB.
> > 
> > Yet, imagining that such application exists, then, IMHO, it is better
> > to break compilation for it, as probably such application was written for
> > some OOT driver that might be using its own version of the media
> > controller implementation.
> 
> OK. I'll remember that argument the next time I want to break a kernel API 
> though ;-)

:)

Actually, we're not breaking the Kernel API here, as DVB support
inside the media controller were never added.

Next time, we should be sure to not add provision for an API at
the Kernel without actually implementing it ;)

Btw, eventually we'll end facing the very same issue when we
merge support for ALSA. IMHO, it is just easier to use major,minor
for all devnodes than to use anything else.

Yet, you're right: maybe we should do, instead:


	union {
		struct {
			u32 major;
			u32 minor;
		} dev;

		/* DEPRECATED: old node specifications */
		struct {
			u32 major;
			u32 minor;
		} v4l;
		struct {
			u32 major;
			u32 minor;
		} fb;
		struct {
			u32 card;
			u32 device;
			u32 subdevice;
		} alsa;
		int dvb;

		/* Sub-device specifications */
		/* Nothing needed yet */
	} info;

And change media-ctl to use info.dev for all devnodes. This will
provide a fix when we add support for alsa devnodes too.

Regards,
Mauro

[PATCHv2 2/9] media: add new types for DVB devnodes

[Mauro Carvalho Chehab]

Most of the DVB subdevs have already their own devnode.

Add support for them at the media controller API.

Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 7902e800f019..707db275f92b 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -50,7 +50,14 @@ struct media_device_info {
 #define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
 #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
 #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
-#define MEDIA_ENT_T_DEVNODE_DVB		(MEDIA_ENT_T_DEVNODE + 4)
+#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE + 4)
+#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE + 5)
+#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE + 6)
+#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE + 7)
+#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE + 8)
+
+/* Legacy symbol. Use it to avoid userspace compilation breakages */
+#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
 
 #define MEDIA_ENT_T_V4L2_SUBDEV		(2 << MEDIA_ENT_TYPE_SHIFT)
 #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV + 1)
-- 
2.1.0