Hi,
On Tue, 2016-12-13 at 12:39 -0300, Javier Martinez Canillas wrote:
> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> added input signals support for the tvp5150, but the approach was found
> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> ("[media] tvp5150: document input connectors DT bindings") was reverted.
>
> This left the driver with an undocumented (and wrong) DT parsing logic,
> so lets get rid of this code as well until the input connectors support
> is implemented properly.
>
> It's a partial revert due other patches added on top of mentioned commit
> not allowing the commit to be reverted cleanly anymore. But all the code
> related to the DT parsing logic and input entities creation are removed.
>
> > Suggested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
what is the state of this patch? Was it forgotten or was the revert
deemed unnecessary?
regards
Philipp
> ---
>
> Hello Laurent,
>
> I've tested this patch on top of media/master on my IGEPv2 + tvp5150
> with the following:
>
> $ media-ctl -r -l '"tvp5150 1-005c":1->"OMAP3 ISP CCDC":0[1], "OMAP3 ISP CCDC":1->"OMAP3 ISP CCDC output":0[1]'
> $ media-ctl -v --set-format '"OMAP3 ISP CCDC":0 [UYVY2X8 720x240 field:alternate]'
> $ media-ctl -v --set-format '"OMAP3 ISP CCDC":1 [UYVY2X8 720x240 field:interlaced-tb]'
> $ yavta -f UYVY -s 720x480 -n 1 --field interlaced-tb --capture=1 -F /dev/video2
> $ raw2rgbpnm -f UYVY -s 720x480 frame-000000.bin frame.pnm
>
> I've also tested the other composite input with the following change:
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 5fe5faefe212..973be68ff78c 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1371,7 +1371,7 @@ static int tvp5150_probe(struct i2c_client *c,
> return res;
>
> core->norm = V4L2_STD_ALL; /* Default is autodetect */
> - core->input = TVP5150_COMPOSITE1;
> + core->input = TVP5150_COMPOSITE0;
> core->enable = true;
>
> v4l2_ctrl_handler_init(&core->hdl, 5);
>
> But as mentioned, it also worked for me without the revert so please let
> me know if the driver works with your omap3 board.
>
> Best regards,
> Javier
>
> drivers/media/i2c/tvp5150.c | 142 --------------------------------------------
> 1 file changed, 142 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 48646a7f3fb0..5fe5faefe212 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -42,8 +42,6 @@ struct tvp5150 {
> > struct v4l2_subdev sd;
> #ifdef CONFIG_MEDIA_CONTROLLER
> > struct media_pad pads[DEMOD_NUM_PADS];
> > - struct media_entity input_ent[TVP5150_INPUT_NUM];
> > - struct media_pad input_pad[TVP5150_INPUT_NUM];
> #endif
> > struct v4l2_ctrl_handler hdl;
> > struct v4l2_rect rect;
> @@ -1018,40 +1016,6 @@ static int tvp5150_enum_frame_size(struct v4l2_subdev *sd,
> }
>
> /****************************************************************************
> > - Media entity ops
> - ****************************************************************************/
> -
> -#ifdef CONFIG_MEDIA_CONTROLLER
> -static int tvp5150_link_setup(struct media_entity *entity,
> > - const struct media_pad *local,
> > - const struct media_pad *remote, u32 flags)
> -{
> > - struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> > - struct tvp5150 *decoder = to_tvp5150(sd);
> > - int i;
> -
> > - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> > - if (remote->entity == &decoder->input_ent[i])
> > - break;
> > - }
> -
> > - /* Do nothing for entities that are not input connectors */
> > - if (i == TVP5150_INPUT_NUM)
> > - return 0;
> -
> > - decoder->input = i;
> -
> > - tvp5150_selmux(sd);
> -
> > - return 0;
> -}
> -
> -static const struct media_entity_operations tvp5150_sd_media_ops = {
> > - .link_setup = tvp5150_link_setup,
> -};
> -#endif
> -
> -/****************************************************************************
> > I2C Command
> ****************************************************************************/
>
> @@ -1188,42 +1152,6 @@ static int tvp5150_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
> > return 0;
> }
>
> -static int tvp5150_registered(struct v4l2_subdev *sd)
> -{
> -#ifdef CONFIG_MEDIA_CONTROLLER
> > - struct tvp5150 *decoder = to_tvp5150(sd);
> > - int ret = 0;
> > - int i;
> -
> > - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> > - struct media_entity *input = &decoder->input_ent[i];
> > - struct media_pad *pad = &decoder->input_pad[i];
> -
> > - if (!input->name)
> > - continue;
> -
> > - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE;
> -
> > - ret = media_entity_pads_init(input, 1, pad);
> > - if (ret < 0)
> > - return ret;
> -
> > - ret = media_device_register_entity(sd->v4l2_dev->mdev, input);
> > - if (ret < 0)
> > - return ret;
> -
> > - ret = media_create_pad_link(input, 0, &sd->entity,
> > - DEMOD_PAD_IF_INPUT, 0);
> > - if (ret < 0) {
> > - media_device_unregister_entity(input);
> > - return ret;
> > - }
> > - }
> -#endif
> -
> > - return 0;
> -}
> -
> /* ----------------------------------------------------------------------- */
>
> static const struct v4l2_ctrl_ops tvp5150_ctrl_ops = {
> @@ -1274,11 +1202,6 @@ static const struct v4l2_subdev_ops tvp5150_ops = {
> > .pad = &tvp5150_pad_ops,
> };
>
> -static const struct v4l2_subdev_internal_ops tvp5150_internal_ops = {
> > - .registered = tvp5150_registered,
> -};
> -
> -
> /****************************************************************************
> > I2C Client & Driver
> ****************************************************************************/
> @@ -1360,12 +1283,6 @@ static int tvp5150_parse_dt(struct tvp5150 *decoder, struct device_node *np)
> {
> > struct v4l2_of_endpoint bus_cfg;
> > struct device_node *ep;
> -#ifdef CONFIG_MEDIA_CONTROLLER
> > - struct device_node *connectors, *child;
> > - struct media_entity *input;
> > - const char *name;
> > - u32 input_type;
> -#endif
> > unsigned int flags;
> > int ret = 0;
>
> @@ -1389,63 +1306,6 @@ static int tvp5150_parse_dt(struct tvp5150 *decoder, struct device_node *np)
>
> > decoder->mbus_type = bus_cfg.bus_type;
>
> -#ifdef CONFIG_MEDIA_CONTROLLER
> > - connectors = of_get_child_by_name(np, "connectors");
> -
> > - if (!connectors)
> > - goto err;
> -
> > - for_each_available_child_of_node(connectors, child) {
> > - ret = of_property_read_u32(child, "input", &input_type);
> > - if (ret) {
> > - dev_err(decoder->sd.dev,
> > - "missing type property in node %s\n",
> > - child->name);
> > - goto err_connector;
> > - }
> -
> > - if (input_type >= TVP5150_INPUT_NUM) {
> > - ret = -EINVAL;
> > - goto err_connector;
> > - }
> -
> > - input = &decoder->input_ent[input_type];
> -
> > - /* Each input connector can only be defined once */
> > - if (input->name) {
> > - dev_err(decoder->sd.dev,
> > - "input %s with same type already exists\n",
> > - input->name);
> > - ret = -EINVAL;
> > - goto err_connector;
> > - }
> -
> > - switch (input_type) {
> > - case TVP5150_COMPOSITE0:
> > - case TVP5150_COMPOSITE1:
> > - input->function = MEDIA_ENT_F_CONN_COMPOSITE;
> > - break;
> > - case TVP5150_SVIDEO:
> > - input->function = MEDIA_ENT_F_CONN_SVIDEO;
> > - break;
> > - }
> -
> > - input->flags = MEDIA_ENT_FL_CONNECTOR;
> -
> > - ret = of_property_read_string(child, "label", &name);
> > - if (ret < 0) {
> > - dev_err(decoder->sd.dev,
> > - "missing label property in node %s\n",
> > - child->name);
> > - goto err_connector;
> > - }
> -
> > - input->name = name;
> > - }
> -
> -err_connector:
> > - of_node_put(connectors);
> -#endif
> err:
> > of_node_put(ep);
> > return ret;
> @@ -1491,7 +1351,6 @@ static int tvp5150_probe(struct i2c_client *c,
> > }
>
> > v4l2_i2c_subdev_init(sd, c, &tvp5150_ops);
> > - sd->internal_ops = &tvp5150_internal_ops;
> > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> @@ -1505,7 +1364,6 @@ static int tvp5150_probe(struct i2c_client *c,
> > if (res < 0)
> > return res;
>
> > - sd->entity.ops = &tvp5150_sd_media_ops;
> #endif
>
> > res = tvp5150_detect_version(core);
