Re: [PATCH v6 00/17] media: imx: Switch to subdev notifiers
Hi Steve, On Mon, Jul 09, 2018 at 03:39:00PM -0700, Steve Longerbeam wrote: > This patchset converts the imx-media driver and its dependent > subdevs to use subdev notifiers. > > There are a couple shortcomings in v4l2-core that prevented > subdev notifiers from working correctly in imx-media: > > 1. v4l2_async_notifier_fwnode_parse_endpoint() treats a fwnode >endpoint that is not connected to a remote device as an error. >But in the case of the video-mux subdev, this is not an error, >it is OK if some of the mux inputs have no connection. Also, >Documentation/devicetree/bindings/media/video-interfaces.txt explicitly >states that the 'remote-endpoint' property is optional. So the first >patch is a small modification to ignore empty endpoints in >v4l2_async_notifier_fwnode_parse_endpoint() and allow >__v4l2_async_notifier_parse_fwnode_endpoints() to continue to >parse the remaining port endpoints of the device. > > 2. In the imx-media graph, multiple subdevs will encounter the same >upstream subdev (such as the imx6-mipi-csi2 receiver), and so >v4l2_async_notifier_parse_fwnode_endpoints() will add imx6-mipi-csi2 >multiple times. This is treated as an error by >v4l2_async_notifier_register() later. > >To get around this problem, add an v4l2_async_notifier_add_subdev() >which first verifies the provided asd does not already exist in the >given notifier asd list or in other registered notifiers. If the asd >exists, the function returns -EEXIST and it's up to the caller to >decide if that is an error (in imx-media case it is never an error). > >Patches 2-5 deal with adding that support. > > 3. Patch 6 adds v4l2_async_register_fwnode_subdev(), which is a >convenience function for parsing a subdev's fwnode port endpoints >for connected remote subdevs, registering a subdev notifier, and >then registering the sub-device itself. > > 4. Patches 7-14 update the subdev drivers to register a subdev notifier >with endpoint parsing, and the changes to imx-media to support that. > > 5. Finally, the last 3 patches endeavor to completely remove support for >the notifier->subdevs[] array in platform drivers and v4l2 core. All >platform drivers are modified to make use of >v4l2_async_notifier_add_subdev() and its related convenience functions >to add asd's to the notifier @asd_list, and any allocation or reference >to the notifier->subdevs[] array removed. After that large patch, >notifier->subdevs[] array is stripped from v4l2-async and v4l2-subdev >docs are updated to reflect the new method of adding asd's to notifiers. > > > Signed-off-by: Steve Longerbeam > > Patches 07-14 (video-mux and the imx patches) are > Reviewed-by: Philipp Zabel > > Patches 01-14 are > Tested-by: Philipp Zabel > on i.MX6 with Toshiba TC358743 connected via MIPI CSI-2. > > History: > > v6: > - Export v4l2_async_notifier_init(), which must be called by all > drivers before the first call to v4l2_async_notifier_add_subdev(). > Suggested by Sakari Ailus. > - Removed @num_subdevs from struct v4l2_async_notifier, and the macro > V4L2_MAX_SUBDEVS. Sugested by Sakari. > - Fixed a double device node put in vpif_capture.c. Reported by Sakari. > - Fixed wrong printk format qualifiers in xilinx-vipp.c. Reported by > Dan Carpenter. The patches seem good to me. Still, the changes are non-trivial and to allow more time for them to stay in the media tree only I'd like to postpone applying them until we're past the next merge window. I'll add Philipp's Tested-by: and Reviewed-by: tags then. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
[PATCH v6 00/17] media: imx: Switch to subdev notifiers
This patchset converts the imx-media driver and its dependent subdevs to use subdev notifiers. There are a couple shortcomings in v4l2-core that prevented subdev notifiers from working correctly in imx-media: 1. v4l2_async_notifier_fwnode_parse_endpoint() treats a fwnode endpoint that is not connected to a remote device as an error. But in the case of the video-mux subdev, this is not an error, it is OK if some of the mux inputs have no connection. Also, Documentation/devicetree/bindings/media/video-interfaces.txt explicitly states that the 'remote-endpoint' property is optional. So the first patch is a small modification to ignore empty endpoints in v4l2_async_notifier_fwnode_parse_endpoint() and allow __v4l2_async_notifier_parse_fwnode_endpoints() to continue to parse the remaining port endpoints of the device. 2. In the imx-media graph, multiple subdevs will encounter the same upstream subdev (such as the imx6-mipi-csi2 receiver), and so v4l2_async_notifier_parse_fwnode_endpoints() will add imx6-mipi-csi2 multiple times. This is treated as an error by v4l2_async_notifier_register() later. To get around this problem, add an v4l2_async_notifier_add_subdev() which first verifies the provided asd does not already exist in the given notifier asd list or in other registered notifiers. If the asd exists, the function returns -EEXIST and it's up to the caller to decide if that is an error (in imx-media case it is never an error). Patches 2-5 deal with adding that support. 3. Patch 6 adds v4l2_async_register_fwnode_subdev(), which is a convenience function for parsing a subdev's fwnode port endpoints for connected remote subdevs, registering a subdev notifier, and then registering the sub-device itself. 4. Patches 7-14 update the subdev drivers to register a subdev notifier with endpoint parsing, and the changes to imx-media to support that. 5. Finally, the last 3 patches endeavor to completely remove support for the notifier->subdevs[] array in platform drivers and v4l2 core. All platform drivers are modified to make use of v4l2_async_notifier_add_subdev() and its related convenience functions to add asd's to the notifier @asd_list, and any allocation or reference to the notifier->subdevs[] array removed. After that large patch, notifier->subdevs[] array is stripped from v4l2-async and v4l2-subdev docs are updated to reflect the new method of adding asd's to notifiers. Signed-off-by: Steve Longerbeam Patches 07-14 (video-mux and the imx patches) are Reviewed-by: Philipp Zabel Patches 01-14 are Tested-by: Philipp Zabel on i.MX6 with Toshiba TC358743 connected via MIPI CSI-2. History: v6: - Export v4l2_async_notifier_init(), which must be called by all drivers before the first call to v4l2_async_notifier_add_subdev(). Suggested by Sakari Ailus. - Removed @num_subdevs from struct v4l2_async_notifier, and the macro V4L2_MAX_SUBDEVS. Sugested by Sakari. - Fixed a double device node put in vpif_capture.c. Reported by Sakari. - Fixed wrong printk format qualifiers in xilinx-vipp.c. Reported by Dan Carpenter. v5: - see point 5 above. v4: - small non-functional code cleanup in video-mux.c. - strip TODO for comparing custom asd's for equivalence. - add three more convenience functions: v4l2_async_notifier_add_fwnode_subdev, v4l2_async_notifier_add_i2c_subdev, v4l2_async_notifier_add_devname_subdev. - strip support in v4l2_async_register_fwnode_subdev for sub-devices that register from port nodes. v3: - code optimization in asd_equal(), and remove unneeded braces, suggested by Sakari Ailus. - add a NULL asd pointer check to v4l2_async_notifier_asd_valid(). - fix an error-out path in v4l2_async_register_fwnode_subdev() that forgot to put device. v2: - don't pass an empty endpoint to the parse_endpoint callback, v4l2_async_notifier_fwnode_parse_endpoint() now just ignores them and returns success. - Fix a couple compile warnings and errors seen in i386 and sh archs. Steve Longerbeam (17): media: v4l2-fwnode: ignore endpoints that have no remote port parent media: v4l2: async: Allow searching for asd of any type media: v4l2: async: Add v4l2_async_notifier_add_subdev media: v4l2: async: Add convenience functions to allocate and add asd's media: v4l2-fwnode: Switch to v4l2_async_notifier_add_subdev media: v4l2-fwnode: Add a convenience function for registering subdevs with notifiers media: platform: video-mux: Register a subdev notifier media: imx: csi: Register a subdev notifier media: imx: mipi csi-2: Register a subdev notifier media: staging/imx: of: Remove recursive graph walk media: staging/imx: Loop through all registered subdevs for media links media: staging/imx: Rename root notifier media: staging/imx: Switch to v4l2_async_notifier_add_*_subdev media: staging/imx: TODO: Remove one assumption about OF graph parsing media: platform: Switch to v4l2
