Re: [PATCH 03/19] media: adv748x: Use V4L2 streams

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/05/2024 11:31, Laurent Pinchart wrote:

+}
+
+/*
   * We use the internal registered operation to be able to ensure that our
   * incremental subdevices (not connected in the forward path) can be registered
   * against the resulting video path and media device.
@@ -109,6 +132,7 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd)
  }

  static const struct v4l2_subdev_internal_ops adv748x_csi2_internal_ops = {
+	.init_state = adv748x_csi2_init_state,

The .init_state() operation needs to be provided along with the call to
v4l2_subdev_init_finalize() in patch 01/19.

I'll squash, however even if it might be a requirement for having a
fully working implementation, not having init_state() will not lead to
any crash and maybe smaller incremental patches are easier to handle.

	if (sd->internal_ops && sd->internal_ops->init_state) {
		/*
		 * There can be no race at this point, but we lock the state
		 * anyway to satisfy lockdep checks.
		 */
		v4l2_subdev_lock_state(state);
		ret = sd->internal_ops->init_state(sd, state);
		v4l2_subdev_unlock_state(state);

I think it's a mistake in the core to not require .init_state() for
subdevs using the active state. Tomi, what do you think ?

If I'm not mistaken, the v4l2 rules say that a subdev configuration should always be a valid one (for that specific device). To fulfill that, you need .init_state().

So yes, I agree. This is probably one more thing that can be added to the "[PATCH v6 03/11] media: subdev: Add checks for subdev features".

 Tomi





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux