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
