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

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Jun 18, 2026 at 01:36:20AM +0300, Sakari Ailus wrote:
> Hi Frank,
>
> Thanks for the patch.

Thanks for you quick reply.

>
> 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.

>
> - 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);

Frank
>
> --
> Kind regards,
>
> Sakari Ailus