Hi Kieran, On Thu, May 18, 2017 at 01:04:55PM +0100, Kieran Bingham wrote: > Hi Sakari, > > >>> In omap3isp/isp.c: isp_fwnodes_parse() the async notifier is registered looking > >>> at the endpoints parent fwnode: > >>> > >>> isd->asd.match.fwnode.fwnode = > >>> fwnode_graph_get_remote_port_parent(fwnode); > >>> > >>> This would therefore not support ADV748x ... (it wouldn't know which TX/CSI > >>> entity to match against) > >>> > >>> So the way I see it is that all devices which currently register an async > >>> notifier should now register the match against the endpoint instead of the > >>> parent device: > >>> > >>> - isd->asd.match.fwnode.fwnode = fwnode_graph_get_remote_port_parent(fwnode); > >>> + isd->asd.match.fwnode.fwnode = fwnode; > >>> > >>> And then if we adapt the match to check against: > >>> fwnode == fwnode || fwnode == fwnode_graph_get_remote_port_parent(fwnode); > >> > >> That's not enough as a master driver may use device node whereas the > >> sub-device driver uses endpoint node. You need to do it both ways. > >> > > I've worked through this and I'm convinced that it should not be both ways... > > We are matching a Subdevice (SD) to an Async Subdevice Notifier (ASD) > > Bringing in 'endpoints' gives us the following match combinations > > > SD ASD : Result > ep ep : Match ... Subdevices can specify their endpoint node. > Notifiers can be updated to also specify the (remote) endpoint. > > dev ep : Match - We should take the parent of the ASD to match SD, > to support subdevices which default to their device node. > > dev dev : Match - This is the current case for all other drivers > but Notifier ASDs can be converted to EP's > > ep dev : No Match - We should *not* take the parent of the SD > If a subdevice has specified it's endpoint, (ADV748x) > Matching against the parent device is invalid. Is it? You could argue that all CSI-2 receiver drivers that the ADV748x is used need to be converted, but I suppose that other sub-device drivers will gradually get converted as well whilst not all CSI-2 receiver drivers could be converted yet. I guess it would be also possible to perform a mass conversion but the problem in those is always that you can't test all the affected hardware anyway. > > Finding and matching the parent of the SD would allow a driver to register a > notifier ASD with a dev node, and expect to match a subdevice that has > registered it's subdev with an EP node. This would erroneously allow drivers to > register a notifier that would match against *both* of the ADV748x CSI2 entities. You'd first have to have a board that has the two devices, and then ignore what the drivers are doing. :-) I guess an error would be encountered at some point in driver initialisation. Anyway, I don't think this situation should be a lasting one, and that endpoint matching is the way to go. > > I have posted an implementation of this as: > [PATCH v1 3/3] v4l: async: Match parent devices [0] > > I believe this to be correct - but I'm aware that I'm only really considering > the OF side here... Please let me know if there's something I'm not taking into > account. -- Regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
