Hi Laurent,

On Tue, Sep 19, 2017 at 03:46:18PM +0300, Laurent Pinchart wrote:

Hi Sakari,

On Tuesday, 19 September 2017 15:43:26 EEST Sakari Ailus wrote:

quoted

On Tue, Sep 19, 2017 at 02:40:29PM +0300, Laurent Pinchart wrote:

quoted

quoted

@@ -2256,7 +2210,9 @@ static int isp_probe(struct platform_device *pdev)

 	if (ret)
 	
 		return ret;

-	ret = isp_fwnodes_parse(&pdev->dev, &isp->notifier);
+	ret = v4l2_async_notifier_parse_fwnode_endpoints(
+		&pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),
+		isp_fwnode_parse);

 	if (ret < 0)

The documentation in patch 05/25 states that v4l2_async_notifier_release()
should be called even if v4l2_async_notifier_parse_fwnode_endpoints()
fails. I don't think that's needed here, so you might want to update the
documentation (and possibly the implementation of the function).

It is. If parsing fails, async sub-devices may have been already set up.
This happens e.g. when the parsing fails after the first one has been
successfully set up already.

But for v4l2_async_notifier_parse_fwnode_endpoints() we could clean up 
internally when an error occurs. Otherwise you need to call 
v4l2_async_notifier_release() here.

The functions that set up async sub-devices can be called multiple times
(on separate references). This is quite alike setting up a control handler
really, so I adopted the same pattern.

If there is a failure, how many async sub-devices should be cleaned up, if
there have been async sub-devices already set up before calling this
function?

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html