From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from galahad.ideasonboard.com ([185.26.127.97]:39157 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751549AbbKWUBP (ORCPT ); Mon, 23 Nov 2015 15:01:15 -0500 From: Laurent Pinchart To: Mauro Carvalho Chehab Cc: Linux Media Mailing List , Hans Verkuil , Rafael =?ISO-8859-1?Q?Louren=E7o?= de Lima Chehab , Shuah Khan , Matthias Schwarzott , Antti Palosaari , Olli Salonen , Tommi Rantala , "Lad, Prabhakar" , Sakari Ailus , Krzysztof Kozlowski Subject: Re: [PATCH 01/18] [media] tuner-core: add an input pad Date: Mon, 23 Nov 2015 22:01:23 +0200 Message-ID: <2280184.FRmr0h2MEz@avalon> In-Reply-To: <7e90c4ecdcdc15ebb3b32ac075168a93a6b63f4f.1441559233.git.mchehab@osg.samsung.com> References: <7e90c4ecdcdc15ebb3b32ac075168a93a6b63f4f.1441559233.git.mchehab@osg.samsung.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-media-owner@vger.kernel.org List-ID: Hi Mauro, Thank you for the patch. On Sunday 06 September 2015 14:30:44 Mauro Carvalho Chehab wrote: > Tuners actually have at least one connector on its > input. > > Add a PAD to connect it. The patch looks fine to me, but have you checked that there are no driver instantiating a tuner that would get confused by this additional pad, for instance in graph traversal code ? Additionally it should be documented somewhere that drivers instantiating tuners are responsible for creating and linking a connector to the tuner input. > Signed-off-by: Mauro Carvalho Chehab > > diff --git a/drivers/media/dvb-core/dvbdev.c > b/drivers/media/dvb-core/dvbdev.c index f00f1a5f279c..a8e7e2398f7a 100644 > --- a/drivers/media/dvb-core/dvbdev.c > +++ b/drivers/media/dvb-core/dvbdev.c > @@ -34,6 +34,9 @@ > #include > #include "dvbdev.h" > > +/* Due to enum tuner_pad_index */ > +#include > + > static DEFINE_MUTEX(dvbdev_mutex); > static int dvbdev_debug; > > @@ -552,7 +555,7 @@ void dvb_create_media_graph(struct dvb_adapter *adap) > } > > if (tuner && demod) > - media_create_pad_link(tuner, 0, demod, 0, 0); > + media_create_pad_link(tuner, TUNER_PAD_IF_OUTPUT, demod, 0, 0); > > if (demod && demux) > media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED); > diff --git a/drivers/media/usb/au0828/au0828-core.c > b/drivers/media/usb/au0828/au0828-core.c index e28cabe65934..f54c7d10f350 > 100644 > --- a/drivers/media/usb/au0828/au0828-core.c > +++ b/drivers/media/usb/au0828/au0828-core.c > @@ -27,6 +27,9 @@ > #include > #include > > +/* Due to enum tuner_pad_index */ > +#include > + > /* > * 1 = General debug messages > * 2 = USB handling > @@ -260,7 +263,7 @@ static void au0828_create_media_graph(struct au0828_dev > *dev) return; > > if (tuner) > - media_create_pad_link(tuner, 0, decoder, 0, > + media_create_pad_link(tuner, TUNER_PAD_IF_OUTPUT, decoder, 0, > MEDIA_LNK_FL_ENABLED); > media_create_pad_link(decoder, 1, &dev->vdev.entity, 0, > MEDIA_LNK_FL_ENABLED); > diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c > b/drivers/media/usb/cx231xx/cx231xx-cards.c index > 3b5c9ae39ad3..1070d87efc65 100644 > --- a/drivers/media/usb/cx231xx/cx231xx-cards.c > +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c > @@ -1264,7 +1264,7 @@ static void cx231xx_create_media_graph(struct cx231xx > *dev) return; > > if (tuner) > - media_create_pad_link(tuner, 0, decoder, 0, > + media_create_pad_link(tuner, TUNER_PAD_IF_OUTPUT, decoder, 0, > MEDIA_LNK_FL_ENABLED); > media_create_pad_link(decoder, 1, &dev->vdev.entity, 0, > MEDIA_LNK_FL_ENABLED); > diff --git a/drivers/media/v4l2-core/tuner-core.c > b/drivers/media/v4l2-core/tuner-core.c index 100b8f069640..b90f2a52db96 > 100644 > --- a/drivers/media/v4l2-core/tuner-core.c > +++ b/drivers/media/v4l2-core/tuner-core.c > @@ -134,8 +134,9 @@ struct tuner { > unsigned int type; /* chip type id */ > void *config; > const char *name; > + > #if defined(CONFIG_MEDIA_CONTROLLER) > - struct media_pad pad; > + struct media_pad pad[TUNER_NUM_PADS]; > #endif > }; > > @@ -695,11 +696,12 @@ static int tuner_probe(struct i2c_client *client, > /* Should be just before return */ > register_client: > #if defined(CONFIG_MEDIA_CONTROLLER) > - t->pad.flags = MEDIA_PAD_FL_SOURCE; > + t->pad[TUNER_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK; > + t->pad[TUNER_PAD_IF_OUTPUT].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); > + ret = media_entity_init(&t->sd.entity, TUNER_NUM_PADS, &t->pad[0]); > if (ret < 0) { > tuner_err("failed to initialize media entity!\n"); > kfree(t); > diff --git a/include/media/tuner.h b/include/media/tuner.h > index b46ebb48fe74..95835c8069dd 100644 > --- a/include/media/tuner.h > +++ b/include/media/tuner.h > @@ -25,6 +25,14 @@ > > #include > > +/* Tuner PADs */ > +/* FIXME: is this the right place for it? */ > +enum tuner_pad_index { > + TUNER_PAD_RF_INPUT, > + TUNER_PAD_IF_OUTPUT, > + TUNER_NUM_PADS > +}; > + > #define ADDR_UNSET (255) > > #define TUNER_TEMIC_PAL 0 /* 4002 FH5 (3X 7756, 9483) */ -- Regards, Laurent Pinchart