[{"id":1767606,"web_url":"http://patchwork.ozlabs.org/comment/1767606/","msgid":"<0b73f576-c37b-ad58-6f74-71ffc3f8f176@xs4all.nl>","list_archive_url":null,"date":"2017-09-13T07:01:38","subject":"Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph\n\tendpoints, per port","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> This is just like like v4l2_async_notifier_parse_fwnode_endpoints but it\n> only parses endpoints on a single port. The behaviour is useful on devices\n> that have both sinks and sources, in other words only some of these should\n> be parsed.\n\nI think it would be better to merge this patch with the preceding patch. It\ndoes not make a lot of sense to have this separate IMHO.\n\nThat said:\n\nAcked-by: Hans Verkuil \n\nRegards,\n\n\tHans\n\n> \n> Signed-off-by: Sakari Ailus \n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 59 ++++++++++++++++++++++++++++++++---\n> include/media/v4l2-fwnode.h | 59 +++++++++++++++++++++++++++++++++++\n> 2 files changed, 113 insertions(+), 5 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index d978f2d714ca..44ee35f6aad5 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -398,9 +398,9 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(\n> \treturn ret == -ENOTCONN ? 0 : ret;\n> }\n> \n> -int v4l2_async_notifier_parse_fwnode_endpoints(\n> +static int __v4l2_async_notifier_parse_fwnode_endpoints(\n> \tstruct device *dev, struct v4l2_async_notifier *notifier,\n> -\tsize_t asd_struct_size,\n> +\tsize_t asd_struct_size, unsigned int port, bool has_port,\n> \tint (*parse_endpoint)(struct device *dev,\n> \t\t\t struct v4l2_fwnode_endpoint *vep,\n> \t\t\t struct v4l2_async_subdev *asd))\n> @@ -413,10 +413,25 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> \t\treturn -EINVAL;\n> \n> \tfor (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(\n> -\t\t\t\t dev_fwnode(dev), fwnode)); )\n> -\t\tif (fwnode_device_is_available(\n> +\t\t\t\t dev_fwnode(dev), fwnode)); ) {\n> +\t\tif (!fwnode_device_is_available(\n> \t\t\t fwnode_graph_get_port_parent(fwnode)))\n> -\t\t\tmax_subdevs++;\n> +\t\t\tcontinue;\n> +\n> +\t\tif (has_port) {\n> +\t\t\tstruct fwnode_endpoint ep;\n> +\n> +\t\t\tret = fwnode_graph_parse_endpoint(fwnode, &ep);\n> +\t\t\tif (ret) {\n> +\t\t\t\tfwnode_handle_put(fwnode);\n> +\t\t\t\treturn ret;\n> +\t\t\t}\n> +\n> +\t\t\tif (ep.port != port)\n> +\t\t\t\tcontinue;\n> +\t\t}\n> +\t\tmax_subdevs++;\n> +\t}\n> \n> \t/* No subdevs to add? Return here. */\n> \tif (max_subdevs == notifier->max_subdevs)\n> @@ -437,6 +452,17 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> \t\t\tbreak;\n> \t\t}\n> \n> +\t\tif (has_port) {\n> +\t\t\tstruct fwnode_endpoint ep;\n> +\n> +\t\t\tret = fwnode_graph_parse_endpoint(fwnode, &ep);\n> +\t\t\tif (ret)\n> +\t\t\t\tbreak;\n> +\n> +\t\t\tif (ep.port != port)\n> +\t\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\tret = v4l2_async_notifier_fwnode_parse_endpoint(\n> \t\t\tdev, notifier, fwnode, asd_struct_size, parse_endpoint);\n> \t\tif (ret < 0)\n> @@ -447,8 +473,31 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> \n> \treturn ret;\n> }\n> +\n> +int v4l2_async_notifier_parse_fwnode_endpoints(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tsize_t asd_struct_size,\n> +\tint (*parse_endpoint)(struct device *dev,\n> +\t\t\t struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t struct v4l2_async_subdev *asd))\n> +{\n> +\treturn __v4l2_async_notifier_parse_fwnode_endpoints(\n> +\t\tdev, notifier, asd_struct_size, 0, false, parse_endpoint);\n> +}\n> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\n> \n> +int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tsize_t asd_struct_size, unsigned int port,\n> +\tint (*parse_endpoint)(struct device *dev,\n> +\t\t\t struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t struct v4l2_async_subdev *asd))\n> +{\n> +\treturn __v4l2_async_notifier_parse_fwnode_endpoints(\n> +\t\tdev, notifier, asd_struct_size, port, true, parse_endpoint);\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);\n> +\n> MODULE_LICENSE(\"GPL\");\n> MODULE_AUTHOR(\"Sakari Ailus \");\n> MODULE_AUTHOR(\"Sylwester Nawrocki \");\n> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> index 31fb77e470fa..b2eed4f33e6a 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -257,4 +257,63 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> \t\t\t struct v4l2_fwnode_endpoint *vep,\n> \t\t\t struct v4l2_async_subdev *asd));\n> \n> +/**\n> + * v4l2_async_notifier_parse_fwnode_endpoints_by_port - Parse V4L2 fwnode\n> + *\t\t\t\t\t\t\tendpoints of a port in a\n> + *\t\t\t\t\t\t\tdevice node\n> + * @dev: the device the endpoints of which are to be parsed\n> + * @notifier: notifier for @dev\n> + * @asd_struct_size: size of the driver's async sub-device struct, including\n> + *\t\t sizeof(struct v4l2_async_subdev). The &struct\n> + *\t\t v4l2_async_subdev shall be the first member of\n> + *\t\t the driver's async sub-device struct, i.e. both\n> + *\t\t begin at the same memory address.\n> + * @port: port number where endpoints are to be parsed\n> + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode\n> + *\t\t endpoint. Optional.\n> + *\t\t Return: %0 on success\n> + *\t\t\t %-ENOTCONN if the endpoint is to be skipped but this\n> + *\t\t\t\t should not be considered as an error\n> + *\t\t\t %-EINVAL if the endpoint configuration is invalid\n> + *\n> + * This function is just like @v4l2_async_notifier_parse_fwnode_endpoints with\n> + * the exception that it only parses endpoints in a given port.\n> + *\n> + * Parse the fwnode endpoints of the @dev device on a given @port and populate\n> + * the async sub-devices array of the notifier. The @parse_endpoint callback\n> + * function is called for each endpoint with the corresponding async sub-device\n> + * pointer to let the caller initialize the driver-specific part of the async\n> + * sub-device structure.\n> + *\n> + * The notifier memory shall be zeroed before this function is called on the\n> + * notifier.\n> + *\n> + * This function may not be called on a registered notifier and may be called on\n> + * a notifier only once, per port.\n> + *\n> + * Do not change the notifier's subdevs array, take references to the subdevs\n> + * array itself or change the notifier's num_subdevs field. This is because this\n> + * function allocates and reallocates the subdevs array based on parsing\n> + * endpoints.\n> + *\n> + * The @struct v4l2_fwnode_endpoint passed to the callback function\n> + * @parse_endpoint is released once the function is finished. If there is a need\n> + * to retain that configuration, the user needs to allocate memory for it.\n> + *\n> + * Any notifier populated using this function must be released with a call to\n> + * v4l2_async_notifier_release() after it has been unregistered and the async\n> + * sub-devices are no longer in use, even if the function returned an error.\n> + *\n> + * Return: %0 on success, including when no async sub-devices are found\n> + *\t %-ENOMEM if memory allocation failed\n> + *\t %-EINVAL if graph or endpoint parsing failed\n> + *\t Other error codes as returned by @parse_endpoint\n> + */\n> +int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tsize_t asd_struct_size, unsigned int port,\n> +\tint (*parse_endpoint)(struct device *dev,\n> +\t\t\t struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t struct v4l2_async_subdev *asd));\n> +\n> #endif /* _V4L2_FWNODE_H */\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsXdn6Fwlz9sPs\n\tfor ;\n\tWed, 13 Sep 2017 17:01:49 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751316AbdIMHBs (ORCPT\n\t);\n\tWed, 13 Sep 2017 03:01:48 -0400","from lb2-smtp-cloud8.xs4all.net ([194.109.24.25]:45443 \"EHLO\n\tlb2-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1750999AbdIMHBr (ORCPT\n\t); Wed, 13 Sep 2017 03:01:47 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s1g2d04YgcQyLs1g5dwz6O; Wed, 13 Sep 2017 09:01:45 +0200"],"Subject":"Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph\n\tendpoints, per port","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, maxime.ripard@free-electrons.com,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-7-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<0b73f576-c37b-ad58-6f74-71ffc3f8f176@xs4all.nl>","Date":"Wed, 13 Sep 2017 09:01:38 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170912134200.19556-7-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfIhJp2fNSGm7MRP4iZoarG5Sq9uGJSMlxBgv3m6AJwIDqpU1w3ZXqJLScGFqnKQAEc9X2XDHlsLmD5XPmBIlPgjr6oNd5VOg48TTYmbu82bpdmhP5ocM\n\tSfshma/5odRWM7l77wqAVzFEIsHF7s8ES9qxptQ+T47e5Tc5Jba+ebikXB3OsqQKZGVvomdvXMbAvTFg9iAH2FMlb48UPCUM5j/MT/b05Cr8B3Qc/4V2mcLp\n\tw7PZTXBErMUi6BVhG7I2TdplAuvjfHleK7hIBtrs7kRki7TRg6Z4XwR6rHP2PjcdotljtsDkMxK39ake2MGuNuSeS7XBEpEkihDDDi/V3VfGkAwHPVyRHzKd\n\tfz8Ube8WIcvMHH4K7tQwh9EN/Ll9wtZx8sPz9RXQ5ZJGGGA1CJwtHUHTXyNnpxmsfsJ33Zx9nEV1AnkEzcmYtQ1yGSwwC0/V3HIAzXCm8noxc9UxdpMMpCqh\n\trxEqqDlm+KG5l0+cWCNPnQeOyHd7n3RvHq8TJiFQaGfQThmac0W8iqmcNBE=","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767608,"web_url":"http://patchwork.ozlabs.org/comment/1767608/","msgid":"","list_archive_url":null,"date":"2017-09-13T07:06:11","subject":"Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph\n\tendpoints, per port","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/13/2017 09:01 AM, Hans Verkuil wrote:\n> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n>> This is just like like v4l2_async_notifier_parse_fwnode_endpoints but it\n>> only parses endpoints on a single port. The behaviour is useful on devices\n>> that have both sinks and sources, in other words only some of these should\n>> be parsed.\n\nI forgot to mention that I think the log message can be improved: it is not\nclear why you would only parse some of the ports for devices that have both\nsinks and sources. That should be explained better. And probably also in the\nheader documentation.\n\n> \n> I think it would be better to merge this patch with the preceding patch. It\n> does not make a lot of sense to have this separate IMHO.\n> \n> That said:\n> \n> Acked-by: Hans Verkuil \n\nSorry, I'm retracting this. It needs a bit more work w.r.t. commit log and\nheader comments.\n\nRegards,\n\n\tHans\n\n> \n> Regards,\n> \n> \tHans\n> \n>>\n>> Signed-off-by: Sakari Ailus \n>> ---\n>> drivers/media/v4l2-core/v4l2-fwnode.c | 59 ++++++++++++++++++++++++++++++++---\n>> include/media/v4l2-fwnode.h | 59 +++++++++++++++++++++++++++++++++++\n>> 2 files changed, 113 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n>> index d978f2d714ca..44ee35f6aad5 100644\n>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n>> @@ -398,9 +398,9 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(\n>> \treturn ret == -ENOTCONN ? 0 : ret;\n>> }\n>> \n>> -int v4l2_async_notifier_parse_fwnode_endpoints(\n>> +static int __v4l2_async_notifier_parse_fwnode_endpoints(\n>> \tstruct device *dev, struct v4l2_async_notifier *notifier,\n>> -\tsize_t asd_struct_size,\n>> +\tsize_t asd_struct_size, unsigned int port, bool has_port,\n>> \tint (*parse_endpoint)(struct device *dev,\n>> \t\t\t struct v4l2_fwnode_endpoint *vep,\n>> \t\t\t struct v4l2_async_subdev *asd))\n>> @@ -413,10 +413,25 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n>> \t\treturn -EINVAL;\n>> \n>> \tfor (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(\n>> -\t\t\t\t dev_fwnode(dev), fwnode)); )\n>> -\t\tif (fwnode_device_is_available(\n>> +\t\t\t\t dev_fwnode(dev), fwnode)); ) {\n>> +\t\tif (!fwnode_device_is_available(\n>> \t\t\t fwnode_graph_get_port_parent(fwnode)))\n>> -\t\t\tmax_subdevs++;\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tif (has_port) {\n>> +\t\t\tstruct fwnode_endpoint ep;\n>> +\n>> +\t\t\tret = fwnode_graph_parse_endpoint(fwnode, &ep);\n>> +\t\t\tif (ret) {\n>> +\t\t\t\tfwnode_handle_put(fwnode);\n>> +\t\t\t\treturn ret;\n>> +\t\t\t}\n>> +\n>> +\t\t\tif (ep.port != port)\n>> +\t\t\t\tcontinue;\n>> +\t\t}\n>> +\t\tmax_subdevs++;\n>> +\t}\n>> \n>> \t/* No subdevs to add? Return here. */\n>> \tif (max_subdevs == notifier->max_subdevs)\n>> @@ -437,6 +452,17 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n>> \t\t\tbreak;\n>> \t\t}\n>> \n>> +\t\tif (has_port) {\n>> +\t\t\tstruct fwnode_endpoint ep;\n>> +\n>> +\t\t\tret = fwnode_graph_parse_endpoint(fwnode, &ep);\n>> +\t\t\tif (ret)\n>> +\t\t\t\tbreak;\n>> +\n>> +\t\t\tif (ep.port != port)\n>> +\t\t\t\tcontinue;\n>> +\t\t}\n>> +\n>> \t\tret = v4l2_async_notifier_fwnode_parse_endpoint(\n>> \t\t\tdev, notifier, fwnode, asd_struct_size, parse_endpoint);\n>> \t\tif (ret < 0)\n>> @@ -447,8 +473,31 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n>> \n>> \treturn ret;\n>> }\n>> +\n>> +int v4l2_async_notifier_parse_fwnode_endpoints(\n>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n>> +\tsize_t asd_struct_size,\n>> +\tint (*parse_endpoint)(struct device *dev,\n>> +\t\t\t struct v4l2_fwnode_endpoint *vep,\n>> +\t\t\t struct v4l2_async_subdev *asd))\n>> +{\n>> +\treturn __v4l2_async_notifier_parse_fwnode_endpoints(\n>> +\t\tdev, notifier, asd_struct_size, 0, false, parse_endpoint);\n>> +}\n>> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\n>> \n>> +int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n>> +\tsize_t asd_struct_size, unsigned int port,\n>> +\tint (*parse_endpoint)(struct device *dev,\n>> +\t\t\t struct v4l2_fwnode_endpoint *vep,\n>> +\t\t\t struct v4l2_async_subdev *asd))\n>> +{\n>> +\treturn __v4l2_async_notifier_parse_fwnode_endpoints(\n>> +\t\tdev, notifier, asd_struct_size, port, true, parse_endpoint);\n>> +}\n>> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);\n>> +\n>> MODULE_LICENSE(\"GPL\");\n>> MODULE_AUTHOR(\"Sakari Ailus \");\n>> MODULE_AUTHOR(\"Sylwester Nawrocki \");\n>> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n>> index 31fb77e470fa..b2eed4f33e6a 100644\n>> --- a/include/media/v4l2-fwnode.h\n>> +++ b/include/media/v4l2-fwnode.h\n>> @@ -257,4 +257,63 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n>> \t\t\t struct v4l2_fwnode_endpoint *vep,\n>> \t\t\t struct v4l2_async_subdev *asd));\n>> \n>> +/**\n>> + * v4l2_async_notifier_parse_fwnode_endpoints_by_port - Parse V4L2 fwnode\n>> + *\t\t\t\t\t\t\tendpoints of a port in a\n>> + *\t\t\t\t\t\t\tdevice node\n>> + * @dev: the device the endpoints of which are to be parsed\n>> + * @notifier: notifier for @dev\n>> + * @asd_struct_size: size of the driver's async sub-device struct, including\n>> + *\t\t sizeof(struct v4l2_async_subdev). The &struct\n>> + *\t\t v4l2_async_subdev shall be the first member of\n>> + *\t\t the driver's async sub-device struct, i.e. both\n>> + *\t\t begin at the same memory address.\n>> + * @port: port number where endpoints are to be parsed\n>> + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode\n>> + *\t\t endpoint. Optional.\n>> + *\t\t Return: %0 on success\n>> + *\t\t\t %-ENOTCONN if the endpoint is to be skipped but this\n>> + *\t\t\t\t should not be considered as an error\n>> + *\t\t\t %-EINVAL if the endpoint configuration is invalid\n>> + *\n>> + * This function is just like @v4l2_async_notifier_parse_fwnode_endpoints with\n>> + * the exception that it only parses endpoints in a given port.\n>> + *\n>> + * Parse the fwnode endpoints of the @dev device on a given @port and populate\n>> + * the async sub-devices array of the notifier. The @parse_endpoint callback\n>> + * function is called for each endpoint with the corresponding async sub-device\n>> + * pointer to let the caller initialize the driver-specific part of the async\n>> + * sub-device structure.\n>> + *\n>> + * The notifier memory shall be zeroed before this function is called on the\n>> + * notifier.\n>> + *\n>> + * This function may not be called on a registered notifier and may be called on\n>> + * a notifier only once, per port.\n>> + *\n>> + * Do not change the notifier's subdevs array, take references to the subdevs\n>> + * array itself or change the notifier's num_subdevs field. This is because this\n>> + * function allocates and reallocates the subdevs array based on parsing\n>> + * endpoints.\n>> + *\n>> + * The @struct v4l2_fwnode_endpoint passed to the callback function\n>> + * @parse_endpoint is released once the function is finished. If there is a need\n>> + * to retain that configuration, the user needs to allocate memory for it.\n>> + *\n>> + * Any notifier populated using this function must be released with a call to\n>> + * v4l2_async_notifier_release() after it has been unregistered and the async\n>> + * sub-devices are no longer in use, even if the function returned an error.\n>> + *\n>> + * Return: %0 on success, including when no async sub-devices are found\n>> + *\t %-ENOMEM if memory allocation failed\n>> + *\t %-EINVAL if graph or endpoint parsing failed\n>> + *\t Other error codes as returned by @parse_endpoint\n>> + */\n>> +int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n>> +\tsize_t asd_struct_size, unsigned int port,\n>> +\tint (*parse_endpoint)(struct device *dev,\n>> +\t\t\t struct v4l2_fwnode_endpoint *vep,\n>> +\t\t\t struct v4l2_async_subdev *asd));\n>> +\n>> #endif /* _V4L2_FWNODE_H */\n>>\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsXl03pxCz9sPs\n\tfor ;\n\tWed, 13 Sep 2017 17:06:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751090AbdIMHGR (ORCPT\n\t);\n\tWed, 13 Sep 2017 03:06:17 -0400","from lb3-smtp-cloud8.xs4all.net ([194.109.24.29]:59765 \"EHLO\n\tlb3-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1750993AbdIMHGQ (ORCPT\n\t); Wed, 13 Sep 2017 03:06:16 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s1kRd07mFcQyLs1kUdx0Uc; Wed, 13 Sep 2017 09:06:15 +0200"],"Subject":"Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph\n\tendpoints, per port","From":"Hans Verkuil ","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, maxime.ripard@free-electrons.com,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-7-sakari.ailus@linux.intel.com>\n\t<0b73f576-c37b-ad58-6f74-71ffc3f8f176@xs4all.nl>","Message-ID":"","Date":"Wed, 13 Sep 2017 09:06:11 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<0b73f576-c37b-ad58-6f74-71ffc3f8f176@xs4all.nl>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfFNXxsO4zg6RCJtmipuLcUNOXajfV5SJK6ZHJ0+Nmf+Q1AfK5JJDMyXfiEAOudjr7PIimCVMg1YkmReBI+puGAH9XlR1/5/DdBCYQlCCtM9MZs6WM8Wd\n\t+LeX9mmRVzJxoqybU7M9snMIDx5Jm/BRUhTLhQMThz0/4DCt8A2cWd41AjEPd24uJIxEy17LmGTALR7SeGtBXE7TMIDXNs0eQYWIgAMS8vBB81lEAoVOzyrp\n\tA1XEGVnHPR6sqID60h4oDcRv/AU2gq/NUl/C2+ZTtO1YS1yNVhIAUCZQuTf3Okx9UTaWwyTQOn05kmcKJlz4TkJ1Do/BN8LIKO7s9IfQ7tkV6OjItjrKfJxn\n\tmvmtXXjcp3/maaz2YEVSX8bEpBx4A2ar3dKptOg1JakzbycUmR1SJ8m16TOK0egGTzwwEwL1C/wqg7gQ2/ePXbYtMQU6d49M7drRvKuNKchB7Q4bjVgxkVF4\n\tCQzFOp/VMIV2UFMm4918QImkaIGuJ3HUXvX/1RohONcWlfmg05oEq663Mb4=","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767617,"web_url":"http://patchwork.ozlabs.org/comment/1767617/","msgid":"<575bf15b-62d2-3a51-d550-d462578471f7@xs4all.nl>","list_archive_url":null,"date":"2017-09-13T07:17:08","subject":"Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to\n\tsub-devices","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> Registering a notifier has required the knowledge of struct v4l2_device\n> for the reason that sub-devices generally are registered to the\n> v4l2_device (as well as the media device, also available through\n> v4l2_device).\n> \n> This information is not available for sub-device drivers at probe time.\n> \n> What this patch does is that it allows registering notifiers without\n> having v4l2_device around. Instead the sub-device pointer is stored in the\n> notifier. Once the sub-device of the driver that registered the notifier\n> is registered, the notifier will gain the knowledge of the v4l2_device,\n> and the binding of async sub-devices from the sub-device driver's notifier\n> may proceed.\n> \n> The root notifier's complete callback is only called when all sub-device\n> notifiers are completed.\n> \n> Signed-off-by: Sakari Ailus \n\nJust two small comments (see below).\n\nAfter changing those (the first is up to you) you can add my:\n\nAcked-by: Hans Verkuil \n\nRegards,\n\n\tHans\n\n> ---\n> drivers/media/v4l2-core/v4l2-async.c | 219 ++++++++++++++++++++++++++++++-----\n> include/media/v4l2-async.h | 16 ++-\n> 2 files changed, 204 insertions(+), 31 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> index 4525b03d59c1..5082b01d2b96 100644\n> --- a/drivers/media/v4l2-core/v4l2-async.c\n> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> @@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n)\n> \treturn n->ops->complete(n);\n> }\n> \n> +static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n> +\t\t\t\t struct v4l2_subdev *sd,\n> +\t\t\t\t struct v4l2_async_subdev *asd);\n> +\n> static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)\n> {\n> #if IS_ENABLED(CONFIG_I2C)\n> @@ -124,14 +128,128 @@ static struct v4l2_async_subdev *v4l2_async_find_match(\n> \treturn NULL;\n> }\n> \n> +/* Find the sub-device notifier registered by a sub-device driver. */\n> +static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier(\n> +\tstruct v4l2_subdev *sd)\n> +{\n> +\tstruct v4l2_async_notifier *n;\n> +\n> +\tlist_for_each_entry(n, ¬ifier_list, list)\n> +\t\tif (n->sd == sd)\n> +\t\t\treturn n;\n> +\n> +\treturn NULL;\n> +}\n> +\n> +/* Return true if all sub-device notifiers are complete, false otherwise. */\n> +static bool v4l2_async_subdev_notifiers_complete(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tstruct v4l2_subdev *sd;\n> +\n> +\tif (!list_empty(¬ifier->waiting))\n> +\t\treturn false;\n> +\n> +\tlist_for_each_entry(sd, ¬ifier->done, async_list) {\n> +\t\tstruct v4l2_async_notifier *subdev_notifier =\n> +\t\t\tv4l2_async_find_subdev_notifier(sd);\n> +\n> +\t\tif (!subdev_notifier)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (!v4l2_async_subdev_notifiers_complete(subdev_notifier))\n\nI think it is better to change this to:\n\n\t\tif (subdev_notifier &&\n\t\t !v4l2_async_subdev_notifiers_complete(subdev_notifier))\n\nand drop this if..continue above. That's a bit overkill in this simple case.\n\nIt's up to you, though.\n\n> +\t\t\treturn false;\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +/* Get v4l2_device related to the notifier if one can be found. */\n> +static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\twhile (notifier->parent)\n> +\t\tnotifier = notifier->parent;\n> +\n> +\treturn notifier->v4l2_dev;\n> +}\n> +\n> +/* Test all async sub-devices in a notifier for a match. */\n> +static int v4l2_async_notifier_try_all_subdevs(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tstruct v4l2_subdev *sd;\n> +\n> +\tif (!v4l2_async_notifier_find_v4l2_dev(notifier))\n> +\t\treturn 0;\n> +\n> +again:\n> +\tlist_for_each_entry(sd, &subdev_list, async_list) {\n> +\t\tstruct v4l2_async_subdev *asd;\n> +\t\tint ret;\n> +\n> +\t\tasd = v4l2_async_find_match(notifier, sd);\n> +\t\tif (!asd)\n> +\t\t\tcontinue;\n> +\n> +\t\tret = v4l2_async_match_notify(notifier, sd, asd);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\t/*\n> +\t\t * v4l2_async_match_notify() may lead to registering a\n> +\t\t * new notifier and thus changing the async subdevs\n> +\t\t * list. In order to proceed safely from here, restart\n> +\t\t * parsing the list from the beginning.\n> +\t\t */\n> +\t\tgoto again;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +/* Try completing a notifier. */\n> +static int v4l2_async_notifier_try_complete(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tdo {\n> +\t\tint ret;\n> +\n> +\t\t/* Any local async sub-devices left? */\n> +\t\tif (!list_empty(¬ifier->waiting))\n> +\t\t\treturn 0;\n> +\n> +\t\t/*\n> +\t\t * Any sub-device notifiers waiting for async subdevs\n> +\t\t * to be bound?\n> +\t\t */\n> +\t\tif (!v4l2_async_subdev_notifiers_complete(notifier))\n> +\t\t\treturn 0;\n> +\n> +\t\t/* Proceed completing the notifier */\n> +\t\tret = v4l2_async_notifier_call_complete(notifier);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\t/*\n> +\t\t * Obtain notifier's parent. If there is one, repeat\n> +\t\t * the process, otherwise we're done here.\n> +\t\t */\n> +\t} while ((notifier = notifier->parent));\n\nI'd change this to:\n\n\t\tnotifier = notifier->parent;\n\t} while (notifier);\n\nAssignment in a condition is frowned upon, and there is no need to do that\nhere.\n\n> +\n> +\treturn 0;\n> +}\n> +\n> static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n> \t\t\t\t struct v4l2_subdev *sd,\n> \t\t\t\t struct v4l2_async_subdev *asd)\n> {\n> +\tstruct v4l2_async_notifier *subdev_notifier;\n> \tint ret;\n> \n> -\tret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);\n> -\tif (ret < 0)\n> +\tret = v4l2_device_register_subdev(\n> +\t\tv4l2_async_notifier_find_v4l2_dev(notifier), sd);\n> +\tif (ret)\n> \t\treturn ret;\n> \n> \tret = v4l2_async_notifier_call_bound(notifier, sd, asd);\n> @@ -148,10 +266,20 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n> \t/* Move from the global subdevice list to notifier's done */\n> \tlist_move(&sd->async_list, ¬ifier->done);\n> \n> -\tif (list_empty(¬ifier->waiting))\n> -\t\treturn v4l2_async_notifier_call_complete(notifier);\n> +\t/*\n> +\t * See if the sub-device has a notifier. If it does, proceed\n> +\t * with checking for its async sub-devices.\n> +\t */\n> +\tsubdev_notifier = v4l2_async_find_subdev_notifier(sd);\n> +\tif (subdev_notifier && !subdev_notifier->parent) {\n> +\t\tsubdev_notifier->parent = notifier;\n> +\t\tret = v4l2_async_notifier_try_all_subdevs(subdev_notifier);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> \n> -\treturn 0;\n> +\t/* Try completing the notifier and its parent(s). */\n> +\treturn v4l2_async_notifier_try_complete(notifier);\n> }\n> \n> static void v4l2_async_cleanup(struct v4l2_subdev *sd)\n> @@ -163,17 +291,15 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)\n> \tsd->dev = NULL;\n> }\n> \n> -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n> -\t\t\t\t struct v4l2_async_notifier *notifier)\n> +static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)\n> {\n> -\tstruct v4l2_subdev *sd, *tmp;\n> \tstruct v4l2_async_subdev *asd;\n> +\tint ret;\n> \tint i;\n> \n> -\tif (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)\n> +\tif (notifier->num_subdevs > V4L2_MAX_SUBDEVS)\n> \t\treturn -EINVAL;\n> \n> -\tnotifier->v4l2_dev = v4l2_dev;\n> \tINIT_LIST_HEAD(¬ifier->waiting);\n> \tINIT_LIST_HEAD(¬ifier->done);\n> \n> @@ -200,18 +326,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n> \n> \tmutex_lock(&list_lock);\n> \n> -\tlist_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {\n> -\t\tint ret;\n> -\n> -\t\tasd = v4l2_async_find_match(notifier, sd);\n> -\t\tif (!asd)\n> -\t\t\tcontinue;\n> -\n> -\t\tret = v4l2_async_match_notify(notifier, sd, asd);\n> -\t\tif (ret < 0) {\n> -\t\t\tmutex_unlock(&list_lock);\n> -\t\t\treturn ret;\n> -\t\t}\n> +\tret = v4l2_async_notifier_try_all_subdevs(notifier);\n> +\tif (ret) {\n> +\t\tmutex_unlock(&list_lock);\n> +\t\treturn ret;\n> \t}\n> \n> \t/* Keep also completed notifiers on the list */\n> @@ -221,29 +339,70 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n> \n> \treturn 0;\n> }\n> +\n> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n> +\t\t\t\t struct v4l2_async_notifier *notifier)\n> +{\n> +\tif (WARN_ON(!v4l2_dev || notifier->sd))\n> +\t\treturn -EINVAL;\n> +\n> +\tnotifier->v4l2_dev = v4l2_dev;\n> +\n> +\treturn __v4l2_async_notifier_register(notifier);\n> +}\n> EXPORT_SYMBOL(v4l2_async_notifier_register);\n> \n> -void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n> +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,\n> +\t\t\t\t\tstruct v4l2_async_notifier *notifier)\n> {\n> -\tstruct v4l2_subdev *sd, *tmp;\n> +\tif (WARN_ON(!sd || notifier->v4l2_dev))\n> +\t\treturn -EINVAL;\n> \n> -\tif (!notifier->v4l2_dev)\n> -\t\treturn;\n> +\tnotifier->sd = sd;\n> \n> -\tmutex_lock(&list_lock);\n> +\treturn __v4l2_async_notifier_register(notifier);\n> +}\n> +EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);\n> \n> -\tlist_del(¬ifier->list);\n> +/* Unbind all sub-devices in the notifier tree. */\n> +static void v4l2_async_notifier_unbind_all_subdevs(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tstruct v4l2_subdev *sd, *tmp;\n> \n> \tlist_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {\n> +\t\tstruct v4l2_async_notifier *subdev_notifier =\n> +\t\t\tv4l2_async_find_subdev_notifier(sd);\n> +\n> +\t\tif (subdev_notifier)\n> +\t\t\tv4l2_async_notifier_unbind_all_subdevs(subdev_notifier);\n> +\n> \t\tv4l2_async_cleanup(sd);\n> \n> \t\tv4l2_async_notifier_call_unbind(notifier, sd, sd->asd);\n> -\t}\n> \n> -\tmutex_unlock(&list_lock);\n> +\t\tlist_del(&sd->async_list);\n> +\t\tlist_add(&sd->async_list, &subdev_list);\n> +\t}\n> \n> +\tnotifier->parent = NULL;\n> +\tnotifier->sd = NULL;\n> \tnotifier->v4l2_dev = NULL;\n> }\n> +\n> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n> +{\n> +\tif (!notifier->v4l2_dev && !notifier->sd)\n> +\t\treturn;\n> +\n> +\tmutex_lock(&list_lock);\n> +\n> +\tv4l2_async_notifier_unbind_all_subdevs(notifier);\n> +\n> +\tlist_del(¬ifier->list);\n> +\n> +\tmutex_unlock(&list_lock);\n> +}\n> EXPORT_SYMBOL(v4l2_async_notifier_unregister);\n> \n> void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)\n> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> index 3bc8a7c0d83f..a13803a6371d 100644\n> --- a/include/media/v4l2-async.h\n> +++ b/include/media/v4l2-async.h\n> @@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations {\n> * @num_subdevs: number of subdevices used in the subdevs array\n> * @max_subdevs: number of subdevices allocated in the subdevs array\n> * @subdevs:\tarray of pointers to subdevice descriptors\n> - * @v4l2_dev:\tpointer to struct v4l2_device\n> + * @v4l2_dev:\tv4l2_device of the root notifier, NULL otherwise\n> + * @sd:\t\tsub-device that registered the notifier, NULL otherwise\n> + * @parent:\tparent notifier\n> * @waiting:\tlist of struct v4l2_async_subdev, waiting for their drivers\n> * @done:\tlist of struct v4l2_subdev, already probed\n> * @list:\tmember in a global list of notifiers\n> @@ -113,6 +115,8 @@ struct v4l2_async_notifier {\n> \tunsigned int max_subdevs;\n> \tstruct v4l2_async_subdev **subdevs;\n> \tstruct v4l2_device *v4l2_dev;\n> +\tstruct v4l2_subdev *sd;\n> +\tstruct v4l2_async_notifier *parent;\n> \tstruct list_head waiting;\n> \tstruct list_head done;\n> \tstruct list_head list;\n> @@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n> \t\t\t\t struct v4l2_async_notifier *notifier);\n> \n> /**\n> + * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous\n> + *\t\t\t\t\t notifier for a sub-device\n> + *\n> + * @sd: pointer to &struct v4l2_subdev\n> + * @notifier: pointer to &struct v4l2_async_notifier\n> + */\n> +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,\n> +\t\t\t\t\tstruct v4l2_async_notifier *notifier);\n> +\n> +/**\n> * v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier\n> *\n> * @notifier: pointer to &struct v4l2_async_notifier\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsXzb5g1cz9sNw\n\tfor ;\n\tWed, 13 Sep 2017 17:17:15 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751854AbdIMHRN (ORCPT\n\t);\n\tWed, 13 Sep 2017 03:17:13 -0400","from lb2-smtp-cloud8.xs4all.net ([194.109.24.25]:52805 \"EHLO\n\tlb2-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751842AbdIMHRN (ORCPT\n\t); Wed, 13 Sep 2017 03:17:13 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s1v2d0FJlcQyLs1v5dx40T; Wed, 13 Sep 2017 09:17:11 +0200"],"Subject":"Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to\n\tsub-devices","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, maxime.ripard@free-electrons.com,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-16-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<575bf15b-62d2-3a51-d550-d462578471f7@xs4all.nl>","Date":"Wed, 13 Sep 2017 09:17:08 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170912134200.19556-16-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfPKx6B18ivtd0J/G9lbQGi21Aw75ph1H9zTHtP4NsKb55pkqFAOvvCargAiR77ZzTxJCywy0oHM2HEPqzBkwCIrLHBCdEe/sTZNx3NcCD0HQJNknTGdJ\n\tvJ3MFyD7nVZxuTF+nIhRnWcPtTh/sE2+9xBrYsq30T2n6AE30rqKG338fI0DPWsJRcygOz14pK+AsOz8KW7q50PMOMF2HKYxv4UjZbyjuRdeKQEQn3/tbJ+c\n\toER+0s2h+J6vbVvQiUN4EVJ/ycEe53+8pa/ZTTZa/7i5Iz4Jt009ko7Z5bZMtmb9jyRnIB7gHkwYCTs8PBPqF5HWwlRx5mEog5eYBpbJBnRVnThsaBU2Vhn3\n\t5dRTo3ZexDFgYPK/nesEVO+5oxGR8Fv+Q8xzAfBwchHOaFtFfiPzz6dGILgImppqnIsZ0bne3TSYNW33/1QP398WfUZWyTN8sp4rNk6HTu+t890rFMQalMvX\n\twhrVfqbHlHwvk0GD0kMKbbzP/BwEOWm/x8OQg2wC93wJyiThRp0VMiRDumE=","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767624,"web_url":"http://patchwork.ozlabs.org/comment/1767624/","msgid":"<020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl>","list_archive_url":null,"date":"2017-09-13T07:27:34","subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> Add function v4l2_fwnode_reference_count() for counting external\n\n???? There is no function v4l2_fwnode_reference_count()?!\n\n> references and v4l2_fwnode_reference_parse() for parsing them as async\n> sub-devices.\n> \n> This can be done on e.g. flash or lens async sub-devices that are not part\n> of but are associated with a sensor.\n> \n> struct v4l2_async_notifier.max_subdevs field is added to contain the\n> maximum number of sub-devices in a notifier to reflect the memory\n> allocated for the subdevs array.\n\nYou forgot to remove this outdated paragraph.\n\n> \n> Signed-off-by: Sakari Ailus \n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 69 +++++++++++++++++++++++++++++++++++\n> 1 file changed, 69 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index 44ee35f6aad5..a32473f95be1 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n> }\n> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);\n> \n> +/*\n> + * v4l2_fwnode_reference_parse - parse references for async sub-devices\n> + * @dev: the device node the properties of which are parsed for references\n> + * @notifier: the async notifier where the async subdevs will be added\n> + * @prop: the name of the property\n> + *\n> + * Return: 0 on success\n> + *\t -ENOENT if no entries were found\n> + *\t -ENOMEM if memory allocation failed\n> + *\t -EINVAL if property parsing failed\n> + */\n> +static int v4l2_fwnode_reference_parse(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tconst char *prop)\n> +{\n> +\tstruct fwnode_reference_args args;\n> +\tunsigned int index;\n> +\tint ret;\n> +\n> +\tfor (index = 0;\n> +\t !(ret = fwnode_property_get_reference_args(\n> +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args));\n> +\t index++)\n> +\t\tfwnode_handle_put(args.fwnode);\n> +\n> +\tif (!index)\n> +\t\treturn -ENOENT;\n> +\n> +\t/*\n> +\t * To-do: handle -ENODATA when \"device property: Align return\n> +\t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n> +\t */\n> +\tif (ret != -ENOENT && ret != -ENODATA)\n\nSo while that patch referenced in the To-do above is not merged yet,\nwhat does fwnode_property_get_reference_args return? ENOENT or ENODATA?\nOr ENOENT now and ENODATA later? Or vice versa?\n\nI can't tell based on that information whether this code is correct or not.\n\nThe comment needs to explain this a bit better.\n\n> +\t\treturn ret;\n> +\n> +\tret = v4l2_async_notifier_realloc(notifier,\n> +\t\t\t\t\t notifier->num_subdevs + index);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tfor (index = 0; !fwnode_property_get_reference_args(\n> +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args);\n> +\t index++) {\n> +\t\tstruct v4l2_async_subdev *asd;\n> +\n> +\t\tif (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {\n> +\t\t\tret = -EINVAL;\n> +\t\t\tgoto error;\n> +\t\t}\n> +\n> +\t\tasd = kzalloc(sizeof(*asd), GFP_KERNEL);\n> +\t\tif (!asd) {\n> +\t\t\tret = -ENOMEM;\n> +\t\t\tgoto error;\n> +\t\t}\n> +\n> +\t\tnotifier->subdevs[notifier->num_subdevs] = asd;\n> +\t\tasd->match.fwnode.fwnode = args.fwnode;\n> +\t\tasd->match_type = V4L2_ASYNC_MATCH_FWNODE;\n> +\t\tnotifier->num_subdevs++;\n> +\t}\n> +\n> +\treturn 0;\n> +\n> +error:\n> +\tfwnode_handle_put(args.fwnode);\n> +\treturn ret;\n> +}\n> +\n> MODULE_LICENSE(\"GPL\");\n> MODULE_AUTHOR(\"Sakari Ailus \");\n> MODULE_AUTHOR(\"Sylwester Nawrocki \");\n> \n\nRegards,\n\n\tHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsYCd2vrMz9sPs\n\tfor ;\n\tWed, 13 Sep 2017 17:27:41 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751442AbdIMH1j (ORCPT\n\t);\n\tWed, 13 Sep 2017 03:27:39 -0400","from lb3-smtp-cloud8.xs4all.net ([194.109.24.29]:51729 \"EHLO\n\tlb3-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1750949AbdIMH1j (ORCPT\n\t); Wed, 13 Sep 2017 03:27:39 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s258d0MYfcQyLs25Bdx7Rf; Wed, 13 Sep 2017 09:27:38 +0200"],"Subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, maxime.ripard@free-electrons.com,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-19-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl>","Date":"Wed, 13 Sep 2017 09:27:34 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170912134200.19556-19-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfNeTdoq5qv3g6Lpgf9QiSdM9wl0D/PgaGmgMUNsRboOY6XRlKoGQ4x9K4RSsjD5shflqN6y/Tku3OdPfNLg38Uf/8fq07LQbgdehrbWelCOOcswcsuJy\n\tRRPTNaAqeEDK2M7wHXMM4Xb5VPQ6zD+PUJRPLUR6WcKFrRdZ4nx1FITpseEyf2+Vo2SukYaYn01e/eQD2nOBbtmgzHhn4LK89lWIYzPDXR/RJdvbUw1MjSYK\n\tmHs2Y2cYGUSG2a3ylusJi4RswFFDkGh5MHtVfpep9OyIPTXMacR/2huGnjaG1hMUzKRvFVl3aaJnbvevl0j222zAlvHl/0c1NjU9oPn3mYSwTVD/CQoFMk7j\n\tTcsiSbL1c+WGjKg5agV40NI0fJn4/k3elfq2colsT6y+GsoktgZZE9tHc7Rw4/hIfvigcsCxwHUGZLtmpVukEzk/o0O1NmysEyaFBXSDJ/gJwBKwwmm2tBtI\n\tMC/vNsBNWIemeiOFxRjiXYpNVEBQ8wlYVWvnaV4xCbrE7Xy9kMuKGvSwFwg=","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767651,"web_url":"http://patchwork.ozlabs.org/comment/1767651/","msgid":"","list_archive_url":null,"date":"2017-09-13T07:57:52","subject":"Re: [PATCH v12 19/26] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"The subject still has the 'interger' typo.\n\nOn 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under\n> the device's own fwnode, it will follow child fwnodes with the given\n> property -- value pair and return the resulting fwnode.\n\nAs suggested before: 'property-value' is easier to read than ' -- '.\n\n> \n> Signed-off-by: Sakari Ailus \n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 145 ++++++++++++++++++++++++++++++++++\n> 1 file changed, 145 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index a32473f95be1..a07599a8f647 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -567,6 +567,151 @@ static int v4l2_fwnode_reference_parse(\n> \treturn ret;\n> }\n> \n> +/*\n> + * v4l2_fwnode_reference_get_int_prop - parse a reference with integer\n> + *\t\t\t\t\targuments\n> + * @dev: struct device pointer\n> + * @notifier: notifier for @dev\n> + * @prop: the name of the property\n\n@index is not documented.\n\n> + * @props: the array of integer property names\n> + * @nprops: the number of integer properties\n\nproperties -> property names in @props\n\n> + *\n> + * Find fwnodes referred to by a property @prop, then under that iteratively\n> + * follow each child node which has a property the value matches the integer\n\n\"the value\" -> \"whose value\" or \"with a value that\"\n\nAt least, I think that's what you mean here.\n\nHow is @props/@nprops used?\n\n> + * argument at an index.\n\nI assume this should be \"the @index\"?\n\n> + *\n> + * Return: 0 on success\n> + *\t -ENOENT if no entries (or the property itself) were found\n> + *\t -EINVAL if property parsing otherwisefailed\n\nMissing space before \"failed\"\n\n> + *\t -ENOMEM if memory allocation failed\n> + */\n> +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(\n> +\tstruct fwnode_handle *fwnode, const char *prop, unsigned int index,\n> +\tconst char **props, unsigned int nprops)\n> +{\n> +\tstruct fwnode_reference_args fwnode_args;\n> +\tunsigned int *args = fwnode_args.args;\n> +\tstruct fwnode_handle *child;\n> +\tint ret;\n> +\n> +\t/*\n> +\t * Obtain remote fwnode as well as the integer arguments.\n> +\t *\n> +\t * To-do: handle -ENODATA when \"device property: Align return\n> +\t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n> +\t */\n> +\tret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,\n> +\t\t\t\t\t\t index, &fwnode_args);\n> +\tif (ret)\n> +\t\treturn ERR_PTR(ret == -ENODATA ? -ENOENT : ret);\n> +\n> +\t/*\n> +\t * Find a node in the tree under the referred fwnode corresponding the\n\nthe -> to the\n\n> +\t * integer arguments.\n> +\t */\n> +\tfwnode = fwnode_args.fwnode;\n> +\twhile (nprops) {\n\nThis can be 'while (nprops--) {'.\n\n> +\t\tu32 val;\n> +\n> +\t\t/* Loop over all child nodes under fwnode. */\n> +\t\tfwnode_for_each_child_node(fwnode, child) {\n> +\t\t\tif (fwnode_property_read_u32(child, *props, &val))\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\t/* Found property, see if its value matches. */\n> +\t\t\tif (val == *args)\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tfwnode_handle_put(fwnode);\n> +\n> +\t\t/* No property found; return an error here. */\n> +\t\tif (!child) {\n> +\t\t\tfwnode = ERR_PTR(-ENOENT);\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tprops++;\n> +\t\targs++;\n> +\t\tfwnode = child;\n> +\t\tnprops--;\n> +\t}\n> +\n> +\treturn fwnode;\n> +}\n\nYou really need to add an ACPI example as comment for this source code.\n\nI still don't understand the code. I know you pointed me to an example,\nbut I can't remember/find what it was. Either copy the example here or\npoint to the file containing the example (copying is best IMHO).\n\nRegards,\n\n\tHans\n\n> +\n> +/*\n> + * v4l2_fwnode_reference_parse_int_props - parse references for async sub-devices\n> + * @dev: struct device pointer\n> + * @notifier: notifier for @dev\n> + * @prop: the name of the property\n> + * @props: the array of integer property names\n> + * @nprops: the number of integer properties\n> + *\n> + * Use v4l2_fwnode_reference_get_int_prop to find fwnodes through reference in\n> + * property @prop with integer arguments with child nodes matching in properties\n> + * @props. Then, set up V4L2 async sub-devices for those fwnodes in the notifier\n> + * accordingly.\n> + *\n> + * While it is technically possible to use this function on DT, it is only\n> + * meaningful on ACPI. On Device tree you can refer to any node in the tree but\n> + * on ACPI the references are limited to devices.\n> + *\n> + * Return: 0 on success\n> + *\t -ENOENT if no entries (or the property itself) were found\n> + *\t -EINVAL if property parsing otherwisefailed\n> + *\t -ENOMEM if memory allocation failed\n> + */\n> +static int v4l2_fwnode_reference_parse_int_props(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tconst char *prop, const char **props, unsigned int nprops)\n> +{\n> +\tstruct fwnode_handle *fwnode;\n> +\tunsigned int index;\n> +\tint ret;\n> +\n> +\tfor (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(\n> +\t\t\t\t\t dev_fwnode(dev), prop, index, props,\n> +\t\t\t\t\t nprops))); index++)\n> +\t\tfwnode_handle_put(fwnode);\n> +\n> +\tif (PTR_ERR(fwnode) != -ENOENT)\n> +\t\treturn PTR_ERR(fwnode);\n> +\n> +\tret = v4l2_async_notifier_realloc(notifier,\n> +\t\t\t\t\t notifier->num_subdevs + index);\n> +\tif (ret)\n> +\t\treturn -ENOMEM;\n> +\n> +\tfor (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(\n> +\t\t\t\t\t dev_fwnode(dev), prop, index, props,\n> +\t\t\t\t\t nprops))); index++) {\n> +\t\tstruct v4l2_async_subdev *asd;\n> +\n> +\t\tif (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {\n> +\t\t\tret = -EINVAL;\n> +\t\t\tgoto error;\n> +\t\t}\n> +\n> +\t\tasd = kzalloc(sizeof(struct v4l2_async_subdev), GFP_KERNEL);\n> +\t\tif (!asd) {\n> +\t\t\tret = -ENOMEM;\n> +\t\t\tgoto error;\n> +\t\t}\n> +\n> +\t\tnotifier->subdevs[notifier->num_subdevs] = asd;\n> +\t\tasd->match.fwnode.fwnode = fwnode;\n> +\t\tasd->match_type = V4L2_ASYNC_MATCH_FWNODE;\n> +\t\tnotifier->num_subdevs++;\n> +\t}\n> +\n> +\treturn PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);\n> +\n> +error:\n> +\tfwnode_handle_put(fwnode);\n> +\treturn ret;\n> +}\n> +\n> MODULE_LICENSE(\"GPL\");\n> MODULE_AUTHOR(\"Sakari Ailus \");\n> MODULE_AUTHOR(\"Sylwester Nawrocki \");\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsYtc4xWVz9sPs\n\tfor ;\n\tWed, 13 Sep 2017 17:58:00 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751052AbdIMH56 (ORCPT\n\t);\n\tWed, 13 Sep 2017 03:57:58 -0400","from lb1-smtp-cloud8.xs4all.net ([194.109.24.21]:48106 \"EHLO\n\tlb1-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751031AbdIMH55 (ORCPT\n\t); Wed, 13 Sep 2017 03:57:57 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s2YSd0iIycQyLs2YWdxHqq; Wed, 13 Sep 2017 09:57:56 +0200"],"Subject":"Re: [PATCH v12 19/26] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, maxime.ripard@free-electrons.com,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-20-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"","Date":"Wed, 13 Sep 2017 09:57:52 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170912134200.19556-20-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfEZ3/cDMRieK15kBYnxLCgQv1gsZOyKucQ3d/BHEYGLoFA3e457RD04Rvo3koTOI4uZ9lz+2H9gdGTaLyd0wMHRVaEIuD8ANixwmFA9ir7aBEIM6B87a\n\tV/7A0UhWW1V/TB+yxpxC4xR0DHUHU6m+5ul2eGWWlIZlEhswpxbjGiljTzUNESI9NZ2k/z78Mg5NAah2Zz+/tTkAeg0ha85a/S+XIfraoKE9lHKq8GROur2H\n\tVc6lMkY3EaEjBdOuSPoxbAbG73+BzJcIw3W+2MXZFBmf607hR7m+ydMcw1OzjwgDHMVy909sWuVZMgzlpj0WyoZYVK+m/T6fHzNjlXgjijVmYUrcmmBuoEfI\n\tCVXVP6xbGlPKs2OoOREL7v88KGvReVAKmZVCTQaov+4g8jBSSC0npfJHym3yTc4KVCoQMOqGIXi09hoOdgoV1BvvYarHRt/FIN0EzAEnN356T3n/VBa8UeBk\n\tokASZU/anw12EcetNVwklm9c+CsKu5YsFBjlFwIOjuZbZo1KzjOqcZGCU8M=","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767655,"web_url":"http://patchwork.ozlabs.org/comment/1767655/","msgid":"","list_archive_url":null,"date":"2017-09-13T07:59:08","subject":"Re: [PATCH v12 23/26] et8ek8: Add support for flash and lens devices","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> From: Pavel Machek \n> \n> Parse async sub-devices by using\n> v4l2_subdev_fwnode_reference_parse_sensor_common().\n> \n> These types devices aren't directly related to the sensor, but are\n> nevertheless handled by the et8ek8 driver due to the relationship of these\n> component to the main part of the camera module --- the sensor.\n> \n> [Sakari Ailus: Rename fwnode function, check for ret < 0 only.]\n> Signed-off-by: Pavel Machek \n> Signed-off-by: Sakari Ailus \n\nAcked-by: Hans Verkuil \n\nRegards,\n\n\tHans\n\n> ---\n> drivers/media/i2c/et8ek8/et8ek8_driver.c | 21 ++++++++++++++++++++-\n> 1 file changed, 20 insertions(+), 1 deletion(-)\n> \n> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c\n> index c14f0fd6ded3..0ef1b8025935 100644\n> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c\n> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c\n> @@ -34,10 +34,12 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> #include \n> #include \n> +#include \n> \n> #include \"et8ek8_reg.h\"\n> \n> @@ -46,6 +48,7 @@\n> #define ET8EK8_MAX_MSG\t\t8\n> \n> struct et8ek8_sensor {\n> +\tstruct v4l2_async_notifier notifier;\n> \tstruct v4l2_subdev subdev;\n> \tstruct media_pad pad;\n> \tstruct v4l2_mbus_framefmt format;\n> @@ -1446,6 +1449,11 @@ static int et8ek8_probe(struct i2c_client *client,\n> \tsensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;\n> \tsensor->subdev.internal_ops = &et8ek8_internal_ops;\n> \n> +\tret = v4l2_async_notifier_parse_fwnode_sensor_common(\n> +\t\t&client->dev, &sensor->notifier);\n> +\tif (ret < 0)\n> +\t\tgoto err_release;\n> +\n> \tsensor->pad.flags = MEDIA_PAD_FL_SOURCE;\n> \tret = media_entity_pads_init(&sensor->subdev.entity, 1, &sensor->pad);\n> \tif (ret < 0) {\n> @@ -1453,18 +1461,27 @@ static int et8ek8_probe(struct i2c_client *client,\n> \t\tgoto err_mutex;\n> \t}\n> \n> +\tret = v4l2_async_subdev_notifier_register(&sensor->subdev,\n> +\t\t\t\t\t\t &sensor->notifier);\n> +\tif (ret)\n> +\t\tgoto err_entity;\n> +\n> \tret = v4l2_async_register_subdev(&sensor->subdev);\n> \tif (ret < 0)\n> -\t\tgoto err_entity;\n> +\t\tgoto err_async;\n> \n> \tdev_dbg(dev, \"initialized!\\n\");\n> \n> \treturn 0;\n> \n> +err_async:\n> +\tv4l2_async_notifier_unregister(&sensor->notifier);\n> err_entity:\n> \tmedia_entity_cleanup(&sensor->subdev.entity);\n> err_mutex:\n> \tmutex_destroy(&sensor->power_lock);\n> +err_release:\n> +\tv4l2_async_notifier_release(&sensor->notifier);\n> \treturn ret;\n> }\n> \n> @@ -1480,6 +1497,8 @@ static int __exit et8ek8_remove(struct i2c_client *client)\n> \t}\n> \n> \tv4l2_device_unregister_subdev(&sensor->subdev);\n> +\tv4l2_async_notifier_unregister(&sensor->notifier);\n> +\tv4l2_async_notifier_release(&sensor->notifier);\n> \tdevice_remove_file(&client->dev, &dev_attr_priv_mem);\n> \tv4l2_ctrl_handler_free(&sensor->ctrl_handler);\n> \tv4l2_async_unregister_subdev(&sensor->subdev);\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsYw21QYjz9sPm\n\tfor ;\n\tWed, 13 Sep 2017 17:59:14 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751238AbdIMH7N (ORCPT\n\t);\n\tWed, 13 Sep 2017 03:59:13 -0400","from lb1-smtp-cloud8.xs4all.net ([194.109.24.21]:59923 \"EHLO\n\tlb1-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751031AbdIMH7M (ORCPT\n\t); Wed, 13 Sep 2017 03:59:12 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s2Zgd0jGrcQyLs2ZjdxIJl; Wed, 13 Sep 2017 09:59:11 +0200"],"Subject":"Re: [PATCH v12 23/26] et8ek8: Add support for flash and lens devices","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, maxime.ripard@free-electrons.com,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-24-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"","Date":"Wed, 13 Sep 2017 09:59:08 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170912134200.19556-24-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfMzmdHDRvBbPS0Vb70ZYKaQYji1jJJAh7j799zrxM18ouX+5ALdtj1+pqgSwQLTJ8LjXAPVEe6aEYNHAh0rbW4dc/sWRSBB01QWQx12nq50JjFBCY2Po\n\tEwG4a3l2PIuiHc6WPu9jvlt/ZUYv3VJRUZzqJvrE5NzdrHdcYB7xB7HqFWBkgD1iaHuzIRtxZ5Bz9Z5bAP9ba0ru5y2XM6zfIlVGGJl4g8IkVNqwH2jdXMT+\n\tL4TCZsvDArSx1WnuZYLW06Oan/EkuT/96Dcdr4tpqSurhsjvuVVVTeQs5g7Pjo4MjGOIVXg0lFDBfIqUZ3BGe5Nr0I+j03+23ST1T/u5/mcSBi3COlRbQPuB\n\tXfYhriK87W+PzHEd7JsTX61JdAXQ4Q==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767667,"web_url":"http://patchwork.ozlabs.org/comment/1767667/","msgid":"<20170913081821.4kqf2kfmkidkaqah@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-13T08:18:21","subject":"Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph\n\tendpoints, per port","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Hans,\n\nThanks for the review.\n\nOn Wed, Sep 13, 2017 at 09:06:11AM +0200, Hans Verkuil wrote:\n> On 09/13/2017 09:01 AM, Hans Verkuil wrote:\n> > On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> >> This is just like like v4l2_async_notifier_parse_fwnode_endpoints but it\n> >> only parses endpoints on a single port. The behaviour is useful on devices\n> >> that have both sinks and sources, in other words only some of these should\n> >> be parsed.\n> \n> I forgot to mention that I think the log message can be improved: it is not\n> clear why you would only parse some of the ports for devices that have both\n> sinks and sources. That should be explained better. And probably also in the\n> header documentation.\n\nI'll add both.","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsZLD03fZz9sNr\n\tfor ;\n\tWed, 13 Sep 2017 18:18:27 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750776AbdIMIS0 (ORCPT\n\t);\n\tWed, 13 Sep 2017 04:18:26 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:46280 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1750737AbdIMISZ (ORCPT\n\t); Wed, 13 Sep 2017 04:18:25 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 7D854600BE;\n\tWed, 13 Sep 2017 11:18:22 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1ds2sH-00045b-Uv; Wed, 13 Sep 2017 11:18:22 +0300"],"Date":"Wed, 13 Sep 2017 11:18:21 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph\n\tendpoints, per port","Message-ID":"<20170913081821.4kqf2kfmkidkaqah@valkosipuli.retiisi.org.uk>","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-7-sakari.ailus@linux.intel.com>\n\t<0b73f576-c37b-ad58-6f74-71ffc3f8f176@xs4all.nl>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767672,"web_url":"http://patchwork.ozlabs.org/comment/1767672/","msgid":"<20170913082901.fbxxphn7s3ljn3mc@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-13T08:29:02","subject":"Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to\n\tsub-devices","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Hans,\n\nOn Wed, Sep 13, 2017 at 09:17:08AM +0200, Hans Verkuil wrote:\n> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> > Registering a notifier has required the knowledge of struct v4l2_device\n> > for the reason that sub-devices generally are registered to the\n> > v4l2_device (as well as the media device, also available through\n> > v4l2_device).\n> > \n> > This information is not available for sub-device drivers at probe time.\n> > \n> > What this patch does is that it allows registering notifiers without\n> > having v4l2_device around. Instead the sub-device pointer is stored in the\n> > notifier. Once the sub-device of the driver that registered the notifier\n> > is registered, the notifier will gain the knowledge of the v4l2_device,\n> > and the binding of async sub-devices from the sub-device driver's notifier\n> > may proceed.\n> > \n> > The root notifier's complete callback is only called when all sub-device\n> > notifiers are completed.\n> > \n> > Signed-off-by: Sakari Ailus \n> \n> Just two small comments (see below).\n> \n> After changing those (the first is up to you) you can add my:\n> \n> Acked-by: Hans Verkuil \n\nThanks; please see my comments below.\n\n...\n\n> > +/* Return true if all sub-device notifiers are complete, false otherwise. */\n> > +static bool v4l2_async_subdev_notifiers_complete(\n> > +\tstruct v4l2_async_notifier *notifier)\n> > +{\n> > +\tstruct v4l2_subdev *sd;\n> > +\n> > +\tif (!list_empty(¬ifier->waiting))\n> > +\t\treturn false;\n> > +\n> > +\tlist_for_each_entry(sd, ¬ifier->done, async_list) {\n> > +\t\tstruct v4l2_async_notifier *subdev_notifier =\n> > +\t\t\tv4l2_async_find_subdev_notifier(sd);\n> > +\n> > +\t\tif (!subdev_notifier)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tif (!v4l2_async_subdev_notifiers_complete(subdev_notifier))\n> \n> I think it is better to change this to:\n> \n> \t\tif (subdev_notifier &&\n> \t\t !v4l2_async_subdev_notifiers_complete(subdev_notifier))\n> \n> and drop this if..continue above. That's a bit overkill in this simple case.\n> \n> It's up to you, though.\n\nYes, makes sense.\n\n...\n\n> > +/* Try completing a notifier. */\n> > +static int v4l2_async_notifier_try_complete(\n> > +\tstruct v4l2_async_notifier *notifier)\n> > +{\n> > +\tdo {\n> > +\t\tint ret;\n> > +\n> > +\t\t/* Any local async sub-devices left? */\n> > +\t\tif (!list_empty(¬ifier->waiting))\n> > +\t\t\treturn 0;\n> > +\n> > +\t\t/*\n> > +\t\t * Any sub-device notifiers waiting for async subdevs\n> > +\t\t * to be bound?\n> > +\t\t */\n> > +\t\tif (!v4l2_async_subdev_notifiers_complete(notifier))\n> > +\t\t\treturn 0;\n> > +\n> > +\t\t/* Proceed completing the notifier */\n> > +\t\tret = v4l2_async_notifier_call_complete(notifier);\n> > +\t\tif (ret < 0)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\t/*\n> > +\t\t * Obtain notifier's parent. If there is one, repeat\n> > +\t\t * the process, otherwise we're done here.\n> > +\t\t */\n> > +\t} while ((notifier = notifier->parent));\n> \n> I'd change this to:\n> \n> \t\tnotifier = notifier->parent;\n> \t} while (notifier);\n> \n> Assignment in a condition is frowned upon, and there is no need to do that\n> here.\n\nWouldn't that equally apply to any statement with side effects? In other\nwords, what you're suggesting for patch 19? :-)","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsZZZ1Ypcz9sNr\n\tfor ;\n\tWed, 13 Sep 2017 18:29:10 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751120AbdIMI3I (ORCPT\n\t);\n\tWed, 13 Sep 2017 04:29:08 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:46404 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751020AbdIMI3F (ORCPT\n\t); Wed, 13 Sep 2017 04:29:05 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 6E253600D5;\n\tWed, 13 Sep 2017 11:29:03 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1ds32c-00045j-S0; Wed, 13 Sep 2017 11:29:03 +0300"],"Date":"Wed, 13 Sep 2017 11:29:02 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to\n\tsub-devices","Message-ID":"<20170913082901.fbxxphn7s3ljn3mc@valkosipuli.retiisi.org.uk>","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-16-sakari.ailus@linux.intel.com>\n\t<575bf15b-62d2-3a51-d550-d462578471f7@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<575bf15b-62d2-3a51-d550-d462578471f7@xs4all.nl>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767710,"web_url":"http://patchwork.ozlabs.org/comment/1767710/","msgid":"<3cb2b5ad-97e2-0f60-d4a3-2f36431dc358@xs4all.nl>","list_archive_url":null,"date":"2017-09-13T09:25:27","subject":"Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to\n\tsub-devices","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/13/17 10:29, Sakari Ailus wrote:\n> Hi Hans,\n> \n> On Wed, Sep 13, 2017 at 09:17:08AM +0200, Hans Verkuil wrote:\n>> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n>>> Registering a notifier has required the knowledge of struct v4l2_device\n>>> for the reason that sub-devices generally are registered to the\n>>> v4l2_device (as well as the media device, also available through\n>>> v4l2_device).\n>>>\n>>> This information is not available for sub-device drivers at probe time.\n>>>\n>>> What this patch does is that it allows registering notifiers without\n>>> having v4l2_device around. Instead the sub-device pointer is stored in the\n>>> notifier. Once the sub-device of the driver that registered the notifier\n>>> is registered, the notifier will gain the knowledge of the v4l2_device,\n>>> and the binding of async sub-devices from the sub-device driver's notifier\n>>> may proceed.\n>>>\n>>> The root notifier's complete callback is only called when all sub-device\n>>> notifiers are completed.\n>>>\n>>> Signed-off-by: Sakari Ailus \n>>\n>> Just two small comments (see below).\n>>\n>> After changing those (the first is up to you) you can add my:\n>>\n>> Acked-by: Hans Verkuil \n> \n> Thanks; please see my comments below.\n> \n> ...\n> \n>>> +/* Return true if all sub-device notifiers are complete, false otherwise. */\n>>> +static bool v4l2_async_subdev_notifiers_complete(\n>>> +\tstruct v4l2_async_notifier *notifier)\n>>> +{\n>>> +\tstruct v4l2_subdev *sd;\n>>> +\n>>> +\tif (!list_empty(¬ifier->waiting))\n>>> +\t\treturn false;\n>>> +\n>>> +\tlist_for_each_entry(sd, ¬ifier->done, async_list) {\n>>> +\t\tstruct v4l2_async_notifier *subdev_notifier =\n>>> +\t\t\tv4l2_async_find_subdev_notifier(sd);\n>>> +\n>>> +\t\tif (!subdev_notifier)\n>>> +\t\t\tcontinue;\n>>> +\n>>> +\t\tif (!v4l2_async_subdev_notifiers_complete(subdev_notifier))\n>>\n>> I think it is better to change this to:\n>>\n>> \t\tif (subdev_notifier &&\n>> \t\t !v4l2_async_subdev_notifiers_complete(subdev_notifier))\n>>\n>> and drop this if..continue above. That's a bit overkill in this simple case.\n>>\n>> It's up to you, though.\n> \n> Yes, makes sense.\n> \n> ...\n> \n>>> +/* Try completing a notifier. */\n>>> +static int v4l2_async_notifier_try_complete(\n>>> +\tstruct v4l2_async_notifier *notifier)\n>>> +{\n>>> +\tdo {\n>>> +\t\tint ret;\n>>> +\n>>> +\t\t/* Any local async sub-devices left? */\n>>> +\t\tif (!list_empty(¬ifier->waiting))\n>>> +\t\t\treturn 0;\n>>> +\n>>> +\t\t/*\n>>> +\t\t * Any sub-device notifiers waiting for async subdevs\n>>> +\t\t * to be bound?\n>>> +\t\t */\n>>> +\t\tif (!v4l2_async_subdev_notifiers_complete(notifier))\n>>> +\t\t\treturn 0;\n>>> +\n>>> +\t\t/* Proceed completing the notifier */\n>>> +\t\tret = v4l2_async_notifier_call_complete(notifier);\n>>> +\t\tif (ret < 0)\n>>> +\t\t\treturn ret;\n>>> +\n>>> +\t\t/*\n>>> +\t\t * Obtain notifier's parent. If there is one, repeat\n>>> +\t\t * the process, otherwise we're done here.\n>>> +\t\t */\n>>> +\t} while ((notifier = notifier->parent));\n>>\n>> I'd change this to:\n>>\n>> \t\tnotifier = notifier->parent;\n>> \t} while (notifier);\n>>\n>> Assignment in a condition is frowned upon, and there is no need to do that\n>> here.\n> \n> Wouldn't that equally apply to any statement with side effects? In other\n> words, what you're suggesting for patch 19? :-)\n\nI don't like it there either, but rewriting that would make the code quite a\nbit longer and you enter a gray area between 'no side-effects' and 'readability'.\nIn cases like that I tend to accept the preference of the author of the code.\n\nIn the case of this do...while the 'no side-effects' version is just as readable\nif not more so than the 'side-effect' version.\n\nAt least, that's my reasoning.\n\nRegards,\n\n\tHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsbqg37RMz9sNr\n\tfor ;\n\tWed, 13 Sep 2017 19:25:35 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751662AbdIMJZd (ORCPT\n\t);\n\tWed, 13 Sep 2017 05:25:33 -0400","from lb1-smtp-cloud8.xs4all.net ([194.109.24.21]:38300 \"EHLO\n\tlb1-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751062AbdIMJZd (ORCPT\n\t); Wed, 13 Sep 2017 05:25:33 -0400","from [IPv6:2001:420:44c1:2579:c460:2dc1:1388:52dc]\n\t([IPv6:2001:420:44c1:2579:c460:2dc1:1388:52dc])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s3vDd1mcNcQyLs3vHdxqOI; Wed, 13 Sep 2017 11:25:31 +0200"],"Subject":"Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to\n\tsub-devices","To":"Sakari Ailus ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-16-sakari.ailus@linux.intel.com>\n\t<575bf15b-62d2-3a51-d550-d462578471f7@xs4all.nl>\n\t<20170913082901.fbxxphn7s3ljn3mc@valkosipuli.retiisi.org.uk>","From":"Hans Verkuil ","Message-ID":"<3cb2b5ad-97e2-0f60-d4a3-2f36431dc358@xs4all.nl>","Date":"Wed, 13 Sep 2017 11:25:27 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170913082901.fbxxphn7s3ljn3mc@valkosipuli.retiisi.org.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfIipATAuosj48xh0rz4bjdKCPZlLcYB51oKX2IkUCiCKckVUBhWRbvEOo0Q+nM5fDDIAZ26oQb+WVPcmf8a+ccHmwRnaY4AsIEuVFakcUOtOFcn7t1FB\n\taf8iyEkmWix8upfCLXVDn/pdxcQYyG9fINH7cEKz8TR572sERnsyrl/9qWrc7uyfxBGQYdcFBdGnVxLBmmQ0Pq9uPmiE5FIqZ0Gju3MHA4a8p7sEkrLuEZUV\n\tKzT6fjB+gvbzh1DxvJuc+XqJJHYTz5YrUu4RgKi4ZIfdTAbNAeibI2gn+OtqdLbfEUXHZTuI/cmt+VQvvWnhA4o/+I/hJc4uQCCJJhjY2gmz6oA/Sx5uYw6/\n\tXQGVQ2oR8lMEO6PdDSm+iUFZeRe9XDcz0zg34SzPgP1OSTRqHel/eMX0vZZdN+7jz30EvjIVrDCTblhIkULCBP+9F1z5uOKebOMxzWtShep3FxfWQQxrEXXw\n\tcoXjcgYZTkIOBF9qBWSAZheURzVZQAbR04jPLO2ovoGLiSxLS4U0eb0clenQtxpOOfHcDfPhO/8Kd7Ie","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767713,"web_url":"http://patchwork.ozlabs.org/comment/1767713/","msgid":"","list_archive_url":null,"date":"2017-09-13T09:28:44","subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/13/17 11:24, Sakari Ailus wrote:\n> Hi Hans,\n> \n> Thanks for the review!\n> \n> On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:\n>> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n>>> Add function v4l2_fwnode_reference_count() for counting external\n>>\n>> ???? There is no function v4l2_fwnode_reference_count()?!\n> \n> It got removed during the revisions but the commit message was not changed\n> accordingly, I do that now.\n> \n>>\n>>> references and v4l2_fwnode_reference_parse() for parsing them as async\n>>> sub-devices.\n>>>\n>>> This can be done on e.g. flash or lens async sub-devices that are not part\n>>> of but are associated with a sensor.\n>>>\n>>> struct v4l2_async_notifier.max_subdevs field is added to contain the\n>>> maximum number of sub-devices in a notifier to reflect the memory\n>>> allocated for the subdevs array.\n>>\n>> You forgot to remove this outdated paragraph.\n> \n> Oops. Removed it now.\n> \n>>\n>>>\n>>> Signed-off-by: Sakari Ailus \n>>> ---\n>>> drivers/media/v4l2-core/v4l2-fwnode.c | 69 +++++++++++++++++++++++++++++++++++\n>>> 1 file changed, 69 insertions(+)\n>>>\n>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> index 44ee35f6aad5..a32473f95be1 100644\n>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n>>> }\n>>> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);\n>>> \n>>> +/*\n>>> + * v4l2_fwnode_reference_parse - parse references for async sub-devices\n>>> + * @dev: the device node the properties of which are parsed for references\n>>> + * @notifier: the async notifier where the async subdevs will be added\n>>> + * @prop: the name of the property\n>>> + *\n>>> + * Return: 0 on success\n>>> + *\t -ENOENT if no entries were found\n>>> + *\t -ENOMEM if memory allocation failed\n>>> + *\t -EINVAL if property parsing failed\n>>> + */\n>>> +static int v4l2_fwnode_reference_parse(\n>>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n>>> +\tconst char *prop)\n>>> +{\n>>> +\tstruct fwnode_reference_args args;\n>>> +\tunsigned int index;\n>>> +\tint ret;\n>>> +\n>>> +\tfor (index = 0;\n>>> +\t !(ret = fwnode_property_get_reference_args(\n>>> +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args));\n>>> +\t index++)\n>>> +\t\tfwnode_handle_put(args.fwnode);\n>>> +\n>>> +\tif (!index)\n>>> +\t\treturn -ENOENT;\n>>> +\n>>> +\t/*\n>>> +\t * To-do: handle -ENODATA when \"device property: Align return\n>>> +\t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n>>> +\t */\n>>> +\tif (ret != -ENOENT && ret != -ENODATA)\n>>\n>> So while that patch referenced in the To-do above is not merged yet,\n>> what does fwnode_property_get_reference_args return? ENOENT or ENODATA?\n>> Or ENOENT now and ENODATA later? Or vice versa?\n>>\n>> I can't tell based on that information whether this code is correct or not.\n>>\n>> The comment needs to explain this a bit better.\n> \n> I'll add this:\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index a32473f95be1..74fcc3ba9ebd 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(\n> \t/*\n> \t * To-do: handle -ENODATA when \"device property: Align return\n> \t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n\nSo after this patch referred to in the To-do is applied it will only\nreturn ENODATA?\n\nIn that case, change 'handle' to 'handle only'.\n\n> +\t * Right now, both -ENODATA and -ENOENT signal the end of\n> +\t * references where only a single error code should be used\n> +\t * for the purpose.\n> \t */\n> \tif (ret != -ENOENT && ret != -ENODATA)\n> \t\treturn ret;\n> \n\nRegards,\n\n\tHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsbvW5jLKz9sPm\n\tfor ;\n\tWed, 13 Sep 2017 19:28:55 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752278AbdIMJ2w (ORCPT\n\t);\n\tWed, 13 Sep 2017 05:28:52 -0400","from lb1-smtp-cloud8.xs4all.net ([194.109.24.21]:53956 \"EHLO\n\tlb1-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1752279AbdIMJ2t (ORCPT\n\t); Wed, 13 Sep 2017 05:28:49 -0400","from [IPv6:2001:420:44c1:2579:c460:2dc1:1388:52dc]\n\t([IPv6:2001:420:44c1:2579:c460:2dc1:1388:52dc])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid s3yOd1p9ycQyLs3yRdxrgQ; Wed, 13 Sep 2017 11:28:48 +0200"],"Subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","To":"Sakari Ailus ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-19-sakari.ailus@linux.intel.com>\n\t<020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl>\n\t<20170913092430.cbdgerkhiuxakbxv@valkosipuli.retiisi.org.uk>","From":"Hans Verkuil ","Message-ID":"","Date":"Wed, 13 Sep 2017 11:28:44 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170913092430.cbdgerkhiuxakbxv@valkosipuli.retiisi.org.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfK2Sv3db4/TX79+/SRQf+uVeH6I0t2+MTafcUBdYS6eoyKgLY9tS6BujW2heWKNotbXgakd8ozd3XKiz6zRhO1ru2fShiYbwc7RbbotWXfa7Zi4H6L31\n\txXzqLsKtrRgAKNQoU1Wt9TqSlroOuTC4o69co6APO9cNRHPM5JqRKiIDvPh7yF8gk2+rFYD1tEJSaJVnqIkdrD4NeDUel9UzbSW6rPo6ZFyVVwSNWUnP90/U\n\tWc9ig8PbzmIMfV5QhBbZ+UB1Sk+aq5LRnXRjbCpH74LZwqCubtbuMsNwUBiblsduKCCm+YMD3K18pi7cD/3r9KCB+jpALMXREdYC/7rr+rShr2QLVuOHBxJp\n\tbkfRRmnPk8bWkTebLmMfzOvsCxstJSdjD5zGdK40kCtKWWZiCnr6IM/xrcCC/drdSfzv06wvZT7d5mA4R96IOfw0bnOgrLgEzyuuzpfPRVFlupvX+ZvF3f3f\n\tSIElSWUYEHKt29n8ZF8RPQcw1MOvvRz3J1mvzquDr4ebVzOn3RJMOZK5LnGuyIZSqqmNDRFOHh9uS/Fk","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767751,"web_url":"http://patchwork.ozlabs.org/comment/1767751/","msgid":"<20170913100449.7pgg6pdglo3b43ml@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-13T10:04:50","subject":"Re: [PATCH v12 19/26] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Hans,\n\nOn Wed, Sep 13, 2017 at 09:57:52AM +0200, Hans Verkuil wrote:\n> The subject still has the 'interger' typo.\n> \n> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> > v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under\n> > the device's own fwnode, it will follow child fwnodes with the given\n> > property -- value pair and return the resulting fwnode.\n> \n> As suggested before: 'property-value' is easier to read than ' -- '.\n\nOops. I must have missed some of your comments while making changes.\nApologies for that.\n\n> \n> > \n> > Signed-off-by: Sakari Ailus \n> > ---\n> > drivers/media/v4l2-core/v4l2-fwnode.c | 145 ++++++++++++++++++++++++++++++++++\n> > 1 file changed, 145 insertions(+)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > index a32473f95be1..a07599a8f647 100644\n> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > @@ -567,6 +567,151 @@ static int v4l2_fwnode_reference_parse(\n> > \treturn ret;\n> > }\n> > \n> > +/*\n> > + * v4l2_fwnode_reference_get_int_prop - parse a reference with integer\n> > + *\t\t\t\t\targuments\n> > + * @dev: struct device pointer\n> > + * @notifier: notifier for @dev\n> > + * @prop: the name of the property\n> \n> @index is not documented.\n\nFixed.\n\n> \n> > + * @props: the array of integer property names\n> > + * @nprops: the number of integer properties\n> \n> properties -> property names in @props\n\nFixed.\n\n> \n> > + *\n> > + * Find fwnodes referred to by a property @prop, then under that iteratively\n> > + * follow each child node which has a property the value matches the integer\n> \n> \"the value\" -> \"whose value\" or \"with a value that\"\n> \n> At least, I think that's what you mean here.\n> \n> How is @props/@nprops used?\n> \n> > + * argument at an index.\n> \n> I assume this should be \"the @index\"?\n\nUm, no. This is the index to the @props array. I'll clarify the\ndocumentation for this function.\n\n> \n> > + *\n> > + * Return: 0 on success\n> > + *\t -ENOENT if no entries (or the property itself) were found\n> > + *\t -EINVAL if property parsing otherwisefailed\n> \n> Missing space before \"failed\"\n\nFixed.\n\n> \n> > + *\t -ENOMEM if memory allocation failed\n> > + */\n> > +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(\n> > +\tstruct fwnode_handle *fwnode, const char *prop, unsigned int index,\n> > +\tconst char **props, unsigned int nprops)\n> > +{\n> > +\tstruct fwnode_reference_args fwnode_args;\n> > +\tunsigned int *args = fwnode_args.args;\n> > +\tstruct fwnode_handle *child;\n> > +\tint ret;\n> > +\n> > +\t/*\n> > +\t * Obtain remote fwnode as well as the integer arguments.\n> > +\t *\n> > +\t * To-do: handle -ENODATA when \"device property: Align return\n> > +\t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n> > +\t */\n> > +\tret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,\n> > +\t\t\t\t\t\t index, &fwnode_args);\n> > +\tif (ret)\n> > +\t\treturn ERR_PTR(ret == -ENODATA ? -ENOENT : ret);\n> > +\n> > +\t/*\n> > +\t * Find a node in the tree under the referred fwnode corresponding the\n> \n> the -> to the\n\nFixed.\n\n> \n> > +\t * integer arguments.\n> > +\t */\n> > +\tfwnode = fwnode_args.fwnode;\n> > +\twhile (nprops) {\n> \n> This can be 'while (nprops--) {'.\n\nChanged.\n\n> \n> > +\t\tu32 val;\n> > +\n> > +\t\t/* Loop over all child nodes under fwnode. */\n> > +\t\tfwnode_for_each_child_node(fwnode, child) {\n> > +\t\t\tif (fwnode_property_read_u32(child, *props, &val))\n> > +\t\t\t\tcontinue;\n> > +\n> > +\t\t\t/* Found property, see if its value matches. */\n> > +\t\t\tif (val == *args)\n> > +\t\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tfwnode_handle_put(fwnode);\n> > +\n> > +\t\t/* No property found; return an error here. */\n> > +\t\tif (!child) {\n> > +\t\t\tfwnode = ERR_PTR(-ENOENT);\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tprops++;\n> > +\t\targs++;\n> > +\t\tfwnode = child;\n> > +\t\tnprops--;\n> > +\t}\n> > +\n> > +\treturn fwnode;\n> > +}\n> \n> You really need to add an ACPI example as comment for this source code.\n\nI can copy the relevant portions of the LED flash example, and remove them\nonce the example is merged to mainline. That example really doesn't belong\nhere though.\n\n> \n> I still don't understand the code. I know you pointed me to an example,\n> but I can't remember/find what it was. Either copy the example here or\n> point to the file containing the example (copying is best IMHO).\n\n:-)","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xscj36Ycjz9sPm\n\tfor ;\n\tWed, 13 Sep 2017 20:04:55 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752025AbdIMKEy (ORCPT\n\t);\n\tWed, 13 Sep 2017 06:04:54 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:47422 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751714AbdIMKEx (ORCPT\n\t); Wed, 13 Sep 2017 06:04:53 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 75832600F5;\n\tWed, 13 Sep 2017 13:04:51 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1ds4XK-00046M-7X; Wed, 13 Sep 2017 13:04:50 +0300"],"Date":"Wed, 13 Sep 2017 13:04:50 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v12 19/26] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","Message-ID":"<20170913100449.7pgg6pdglo3b43ml@valkosipuli.retiisi.org.uk>","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-20-sakari.ailus@linux.intel.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767752,"web_url":"http://patchwork.ozlabs.org/comment/1767752/","msgid":"<20170913100714.5qy32qly3vlxguxz@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-13T10:07:15","subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Hans,\n\nOn Wed, Sep 13, 2017 at 11:28:44AM +0200, Hans Verkuil wrote:\n> On 09/13/17 11:24, Sakari Ailus wrote:\n> > Hi Hans,\n> > \n> > Thanks for the review!\n> > \n> > On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:\n> >> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> >>> Add function v4l2_fwnode_reference_count() for counting external\n> >>\n> >> ???? There is no function v4l2_fwnode_reference_count()?!\n> > \n> > It got removed during the revisions but the commit message was not changed\n> > accordingly, I do that now.\n> > \n> >>\n> >>> references and v4l2_fwnode_reference_parse() for parsing them as async\n> >>> sub-devices.\n> >>>\n> >>> This can be done on e.g. flash or lens async sub-devices that are not part\n> >>> of but are associated with a sensor.\n> >>>\n> >>> struct v4l2_async_notifier.max_subdevs field is added to contain the\n> >>> maximum number of sub-devices in a notifier to reflect the memory\n> >>> allocated for the subdevs array.\n> >>\n> >> You forgot to remove this outdated paragraph.\n> > \n> > Oops. Removed it now.\n> > \n> >>\n> >>>\n> >>> Signed-off-by: Sakari Ailus \n> >>> ---\n> >>> drivers/media/v4l2-core/v4l2-fwnode.c | 69 +++++++++++++++++++++++++++++++++++\n> >>> 1 file changed, 69 insertions(+)\n> >>>\n> >>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> index 44ee35f6aad5..a32473f95be1 100644\n> >>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n> >>> }\n> >>> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);\n> >>> \n> >>> +/*\n> >>> + * v4l2_fwnode_reference_parse - parse references for async sub-devices\n> >>> + * @dev: the device node the properties of which are parsed for references\n> >>> + * @notifier: the async notifier where the async subdevs will be added\n> >>> + * @prop: the name of the property\n> >>> + *\n> >>> + * Return: 0 on success\n> >>> + *\t -ENOENT if no entries were found\n> >>> + *\t -ENOMEM if memory allocation failed\n> >>> + *\t -EINVAL if property parsing failed\n> >>> + */\n> >>> +static int v4l2_fwnode_reference_parse(\n> >>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> >>> +\tconst char *prop)\n> >>> +{\n> >>> +\tstruct fwnode_reference_args args;\n> >>> +\tunsigned int index;\n> >>> +\tint ret;\n> >>> +\n> >>> +\tfor (index = 0;\n> >>> +\t !(ret = fwnode_property_get_reference_args(\n> >>> +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args));\n> >>> +\t index++)\n> >>> +\t\tfwnode_handle_put(args.fwnode);\n> >>> +\n> >>> +\tif (!index)\n> >>> +\t\treturn -ENOENT;\n> >>> +\n> >>> +\t/*\n> >>> +\t * To-do: handle -ENODATA when \"device property: Align return\n> >>> +\t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n> >>> +\t */\n> >>> +\tif (ret != -ENOENT && ret != -ENODATA)\n> >>\n> >> So while that patch referenced in the To-do above is not merged yet,\n> >> what does fwnode_property_get_reference_args return? ENOENT or ENODATA?\n> >> Or ENOENT now and ENODATA later? Or vice versa?\n> >>\n> >> I can't tell based on that information whether this code is correct or not.\n> >>\n> >> The comment needs to explain this a bit better.\n> > \n> > I'll add this:\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > index a32473f95be1..74fcc3ba9ebd 100644\n> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(\n> > \t/*\n> > \t * To-do: handle -ENODATA when \"device property: Align return\n> > \t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n> \n> So after this patch referred to in the To-do is applied it will only\n> return ENODATA?\n> \n> In that case, change 'handle' to 'handle only'.\n\nThat depends a bit in which form the patch will be eventually accepted. The\nunderlying issue there is that different error codes are used to signal\nconditions for out-of-bounds access and missing entry. After aligning them\nthe code here can be updated.\n\n> \n> > +\t * Right now, both -ENODATA and -ENOENT signal the end of\n> > +\t * references where only a single error code should be used\n> > +\t * for the purpose.\n> > \t */\n> > \tif (ret != -ENOENT && ret != -ENODATA)\n> > \t\treturn ret;\n> >","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsclr1bTGz9sBW\n\tfor ;\n\tWed, 13 Sep 2017 20:07:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752057AbdIMKHS (ORCPT\n\t);\n\tWed, 13 Sep 2017 06:07:18 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:47466 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1752012AbdIMKHR (ORCPT\n\t); Wed, 13 Sep 2017 06:07:17 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id BBEAD600F5;\n\tWed, 13 Sep 2017 13:07:15 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1ds4Zf-00046S-7l; Wed, 13 Sep 2017 13:07:15 +0300"],"Date":"Wed, 13 Sep 2017 13:07:15 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","Message-ID":"<20170913100714.5qy32qly3vlxguxz@valkosipuli.retiisi.org.uk>","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-19-sakari.ailus@linux.intel.com>\n\t<020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl>\n\t<20170913092430.cbdgerkhiuxakbxv@valkosipuli.retiisi.org.uk>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767769,"web_url":"http://patchwork.ozlabs.org/comment/1767769/","msgid":"","list_archive_url":null,"date":"2017-09-13T10:32:17","subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/13/17 12:07, Sakari Ailus wrote:\n> Hi Hans,\n> \n> On Wed, Sep 13, 2017 at 11:28:44AM +0200, Hans Verkuil wrote:\n>> On 09/13/17 11:24, Sakari Ailus wrote:\n>>> Hi Hans,\n>>>\n>>> Thanks for the review!\n>>>\n>>> On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:\n>>>> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n>>>>> Add function v4l2_fwnode_reference_count() for counting external\n>>>>\n>>>> ???? There is no function v4l2_fwnode_reference_count()?!\n>>>\n>>> It got removed during the revisions but the commit message was not changed\n>>> accordingly, I do that now.\n>>>\n>>>>\n>>>>> references and v4l2_fwnode_reference_parse() for parsing them as async\n>>>>> sub-devices.\n>>>>>\n>>>>> This can be done on e.g. flash or lens async sub-devices that are not part\n>>>>> of but are associated with a sensor.\n>>>>>\n>>>>> struct v4l2_async_notifier.max_subdevs field is added to contain the\n>>>>> maximum number of sub-devices in a notifier to reflect the memory\n>>>>> allocated for the subdevs array.\n>>>>\n>>>> You forgot to remove this outdated paragraph.\n>>>\n>>> Oops. Removed it now.\n>>>\n>>>>\n>>>>>\n>>>>> Signed-off-by: Sakari Ailus \n>>>>> ---\n>>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 69 +++++++++++++++++++++++++++++++++++\n>>>>> 1 file changed, 69 insertions(+)\n>>>>>\n>>>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>>>> index 44ee35f6aad5..a32473f95be1 100644\n>>>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n>>>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>>>> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n>>>>> }\n>>>>> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);\n>>>>> \n>>>>> +/*\n>>>>> + * v4l2_fwnode_reference_parse - parse references for async sub-devices\n>>>>> + * @dev: the device node the properties of which are parsed for references\n>>>>> + * @notifier: the async notifier where the async subdevs will be added\n>>>>> + * @prop: the name of the property\n>>>>> + *\n>>>>> + * Return: 0 on success\n>>>>> + *\t -ENOENT if no entries were found\n>>>>> + *\t -ENOMEM if memory allocation failed\n>>>>> + *\t -EINVAL if property parsing failed\n>>>>> + */\n>>>>> +static int v4l2_fwnode_reference_parse(\n>>>>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n>>>>> +\tconst char *prop)\n>>>>> +{\n>>>>> +\tstruct fwnode_reference_args args;\n>>>>> +\tunsigned int index;\n>>>>> +\tint ret;\n>>>>> +\n>>>>> +\tfor (index = 0;\n>>>>> +\t !(ret = fwnode_property_get_reference_args(\n>>>>> +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args));\n>>>>> +\t index++)\n>>>>> +\t\tfwnode_handle_put(args.fwnode);\n>>>>> +\n>>>>> +\tif (!index)\n>>>>> +\t\treturn -ENOENT;\n>>>>> +\n>>>>> +\t/*\n>>>>> +\t * To-do: handle -ENODATA when \"device property: Align return\n>>>>> +\t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n>>>>> +\t */\n>>>>> +\tif (ret != -ENOENT && ret != -ENODATA)\n>>>>\n>>>> So while that patch referenced in the To-do above is not merged yet,\n>>>> what does fwnode_property_get_reference_args return? ENOENT or ENODATA?\n>>>> Or ENOENT now and ENODATA later? Or vice versa?\n>>>>\n>>>> I can't tell based on that information whether this code is correct or not.\n>>>>\n>>>> The comment needs to explain this a bit better.\n>>>\n>>> I'll add this:\n>>>\n>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> index a32473f95be1..74fcc3ba9ebd 100644\n>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(\n>>> \t/*\n>>> \t * To-do: handle -ENODATA when \"device property: Align return\n>>> \t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n>>\n>> So after this patch referred to in the To-do is applied it will only\n>> return ENODATA?\n>>\n>> In that case, change 'handle' to 'handle only'.\n> \n> That depends a bit in which form the patch will be eventually accepted. The\n> underlying issue there is that different error codes are used to signal\n> conditions for out-of-bounds access and missing entry. After aligning them\n> the code here can be updated.\n\nAh. In that case I'd drop the 'To-do' sentence.\n\n> \n>>\n>>> +\t * Right now, both -ENODATA and -ENOENT signal the end of\n>>> +\t * references where only a single error code should be used\n>>> +\t * for the purpose.\n\nAnd add something like: \"This might change in the future, in which\ncase this code should be updated.\"\n\n>>> \t */\n>>> \tif (ret != -ENOENT && ret != -ENODATA)\n>>> \t\treturn ret;\n>>>\n> \n\nRegards,\n\n\tHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsdJr1NrTz9sNV\n\tfor ;\n\tWed, 13 Sep 2017 20:32:28 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751705AbdIMKcZ (ORCPT\n\t);\n\tWed, 13 Sep 2017 06:32:25 -0400","from lb2-smtp-cloud7.xs4all.net ([194.109.24.28]:53126 \"EHLO\n\tlb2-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751661AbdIMKcZ (ORCPT\n\t); Wed, 13 Sep 2017 06:32:25 -0400","from [IPv6:2001:420:44c1:2579:c460:2dc1:1388:52dc]\n\t([IPv6:2001:420:44c1:2579:c460:2dc1:1388:52dc])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid s4xtdaAxEb2sns4xxdGMzS; Wed, 13 Sep 2017 12:32:23 +0200"],"Subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","To":"Sakari Ailus ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-19-sakari.ailus@linux.intel.com>\n\t<020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl>\n\t<20170913092430.cbdgerkhiuxakbxv@valkosipuli.retiisi.org.uk>\n\t\n\t<20170913100714.5qy32qly3vlxguxz@valkosipuli.retiisi.org.uk>","From":"Hans Verkuil ","Message-ID":"","Date":"Wed, 13 Sep 2017 12:32:17 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170913100714.5qy32qly3vlxguxz@valkosipuli.retiisi.org.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfD+dqKIoU5G9TihTno9JSVlPljR/hwtI38YvHg8LhaBXaAi8HOZa1xXAlH/sg3TVQUhFt6apbfaetqrIMKL61qLiQbnZqwwQUG76oUNlJgR6CstjsGHF\n\trIsuXIUkHA5Kdxrp7+arxuMYpHCFNw/HmXrUxWnptTHT3HuezQRJR4Gc6YsEp7qzSYv/SZYs36vw3o9VbNVGSJfk82RIvwZQf8iY3hZJ7MMl1QroIZwwnjHG\n\t98asIGelzUWn5fMKamTLtPAX+gt/RbAr35pGJJ36lab7MdfBceg1ynRVgAHOktlunAIT4w0Wj/IPx8wjiwz/bb6GVRCfp4O9fCQxyuGHQpAYB0FtQJBU1KSN\n\tME9yKhs9SIE57FLiQx5mUj/5dhj0SsGKjn8Hf32o8vxmTi/JaZwdh9sF60N2Z8q5Jx2Cwb/shNqwtPpZ6jz5Fp2m1/wAroVybawWPfF3KU4AX5sifDy2wQtO\n\tkfnew/bbcHC15Hylzq92Z8Q94ZWRZXlgn8tyFvQPWHS8BNjuJ4QeW5rVE7npVfs6lHaiej6gmLsrQ/Xf","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1769209,"web_url":"http://patchwork.ozlabs.org/comment/1769209/","msgid":"<20170915140512.nld3qfn6xen3hx5q@lanttu.localdomain>","list_archive_url":null,"date":"2017-09-15T14:05:13","subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Hans,\n\nOn Wed, Sep 13, 2017 at 12:32:17PM +0200, Hans Verkuil wrote:\n> On 09/13/17 12:07, Sakari Ailus wrote:\n> > Hi Hans,\n> > \n> > On Wed, Sep 13, 2017 at 11:28:44AM +0200, Hans Verkuil wrote:\n> >> On 09/13/17 11:24, Sakari Ailus wrote:\n> >>> Hi Hans,\n> >>>\n> >>> Thanks for the review!\n> >>>\n> >>> On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:\n> >>>> On 09/12/2017 03:41 PM, Sakari Ailus wrote:\n> >>>>> Add function v4l2_fwnode_reference_count() for counting external\n> >>>>\n> >>>> ???? There is no function v4l2_fwnode_reference_count()?!\n> >>>\n> >>> It got removed during the revisions but the commit message was not changed\n> >>> accordingly, I do that now.\n> >>>\n> >>>>\n> >>>>> references and v4l2_fwnode_reference_parse() for parsing them as async\n> >>>>> sub-devices.\n> >>>>>\n> >>>>> This can be done on e.g. flash or lens async sub-devices that are not part\n> >>>>> of but are associated with a sensor.\n> >>>>>\n> >>>>> struct v4l2_async_notifier.max_subdevs field is added to contain the\n> >>>>> maximum number of sub-devices in a notifier to reflect the memory\n> >>>>> allocated for the subdevs array.\n> >>>>\n> >>>> You forgot to remove this outdated paragraph.\n> >>>\n> >>> Oops. Removed it now.\n> >>>\n> >>>>\n> >>>>>\n> >>>>> Signed-off-by: Sakari Ailus \n> >>>>> ---\n> >>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 69 +++++++++++++++++++++++++++++++++++\n> >>>>> 1 file changed, 69 insertions(+)\n> >>>>>\n> >>>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>>>> index 44ee35f6aad5..a32473f95be1 100644\n> >>>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>>>> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(\n> >>>>> }\n> >>>>> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);\n> >>>>> \n> >>>>> +/*\n> >>>>> + * v4l2_fwnode_reference_parse - parse references for async sub-devices\n> >>>>> + * @dev: the device node the properties of which are parsed for references\n> >>>>> + * @notifier: the async notifier where the async subdevs will be added\n> >>>>> + * @prop: the name of the property\n> >>>>> + *\n> >>>>> + * Return: 0 on success\n> >>>>> + *\t -ENOENT if no entries were found\n> >>>>> + *\t -ENOMEM if memory allocation failed\n> >>>>> + *\t -EINVAL if property parsing failed\n> >>>>> + */\n> >>>>> +static int v4l2_fwnode_reference_parse(\n> >>>>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> >>>>> +\tconst char *prop)\n> >>>>> +{\n> >>>>> +\tstruct fwnode_reference_args args;\n> >>>>> +\tunsigned int index;\n> >>>>> +\tint ret;\n> >>>>> +\n> >>>>> +\tfor (index = 0;\n> >>>>> +\t !(ret = fwnode_property_get_reference_args(\n> >>>>> +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args));\n> >>>>> +\t index++)\n> >>>>> +\t\tfwnode_handle_put(args.fwnode);\n> >>>>> +\n> >>>>> +\tif (!index)\n> >>>>> +\t\treturn -ENOENT;\n> >>>>> +\n> >>>>> +\t/*\n> >>>>> +\t * To-do: handle -ENODATA when \"device property: Align return\n> >>>>> +\t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n> >>>>> +\t */\n> >>>>> +\tif (ret != -ENOENT && ret != -ENODATA)\n> >>>>\n> >>>> So while that patch referenced in the To-do above is not merged yet,\n> >>>> what does fwnode_property_get_reference_args return? ENOENT or ENODATA?\n> >>>> Or ENOENT now and ENODATA later? Or vice versa?\n> >>>>\n> >>>> I can't tell based on that information whether this code is correct or not.\n> >>>>\n> >>>> The comment needs to explain this a bit better.\n> >>>\n> >>> I'll add this:\n> >>>\n> >>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> index a32473f95be1..74fcc3ba9ebd 100644\n> >>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(\n> >>> \t/*\n> >>> \t * To-do: handle -ENODATA when \"device property: Align return\n> >>> \t * codes of acpi_fwnode_get_reference_with_args\" is merged.\n> >>\n> >> So after this patch referred to in the To-do is applied it will only\n> >> return ENODATA?\n> >>\n> >> In that case, change 'handle' to 'handle only'.\n> > \n> > That depends a bit in which form the patch will be eventually accepted. The\n> > underlying issue there is that different error codes are used to signal\n> > conditions for out-of-bounds access and missing entry. After aligning them\n> > the code here can be updated.\n> \n> Ah. In that case I'd drop the 'To-do' sentence.\n> \n> > \n> >>\n> >>> +\t * Right now, both -ENODATA and -ENOENT signal the end of\n> >>> +\t * references where only a single error code should be used\n> >>> +\t * for the purpose.\n> \n> And add something like: \"This might change in the future, in which\n> case this code should be updated.\"\n\nI'll use:\n\n\t/*\n\t * Note that right now both -ENODATA and -ENOENT may signal\n\t * out-of-bounds access. Return the error in cases other than that.\n\t */","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xtxyr3NHGz9s7m\n\tfor ;\n\tSat, 16 Sep 2017 00:06:28 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751884AbdIOOGZ (ORCPT\n\t);\n\tFri, 15 Sep 2017 10:06:25 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:45192 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751715AbdIOOFT (ORCPT\n\t); Fri, 15 Sep 2017 10:05:19 -0400","from lanttu.localdomain (unknown\n\t[IPv6:2001:1bc8:1a6:d3d5::e1:1002])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 393D36011E;\n\tFri, 15 Sep 2017 17:05:16 +0300 (EEST)","from sailus by lanttu.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dsrF3-000571-Cm; Fri, 15 Sep 2017 17:05:13 +0300"],"Date":"Fri, 15 Sep 2017 17:05:13 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tmaxime.ripard@free-electrons.com, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","Message-ID":"<20170915140512.nld3qfn6xen3hx5q@lanttu.localdomain>","References":"<20170912134200.19556-1-sakari.ailus@linux.intel.com>\n\t<20170912134200.19556-19-sakari.ailus@linux.intel.com>\n\t<020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl>\n\t<20170913092430.cbdgerkhiuxakbxv@valkosipuli.retiisi.org.uk>\n\t\n\t<20170913100714.5qy32qly3vlxguxz@valkosipuli.retiisi.org.uk>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}}]