Re: [PATCH] media: v4l2-async: fix binding async subdevs with multiple source ports

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

Hi Philipp,

On Mon, Jan 16, 2023 at 02:46:07PM +0100, Philipp Zabel wrote:
> On Mon, Jan 16, 2023 at 03:08:27AM +0200, Laurent Pinchart wrote:
> [...]
> > To make sure I understand this correctly, you need both changes, with
> > the change in __v4l2_async_nf_has_async_subdev() meant to address asds
> > being added after the subdev has been registered, and the change in
> > v4l2_async_register_subdev() meant to address asds that have been added
> > before ?
> 
> Yes, your understanding is correct.
> 
> > The imx6 ipu drivers implement a "clever hack" to handle the
> > multi-endpoint issue that was never officially supported by v4l2-async.
> 
> That belongs in parentheses all right. I think it might even have been
> accidental.

There are explicit checks in the video-mux driver and in the ipu driver
for -EEXIST, so it was likely done on purpose :-)

> > Obviously, as it has worked so far, leaving it broken isn't a very nice
> > option. The fix feels a bit like a hack though, and a better solution
> > would be to allow subdevs to be matched multiple times, by multiple
> > consumers. That's a more intrusive change though, so I could be OK with
> > this as a short term fix, assuming it doesn't break anything else.
> 
> Right, the intention was that this patch could be applied on stable
> kernels, as I only had noticed the issue after the release.
> 
> > I would however want to ensure this doesn't get abused by new drivers.
> > Could we add a dev_warn() somewhere to indicate that multi-endpoint
> > matching is not supported and shouldn't be used until fixed ? Sakari,
> > what do you think ?
> [...]
> 
> pr_warn_once() in both places, perhaps?

I'm fine with that.

-- 
Regards,

Laurent Pinchart