Re: [PATCH 03/18] media: v4l: async: Simplify async sub-device fwnode matching

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Niklas,

On Mon, Apr 24, 2023 at 09:20:22PM +0200, Niklas Söderlund wrote:
> Hi Sakari,
> 
> On 2023-04-14 14:07:40 +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> > 
> > On Thu, Apr 13, 2023 at 06:50:04PM +0200, Jacopo Mondi wrote:
> > > Hi Sakari
> > > 
> > > On Thu, Mar 30, 2023 at 02:58:38PM +0300, Sakari Ailus wrote:
> > > > V4L2 async sub-device matching originally used the device nodes only.
> > > > Endpoint nodes were taken into use instead as using the device nodes was
> > > > problematic for it was in some cases ambiguous which link might have been
> > > > in question.
> > > >
> > > > There is however no need to use endpoint nodes on both sides, as the async
> > > > sub-device's fwnode can always be trivially obtained using
> > > > fwnode_graph_get_remote_endpoint() when needed while what counts is
> > > > whether or not the link is between two device nodes, i.e. the device nodes
> > > > match.
> > > >
> > > 
> > > As you know I'm a bit debated.
> > > 
> > > Strict endpoint-matching requires one subdev to be registed per each
> > > endpoint, and this is tedious for drivers that have to register a
> > > subdev for each of its endpoints
> > > 
> > > Allowing a subdev to be matched multiple times on different endpoints
> > > gives a way for lazy drivers to take a shortcut and simplify their
> > > topologies to a single subdev, when they would actually need more.
> > 
> > I'd say this is really about interface design, not being "lazy". It depends
> > on the sub-device. Ideally the framework should be also as easy for drivers
> > drivers to use as possible.
> > 
> > What is not supported, though, is multiple sub-devices with a single device
> > node. Do we need that? At least I don't think I came across a driver that
> > would.
> 
> If I understand you correctly about multiple sub-device from a single 
> device node, this exists today. The ADV748x driver have a single device 
> node in DT and register multiple sub-devices, one for each source 
> endpoint.
> 
> The ADV748x have two CSI-2 transmitters, one 4-lane and one 1-lane as 
> well as two different video capture "ports" one HDMI and one CVBS. Both 
> capture ports can be active at the same time and routed internally 
> inside the ADV748x to the two different CSI-2 transmitters.
> 
> In order todo this the ADV748x register multiple subdevices and modifies 
> the fwnode to be the endpoint instead of the device node. So the change 
> in this patch for ADV748x driver would break that driver.

Ah, indeed. I guess I'll need to support that case as well then. It doesn't
seem to be troublesome to implement, but I'm tempted making it a special
case: every other driver would apparently be fine matching with device
fwnode whereas doing endpoint-to-endpoint matching adds complexity to the
drivers. This patch removes about 100 lines of rather ugly code largely
from v4l2-async.

There are other issues in the set with connection-subdev relations, I'll
post v2 to address those as well.

-- 
Kind regards,

Sakari Ailus