Hi Marco,

On Mon, Aug 13, 2018 at 11:25:02AM +0200, Marco Felsch wrote:
...
> +static void tvp5150_dt_cleanup(struct tvp5150 *decoder)
> +{
> +     unsigned int i;
> +
> +     for (i = 0; i < TVP5150_NUM_PADS; i++)
> +             of_node_put(decoder->endpoints[i]);
> +}
> +
>  static const char * const tvp5150_test_patterns[2] = {
>       "Disabled",
>       "Black screen"
> @@ -1586,7 +1996,7 @@ static int tvp5150_probe(struct i2c_client *c,
>               res = tvp5150_parse_dt(core, np);
>               if (res) {
>                       dev_err(sd->dev, "DT parsing error: %d\n", res);
> -                     return res;
> +                     goto err_cleanup_dt;
>               }
>       } else {
>               /* Default to BT.656 embedded sync */
> @@ -1594,25 +2004,16 @@ 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)
> -     core->pads[TVP5150_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
> -     core->pads[TVP5150_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG;
> -     core->pads[TVP5150_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
> -     core->pads[TVP5150_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV;
> -
> -     sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
> -
> -     res = media_entity_pads_init(&sd->entity, TVP5150_NUM_PADS, core->pads);
> -     if (res < 0)
> -             return res;
> -
> -#endif
> +     res = tvp5150_mc_init(sd);
> +     if (res)
> +             goto err_cleanup_dt;
>  
>       res = tvp5150_detect_version(core);
>       if (res < 0)
> -             return res;
> +             goto err_cleanup_dt;
>  
>       core->norm = V4L2_STD_ALL;      /* Default is autodetect */
>       core->detected_norm = V4L2_STD_UNKNOWN;
> @@ -1664,6 +2065,9 @@ static int tvp5150_probe(struct i2c_client *c,
>  err:

Now that you have more error labels, you could rename this one.

>       v4l2_ctrl_handler_free(&core->hdl);
>       return res;

Is the above line intended to be kept?

> +err_cleanup_dt:
> +     tvp5150_dt_cleanup(core);
> +     return res;
>  }
>  
>  static int tvp5150_remove(struct i2c_client *c)

-- 
Sakari Ailus
[email protected]

Reply via email to