Hi Tomi, On Thu, Feb 17, 2022 at 06:19:15AM +0200, Tomi Valkeinen wrote: > On 16/02/2022 23:30, Laurent Pinchart wrote: > > On Wed, Feb 16, 2022 at 03:00:48PM +0200, Tomi Valkeinen wrote: > >> Add v4l2_subdev_call_state_active() macro to help calling subdev ops > >> that take a subdev state as a parameter. > > > > We should not encourage subdev drivers to call into each other. There > > may be some valid use cases for such a helper at this point, namely in > > the .start_streaming() implementation in a vb2 queue, but even then, I > > think it would be best to rework the pipeline start API to lock the > > states of all subdevs in the pipeline (I've already improved pipeline > > start on top of your streams series, the locking isn't there yet, but I > > could give it a try). > > > > On the other hand, this macro could help identifying drivers that handle > > locking manually, helping reworking them once the pipeline start API > > handles locking too. > > I use this in the cal-video.c to implement the legacy non-MC support. > That is a bit special case, and I could do that with a CAL internal > helper. So I'm fine with dropping this if there won't be other users. Ah yes, for legacy support we indeed need something. There are likely other drivers that will need this then (why did we write legacy code in the first place, really ? :-D). We could keep the macro, but we should then document it as not being for non-legacy use cases, and not for new drivers. It would be even nicer if we could enforce that at runtime. Actually, thinking more about this, how about moving this macro to a new v4l2-subdev-legacy.h header ? -- Regards, Laurent Pinchart
