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
sakari.ailus@xxxxxxxxxxxxxxx
