Hi Sakari,
thanks for the review.
On 18-09-14 16:31, Sakari Ailus wrote:
> 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.
Hm.. okay make sense, I will change this.
>
> > v4l2_ctrl_handler_free(&core->hdl);
> > return res;
>
> Is the above line intended to be kept?
Nope, sorry I will fix this.
Regards,
Marco
>
> > +err_cleanup_dt:
> > + tvp5150_dt_cleanup(core);
> > + return res;
> > }
> >
> > static int tvp5150_remove(struct i2c_client *c)
>
> --
> Sakari Ailus
> sakari.ai...@linux.intel.com
>
