[PATCH 0/7] add link graph to cx231xx using the media controller

[PATCH 0/7] add link graph to cx231xx using the media controller

[Mauro Carvalho Chehab]

This patch series depends on the patch media controller patch series
sent previously. The full set of patches are at:
	http://git.linuxtv.org/cgit.cgi/mchehab/experimental.git/log/?h=dvb-media-ctl

It adds the logic needed to create the remaining media PADs and the
links to represent the media graph.

TODO:
- The links are now static and don't change when the device is switched
  to DVB mode.

With this change:

Media controller API version 0.1.1

Media device information
------------------------
driver          cx231xx
model           Pixelview PlayTV USB Hybrid
serial          CIR000000000001
bus info        4
hw revision     0x4001
driver version  3.19.0

Device topology
- entity 1: cx25840 19-0044 (3 pads, 3 links)
            type V4L2 subdev subtype Decoder flags 0
	pad0: Sink
		<- "NXP TDA18271HD":0 [ENABLED]
	pad1: Source
		-> "cx231xx #0 video":0 [ENABLED]
	pad2: Source
		-> "cx231xx #0 vbi":0 [ENABLED]

- entity 2: NXP TDA18271HD (1 pad, 2 links)
            type V4L2 subdev subtype Tuner flags 0
	pad0: Source
		-> "cx25840 19-0044":0 [ENABLED]
		-> "Fujitsu mb86A20s":0 []

- entity 3: cx231xx #0 video (1 pad, 1 link)
            type Node subtype V4L flags 0
            device node name /dev/video0
	pad0: Sink
		<- "cx25840 19-0044":1 [ENABLED]

- entity 4: cx231xx #0 vbi (1 pad, 1 link)
            type Node subtype V4L flags 0
            device node name /dev/vbi0
	pad0: Sink
		<- "cx25840 19-0044":2 [ENABLED]

- entity 5: Fujitsu mb86A20s (2 pads, 2 links)
            type Node subtype DVB FE flags 0
            device node name /dev/dvb/adapter0/frontend0
	pad0: Sink
		<- "NXP TDA18271HD":0 []
	pad1: Source
		-> "demux":0 []

- entity 6: demux (2 pads, 2 links)
            type Node subtype DVB DEMUX flags 0
            device node name /dev/dvb/adapter0/demux0
	pad0: Sink
		<- "Fujitsu mb86A20s":1 []
	pad1: Source
		-> "dvr":0 []

- entity 7: dvr (1 pad, 1 link)
            type Node subtype DVB DVR flags 0
            device node name /dev/dvb/adapter0/dvr0
	pad0: Sink
		<- "demux":1 []

- entity 8: dvb net (0 pad, 0 link)
            type Node subtype DVB NET flags 0
            device node name /dev/dvb/adapter0/net0


Mauro Carvalho Chehab (7):
  tuner-core: properly initialize media controller subdev
  cx25840: fill the media controller entity
  cx231xx: initialize video/vbi pads
  cx231xx: create media links for analog mode
  dvbdev: represent frontend with two pads
  dvbdev: add a function to create DVB media graph
  cx231xx: create DVB graph

 drivers/media/dvb-core/dvbdev.c           | 59 ++++++++++++++++++++++++++++---
 drivers/media/dvb-core/dvbdev.h           |  1 +
 drivers/media/i2c/cx25840/cx25840-core.c  | 14 +++++++-
 drivers/media/i2c/cx25840/cx25840-core.h  |  3 ++
 drivers/media/usb/cx231xx/cx231xx-cards.c | 40 +++++++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx-dvb.c   |  1 +
 drivers/media/usb/cx231xx/cx231xx-video.c | 13 ++++++-
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 drivers/media/v4l2-core/tuner-core.c      | 15 ++++++++
 include/uapi/linux/media.h                |  2 ++
 10 files changed, 142 insertions(+), 7 deletions(-)

-- 
2.1.0

DOT file with the graph:

digraph board {
	rankdir=TB
	n00000001 [label="{{<port0> 0} | cx25840 19-0044 | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
	n00000001:port1 -> n00000003
	n00000001:port2 -> n00000004
	n00000002 [label="{{} | NXP TDA18271HD | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
	n00000002:port0 -> n00000001:port0
	n00000002:port0 -> n00000005 [style=dashed]
	n00000003 [label="cx231xx #0 video\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
	n00000004 [label="cx231xx #0 vbi\n/dev/vbi0", shape=box, style=filled, fillcolor=yellow]
	n00000005 [label="Fujitsu mb86A20s\n/dev/dvb/adapter0/frontend0", shape=box, style=filled, fillcolor=yellow]
	n00000005 -> n00000006 [style=dashed]
	n00000006 [label="demux\n/dev/dvb/adapter0/demux0", shape=box, style=filled, fillcolor=yellow]
	n00000006 -> n00000007 [style=dashed]
	n00000007 [label="dvr\n/dev/dvb/adapter0/dvr0", shape=box, style=filled, fillcolor=yellow]
	n00000008 [label="dvb net\n/dev/dvb/adapter0/net0", shape=box, style=filled, fillcolor=yellow]
}

[PATCH 1/7] tuner-core: properly initialize media controller subdev

[Mauro Carvalho Chehab]

Properly initialize tuner core subdev at the media controller.

That requires a new subtype at the media controller API.

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

diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
index 559f8372e2eb..114715ed0110 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -134,6 +134,9 @@ struct tuner {
 	unsigned int        type; /* chip type id */
 	void                *config;
 	const char          *name;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_pad	pad;
+#endif
 };
 
 /*
@@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int type,
 		t->name = analog_ops->info.name;
 	}
 
+	t->sd.entity.name = t->name;
+
 	tuner_dbg("type set to %s\n", t->name);
 
 	t->mode_mask = new_mode_mask;
@@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
 	struct tuner *t;
 	struct tuner *radio;
 	struct tuner *tv;
+	int ret;
 
 	t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
 	if (NULL == t)
@@ -696,6 +702,15 @@ register_client:
 		   t->type,
 		   t->mode_mask & T_RADIO ? " Radio" : "",
 		   t->mode_mask & T_ANALOG_TV ? " TV" : "");
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	t->pad.flags = MEDIA_PAD_FL_SOURCE;
+	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
+	t->sd.entity.name = t->name;
+
+	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
+	if (ret < 0)
+		tuner_err("failed to initialize media entity!\n");
+#endif
 	return 0;
 }
 
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 707db275f92b..5ffde035789b 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -66,6 +66,8 @@ struct media_device_info {
 /* A converter of analogue video to its digital representation. */
 #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
 
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
+
 #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
 
 struct media_entity_desc {
-- 
2.1.0

[PATCH 2/7] cx25840: fill the media controller entity

[Mauro Carvalho Chehab]

Instead of keeping the media controller entity not initialized,
fill it and create the pads for cx25840.

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

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
index 573e08826b9b..831c74b97798 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -5134,7 +5134,7 @@ static int cx25840_probe(struct i2c_client *client,
 {
 	struct cx25840_state *state;
 	struct v4l2_subdev *sd;
-	int default_volume;
+	int default_volume, ret;
 	u32 id;
 	u16 device_id;
 
@@ -5178,6 +5178,18 @@ static int cx25840_probe(struct i2c_client *client,
 
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	/* TODO: need to represent analog inputs too */
+	state->pads[0].flags = MEDIA_PAD_FL_SINK;	/* Tuner or input */
+	state->pads[1].flags = MEDIA_PAD_FL_SOURCE;	/* Video */
+	state->pads[2].flags = MEDIA_PAD_FL_SOURCE;	/* VBI */
+	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
+
+	ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
+				state->pads, 0);
+	if (ret < 0)
+		v4l_info(client, "failed to initialize media entity!\n");
+#endif
 
 	switch (id) {
 	case CX23885_AV:
diff --git a/drivers/media/i2c/cx25840/cx25840-core.h b/drivers/media/i2c/cx25840/cx25840-core.h
index 37bc04217c44..17b409f55445 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.h
+++ b/drivers/media/i2c/cx25840/cx25840-core.h
@@ -64,6 +64,9 @@ struct cx25840_state {
 	wait_queue_head_t fw_wait;    /* wake up when the fw load is finished */
 	struct work_struct fw_work;   /* work entry for fw load */
 	struct cx25840_ir_state *ir_state;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_pad	pads[3];
+#endif
 };
 
 static inline struct cx25840_state *to_state(struct v4l2_subdev *sd)
-- 
2.1.0

[PATCH 3/7] cx231xx: initialize video/vbi pads

[Mauro Carvalho Chehab]

Both video and vbi are sink pads. Initialize them as such.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index ecea76fe07f6..f3d1a488dfa7 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -2121,7 +2121,12 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
 		dev_err(dev->dev, "cannot allocate video_device.\n");
 		return -ENODEV;
 	}
-
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	dev->video_pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_init(&dev->vdev->entity, 1, &dev->video_pad, 0);
+	if (ret < 0)
+		dev_err(dev->dev, "failed to initialize video media entity!\n");
+#endif
 	dev->vdev->ctrl_handler = &dev->ctrl_handler;
 	/* register v4l2 video video_device */
 	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
@@ -2147,6 +2152,12 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
 		dev_err(dev->dev, "cannot allocate video_device.\n");
 		return -ENODEV;
 	}
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	dev->vbi_pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_init(&dev->vbi_dev->entity, 1, &dev->vbi_pad, 0);
+	if (ret < 0)
+		dev_err(dev->dev, "failed to initialize vbi media entity!\n");
+#endif
 	dev->vbi_dev->ctrl_handler = &dev->ctrl_handler;
 	/* register v4l2 vbi video_device */
 	ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index af9d6c4041dc..e0d3106f6b44 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -660,6 +660,7 @@ struct cx231xx {
 
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	struct media_device *media_dev;
+	struct media_pad video_pad, vbi_pad;
 #endif
 
 	unsigned char eedata[256];
-- 
2.1.0

[PATCH 4/7] cx231xx: create media links for analog mode

[Mauro Carvalho Chehab]

Now that we have entities and pads, let's create media links
between them, for analog setup.

We may not have all the links for digital yet, as the dvb extention
may not be loaded yet.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 7e1c73a5172d..5fc7d97df166 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1159,6 +1159,44 @@ static void cx231xx_media_device_register(struct cx231xx *dev,
 #endif
 }
 
+static void cx231xx_create_media_graph(struct cx231xx *dev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	struct media_device *mdev = dev->media_dev;
+	struct media_entity *entity;
+	struct media_entity *tuner = NULL, *decoder = NULL;
+
+	if (!mdev)
+		return;
+
+	media_device_for_each_entity(entity, mdev) {
+		printk("entity %s, type %d\n", entity->name, entity->type);
+
+		switch (entity->type) {
+		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
+			tuner = entity;
+			break;
+		case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
+			decoder = entity;
+			break;
+		}
+	}
+
+	/* Analog setup, using tuner as a link */
+
+	if (!decoder)
+		return;
+
+	if (tuner)
+		media_entity_create_link(tuner, 0, decoder, 0,
+					 MEDIA_LNK_FL_ENABLED);
+	media_entity_create_link(decoder, 1, &dev->vdev->entity, 0,
+				 MEDIA_LNK_FL_ENABLED);
+	media_entity_create_link(decoder, 2, &dev->vbi_dev->entity, 0,
+				 MEDIA_LNK_FL_ENABLED);
+#endif
+}
+
 /*
  * cx231xx_init_dev()
  * allocates and inits the device structs, registers i2c bus and v4l device
@@ -1615,6 +1653,8 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	/* load other modules required */
 	request_modules(dev);
 
+	cx231xx_create_media_graph(dev);
+
 	return 0;
 err_video_alt:
 	/* cx231xx_uninit_dev: */
-- 
2.1.0

[PATCH 5/7] dvbdev: represent frontend with two pads

[Mauro Carvalho Chehab]

While on some devices the tuner is bound inside the frontend,
other devices use a separate subdevice for it.

So, in order to be more generic, better to map it with two
pads.

That will allows to use the media controller to lock the tuner
between the DVB and the V4L2 sub-drivers, on hybrid devices.

While here, change the logic to use pad 0 as sink for devices
with both sink and source pads.

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

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 202c15582fa3..f7294d2ef816 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -200,6 +200,7 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 	switch(type) {
 	case DVB_DEVICE_CA:
 	case DVB_DEVICE_DEMUX:
+	case DVB_DEVICE_FRONTEND:
 		npads = 2;
 		break;
 	case DVB_DEVICE_NET:
@@ -221,12 +222,13 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 	switch(type) {
 	case DVB_DEVICE_FRONTEND:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
-		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DEMUX:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
-		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
-		dvbdev->pads[1].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DVR:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DVR;
@@ -234,8 +236,8 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
 		break;
 	case DVB_DEVICE_CA:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
-		dvbdev->pads[0].flags = MEDIA_PAD_FL_SOURCE;
-		dvbdev->pads[1].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
+		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_NET:
 		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_NET;
-- 
2.1.0

[PATCH 6/7] dvbdev: add a function to create DVB media graph

[Mauro Carvalho Chehab]

We need to create a DVB graph, linking the several DVB devnodes.

Add such function. Please notice that this helper function
doesn't take into account devices with multiple DVB adapters
and frontends.

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

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index f7294d2ef816..7efa157072ed 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -380,6 +380,53 @@ void dvb_unregister_device(struct dvb_device *dvbdev)
 }
 EXPORT_SYMBOL(dvb_unregister_device);
 
+
+void dvb_create_media_graph(struct media_device *mdev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	struct media_entity *entity, *tuner = NULL, *fe = NULL;
+	struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
+
+	if (!mdev)
+		return;
+
+	media_device_for_each_entity(entity, mdev) {
+		printk("entity %s, type %d\n", entity->name, entity->type);
+
+		switch (entity->type) {
+		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
+			tuner = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_FE:
+			fe = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
+			demux = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
+			dvr = entity;
+			break;
+		case MEDIA_ENT_T_DEVNODE_DVB_CA:
+			ca = entity;
+			break;
+		}
+	}
+
+	if (tuner && fe)
+		media_entity_create_link(tuner, 0, fe, 0, 0);
+
+	if (fe && demux)
+		media_entity_create_link(fe, 1, demux, 0, 0);
+
+	if (demux && dvr)
+		media_entity_create_link(demux, 1, dvr, 0, 0);
+
+	if (demux && ca)
+		media_entity_create_link(demux, 1, ca, 0, 0);
+#endif
+}
+EXPORT_SYMBOL_GPL(dvb_create_media_graph);
+
 static int dvbdev_check_free_adapter_num(int num)
 {
 	struct list_head *entry;
diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index 513ca92028dd..df04b55b883a 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -119,6 +119,7 @@ extern int dvb_register_device (struct dvb_adapter *adap,
 				int type);
 
 extern void dvb_unregister_device (struct dvb_device *dvbdev);
+void dvb_create_media_graph(struct media_device *mdev);
 
 extern int dvb_generic_open (struct inode *inode, struct file *file);
 extern int dvb_generic_release (struct inode *inode, struct file *file);
-- 
2.1.0

[PATCH 7/7] cx231xx: create DVB graph

[Mauro Carvalho Chehab]

cx231xx is simple with regards to DVB: all boards have just one
DVB adapter. So, we can use the default DVB helper function to
create the DVB media graph.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 1c0d082fc4ef..3b37324c7613 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -538,6 +538,7 @@ static int register_dvb(struct cx231xx_dvb *dvb,
 	/* register network adapter */
 	dvb->net.mdev = dev->media_dev;
 	dvb_net_init(&dvb->adapter, &dvb->net, &dvb->demux.dmx);
+	dvb_create_media_graph(dev->media_dev);
 	return 0;
 
 fail_fe_conn:
-- 
2.1.0

Re: [PATCH 2/7] cx25840: fill the media controller entity

[Joe Perches]

On Sat, 2015-01-03 at 18:09 -0200, Mauro Carvalho Chehab wrote:
> Instead of keeping the media controller entity not initialized,
> fill it and create the pads for cx25840.

Won't you get an unused variable for ret
without CONFIG_MEDIA_CONTROLLER?

> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
[]
> @@ -5134,7 +5134,7 @@ static int cx25840_probe(struct i2c_client *client,
>  {
>  	struct cx25840_state *state;
>  	struct v4l2_subdev *sd;
> -	int default_volume;
> +	int default_volume, ret;
>  	u32 id;
>  	u16 device_id;
>  
> @@ -5178,6 +5178,18 @@ static int cx25840_probe(struct i2c_client *client,
>  
>  	sd = &state->sd;
>  	v4l2_i2c_subdev_init(sd, client, &cx25840_ops);
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	/* TODO: need to represent analog inputs too */
> +	state->pads[0].flags = MEDIA_PAD_FL_SINK;	/* Tuner or input */
> +	state->pads[1].flags = MEDIA_PAD_FL_SOURCE;	/* Video */
> +	state->pads[2].flags = MEDIA_PAD_FL_SOURCE;	/* VBI */
> +	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +
> +	ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
> +				state->pads, 0);
> +	if (ret < 0)
> +		v4l_info(client, "failed to initialize media entity!\n");
> +#endif
>  
>  	switch (id) {
>  	case CX23885_AV:

Maybe write this without ret at all.

	if (media_entry_init(...) < 0)
		v4l_info(...)

Re: [PATCH 1/7] tuner-core: properly initialize media controller subdev

[Sakari Ailus]

Hi Mauro,

Mauro Carvalho Chehab wrote:
> Properly initialize tuner core subdev at the media controller.
>
> That requires a new subtype at the media controller API.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> index 559f8372e2eb..114715ed0110 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -134,6 +134,9 @@ struct tuner {
>   	unsigned int        type; /* chip type id */
>   	void                *config;
>   	const char          *name;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	struct media_pad	pad;
> +#endif
>   };
>
>   /*
> @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int type,
>   		t->name = analog_ops->info.name;
>   	}
>
> +	t->sd.entity.name = t->name;
> +
>   	tuner_dbg("type set to %s\n", t->name);
>
>   	t->mode_mask = new_mode_mask;
> @@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
>   	struct tuner *t;
>   	struct tuner *radio;
>   	struct tuner *tv;
> +	int ret;

This will emit a compiler warning if CONFIG_MEDIA_CONTROLLER isn't defined.

>   	t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
>   	if (NULL == t)
> @@ -696,6 +702,15 @@ register_client:
>   		   t->type,
>   		   t->mode_mask & T_RADIO ? " Radio" : "",
>   		   t->mode_mask & T_ANALOG_TV ? " TV" : "");
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	t->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> +	t->sd.entity.name = t->name;
> +
> +	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> +	if (ret < 0)
> +		tuner_err("failed to initialize media entity!\n");

I might return the error back to the caller. The failing initialisation 
of a media entity itself might not be a fatal problem, but someone later 
assuming it has been initialised might be.

> +#endif
>   	return 0;
>   }
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 707db275f92b..5ffde035789b 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -66,6 +66,8 @@ struct media_device_info {
>   /* A converter of analogue video to its digital representation. */
>   #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
>
> +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
> +
>   #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
>
>   struct media_entity_desc {
>

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 1/7] tuner-core: properly initialize media controller subdev

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Saturday 03 January 2015 18:09:33 Mauro Carvalho Chehab wrote:
> Properly initialize tuner core subdev at the media controller.
> 
> That requires a new subtype at the media controller API.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/v4l2-core/tuner-core.c
> b/drivers/media/v4l2-core/tuner-core.c index 559f8372e2eb..114715ed0110
> 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -134,6 +134,9 @@ struct tuner {
>  	unsigned int        type; /* chip type id */
>  	void                *config;
>  	const char          *name;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	struct media_pad	pad;
> +#endif

I'm not too familiar with tuners, do they all have a single output only and no 
input ?

>  };
> 
>  /*
> @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int
> type, t->name = analog_ops->info.name;
>  	}
> 
> +	t->sd.entity.name = t->name;
> +

Entity information is not supposed to change at runtime, I'm not sure to be 
comfortable with this change.

set_type() is called at probe time and in tuner_s_type_addr(). The former just 
duplicates the name initialization in tuner_probe(), so isn't really needed. 
The later bothers me.

>  	tuner_dbg("type set to %s\n", t->name);
> 
>  	t->mode_mask = new_mode_mask;
> @@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
>  	struct tuner *t;
>  	struct tuner *radio;
>  	struct tuner *tv;
> +	int ret;
> 
>  	t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
>  	if (NULL == t)
> @@ -696,6 +702,15 @@ register_client:
>  		   t->type,
>  		   t->mode_mask & T_RADIO ? " Radio" : "",
>  		   t->mode_mask & T_ANALOG_TV ? " TV" : "");
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	t->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> +	t->sd.entity.name = t->name;

v4l2_subdev_init(), called by v4l2_i2c_subdev_init(), sets sd->entity.name to 
point to sd->name. Is there a reason why the subdev name can't be used as the 
entity name ?

> +
> +	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> +	if (ret < 0)
> +		tuner_err("failed to initialize media entity!\n");
> +#endif
>  	return 0;
>  }
> 
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 707db275f92b..5ffde035789b 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -66,6 +66,8 @@ struct media_device_info {
>  /* A converter of analogue video to its digital representation. */
>  #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
> 
> +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
> +
>  #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
> 
>  struct media_entity_desc {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/7] tuner-core: properly initialize media controller subdev

[Mauro Carvalho Chehab]

Em Sun, 11 Jan 2015 16:02:41 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Saturday 03 January 2015 18:09:33 Mauro Carvalho Chehab wrote:
> > Properly initialize tuner core subdev at the media controller.
> > 
> > That requires a new subtype at the media controller API.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/drivers/media/v4l2-core/tuner-core.c
> > b/drivers/media/v4l2-core/tuner-core.c index 559f8372e2eb..114715ed0110
> > 100644
> > --- a/drivers/media/v4l2-core/tuner-core.c
> > +++ b/drivers/media/v4l2-core/tuner-core.c
> > @@ -134,6 +134,9 @@ struct tuner {
> >  	unsigned int        type; /* chip type id */
> >  	void                *config;
> >  	const char          *name;
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +	struct media_pad	pad;
> > +#endif
> 
> I'm not too familiar with tuners, do they all have a single output only and no 
> input ?

They have an input: the antenna connector. However, I don't see any need
to map it for most tuners, as there's generally just one input, hardwired
into the tuner chip.

There are some hardware with 2 antenna connectors, but for different
functions (FM and TV). They're selected automatically when the V4L2
driver switches between FM and TV.

In any case, the tuner-core doesn't provide any way to select the
antenna input.

So, if a driver would need to select the input, it would either need
to not use tuner-core or some patch will be required to add such
functionality inside tuner-core.

> >  };
> > 
> >  /*
> > @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int
> > type, t->name = analog_ops->info.name;
> >  	}
> > 
> > +	t->sd.entity.name = t->name;
> > +
> 
> Entity information is not supposed to change at runtime, I'm not sure to be 
> comfortable with this change.
> 
> set_type() is called at probe time and in tuner_s_type_addr(). The former just 
> duplicates the name initialization in tuner_probe(), so isn't really needed. 
> The later bothers me.

The tuner-core driver is a "core subdev" implementation. It handles
the ioctl logic, but the actual driver is a different one. It also
have internally a probe logic that will load the correct tuner subdev.

The tuner_s_type_addr() callback, used only at bridge probing time,
is a way for the bridge driver to provide the name of the tuner driver
that should be loaded, plus its I2C address.

So, once the board is probed, the name shouldn't change.

> 
> >  	tuner_dbg("type set to %s\n", t->name);
> > 
> >  	t->mode_mask = new_mode_mask;
> > @@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
> >  	struct tuner *t;
> >  	struct tuner *radio;
> >  	struct tuner *tv;
> > +	int ret;
> > 
> >  	t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
> >  	if (NULL == t)
> > @@ -696,6 +702,15 @@ register_client:
> >  		   t->type,
> >  		   t->mode_mask & T_RADIO ? " Radio" : "",
> >  		   t->mode_mask & T_ANALOG_TV ? " TV" : "");
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +	t->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> > +	t->sd.entity.name = t->name;
> 
> v4l2_subdev_init(), called by v4l2_i2c_subdev_init(), sets sd->entity.name to 
> point to sd->name. Is there a reason why the subdev name can't be used as the 
> entity name ?

If we don't set entity.name to t->name, the sd->name will be "tuner-core",
instead of the name of the real subdev.

> > +
> > +	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> > +	if (ret < 0)
> > +		tuner_err("failed to initialize media entity!\n");
> > +#endif
> >  	return 0;
> >  }
> > 
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index 707db275f92b..5ffde035789b 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -66,6 +66,8 @@ struct media_device_info {
> >  /* A converter of analogue video to its digital representation. */
> >  #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
> > 
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
> > +
> >  #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
> > 
> >  struct media_entity_desc {
> 

Re: [PATCH 1/7] tuner-core: properly initialize media controller subdev

[Antti Palosaari]

On 01/11/2015 04:25 PM, Mauro Carvalho Chehab wrote:
> Em Sun, 11 Jan 2015 16:02:41 +0200
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

>> I'm not too familiar with tuners, do they all have a single output only and no
>> input ?
>
> They have an input: the antenna connector. However, I don't see any need
> to map it for most tuners, as there's generally just one input, hardwired
> into the tuner chip.
>
> There are some hardware with 2 antenna connectors, but for different
> functions (FM and TV). They're selected automatically when the V4L2
> driver switches between FM and TV.
>
> In any case, the tuner-core doesn't provide any way to select the
> antenna input.
>
> So, if a driver would need to select the input, it would either need
> to not use tuner-core or some patch will be required to add such
> functionality inside tuner-core.

Tuner has antenna as a input and output is intermediate frequency or 
baseband (IF/BB (zero-IF)).

I think most modern silicon tuners actually has more than one physical 
antenna inputs - but those are left unused or same physical antenna 
connector is wired to all those inputs.

Sooner or later there will be receiver having multiple antenna 
connectors which are selectable by software. So let it be at least 
option easy to add later.

regards
Antti

-- 
http://palosaari.fi/