Re: [PATCH v7 11/14] media: v4l2-subdev: Add v4l2_subdev_call_ci_state_{active,try}

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Aug 11, 2026 at 10:20:32AM +0300, Sakari Ailus wrote:
> On Mon, Aug 10, 2026 at 06:32:07PM +0300, Laurent Pinchart wrote:
> > On Mon, Aug 10, 2026 at 04:12:31PM +0200, Hans Verkuil wrote:
> > > On 07/08/2026 14:24, Sakari Ailus wrote:
> > > > Add v4l2_subdev_call_ci_state_active(), and
> > > > v4l2_subdev_call_ci_state_try() to call sub-device pad ops that
> > > > take struct v4l2_subdev_client_info pointer as an argument. These ops
> > > > cannot be called using v4l2_subdev_call_state_active() or
> > > > v4l2_subdev_call_state_try() as the client_info argument precedes the
> > > > state argument.
> > > 
> > > So if we have to jump through all these hoops just because the client_info
> > > pointer precedes the state pointer in the pad op argument list, wouldn't it
> > > be better to swap the order? For example by moving the client_info pointer
> > > as the last argument?
> > > 
> > > Honestly, these macros are getting really hard to follow, and I'm not sure
> > > it is worth it just to keep the client_info before the state pointer. Yes, that's
> > > the logical order, but at the price of some very hard to read defines.
> > > 
> > > Or am I missing something?
> > 
> > Those are exactly the points I raised in the review of v6 :-)
> 
> And my answer then was that I prefer some additional complexity on the
> framework side -- where we have a single implementation of this -- over
> pushing less than ideal APIs to all the drivers.

I'm not convinced, but I won't make that a blocker if it's only me.

> To give some idea, we currently have about 200 drivers implementing the
> set_fmt() pad op.

Does the order of arguments really matter for drivers implementing those
operations ?

> I also expect the number of pad ops where we need client
> info will grow over time, making this quite a bit more visible in the
> drivers than the scope (set_fmt, get_selection and set_selection)
> introduced in this set.

-- 
Regards,

Laurent Pinchart