Re: [RFC PATCH v2 1/4] media: i2c: adv748x: add adv748x driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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.

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.

[0] https://www.spinics.net/lists/linux-media/msg115677.html

Regards
--
Kieran




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux