Messages in this thread | | | Date | Fri, 23 Sep 2022 12:48:54 +0000 | From | Sakari Ailus <> | Subject | Re: [PATCH v3 4/4] media: tc358746: add Toshiba TC358746 Parallel to CSI-2 bridge driver |
| |
Hi Marco,
Thanks for the update. A few small things below.
On Thu, Sep 22, 2022 at 03:48:43PM +0200, Marco Felsch wrote: ... > +static int tc358746_apply_misc_config(struct tc358746 *tc358746) > +{ > + const struct v4l2_mbus_framefmt *mbusfmt; > + struct v4l2_subdev *sd = &tc358746->sd; > + struct v4l2_subdev_state *sink_state; > + const struct tc358746_format *fmt; > + struct device *dev = sd->dev; > + u32 val; > + int err; > + > + sink_state = v4l2_subdev_lock_and_get_active_state(sd); > + mbusfmt = v4l2_subdev_get_pad_format(sd, sink_state, TC358746_SINK); > + v4l2_subdev_unlock_state(sink_state); > + > + fmt = tc358746_get_format_by_code(TC358746_SINK, mbusfmt->code);
You can no longer access mbusfmt here as you've already unlocked subdev state.
> + > + /* Self defined CSI user data type id's are not supported yet */ > + val = PDFMT(fmt->pdformat); > + dev_dbg(dev, "DATAFMT: 0x%x\n", val); > + err = tc358746_write(tc358746, DATAFMT_REG, val); > + if (err) > + return err; > + > + val = PDATAF(fmt->pdataf); > + dev_dbg(dev, "CONFCTL[PDATAF]: 0x%x\n", fmt->pdataf); > + err = tc358746_update_bits(tc358746, CONFCTL_REG, PDATAF_MASK, val); > + if (err) > + return err; > + > + val = tc358746->vb_size / 32; > + dev_dbg(dev, "FIFOCTL: %u (0x%x)\n", val, val); > + err = tc358746_write(tc358746, FIFOCTL_REG, val); > + if (err) > + return err; > + > + /* Total number of bytes for each line/width */ > + val = mbusfmt->width * fmt->bpp / 8; > + dev_dbg(dev, "WORDCNT: %u (0x%x)\n", val, val); > + return tc358746_write(tc358746, WORDCNT_REG, val); > +}
...
> +static int tc358746_s_stream(struct v4l2_subdev *sd, int enable) > +{ > + struct tc358746 *tc358746 = to_tc358746(sd); > + struct v4l2_subdev *src; > + int err; > + > + dev_dbg(sd->dev, "%sable\n", enable ? "en" : "dis"); > + > + src = tc358746_get_remote_sd(&tc358746->pads[TC358746_SINK]); > + if (!src) > + return -EPIPE; > + > + if (enable) { > + err = pm_runtime_resume_and_get(sd->dev); > + if (err) > + return err; > + > + err = tc358746_apply_dphy_config(tc358746); > + if (err) > + goto err_out; > + > + err = tc358746_apply_misc_config(tc358746); > + if (err) > + goto err_out; > + > + err = tc358746_enable_csi_lanes(tc358746, 1); > + if (err) > + goto err_out; > + > + err = tc358746_enable_csi_module(tc358746, 1); > + if (err) > + goto err_out; > + > + err = tc358746_enable_parallel_port(tc358746, 1); > + if (err) > + goto err_out; > + > + err = v4l2_subdev_call(src, video, s_stream, 1); > + if (err) > + goto err_out; > + > + return err;
This can be return 0.
> + > +err_out: > + pm_runtime_mark_last_busy(sd->dev); > + pm_runtime_put_sync_autosuspend(sd->dev); > + > + return err; > + }
-- Kind regards,
Sakari Ailus
| |