| Message ID | f312c9563615f8f5666c1621a20d3fa07831ae89.1420578087.git.mchehab@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Mauro, Thanks for the patch. On Tue, Jan 6, 2015 at 9:08 PM, Mauro Carvalho Chehab <mchehab@osg.samsung.com> wrote: > 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..bdb5bb6b58da 100644 > --- a/drivers/media/i2c/cx25840/cx25840-core.c > +++ b/drivers/media/i2c/cx25840/cx25840-core.c > @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client, > int default_volume; > u32 id; > u16 device_id; > +#if defined(CONFIG_MEDIA_CONTROLLER) > + int ret; > +#endif > > /* Check if the adapter supports the needed features */ > if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > @@ -5178,6 +5181,21 @@ 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 */ | MEDIA_PAD_FL_MUST_CONNECT ? > + state->pads[1].flags = MEDIA_PAD_FL_SOURCE; /* Video */ > + state->pads[2].flags = MEDIA_PAD_FL_SOURCE; /* VBI */ Macros for 0,1,2, and 3 would make it more readable. > + 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"); > + kfree(state); Not required. > + return -ENODEV; return ret; instead ? Thanks, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Prabhakar, Em Wed, 7 Jan 2015 11:44:08 +0000 Prabhakar Lad <prabhakar.csengg@gmail.com> escreveu: > Hi Mauro, > > Thanks for the patch. Thanks for review. > On Tue, Jan 6, 2015 at 9:08 PM, Mauro Carvalho Chehab > <mchehab@osg.samsung.com> wrote: > > 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..bdb5bb6b58da 100644 > > --- a/drivers/media/i2c/cx25840/cx25840-core.c > > +++ b/drivers/media/i2c/cx25840/cx25840-core.c > > @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client, > > int default_volume; > > u32 id; > > u16 device_id; > > +#if defined(CONFIG_MEDIA_CONTROLLER) > > + int ret; > > +#endif > > > > /* Check if the adapter supports the needed features */ > > if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > > @@ -5178,6 +5181,21 @@ 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 */ > > | MEDIA_PAD_FL_MUST_CONNECT ? Maybe. Currently, only omap3 uses this flag. Not sure what difference it will make. > > + state->pads[1].flags = MEDIA_PAD_FL_SOURCE; /* Video */ > > + state->pads[2].flags = MEDIA_PAD_FL_SOURCE; /* VBI */ > > Macros for 0,1,2, and 3 would make it more readable. Yes, but IMHO the best would be to add those macros on some global place, as all analog demod PADs will look the same: one sync to connect to the tuner and two PADs, one for video stream and another one for VBI. A few decoders may have an additional PAD for sliced-VBI. Doing such definition globally helps to support drivers like em28xx, where, depending on the board, there are several different alternatives for the demod, all providing the very same 3 pads. > > + 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"); > > + kfree(state); > Not required. Why not? state were allocated previously in this function. If we don't free it, it will have a memory leak. > > > + return -ENODEV; > return ret; instead ? Yeah, makes sense. I'll fix that on a next spin of the patch. > > Thanks, > --Prabhakar Lad
diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c index 573e08826b9b..bdb5bb6b58da 100644 --- a/drivers/media/i2c/cx25840/cx25840-core.c +++ b/drivers/media/i2c/cx25840/cx25840-core.c @@ -5137,6 +5137,9 @@ static int cx25840_probe(struct i2c_client *client, int default_volume; u32 id; u16 device_id; +#if defined(CONFIG_MEDIA_CONTROLLER) + int ret; +#endif /* Check if the adapter supports the needed features */ if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) @@ -5178,6 +5181,21 @@ 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"); + kfree(state); + return -ENODEV; + } +#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)
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>