Re: [PATCH v5 2/8] media: v4l2-fwnode: Add common helper library for 1-to-1 subdev registration

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

Hi Frank,

On Wed, Jun 17, 2026 at 11:13:55PM -0500, Frank Li wrote:
> On Thu, Jun 18, 2026 at 01:36:20AM +0300, Sakari Ailus wrote:
> > On Wed, Jun 17, 2026 at 03:50:12PM -0400, Frank.Li@oss.nxp.com wrote:
> > > From: Frank Li <Frank.Li@nxp.com>
> > >
> > > Many V4L2 subdev drivers implement the same registration and media pad
> > > setup logic for simple pipelines consisting of a single sink pad and a
> > > single source pad. As a result, the same boilerplate code is duplicated
> > > across multiple drivers.
> > >
> > > Introduce a common helper library for 1-to-1 subdevs to encapsulate the
> > > registration, media entity initialization, and cleanup paths. Drivers
> > > can embed a struct v4l2_subdev_1to1 instance and use the provided helper
> > > APIs instead of open-coding the setup sequence.
> >
> > I appreciate your efforts in trying to reduce the amount of code drivers
> > need simply to get things done but I think there are a few issues with the
> > approach taken in this patch:
> >
> > - The new helpers aren't generic enough, but require two pads; one sink,
> >   one source.
> 
> It can cover many case already, there are many bridge type subdev. after
> glace of all code, many CSI2RX is type device. It should one kind important
> type/case, like sensors.
> 
> And I plan do 1 TO N replicator driver, which duplicate 1 sink pad to N
> source pad (with/without register config), plus exist video-mux driver,
> 
> It think It can cover more than 80% cases.
> 
> > You could provide special helpers for just this case, but
> >   right now it looks like that if there's something you need that the
> >   helper assumes you don't, you can't use the helper at all. In other
> >   words, more modularity would be nice.
> 
> We can add it later if need, which easy to replace 1to1 API, like I did
> for sensor one.
> 
> > - The new helper should work with the existing types and not add new types
> >   (struct v4l2_subdev_1to1).
> 
> May be save vep data into v4l2_subdev to avoid parse it every time. and
> enhence media_entity_pads_init() to avoid refer caller data.

I agree with Sakari about not introducing a new structure.

We could create a version of media_entity_pads_init() that allocates the
pads array dynamically (*not* with a devm_* function !), and free it in
media_entity_cleanup().

> >
> > - There should be a way to provide default V4L2 fwnode endpoint
> >   configuration as well as to validate the obtained configuration.
> 
> Do you means remote_bustype_cap_mask information get from a callback?
> 
> > I don't have a good proposal to address the above but at least one way I
> > can think of making error handling easier would be to use devm_() for
> > teardown in more places we to today. That certainly does have its own
> > issues though.
> 
> I tried it before, media and v4l2's clean up is not revised order of init.
> Sorry, I can't find original thread. I remember laurnet pinchart said there
> are order problem.
> 
> 1  v4l2_subdev_init()
> 2. v4l2_async_subdev_nf_init()
> 3. v4l2_async_nf_register()
> 4. media_entity_pads_init()
> 5  v4l2_async_register_subdev()
> 
> 
> v4l2_async_unregister_subdev(sd);
> v4l2_subdev_cleanup(sd);        // Not sure if it save to move to last step
> media_entity_cleanup(&sd->entity);
> v4l2_async_nf_unregister(&csi2->notifier);
> v4l2_async_nf_cleanup(&csi2->notifier);

The cleanup procedure is really bad, it's a known issue. Fixing that
involves resuming Sakari's work on life time management in V4L2.

-- 
Regards,

Laurent Pinchart