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: > > On Tue, Sep 19, 2017 at 02:40:29PM +0300, Laurent Pinchart wrote: > > > > @@ -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. If a driver uses the variant that parses the endpoints by port, how should that function behave? Release just as many async sub-devices it set up, and leave the rest for the driver to handle? The reason I left it as such as to make the responsibility clear: it belongs to the driver. I can change that if you really think it makes a difference for better. I'm just not that certain about it. -- Regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
