[{"id":1766139,"web_url":"http://patchwork.ozlabs.org/comment/1766139/","msgid":"<19a5c075-92ca-050e-fc50-c78f02579df3@xs4all.nl>","list_archive_url":null,"date":"2017-09-11T08:57:16","subject":"Re: [PATCH v10 14/24] 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/11/2017 09:59 AM, 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> drivers/media/v4l2-core/v4l2-async.c | 217 ++++++++++++++++++++++++++++++-----\n> include/media/v4l2-async.h | 16 ++-\n> 2 files changed, 202 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 9ebc2e079d03..6f788b2e922a 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> +/* Get the sub-device notifier registered by a sub-device driver. */\n> +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(\n\nI prefer to call this v4l2_async_find_subdev_notifier(). 'get' suggests\na getter function, but this actually has to find it. I think this may have\nconfused me during an earlier review of this code. The comment also needs\nupdating: \"Find the sub-device...\".\n\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_get_subdev_notifier(sd);\n\nWould it make sense to add a 'struct v4l2_async_notifier *subdev_notifier'\nfield to struct v4l2_subdev? It's set when a subdev registers a notifier.\n\nThat way you can just use sd->subdev_notifier here.\n\nI wonder if v4l2_async_get_subdev_notifier() is needed at all if you do\nthis.\n\n> +\n> +\t\tif (!subdev_notifier)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (!v4l2_async_subdev_notifiers_complete(subdev_notifier))\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_get_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_get_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> +\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_get_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_get_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,20 +291,18 @@ 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> \tif (!notifier->num_subdevs)\n> \t\treturn v4l2_async_notifier_call_complete(notifier);\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,28 +339,67 @@ 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 (!v4l2_dev || notifier->sd)\n\nShould this be a WARN_ON?\n\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 (!sd || notifier->v4l2_dev)\n\nDitto.\n\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_get_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> +\n> +\t\tlist_del(&sd->async_list);\n> +\t\tlist_add(&sd->async_list, &subdev_list);\n> \t}\n> \n> -\tmutex_unlock(&list_lock);\n> +\tnotifier->parent = NULL;\n\nShouldn't notifier->v4l2_dev and notifier->sd be set to NULL as well?\nI can't really tell.\n\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> -\tnotifier->v4l2_dev = NULL;\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> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> index 3bc8a7c0d83f..cf409d45208c 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 carrying @v4l2_dev\n\nThat's not correct, it only carries v4l2_dev if it is the root notifier.\nI think just 'parent notifier' is sufficient here.\n\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\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 3xrMJ45Qhnz9s8J\n\tfor ;\n\tMon, 11 Sep 2017 18:57:24 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751035AbdIKI5W (ORCPT\n\t);\n\tMon, 11 Sep 2017 04:57:22 -0400","from lb3-smtp-cloud7.xs4all.net ([194.109.24.31]:43740 \"EHLO\n\tlb3-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1750994AbdIKI5V (ORCPT\n\t); Mon, 11 Sep 2017 04:57:21 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rKWqdHfFkb2snrKWtd6c5S; Mon, 11 Sep 2017 10:57:19 +0200"],"Subject":"Re: [PATCH v10 14/24] v4l: async: Allow binding notifiers to\n\tsub-devices","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-15-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<19a5c075-92ca-050e-fc50-c78f02579df3@xs4all.nl>","Date":"Mon, 11 Sep 2017 10:57:16 +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":"<20170911080008.21208-15-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfFyfA9X+C+tfvXhFKfhvhi0mweqKtSB9CmnlWlKzOlM71AY5yzCtfbv6qQhz/z8urpwMT1tqHaihhXfnvG/Bfhj/K4hTUOMIQL/wq9cgM60UuPDwUrWn\n\ttbffwupeQu7Z9hJ3Shd0hiOtPxBIYKJnoHIa3ROCutcS8o9ljRPPEI7wRZ+VM3pgJXwyq8UkQ/yTo7eovLq1+B6U7gaLZDb72exHdfMtpCrFXtSbdxfr50DL\n\tpE2c812SOd9adEIkGKa9QgMm/MG0SfIvJkKOPeXZcZ1vNDmEUWbDQWgI0OY1Co7GY8CCkxhTItxIwzLyEV+AGIHprcxy+Z34Y9rcTmKz+eapShd8qo4EGQSx\n\tPu6Sv0F0Xt1qz2+mfz5NkT0+io+ptPWeiPsvyvbkDQeOg4YfyaytCZAHY2lWbqcQa66rGLxR+SfF49Zm/JIKjTZ32GUTWxGyJp9fFj5wW0XKZlNFWDt4Zmbf\n\tF28XVtOkJKQy0+IBUyoGLnsbNLVv5rcGwotc4Ab/m68V5iJri/hsW6SY01jPBhG7ZjPLcPeOXg4G5JAPgnhloJUmMU3H4c5utttSGg==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766155,"web_url":"http://patchwork.ozlabs.org/comment/1766155/","msgid":"","list_archive_url":null,"date":"2017-09-11T09:14:03","subject":"Re: [PATCH v10 17/24] 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/11/2017 10:00 AM, Sakari Ailus wrote:\n> Add function v4l2_fwnode_reference_count() for counting external\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\nThis paragraph appears to be out-of-date.\n\n> \n> Signed-off-by: Sakari Ailus \n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 47 +++++++++++++++++++++++++++++++++++\n> 1 file changed, 47 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index d978f2d714ca..4821c4989119 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -449,6 +449,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> }\n> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\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 = 0;\n> +\tint ret;\n> +\n> +\tfor (; !fwnode_property_get_reference_args(\n> +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args); index++)\n> +\t\tfwnode_handle_put(args.fwnode);\n> +\n\nIf nothing is found (i.e. index == 0), shouldn't you just return here?\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 (ret = -ENOENT, index = 0;\n\nThere is no reason for the 'ret = -ENOENT' to be in the for(), just set it before\nthe 'for' statement.\n\n> +\t !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\nIf the loop doesn't find anything, then it still returns 0, not -ENOENT.\nSo why set ret to ENOENT? Something weird going on here.\n\nI think you should also add a comment explaining this function.\n\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 3xrMgf4CN8z9s7G\n\tfor ;\n\tMon, 11 Sep 2017 19:14:22 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751291AbdIKJOU (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:14:20 -0400","from lb3-smtp-cloud7.xs4all.net ([194.109.24.31]:47790 \"EHLO\n\tlb3-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751071AbdIKJOI (ORCPT\n\t); Mon, 11 Sep 2017 05:14:08 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rKn5dHp2Qb2snrKn8d6iTG; Mon, 11 Sep 2017 11:14:06 +0200"],"Subject":"Re: [PATCH v10 17/24] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-18-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"","Date":"Mon, 11 Sep 2017 11:14:03 +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":"<20170911080008.21208-18-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfICYWynFu6CI6gZyAxDE1+EDTHXW0hkzK61sZRLxgCyVmdgZJV7gmfZBFOnsLCJ/aIkPCy59iVCPHZrkibUxmlFK96XCUcDhprU6yjSbIGF/HNO3lGhH\n\tZXKfT5lv0YtLGS04x5jAzNXCCREnYAVz6SYFSk/ovpBF156zEOmaQWm6Owmj20gMBnspjCi7/Q9OyNKufvoLphB432aru2w8XXpfqGoQABr8CL4jZVfdCErP\n\tFcrPZTqhrjAJf8IMO1fmounIe1edkzQbd9L0aEzjP2EfCAkhaEeIf7vAgJaxB/5S1BOZyasQj4DXPE0v74UPTaoYBZTq2tNCbHpRuto7+yvdKODFH2H/26Li\n\tI7EvlKfvgRrK9IrCNbeksib8ojzTD+PYdhDh/WgmUeLlYrd4QUIQ6xZibeWMHVVuB3wBjRlLIl/6+xA3/g2SLhN69mPpcZCl6NCej+b2h61TrOWa0Ix2Tflf\n\tPaQPfL2RbTHhXPsnx9BzHdfsKEP6DyDgsjJWDZ7zjsqbizb1SBXGrQ26XkO5k4EKBwZNn2J5FojpRsyZeTw3AD+OwiOBinexoNsyqQ==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766166,"web_url":"http://patchwork.ozlabs.org/comment/1766166/","msgid":"<20170911093024.jmdsi3zrypj5hndc@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-11T09:30:24","subject":"Re: [PATCH v10 14/24] 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\nThanks for the review!\n\nOn Mon, Sep 11, 2017 at 10:57:16AM +0200, Hans Verkuil wrote:\n> On 09/11/2017 09:59 AM, 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> > drivers/media/v4l2-core/v4l2-async.c | 217 ++++++++++++++++++++++++++++++-----\n> > include/media/v4l2-async.h | 16 ++-\n> > 2 files changed, 202 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 9ebc2e079d03..6f788b2e922a 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> > +/* Get the sub-device notifier registered by a sub-device driver. */\n> > +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(\n> \n> I prefer to call this v4l2_async_find_subdev_notifier(). 'get' suggests\n> a getter function, but this actually has to find it. I think this may have\n> confused me during an earlier review of this code. The comment also needs\n> updating: \"Find the sub-device...\".\n\nYes, makes sense. Get also suggests that there would be reference counting\nwhich is not the case here.\n\nI made the corresponding change to v4l2_async_notifier_find_v4l2_dev() as\nwell.\n\n> \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_get_subdev_notifier(sd);\n> \n> Would it make sense to add a 'struct v4l2_async_notifier *subdev_notifier'\n> field to struct v4l2_subdev? It's set when a subdev registers a notifier.\n> \n> That way you can just use sd->subdev_notifier here.\n> \n> I wonder if v4l2_async_get_subdev_notifier() is needed at all if you do\n> this.\n\nI thought of that, but ended up keeping the information in the notifier. As\nthe information is already available elsewhere, I didn't end up adding a\nnew field for the purpose. This is certainly not performance critical\neither.\n\n> \n> > +\n> > +\t\tif (!subdev_notifier)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tif (!v4l2_async_subdev_notifiers_complete(subdev_notifier))\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_get_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_get_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> > +\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_get_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_get_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,20 +291,18 @@ 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> > \tif (!notifier->num_subdevs)\n> > \t\treturn v4l2_async_notifier_call_complete(notifier);\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,28 +339,67 @@ 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 (!v4l2_dev || notifier->sd)\n> \n> Should this be a WARN_ON?\n\nAdded WARN_ON().\n\n> \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 (!sd || notifier->v4l2_dev)\n> \n> Ditto.\n\nIndeed.\n\n> \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_get_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> > +\n> > +\t\tlist_del(&sd->async_list);\n> > +\t\tlist_add(&sd->async_list, &subdev_list);\n> > \t}\n> > \n> > -\tmutex_unlock(&list_lock);\n> > +\tnotifier->parent = NULL;\n> \n> Shouldn't notifier->v4l2_dev and notifier->sd be set to NULL as well?\n> I can't really tell.\n\nWell spotted. Yes. This was actually broken by the patch; v4l2_dev used to\nbe set NULL. Will fix.\n\n> \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> > -\tnotifier->v4l2_dev = NULL;\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> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> > index 3bc8a7c0d83f..cf409d45208c 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 carrying @v4l2_dev\n> \n> That's not correct, it only carries v4l2_dev if it is the root notifier.\n> I think just 'parent notifier' is sufficient here.\n\nWill change.\n\n> \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> >","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 3xrN2H1624z9s75\n\tfor ;\n\tMon, 11 Sep 2017 19:30:31 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751151AbdIKJa3 (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:30:29 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:50170 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751005AbdIKJa2 (ORCPT\n\t); Mon, 11 Sep 2017 05:30:28 -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 9C8E9600E3;\n\tMon, 11 Sep 2017 12:30:25 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1drL2v-0003in-1G; Mon, 11 Sep 2017 12:30:25 +0300"],"Date":"Mon, 11 Sep 2017 12:30:24 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v10 14/24] v4l: async: Allow binding notifiers to\n\tsub-devices","Message-ID":"<20170911093024.jmdsi3zrypj5hndc@valkosipuli.retiisi.org.uk>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-15-sakari.ailus@linux.intel.com>\n\t<19a5c075-92ca-050e-fc50-c78f02579df3@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<19a5c075-92ca-050e-fc50-c78f02579df3@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":1766175,"web_url":"http://patchwork.ozlabs.org/comment/1766175/","msgid":"<11c951eb-0315-0149-829e-ed73d748e783@xs4all.nl>","list_archive_url":null,"date":"2017-09-11T09:38:58","subject":"Re: [PATCH v10 18/24] 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":"Typo in subject: interger -> integer\n\nOn 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under\n> the device's own fwnode, \n\nSorry, you lost me here. Which device are we talking about?\n\n> it will follow child fwnodes with the given\n> property -- value pair and return the resulting fwnode.\n\nproperty-value pair (easier readable that way).\n\nYou only describe v4l2_fwnode_reference_parse_int_prop(), not\nv4l2_fwnode_reference_parse_int_props().\n\n> \n> Signed-off-by: Sakari Ailus \n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++\n> 1 file changed, 93 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index 4821c4989119..56eee5bbd3b5 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(\n> \treturn ret;\n> }\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\nNeed comments describing what this does.\n\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> +\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 == -EINVAL ? -ENOENT : ret);\n\nWhy map EINVAL to ENOENT? Needs a comment, either here or in the function description.\n\n> +\n> +\tfor (fwnode = fwnode_args.fwnode;\n> +\t nprops; nprops--, fwnode = child, props++, args++) {\n\nI think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...\nIt's hard to parse.\n\nI would make this a 'while (nprops)' and write out all the other assignments,\nincrements and decrements.\n\n> +\t\tu32 val;\n> +\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\tif (val == *args)\n> +\t\t\t\tbreak;\n\nI'm lost. This really needs comments and perhaps even an DT or ACPI example\nso you can see what exactly it is we're doing here.\n\n> +\t\t}\n> +\n> +\t\tfwnode_handle_put(fwnode);\n> +\n> +\t\tif (!child) {\n> +\t\t\tfwnode = ERR_PTR(-ENOENT);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\treturn fwnode;\n> +}\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\nNeeds comments describing what this does.\n\n> +{\n> +\tstruct fwnode_handle *fwnode;\n> +\tunsigned int index = 0;\n> +\tint ret;\n> +\n> +\twhile (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(\n> +\t\t\t\tdev_fwnode(dev), prop, index, props,\n> +\t\t\t\tnprops)))) {\n> +\t\tfwnode_handle_put(fwnode);\n> +\t\tindex++;\n> +\t}\n> +\n> +\tif (PTR_ERR(fwnode) != -ENOENT)\n> +\t\treturn PTR_ERR(fwnode);\n\nMissing 'if (index == 0)'?\n\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))); ) {\n\nI'd add 'index++' in this for-loop. It's weird that it is missing.\n\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> +\n> +\t\tfwnode_handle_put(fwnode);\n> +\n> +\t\tindex++;\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\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 3xrNDH6Clrz9s81\n\tfor ;\n\tMon, 11 Sep 2017 19:39:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751651AbdIKJjG (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:39:06 -0400","from lb1-smtp-cloud7.xs4all.net ([194.109.24.24]:40535 \"EHLO\n\tlb1-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751459AbdIKJjD (ORCPT\n\t); Mon, 11 Sep 2017 05:39:03 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rLBCdI46Sb2snrLBFd6sPU; Mon, 11 Sep 2017 11:39:02 +0200"],"Subject":"Re: [PATCH v10 18/24] 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, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-19-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<11c951eb-0315-0149-829e-ed73d748e783@xs4all.nl>","Date":"Mon, 11 Sep 2017 11:38:58 +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":"<20170911080008.21208-19-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfCuVOELSDmwF+RXQVMxXwfl8LR77dHerhQ0TvUmvO4qPTaRXVSXy8vxpJj+MgQIctDBcGncENQNMjlc+xbtBedoPeVBb75XxDvuPAajT6xcQ3GO+TgKT\n\tujp98HqjIlziEhLKkpaVH6LfDFViqi0wc5Oc8JqYxtK3z5Ar0jRXI+IP715Z5+AvobhH875bprUJmjYfYnlCjVKneoYfMEAjrfM/l4geBBCzwb59KmdoaxaB\n\tBBxxuUdtTTnpu+kToZGCABtCdWmJxqVh4Fe4UIDTbiGWggKn1w9QbM9OiMFkksTEe/QatnjWz70S2GhBDgIoKUlgvYAzPewq2UV1juaOyjXTGxhdJMiBj041\n\tyiBgqIw3jpk5xlKMITMVCpJwyX6Dlx347PVDUJ2aVGTPej8ZYZQyhG1iSbQdozGQRURwRTaj/brRIduFpGLyqM6gK21mhzRuqsU/Xhj9zsFA+rE8VkmgVN3m\n\t9xdJPZ+KqG07xFni71zj/sgOPNwf4VTqP3H8kOsqA54rsF6LRWqniHuH+cE4gTc/+pGh2NqM4PVSwICJkZNS1rP/YqzEukXz180h7w==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766179,"web_url":"http://patchwork.ozlabs.org/comment/1766179/","msgid":"","list_archive_url":null,"date":"2017-09-11T09:47:11","subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> Add v4l2_fwnode_parse_reference_sensor_common for parsing common\n> sensor properties that refer to adjacent devices such as flash or lens\n> driver chips.\n> \n> As this is an association only, there's little a regular driver needs to\n> know about these devices as such.\n> \n> Signed-off-by: Sakari Ailus \n> Acked-by: Hans Verkuil \n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 35 +++++++++++++++++++++++++++++++++++\n> include/media/v4l2-fwnode.h | 13 +++++++++++++\n> 2 files changed, 48 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index 56eee5bbd3b5..b9e60a0e8f86 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -589,6 +589,41 @@ static int v4l2_fwnode_reference_parse_int_props(\n> \treturn ret;\n> }\n> \n> +int v4l2_fwnode_reference_parse_sensor_common(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier)\n> +{\n> +\tstatic const char *led_props[] = { \"led\" };\n> +\tstatic const struct {\n> +\t\tconst char *name;\n> +\t\tconst char **props;\n> +\t\tunsigned int nprops;\n> +\t} props[] = {\n> +\t\t{ \"flash-leds\", led_props, ARRAY_SIZE(led_props) },\n> +\t\t{ \"lens-focus\", NULL, 0 },\n> +\t};\n> +\tunsigned int i;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(props); i++) {\n> +\t\tint ret;\n> +\n> +\t\tif (props[i].props && is_acpi_node(dev_fwnode(dev)))\n> +\t\t\tret = v4l2_fwnode_reference_parse_int_props(\n> +\t\t\t\tdev, notifier, props[i].name,\n> +\t\t\t\tprops[i].props, props[i].nprops);\n> +\t\telse\n> +\t\t\tret = v4l2_fwnode_reference_parse(\n> +\t\t\t\tdev, notifier, props[i].name);\n> +\t\tif (ret) {\n> +\t\t\tdev_warn(dev, \"parsing property \\\"%s\\\" failed (%d)\\n\",\n> +\t\t\t\t props[i].name, ret);\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_fwnode_reference_parse_sensor_common);\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 3819a73c3c8a..bcec1ce101dc 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -257,4 +257,17 @@ 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_fwnode_reference_parse_sensor_common - parse common references on\n> + *\t\t\t\t\t sensors 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> + *\n\nI think you should add a note that if this function returns 0 the\ncaller should remember to call v4l2_async_notifier_release. That is not\nimmediately obvious.\n\nRegards,\n\n\tHans\n\n> + * Return: 0 on success\n> + *\t -ENOMEM if memory allocation failed\n> + *\t -EINVAL if property parsing failed\n> + */\n> +int v4l2_fwnode_reference_parse_sensor_common(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier);\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 3xrNPg4L10z9sBZ\n\tfor ;\n\tMon, 11 Sep 2017 19:47:19 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751423AbdIKJrR (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:47:17 -0400","from lb1-smtp-cloud7.xs4all.net ([194.109.24.24]:58461 \"EHLO\n\tlb1-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751005AbdIKJrQ (ORCPT\n\t); Mon, 11 Sep 2017 05:47:16 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rLJ9dI8rSb2snrLJCd6vRH; Mon, 11 Sep 2017 11:47:14 +0200"],"Subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-20-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"","Date":"Mon, 11 Sep 2017 11:47: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":"<20170911080008.21208-20-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfCRXYPTaFWTWOeLMqhv6FISJ/DYLlL54l3ZUsAIBtcjUACLeJpMJeQ3ntL9C+jLy9vF8Z67qSnbuJ6am92mK6PRBVxjnMoYtsCJXBSso9AnMt2ypdW+Q\n\tIExQgHBZKtYrm97VbCeGaJP24b02JqXaAL7OJyXX97jwGxGbuN1WeqKLBhpBtZY28/dGxqnw6LyQmsXh9PSx0uiQb9Z5jZxg70KCgJYzkD/O2h1ZGYGJfDb3\n\trklrGNAGDDtEtNM7CdDFN2kWkVNCNOUSFJCEacs7yBK9ToBxUAZg4aLrzFruRxI34/RAU0gc7X9F49e4yFyVqVw8c85QS4XmT0LPwYy1qe0zgwLCfKz9lhjT\n\tylfWHmINpszSO84MiyTx12lWO5WUD0XMtUAF/ofRvHFfeaoAUuvpul7z9k3p3i6o26em1t+MSZX/YDEZo1UTMIDAYN2NsutuEyN0poIUF3szyNC7c3o6RTzk\n\tyjBlPl1A+zLTRKe8jM+uDfkt8qKcxOGjFPHfZ5oPxiANYzjjISaOrK0pVAEZRliIU6bkCI1DD4KDdkNGwxvE5bAdxcbCsej05A36ag==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766180,"web_url":"http://patchwork.ozlabs.org/comment/1766180/","msgid":"<04981ec0-710e-1367-03fc-501314c45d5f@xs4all.nl>","list_archive_url":null,"date":"2017-09-11T09:48:23","subject":"Re: [PATCH v10 21/24] smiapp: 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/11/2017 10:00 AM, Sakari Ailus wrote:\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 smiapp driver due to the relationship of these\n> component to the main part of the camera module --- the sensor.\n> \n> This does not yet address providing the user space with information on how\n> to associate the sensor or lens devices but the kernel now has the\n> necessary information to do that.\n> \n> Signed-off-by: Sakari Ailus \n\nAcked-by: Hans Verkuil \n\nRegards,\n\n\tHans\n\n> ---\n> drivers/media/i2c/smiapp/smiapp-core.c | 38 +++++++++++++++++++++++++++-------\n> drivers/media/i2c/smiapp/smiapp.h | 4 +++-\n> 2 files changed, 33 insertions(+), 9 deletions(-)\n> \n> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c\n> index 700f433261d0..a65a839135d2 100644\n> --- a/drivers/media/i2c/smiapp/smiapp-core.c\n> +++ b/drivers/media/i2c/smiapp/smiapp-core.c\n> @@ -31,7 +31,7 @@\n> #include \n> #include \n> #include \n> -#include \n> +#include \n> #include \n> #include \n> \n> @@ -2887,17 +2887,24 @@ static int smiapp_probe(struct i2c_client *client,\n> \tv4l2_i2c_subdev_init(&sensor->src->sd, client, &smiapp_ops);\n> \tsensor->src->sd.internal_ops = &smiapp_internal_src_ops;\n> \n> +\trval = v4l2_fwnode_reference_parse_sensor_common(\n> +\t\t&client->dev, &sensor->notifier);\n> +\tif (rval < 0)\n> +\t\treturn rval;\n> +\n> \tsensor->vana = devm_regulator_get(&client->dev, \"vana\");\n> \tif (IS_ERR(sensor->vana)) {\n> \t\tdev_err(&client->dev, \"could not get regulator for vana\\n\");\n> -\t\treturn PTR_ERR(sensor->vana);\n> +\t\trval = PTR_ERR(sensor->vana);\n> +\t\tgoto out_release_async_notifier;\n> \t}\n> \n> \tsensor->ext_clk = devm_clk_get(&client->dev, NULL);\n> \tif (IS_ERR(sensor->ext_clk)) {\n> \t\tdev_err(&client->dev, \"could not get clock (%ld)\\n\",\n> \t\t\tPTR_ERR(sensor->ext_clk));\n> -\t\treturn -EPROBE_DEFER;\n> +\t\trval = -EPROBE_DEFER;\n> +\t\tgoto out_release_async_notifier;\n> \t}\n> \n> \trval = clk_set_rate(sensor->ext_clk, sensor->hwcfg->ext_clk);\n> @@ -2905,17 +2912,19 @@ static int smiapp_probe(struct i2c_client *client,\n> \t\tdev_err(&client->dev,\n> \t\t\t\"unable to set clock freq to %u\\n\",\n> \t\t\tsensor->hwcfg->ext_clk);\n> -\t\treturn rval;\n> +\t\tgoto out_release_async_notifier;\n> \t}\n> \n> \tsensor->xshutdown = devm_gpiod_get_optional(&client->dev, \"xshutdown\",\n> \t\t\t\t\t\t GPIOD_OUT_LOW);\n> -\tif (IS_ERR(sensor->xshutdown))\n> -\t\treturn PTR_ERR(sensor->xshutdown);\n> +\tif (IS_ERR(sensor->xshutdown)) {\n> +\t\trval = PTR_ERR(sensor->xshutdown);\n> +\t\tgoto out_release_async_notifier;\n> +\t}\n> \n> \trval = smiapp_power_on(&client->dev);\n> \tif (rval < 0)\n> -\t\treturn rval;\n> +\t\tgoto out_release_async_notifier;\n> \n> \trval = smiapp_identify_module(sensor);\n> \tif (rval) {\n> @@ -3092,9 +3101,14 @@ static int smiapp_probe(struct i2c_client *client,\n> \tif (rval < 0)\n> \t\tgoto out_media_entity_cleanup;\n> \n> +\trval = v4l2_async_subdev_notifier_register(&sensor->src->sd,\n> +\t\t\t\t\t\t &sensor->notifier);\n> +\tif (rval)\n> +\t\tgoto out_media_entity_cleanup;\n> +\n> \trval = v4l2_async_register_subdev(&sensor->src->sd);\n> \tif (rval < 0)\n> -\t\tgoto out_media_entity_cleanup;\n> +\t\tgoto out_unregister_async_notifier;\n> \n> \tpm_runtime_set_active(&client->dev);\n> \tpm_runtime_get_noresume(&client->dev);\n> @@ -3105,6 +3119,9 @@ static int smiapp_probe(struct i2c_client *client,\n> \n> \treturn 0;\n> \n> +out_unregister_async_notifier:\n> +\tv4l2_async_notifier_unregister(&sensor->notifier);\n> +\n> out_media_entity_cleanup:\n> \tmedia_entity_cleanup(&sensor->src->sd.entity);\n> \n> @@ -3114,6 +3131,9 @@ static int smiapp_probe(struct i2c_client *client,\n> out_power_off:\n> \tsmiapp_power_off(&client->dev);\n> \n> +out_release_async_notifier:\n> +\tv4l2_async_notifier_release(&sensor->notifier);\n> +\n> \treturn rval;\n> }\n> \n> @@ -3124,6 +3144,8 @@ static int smiapp_remove(struct i2c_client *client)\n> \tunsigned int i;\n> \n> \tv4l2_async_unregister_subdev(subdev);\n> +\tv4l2_async_notifier_unregister(&sensor->notifier);\n> +\tv4l2_async_notifier_release(&sensor->notifier);\n> \n> \tpm_runtime_disable(&client->dev);\n> \tif (!pm_runtime_status_suspended(&client->dev))\n> diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h\n> index f74d695018b9..be92cb5713f4 100644\n> --- a/drivers/media/i2c/smiapp/smiapp.h\n> +++ b/drivers/media/i2c/smiapp/smiapp.h\n> @@ -20,9 +20,10 @@\n> #define __SMIAPP_PRIV_H_\n> \n> #include \n> +#include \n> +#include \n> #include \n> #include \n> -#include \n> \n> #include \"smiapp-pll.h\"\n> #include \"smiapp-reg.h\"\n> @@ -172,6 +173,7 @@ struct smiapp_subdev {\n> * struct smiapp_sensor - Main device structure\n> */\n> struct smiapp_sensor {\n> +\tstruct v4l2_async_notifier notifier;\n> \t/*\n> \t * \"mutex\" is used to serialise access to all fields here\n> \t * except v4l2_ctrls at the end of the struct. \"mutex\" is also\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 3xrNRL10DTz9s7M\n\tfor ;\n\tMon, 11 Sep 2017 19:48:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751373AbdIKJs3 (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:48:29 -0400","from lb3-smtp-cloud7.xs4all.net ([194.109.24.31]:38430 \"EHLO\n\tlb3-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751111AbdIKJs2 (ORCPT\n\t); Mon, 11 Sep 2017 05:48:28 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rLKJdI9Y7b2snrLKMd6vtU; Mon, 11 Sep 2017 11:48:27 +0200"],"Subject":"Re: [PATCH v10 21/24] smiapp: Add support for flash and lens devices","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-22-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<04981ec0-710e-1367-03fc-501314c45d5f@xs4all.nl>","Date":"Mon, 11 Sep 2017 11:48:23 +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":"<20170911080008.21208-22-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfDO7hJkFpfV2BciwLQ8ZGjqhAHkjB1r6P1/BMle8iQc4j+rg2GVtFZswRkSYW+B0zEj3vToQV7wdfXBcPMiHOFY6gWfgut9v17aE9Bgf9Gd+xiT/nzlX\n\thYFhiU+oIdstkIFbI3NPhfyQmHDqIM8opWdThT/w8II+MsVkpa0kDHt2TtxZaYugdfVpdzi2FeQQe1j9RGc8zPeO2fXGFRtU7qm9m/hiUKxBDCEF43dAqAFL\n\tblQJ3IozwKPWbkc6Jeqyc7y+j4FzBma9Pk6IVmlxTtamAlObQz+ii60S4tXXiYPOMKNmTs9S8NGfwTejC6dhhpbJyEFPU2iH+ZzyW/rbSNmdnFKOYHMJ+4kP\n\tXdw7EQMcm+etaZ8XxMwOQi1o2rcTgloSfzaiKLdnyuW5n6fXmFnBV8pn8KezFA+NaS3ENLCqV70hPy6Guq6JCtLZU9A0stn64g/yBdtjuL1d0FZrPOarlJPo\n\tOEnGFYJlhiVbwht3UR5794Vh5yXZckY8Yvx8GJBcc0Bbpmtl3fAgmFFexGNAeN1+XmiZI9lYUAKCxklpLWU/cB0ZDzEpMsidBBsxzQ==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766181,"web_url":"http://patchwork.ozlabs.org/comment/1766181/","msgid":"","list_archive_url":null,"date":"2017-09-11T09:49:14","subject":"Re: [PATCH v10 22/24] ov5670: 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/11/2017 10:00 AM, Sakari Ailus wrote:\n> Parse async sub-devices by using\n> v4l2_subdev_fwnode_reference_parse_sensor_common().\n> \n> Signed-off-by: Sakari Ailus \n\nAcked-by: Hans Verkuil \n\nRegards,\n\n\tHans\n\n\n> ---\n> drivers/media/i2c/ov5670.c | 33 +++++++++++++++++++++++++--------\n> 1 file changed, 25 insertions(+), 8 deletions(-)\n> \n> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c\n> index 6f7a1d6d2200..25970307dd75 100644\n> --- a/drivers/media/i2c/ov5670.c\n> +++ b/drivers/media/i2c/ov5670.c\n> @@ -18,6 +18,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #define OV5670_REG_CHIP_ID\t\t0x300a\n> #define OV5670_CHIP_ID\t\t\t0x005670\n> @@ -1807,6 +1808,7 @@ static const struct ov5670_mode supported_modes[] = {\n> struct ov5670 {\n> \tstruct v4l2_subdev sd;\n> \tstruct media_pad pad;\n> +\tstruct v4l2_async_notifier notifier;\n> \n> \tstruct v4l2_ctrl_handler ctrl_handler;\n> \t/* V4L2 Controls */\n> @@ -2473,11 +2475,13 @@ static int ov5670_probe(struct i2c_client *client)\n> \t\treturn -EINVAL;\n> \n> \tov5670 = devm_kzalloc(&client->dev, sizeof(*ov5670), GFP_KERNEL);\n> -\tif (!ov5670) {\n> -\t\tret = -ENOMEM;\n> -\t\terr_msg = \"devm_kzalloc() error\";\n> -\t\tgoto error_print;\n> -\t}\n> +\tif (!ov5670)\n> +\t\treturn -ENOMEM;\n> +\n> +\tret = v4l2_fwnode_reference_parse_sensor_common(\n> +\t\t&client->dev, &ov5670->notifier);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> \n> \t/* Initialize subdev */\n> \tv4l2_i2c_subdev_init(&ov5670->sd, client, &ov5670_subdev_ops);\n> @@ -2486,7 +2490,7 @@ static int ov5670_probe(struct i2c_client *client)\n> \tret = ov5670_identify_module(ov5670);\n> \tif (ret) {\n> \t\terr_msg = \"ov5670_identify_module() error\";\n> -\t\tgoto error_print;\n> +\t\tgoto error_release_notifier;\n> \t}\n> \n> \tmutex_init(&ov5670->mutex);\n> @@ -2513,11 +2517,18 @@ static int ov5670_probe(struct i2c_client *client)\n> \t\tgoto error_handler_free;\n> \t}\n> \n> +\tret = v4l2_async_subdev_notifier_register(&ov5670->sd,\n> +\t\t\t\t\t\t &ov5670->notifier);\n> +\tif (ret) {\n> +\t\terr_msg = \"can't register async notifier\";\n> +\t\tgoto error_entity_cleanup;\n> +\t}\n> +\n> \t/* Async register for subdev */\n> \tret = v4l2_async_register_subdev(&ov5670->sd);\n> \tif (ret < 0) {\n> \t\terr_msg = \"v4l2_async_register_subdev() error\";\n> -\t\tgoto error_entity_cleanup;\n> +\t\tgoto error_unregister_notifier;\n> \t}\n> \n> \tov5670->streaming = false;\n> @@ -2533,6 +2544,9 @@ static int ov5670_probe(struct i2c_client *client)\n> \n> \treturn 0;\n> \n> +error_unregister_notifier:\n> +\tv4l2_async_notifier_unregister(&ov5670->notifier);\n> +\n> error_entity_cleanup:\n> \tmedia_entity_cleanup(&ov5670->sd.entity);\n> \n> @@ -2542,7 +2556,8 @@ static int ov5670_probe(struct i2c_client *client)\n> error_mutex_destroy:\n> \tmutex_destroy(&ov5670->mutex);\n> \n> -error_print:\n> +error_release_notifier:\n> +\tv4l2_async_notifier_release(&ov5670->notifier);\n> \tdev_err(&client->dev, \"%s: %s %d\\n\", __func__, err_msg, ret);\n> \n> \treturn ret;\n> @@ -2554,6 +2569,8 @@ static int ov5670_remove(struct i2c_client *client)\n> \tstruct ov5670 *ov5670 = to_ov5670(sd);\n> \n> \tv4l2_async_unregister_subdev(sd);\n> +\tv4l2_async_notifier_unregister(&ov5670->notifier);\n> +\tv4l2_async_notifier_release(&ov5670->notifier);\n> \tmedia_entity_cleanup(&sd->entity);\n> \tv4l2_ctrl_handler_free(sd->ctrl_handler);\n> \tmutex_destroy(&ov5670->mutex);\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 3xrNS20zhGz9s7M\n\tfor ;\n\tMon, 11 Sep 2017 19:49:22 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751370AbdIKJtU (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:49:20 -0400","from lb2-smtp-cloud7.xs4all.net ([194.109.24.28]:46001 \"EHLO\n\tlb2-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751327AbdIKJtT (ORCPT\n\t); Mon, 11 Sep 2017 05:49:19 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rLL8dIA0xb2snrLLBd6wBh; Mon, 11 Sep 2017 11:49:18 +0200"],"Subject":"Re: [PATCH v10 22/24] ov5670: Add support for flash and lens devices","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-23-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"","Date":"Mon, 11 Sep 2017 11:49:14 +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":"<20170911080008.21208-23-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfH0lEDgpx9mrKIRGC+lIeJaS0r20335bla3fg08CxW6m6d30j/G5XCFD/fNShKWUm1nl1OxQEShvXlnchLpTP1865FhqmAKyEJJrwUhi9zAMJRBYKYaz\n\t4K3rTSFls/6ZjTONWmu2CBHXBRNvVE2oOkt3VJM/KUoCnKhlv5DnxFKN6W2mIlC0FOk6DyxutFKvcOolxfQSt9qWmmWMNmdcTO7P2YWq3rpfwFyrB+ClRCB4\n\tQDOR9fYMZQJFTuoCCKlXOaiFJKKaCfaUNAHXtG/q8X7mlvh8CQzsXV3JzV77yJNUA6VF0o6Xjq8OR1NrfEHFaFL/chBY475QVEz9Z5/1CnIfejzJRmHCIYUd\n\tXAVxTp0N5hQomBxwquwYGF3PGxmXDWnFkbh84yEXh8iAWV0wYT2KjyHnd5mBRWwQvb8MD7Dr3fSPvq5ai34ehKL8snoTRUEDEO9ptS2pOlJ8ya1oxbbVOXRO\n\tiMnyQwt2ReJ6iw/4QCVMP7J/dviqB1sP7TRlVNIMW9KWVT4IfzA4/RFK4pRMGSLfxJixzjp794Fww2k90ZQ4gyOXcrlmPnNjwEwIWQ==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766182,"web_url":"http://patchwork.ozlabs.org/comment/1766182/","msgid":"<38026d9b-53ab-2e58-cfd2-4364e2095cf3@xs4all.nl>","list_archive_url":null,"date":"2017-09-11T09:49:31","subject":"Re: [PATCH v10 23/24] ov13858: Add support for flash and lens\n\tdevices","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> Parse async sub-devices by using\n> v4l2_subdev_fwnode_reference_parse_sensor_common().\n> \n> Signed-off-by: Sakari Ailus \n\nAcked-by: Hans Verkuil \n\nRegards,\n\n\tHans\n\n\n> ---\n> drivers/media/i2c/ov13858.c | 26 +++++++++++++++++++++++---\n> 1 file changed, 23 insertions(+), 3 deletions(-)\n> \n> diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c\n> index af7af0d14c69..0d60defc7492 100644\n> --- a/drivers/media/i2c/ov13858.c\n> +++ b/drivers/media/i2c/ov13858.c\n> @@ -18,6 +18,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #define OV13858_REG_VALUE_08BIT\t\t1\n> #define OV13858_REG_VALUE_16BIT\t\t2\n> @@ -1028,6 +1029,7 @@ static const struct ov13858_mode supported_modes[] = {\n> struct ov13858 {\n> \tstruct v4l2_subdev sd;\n> \tstruct media_pad pad;\n> +\tstruct v4l2_async_notifier notifier;\n> \n> \tstruct v4l2_ctrl_handler ctrl_handler;\n> \t/* V4L2 Controls */\n> @@ -1715,6 +1717,11 @@ static int ov13858_probe(struct i2c_client *client,\n> \tif (!ov13858)\n> \t\treturn -ENOMEM;\n> \n> +\tret = v4l2_fwnode_reference_parse_sensor_common(\n> +\t\t&client->dev, &ov13858->notifier);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> \t/* Initialize subdev */\n> \tv4l2_i2c_subdev_init(&ov13858->sd, client, &ov13858_subdev_ops);\n> \n> @@ -1722,7 +1729,7 @@ static int ov13858_probe(struct i2c_client *client,\n> \tret = ov13858_identify_module(ov13858);\n> \tif (ret) {\n> \t\tdev_err(&client->dev, \"failed to find sensor: %d\\n\", ret);\n> -\t\treturn ret;\n> +\t\tgoto error_notifier_release;\n> \t}\n> \n> \t/* Set default mode to max resolution */\n> @@ -1730,7 +1737,7 @@ static int ov13858_probe(struct i2c_client *client,\n> \n> \tret = ov13858_init_controls(ov13858);\n> \tif (ret)\n> -\t\treturn ret;\n> +\t\tgoto error_notifier_release;\n> \n> \t/* Initialize subdev */\n> \tov13858->sd.internal_ops = &ov13858_internal_ops;\n> @@ -1746,9 +1753,14 @@ static int ov13858_probe(struct i2c_client *client,\n> \t\tgoto error_handler_free;\n> \t}\n> \n> +\tret = v4l2_async_subdev_notifier_register(&ov13858->sd,\n> +\t\t\t\t\t\t &ov13858->notifier);\n> +\tif (ret)\n> +\t\tgoto error_media_entity;\n> +\n> \tret = v4l2_async_register_subdev(&ov13858->sd);\n> \tif (ret < 0)\n> -\t\tgoto error_media_entity;\n> +\t\tgoto error_notifier_unregister;\n> \n> \t/*\n> \t * Device is already turned on by i2c-core with ACPI domain PM.\n> @@ -1761,11 +1773,17 @@ static int ov13858_probe(struct i2c_client *client,\n> \n> \treturn 0;\n> \n> +error_notifier_unregister:\n> +\tv4l2_async_notifier_unregister(&ov13858->notifier);\n> +\n> error_media_entity:\n> \tmedia_entity_cleanup(&ov13858->sd.entity);\n> \n> error_handler_free:\n> \tov13858_free_controls(ov13858);\n> +\n> +error_notifier_release:\n> +\tv4l2_async_notifier_release(&ov13858->notifier);\n> \tdev_err(&client->dev, \"%s failed:%d\\n\", __func__, ret);\n> \n> \treturn ret;\n> @@ -1777,6 +1795,8 @@ static int ov13858_remove(struct i2c_client *client)\n> \tstruct ov13858 *ov13858 = to_ov13858(sd);\n> \n> \tv4l2_async_unregister_subdev(sd);\n> +\tv4l2_async_notifier_unregister(&ov13858->notifier);\n> +\tv4l2_async_notifier_release(&ov13858->notifier);\n> \tmedia_entity_cleanup(&sd->entity);\n> \tov13858_free_controls(ov13858);\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 3xrNSR71MSz9s7M\n\tfor ;\n\tMon, 11 Sep 2017 19:49:43 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751491AbdIKJti (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:49:38 -0400","from lb3-smtp-cloud7.xs4all.net ([194.109.24.31]:60271 \"EHLO\n\tlb3-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751327AbdIKJtg (ORCPT\n\t); Mon, 11 Sep 2017 05:49:36 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rLLPdIABUb2snrLLSd6wIe; Mon, 11 Sep 2017 11:49:35 +0200"],"Subject":"Re: [PATCH v10 23/24] ov13858: Add support for flash and lens\n\tdevices","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-24-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<38026d9b-53ab-2e58-cfd2-4364e2095cf3@xs4all.nl>","Date":"Mon, 11 Sep 2017 11:49:31 +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":"<20170911080008.21208-24-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfKl9SJkfQg6S1xEJirjHP8pl3B3LdirpXWESKgtQL/mDGu8Rh4rTA7ccAFoTE9aoMipFXvKY6++m1/f6yKkYroj8vxJnoub2vI/RfG5B1OoC8WK0AeEs\n\tk1AfSuPN49aKdoQJkwHrvs2hWecj+rm+Xg63tFmtTRr3xiq9tKOEnrvYiQnkA/AJ2Sp0H7qDpqGwfmIXEPl0bobKCKgcnHq2U+hWyAMyxeXeR6cKcjxFNqE7\n\tXcRBpwfhwcTExfTcpgUqITuG8oo2cDIqgx9nd3b0aRbl4JLNlGnI7vFB/YzrJ70WceDqr8gWAuv5B2+m7IWJS0x9cvLRieJ+k51a9Iik4+oH8rt91Ajqh72x\n\t7xk4qZA2/UEtUTT8aiBiYguqFqbHNKYmtigV7pmw100YcZ9E6NdDcq6rtmLFqxPe9HmezrayOaH1S0H9/vU2D8SLUV91FtMTd8eidnEWJwJ8iy6WrVroq4L1\n\tcr5SiWeXMLqoZqssEDsCmu0w2RpNZFhxsf2FZg5vq71dTezkVZdN2SBZj066ovfrw8X8aT0ISHSzdtKisk/fLTYfZoSa23tGVIhU6A==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766184,"web_url":"http://patchwork.ozlabs.org/comment/1766184/","msgid":"<3ea79dc9-9374-008f-7cd9-567a560a0fd5@xs4all.nl>","list_archive_url":null,"date":"2017-09-11T09:50:29","subject":"Re: [PATCH v10 24/24] arm: dts: omap3: N9/N950: Add flash references\n\tto the camera","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> Add flash and indicator LED phandles to the sensor node.\n> \n> Signed-off-by: Sakari Ailus \n\nAcked-by: Hans Verkuil \n\nRegards,\n\n\tHans\n\n\n> ---\n> arch/arm/boot/dts/omap3-n9.dts | 1 +\n> arch/arm/boot/dts/omap3-n950-n9.dtsi | 4 ++--\n> arch/arm/boot/dts/omap3-n950.dts | 1 +\n> 3 files changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts\n> index b9e58c536afd..39e35f8b8206 100644\n> --- a/arch/arm/boot/dts/omap3-n9.dts\n> +++ b/arch/arm/boot/dts/omap3-n9.dts\n> @@ -26,6 +26,7 @@\n> \t\tclocks = <&isp 0>;\n> \t\tclock-frequency = <9600000>;\n> \t\tnokia,nvm-size = <(16 * 64)>;\n> +\t\tflash-leds = <&as3645a_flash &as3645a_indicator>;\n> \t\tport {\n> \t\t\tsmia_1_1: endpoint {\n> \t\t\t\tlink-frequencies = /bits/ 64 <199200000 210000000 499200000>;\n> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi\n> index 1b0bd72945f2..12fbb3da5fce 100644\n> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi\n> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi\n> @@ -271,14 +271,14 @@\n> \t\t#size-cells = <0>;\n> \t\treg = <0x30>;\n> \t\tcompatible = \"ams,as3645a\";\n> -\t\tflash@0 {\n> +\t\tas3645a_flash: flash@0 {\n> \t\t\treg = <0x0>;\n> \t\t\tflash-timeout-us = <150000>;\n> \t\t\tflash-max-microamp = <320000>;\n> \t\t\tled-max-microamp = <60000>;\n> \t\t\tams,input-max-microamp = <1750000>;\n> \t\t};\n> -\t\tindicator@1 {\n> +\t\tas3645a_indicator: indicator@1 {\n> \t\t\treg = <0x1>;\n> \t\t\tled-max-microamp = <10000>;\n> \t\t};\n> diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts\n> index 646601a3ebd8..c354a1ed1e70 100644\n> --- a/arch/arm/boot/dts/omap3-n950.dts\n> +++ b/arch/arm/boot/dts/omap3-n950.dts\n> @@ -60,6 +60,7 @@\n> \t\tclocks = <&isp 0>;\n> \t\tclock-frequency = <9600000>;\n> \t\tnokia,nvm-size = <(16 * 64)>;\n> +\t\tflash-leds = <&as3645a_flash &as3645a_indicator>;\n> \t\tport {\n> \t\t\tsmia_1_1: endpoint {\n> \t\t\t\tlink-frequencies = /bits/ 64 <210000000 333600000 398400000>;\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 3xrNTb1SYcz9s7G\n\tfor ;\n\tMon, 11 Sep 2017 19:50:43 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751558AbdIKJui (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:50:38 -0400","from lb3-smtp-cloud7.xs4all.net ([194.109.24.31]:48118 \"EHLO\n\tlb3-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751553AbdIKJug (ORCPT\n\t); Mon, 11 Sep 2017 05:50:36 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid rLMLdIAmab2snrLMOd6wi0; Mon, 11 Sep 2017 11:50:33 +0200"],"Subject":"Re: [PATCH v10 24/24] arm: dts: omap3: N9/N950: Add flash references\n\tto the camera","To":"Sakari Ailus , linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-25-sakari.ailus@linux.intel.com>","From":"Hans Verkuil ","Message-ID":"<3ea79dc9-9374-008f-7cd9-567a560a0fd5@xs4all.nl>","Date":"Mon, 11 Sep 2017 11:50:29 +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":"<20170911080008.21208-25-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfCc4W8n0KS6I5u02SQM3hcHut2Z5FpfRKHFOpJpCSaQuVaZ/Qtk8PI+WhHg6Xl0t+MjzA72D0p1mBHmA9sdYc7hAwu+X6uagJULmmo2Vea0WAnjyQpjX\n\tHcRCKy5Gt470XutHMUUVcONZmuZck0kxvCdDM5v4FUzY3GJXkTTmKUcdTnnPbpVT+iB+FJL8014oOuKGJPLMA64fnGJxeBC+h63M3ayP/6inDN8NDlnCoNzA\n\tSqgijGlXHb5LW6xNXy72RefH4E1sGt0N1qfdaLm2d8msxaPUw9R+LC8E2UKPJeMjqz1afCAkf4i2cV5aQ5ti6tV+Hh+g78++N5XQ+Nhly+QEFME1EZhmDOVc\n\tdx+0yrdHOXZrdPmAzz/GW244jzxeQaqlINZheabjxU5pPZ8MPYHapGeFKpMXdat5xJolLn8cChX9c4zy2BsR4syRvmfcIc+9nAQ67QQo0ADAyno1LsyY6smD\n\tFZhcuzObFxdLcHQy2h4jdMK1oqyU9NB7t6fCWBnzQl2OwfeWuTNFC9r3mJ3ALf7o+cAWYTJVrHK/2YiTuq1UIFjw/oGNkeCrtpfHfg==","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766190,"web_url":"http://patchwork.ozlabs.org/comment/1766190/","msgid":"<20170911095927.femgxvok2wbe6zdk@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-11T09:59:27","subject":"Re: [PATCH v10 17/24] 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\nThanks for the review!\n\nOn Mon, Sep 11, 2017 at 11:14:03AM +0200, Hans Verkuil wrote:\n> On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> > Add function v4l2_fwnode_reference_count() for counting external\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> This paragraph appears to be out-of-date.\n\nWill remove.\n\n> \n> > \n> > Signed-off-by: Sakari Ailus \n> > ---\n> > drivers/media/v4l2-core/v4l2-fwnode.c | 47 +++++++++++++++++++++++++++++++++++\n> > 1 file changed, 47 insertions(+)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > index d978f2d714ca..4821c4989119 100644\n> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > @@ -449,6 +449,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> > }\n> > EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\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 = 0;\n> > +\tint ret;\n> > +\n> > +\tfor (; !fwnode_property_get_reference_args(\n> > +\t\t dev_fwnode(dev), prop, NULL, 0, index, &args); index++)\n> > +\t\tfwnode_handle_put(args.fwnode);\n> > +\n> \n> If nothing is found (i.e. index == 0), shouldn't you just return here?\n\nYou could. yes. It would actually simplify the code, I could just return\n-ENOENT here.\n\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 (ret = -ENOENT, index = 0;\n> \n> There is no reason for the 'ret = -ENOENT' to be in the for(), just set it before\n> the 'for' statement.\n> \n> > +\t !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> If the loop doesn't find anything, then it still returns 0, not -ENOENT.\n> So why set ret to ENOENT? Something weird going on here.\n\n:-)\n\nI think -ENOENT would make sense indeed if there are no entries found.\n\n> \n> I think you should also add a comment explaining this function.\n\nYes. I had that in the header when it was exported, I'll add the same\ncomments here.\n\n> \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>","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 3xrNgn1Mtkz9s7M\n\tfor ;\n\tMon, 11 Sep 2017 19:59:33 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751435AbdIKJ7b (ORCPT\n\t);\n\tMon, 11 Sep 2017 05:59:31 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:50586 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751253AbdIKJ7a (ORCPT\n\t); Mon, 11 Sep 2017 05:59:30 -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 8BB27600D5;\n\tMon, 11 Sep 2017 12:59:28 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1drLV2-0003iv-3F; Mon, 11 Sep 2017 12:59:28 +0300"],"Date":"Mon, 11 Sep 2017 12:59:27 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v10 17/24] v4l: fwnode: Add a helper function for parsing\n\tgeneric references","Message-ID":"<20170911095927.femgxvok2wbe6zdk@valkosipuli.retiisi.org.uk>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-18-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":1766225,"web_url":"http://patchwork.ozlabs.org/comment/1766225/","msgid":"<20170911110634.cgwrhdvnvzgmvfuk@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-11T11:06:34","subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"On Mon, Sep 11, 2017 at 11:47:11AM +0200, Hans Verkuil wrote:\n> On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> > Add v4l2_fwnode_parse_reference_sensor_common for parsing common\n> > sensor properties that refer to adjacent devices such as flash or lens\n> > driver chips.\n> > \n> > As this is an association only, there's little a regular driver needs to\n> > know about these devices as such.\n> > \n> > Signed-off-by: Sakari Ailus \n> > Acked-by: Hans Verkuil \n> > ---\n> > drivers/media/v4l2-core/v4l2-fwnode.c | 35 +++++++++++++++++++++++++++++++++++\n> > include/media/v4l2-fwnode.h | 13 +++++++++++++\n> > 2 files changed, 48 insertions(+)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > index 56eee5bbd3b5..b9e60a0e8f86 100644\n> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > @@ -589,6 +589,41 @@ static int v4l2_fwnode_reference_parse_int_props(\n> > \treturn ret;\n> > }\n> > \n> > +int v4l2_fwnode_reference_parse_sensor_common(\n> > +\tstruct device *dev, struct v4l2_async_notifier *notifier)\n> > +{\n> > +\tstatic const char *led_props[] = { \"led\" };\n> > +\tstatic const struct {\n> > +\t\tconst char *name;\n> > +\t\tconst char **props;\n> > +\t\tunsigned int nprops;\n> > +\t} props[] = {\n> > +\t\t{ \"flash-leds\", led_props, ARRAY_SIZE(led_props) },\n> > +\t\t{ \"lens-focus\", NULL, 0 },\n> > +\t};\n> > +\tunsigned int i;\n> > +\n> > +\tfor (i = 0; i < ARRAY_SIZE(props); i++) {\n> > +\t\tint ret;\n> > +\n> > +\t\tif (props[i].props && is_acpi_node(dev_fwnode(dev)))\n> > +\t\t\tret = v4l2_fwnode_reference_parse_int_props(\n> > +\t\t\t\tdev, notifier, props[i].name,\n> > +\t\t\t\tprops[i].props, props[i].nprops);\n> > +\t\telse\n> > +\t\t\tret = v4l2_fwnode_reference_parse(\n> > +\t\t\t\tdev, notifier, props[i].name);\n> > +\t\tif (ret) {\n> > +\t\t\tdev_warn(dev, \"parsing property \\\"%s\\\" failed (%d)\\n\",\n> > +\t\t\t\t props[i].name, ret);\n> > +\t\t\treturn ret;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +EXPORT_SYMBOL_GPL(v4l2_fwnode_reference_parse_sensor_common);\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 3819a73c3c8a..bcec1ce101dc 100644\n> > --- a/include/media/v4l2-fwnode.h\n> > +++ b/include/media/v4l2-fwnode.h\n> > @@ -257,4 +257,17 @@ 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_fwnode_reference_parse_sensor_common - parse common references on\n> > + *\t\t\t\t\t sensors 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> > + *\n> \n> I think you should add a note that if this function returns 0 the\n> caller should remember to call v4l2_async_notifier_release. That is not\n> immediately obvious.\n\nI'll add that, plus a note to v4l2_async_notifier_release() as well.","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 3xrQ9D5ym4z9s7G\n\tfor ;\n\tMon, 11 Sep 2017 21:06:40 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751076AbdIKLGj (ORCPT\n\t);\n\tMon, 11 Sep 2017 07:06:39 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:51240 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1750967AbdIKLGi (ORCPT\n\t); Mon, 11 Sep 2017 07:06:38 -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 A7491600DC;\n\tMon, 11 Sep 2017 14:06:35 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1drMXz-0003jF-4q; Mon, 11 Sep 2017 14:06:35 +0300"],"Date":"Mon, 11 Sep 2017 14:06:34 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","Message-ID":"<20170911110634.cgwrhdvnvzgmvfuk@valkosipuli.retiisi.org.uk>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-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":1766228,"web_url":"http://patchwork.ozlabs.org/comment/1766228/","msgid":"<20170911111258.GB28095@amd>","list_archive_url":null,"date":"2017-09-11T11:12:58","subject":"Re: [PATCH v10 24/24] arm: dts: omap3: N9/N950: Add flash references\n\tto the camera","submitter":{"id":2109,"url":"http://patchwork.ozlabs.org/api/people/2109/","name":"Pavel Machek","email":"pavel@ucw.cz"},"content":"On Mon 2017-09-11 11:00:08, Sakari Ailus wrote:\n> Add flash and indicator LED phandles to the sensor node.\n> \n> Signed-off-by: Sakari Ailus \n\nAcked-by: Pavel Machek ","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 3xrQJd752Fz9s7G\n\tfor ;\n\tMon, 11 Sep 2017 21:13:05 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751224AbdIKLNA (ORCPT\n\t);\n\tMon, 11 Sep 2017 07:13:00 -0400","from atrey.karlin.mff.cuni.cz ([195.113.26.193]:54286 \"EHLO\n\tatrey.karlin.mff.cuni.cz\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751078AbdIKLNA (ORCPT\n\t); Mon, 11 Sep 2017 07:13:00 -0400","by atrey.karlin.mff.cuni.cz (Postfix, from userid 512)\n\tid C58C0824C7; Mon, 11 Sep 2017 13:12:58 +0200 (CEST)"],"Date":"Mon, 11 Sep 2017 13:12:58 +0200","From":"Pavel Machek ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org, sre@kernel.org","Subject":"Re: [PATCH v10 24/24] arm: dts: omap3: N9/N950: Add flash references\n\tto the camera","Message-ID":"<20170911111258.GB28095@amd>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-25-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"tjCHc7DPkfUGtrlw\"","Content-Disposition":"inline","In-Reply-To":"<20170911080008.21208-25-sakari.ailus@linux.intel.com>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766231,"web_url":"http://patchwork.ozlabs.org/comment/1766231/","msgid":"<20170911111746.GC28095@amd>","list_archive_url":null,"date":"2017-09-11T11:17:46","subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","submitter":{"id":2109,"url":"http://patchwork.ozlabs.org/api/people/2109/","name":"Pavel Machek","email":"pavel@ucw.cz"},"content":"On Mon 2017-09-11 11:00:03, Sakari Ailus wrote:\n> Add v4l2_fwnode_parse_reference_sensor_common for parsing common\n> sensor properties that refer to adjacent devices such as flash or lens\n> driver chips.\n> \n> As this is an association only, there's little a regular driver needs to\n> know about these devices as such.\n> \n> Signed-off-by: Sakari Ailus \n> Acked-by: Hans Verkuil \n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 35 +++++++++++++++++++++++++++++++++++\n> include/media/v4l2-fwnode.h | 13 +++++++++++++\n> 2 files changed, 48 insertions(+)\n> \n> \n> +/**\n> + * v4l2_fwnode_reference_parse_sensor_common - parse common references on\n> + *\t\t\t\t\t sensors 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> + *\n> + * Return: 0 on success\n> + *\t -ENOMEM if memory allocation failed\n> + *\t -EINVAL if property parsing failed\n> + */\n\nReturns: would sound more correct to me, but it seems kernel is split\non that.\n\nAcked-by: Pavel Machek ","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 3xrQQH5FZrz9s7G\n\tfor ;\n\tMon, 11 Sep 2017 21:17:59 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751673AbdIKLRz (ORCPT\n\t);\n\tMon, 11 Sep 2017 07:17:55 -0400","from atrey.karlin.mff.cuni.cz ([195.113.26.193]:54507 \"EHLO\n\tatrey.karlin.mff.cuni.cz\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751707AbdIKLRt (ORCPT\n\t); Mon, 11 Sep 2017 07:17:49 -0400","by atrey.karlin.mff.cuni.cz (Postfix, from userid 512)\n\tid 32E14824C7; Mon, 11 Sep 2017 13:17:47 +0200 (CEST)"],"Date":"Mon, 11 Sep 2017 13:17:46 +0200","From":"Pavel Machek ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org, sre@kernel.org","Subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","Message-ID":"<20170911111746.GC28095@amd>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-20-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"lCAWRPmW1mITcIfM\"","Content-Disposition":"inline","In-Reply-To":"<20170911080008.21208-20-sakari.ailus@linux.intel.com>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766272,"web_url":"http://patchwork.ozlabs.org/comment/1766272/","msgid":"<20170911122820.fkbd2rnaddiestab@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-11T12:28:20","subject":"Re: [PATCH v10 18/24] 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\nThanks for the review.\n\nOn Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:\n> Typo in subject: interger -> integer\n> \n> On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> > v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under\n> > the device's own fwnode, \n> \n> Sorry, you lost me here. Which device are we talking about?\n\nThe fwnode related a struct device, in other words what dev_fwnode(dev)\ngives you. This is either struct device.fwnode or struct\ndevice.of_node.fwnode, depending on which firmware interface was used to\ncreate the device.\n\nI'll add a note of this.\n\n> \n> > it will follow child fwnodes with the given\n> > property -- value pair and return the resulting fwnode.\n> \n> property-value pair (easier readable that way).\n> \n> You only describe v4l2_fwnode_reference_parse_int_prop(), not\n> v4l2_fwnode_reference_parse_int_props().\n\nYes, I think I changed the naming but forgot to update the commit. I'll do\nthat now.\n\n> \n> > \n> > Signed-off-by: Sakari Ailus \n> > ---\n> > drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++\n> > 1 file changed, 93 insertions(+)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > index 4821c4989119..56eee5bbd3b5 100644\n> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(\n> > \treturn ret;\n> > }\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> Need comments describing what this does.\n\nYes. I'll also rename it (get -> read) for consistency with the async\nchanges.\n\n> \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> > +\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 == -EINVAL ? -ENOENT : ret);\n> \n> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.\n\nfwnode_property_get_reference_args() returns currently a little bit\ndifferent error codes in ACPI / DT. This is worth documenting there and\nfixing as well.\n\n> \n> > +\n> > +\tfor (fwnode = fwnode_args.fwnode;\n> > +\t nprops; nprops--, fwnode = child, props++, args++) {\n> \n> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...\n> It's hard to parse.\n\nHmm. I'm not sure if that really helps; the function is just handling each\nentry in the array and related array pointers are changed accordingly. The\nfwnode = child assignment is there to move to the child node. I.e. what you\nneed for handling the loop itself.\n\nI can change this though if you think it really makes a difference for\nbetter.\n\n> \n> I would make this a 'while (nprops)' and write out all the other assignments,\n> increments and decrements.\n> \n> > +\t\tu32 val;\n> > +\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\tif (val == *args)\n> > +\t\t\t\tbreak;\n> \n> I'm lost. This really needs comments and perhaps even an DT or ACPI example\n> so you can see what exactly it is we're doing here.\n\nI'll add comments to the code. A good example will be ACPI documentation\nfor LEDs, see 17th patch in v9. That will go through the linux-pm tree so\nit won't be available in the same tree for a while.\n\n> \n> > +\t\t}\n> > +\n> > +\t\tfwnode_handle_put(fwnode);\n> > +\n> > +\t\tif (!child) {\n> > +\t\t\tfwnode = ERR_PTR(-ENOENT);\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\treturn fwnode;\n> > +}\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> Needs comments describing what this does.\n\nWill add.\n\n> \n> > +{\n> > +\tstruct fwnode_handle *fwnode;\n> > +\tunsigned int index = 0;\n> > +\tint ret;\n> > +\n> > +\twhile (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(\n> > +\t\t\t\tdev_fwnode(dev), prop, index, props,\n> > +\t\t\t\tnprops)))) {\n> > +\t\tfwnode_handle_put(fwnode);\n> > +\t\tindex++;\n> > +\t}\n> > +\n> > +\tif (PTR_ERR(fwnode) != -ENOENT)\n> > +\t\treturn PTR_ERR(fwnode);\n> \n> Missing 'if (index == 0)'?\n\nYes, will add.\n\n> \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))); ) {\n> \n> I'd add 'index++' in this for-loop. It's weird that it is missing.\n\nAgreed, I'll move it there.\n\n> \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> > +\n> > +\t\tfwnode_handle_put(fwnode);\n> > +\n> > +\t\tindex++;\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> >","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 3xrRzp2wPkz9s4q\n\tfor ;\n\tMon, 11 Sep 2017 22:28:38 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751787AbdIKM2Z (ORCPT\n\t);\n\tMon, 11 Sep 2017 08:28:25 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:51962 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751317AbdIKM2Y (ORCPT\n\t); Mon, 11 Sep 2017 08:28:24 -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 7DF9860102;\n\tMon, 11 Sep 2017 15:28:21 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1drNp6-0003kU-W1; Mon, 11 Sep 2017 15:28:21 +0300"],"Date":"Mon, 11 Sep 2017 15:28:20 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","Message-ID":"<20170911122820.fkbd2rnaddiestab@valkosipuli.retiisi.org.uk>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-19-sakari.ailus@linux.intel.com>\n\t<11c951eb-0315-0149-829e-ed73d748e783@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<11c951eb-0315-0149-829e-ed73d748e783@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":1766280,"web_url":"http://patchwork.ozlabs.org/comment/1766280/","msgid":"<2e2eba02-39bc-11e1-d9f1-b83a6f580667@xs4all.nl>","list_archive_url":null,"date":"2017-09-11T12:38:23","subject":"Re: [PATCH v10 18/24] 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":"On 09/11/2017 02:28 PM, Sakari Ailus wrote:\n> Hi Hans,\n> \n> Thanks for the review.\n> \n> On Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:\n>> Typo in subject: interger -> integer\n>>\n>> On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n>>> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under\n>>> the device's own fwnode, \n>>\n>> Sorry, you lost me here. Which device are we talking about?\n> \n> The fwnode related a struct device, in other words what dev_fwnode(dev)\n> gives you. This is either struct device.fwnode or struct\n> device.of_node.fwnode, depending on which firmware interface was used to\n> create the device.\n> \n> I'll add a note of this.\n> \n>>\n>>> it will follow child fwnodes with the given\n>>> property -- value pair and return the resulting fwnode.\n>>\n>> property-value pair (easier readable that way).\n>>\n>> You only describe v4l2_fwnode_reference_parse_int_prop(), not\n>> v4l2_fwnode_reference_parse_int_props().\n> \n> Yes, I think I changed the naming but forgot to update the commit. I'll do\n> that now.\n> \n>>\n>>>\n>>> Signed-off-by: Sakari Ailus \n>>> ---\n>>> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++\n>>> 1 file changed, 93 insertions(+)\n>>>\n>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> index 4821c4989119..56eee5bbd3b5 100644\n>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(\n>>> \treturn ret;\n>>> }\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>> Need comments describing what this does.\n> \n> Yes. I'll also rename it (get -> read) for consistency with the async\n> changes.\n\nWhich async changes? Since the fwnode_handle that's returned is refcounted\nI wonder if 'get' isn't the right name in this case.\n\n> \n>>\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>>> +\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 == -EINVAL ? -ENOENT : ret);\n>>\n>> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.\n> \n> fwnode_property_get_reference_args() returns currently a little bit\n> different error codes in ACPI / DT. This is worth documenting there and\n> fixing as well.\n> \n>>\n>>> +\n>>> +\tfor (fwnode = fwnode_args.fwnode;\n>>> +\t nprops; nprops--, fwnode = child, props++, args++) {\n>>\n>> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...\n>> It's hard to parse.\n> \n> Hmm. I'm not sure if that really helps; the function is just handling each\n> entry in the array and related array pointers are changed accordingly. The\n> fwnode = child assignment is there to move to the child node. I.e. what you\n> need for handling the loop itself.\n> \n> I can change this though if you think it really makes a difference for\n> better.\n\nI think so, yes. I noticed you like complex for-loops :-)\n\n> \n>>\n>> I would make this a 'while (nprops)' and write out all the other assignments,\n>> increments and decrements.\n>>\n>>> +\t\tu32 val;\n>>> +\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\tif (val == *args)\n>>> +\t\t\t\tbreak;\n>>\n>> I'm lost. This really needs comments and perhaps even an DT or ACPI example\n>> so you can see what exactly it is we're doing here.\n> \n> I'll add comments to the code. A good example will be ACPI documentation\n> for LEDs, see 17th patch in v9. That will go through the linux-pm tree so\n> it won't be available in the same tree for a while.\n\nIdeally an ACPI and an equivalent DT example would be nice to have, but I might\nbe asking too much. I'm not that familiar with ACPI, so for me a DT example\nis easier.\n\n> \n>>\n>>> +\t\t}\n>>> +\n>>> +\t\tfwnode_handle_put(fwnode);\n>>> +\n>>> +\t\tif (!child) {\n>>> +\t\t\tfwnode = ERR_PTR(-ENOENT);\n>>> +\t\t\tbreak;\n>>> +\t\t}\n>>> +\t}\n>>> +\n>>> +\treturn fwnode;\n>>> +}\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>> Needs comments describing what this does.\n> \n> Will add.\n> \n>>\n>>> +{\n>>> +\tstruct fwnode_handle *fwnode;\n>>> +\tunsigned int index = 0;\n>>> +\tint ret;\n>>> +\n>>> +\twhile (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(\n>>> +\t\t\t\tdev_fwnode(dev), prop, index, props,\n>>> +\t\t\t\tnprops)))) {\n>>> +\t\tfwnode_handle_put(fwnode);\n>>> +\t\tindex++;\n>>> +\t}\n>>> +\n>>> +\tif (PTR_ERR(fwnode) != -ENOENT)\n>>> +\t\treturn PTR_ERR(fwnode);\n>>\n>> Missing 'if (index == 0)'?\n> \n> Yes, will add.\n> \n>>\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))); ) {\n>>\n>> I'd add 'index++' in this for-loop. It's weird that it is missing.\n> \n> Agreed, I'll move it there.\n> \n>>\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>>> +\n>>> +\t\tfwnode_handle_put(fwnode);\n>>> +\n>>> +\t\tindex++;\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\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 3xrSCJ0CPmz9s4q\n\tfor ;\n\tMon, 11 Sep 2017 22:38:36 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751672AbdIKMie (ORCPT\n\t);\n\tMon, 11 Sep 2017 08:38:34 -0400","from lb1-smtp-cloud8.xs4all.net ([194.109.24.21]:45167 \"EHLO\n\tlb1-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751658AbdIKMid (ORCPT\n\t); Mon, 11 Sep 2017 08:38:33 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid rNypddJ8bcQyLrNysdpJoH; Mon, 11 Sep 2017 14:38:32 +0200"],"Subject":"Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","To":"Sakari Ailus ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-19-sakari.ailus@linux.intel.com>\n\t<11c951eb-0315-0149-829e-ed73d748e783@xs4all.nl>\n\t<20170911122820.fkbd2rnaddiestab@valkosipuli.retiisi.org.uk>","From":"Hans Verkuil ","Message-ID":"<2e2eba02-39bc-11e1-d9f1-b83a6f580667@xs4all.nl>","Date":"Mon, 11 Sep 2017 14:38:23 +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":"<20170911122820.fkbd2rnaddiestab@valkosipuli.retiisi.org.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfGjONWklY/yonuD8d8UTEdUBScsrYTpc4LpS3efC5ZtoQJ0KhTrd549IExkzNfdjxLPZHIwXXfa0YK/02gMBwKV2tC5kfePVFCXK7IY7vAInSpf0/3pW\n\t/JHVS8lqKcPD9lriUfEoZnlb0VeA+lMfX6hzGCyr+t0+ScXS75IZ3qL4An0s8Qt1JFVGM2rhM5WfmL4MK665jZJ0VEwJupHmx0UI2DcSDgXwX4j+bF7BbO89\n\t8+cYlgJogfoLlJy+WUM9m6DjNm5llyEXAPRSRZL/Q+mBAnVJ7hvQ5NaNLDKhja2rNOVg+68Jk29CwtLykpDNxk/yL4SKzLilVEJvNmgwDOG6xXlPGGu4ibkt\n\tjvfOBQq4KKM41L1eJtn/Kp86qbChu3qNN+v0rZjsge/Atd75QhG0VDQR30AKzBXBhKqVYLDZviPk4K34FJO/ZWpQ6RJXuIBZlAmqfuBVHj0dGVrrvcYDug/Q\n\tMy/RhMVZFKLtJD4p","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766302,"web_url":"http://patchwork.ozlabs.org/comment/1766302/","msgid":"<20170911132710.mcgqn6tbiabzvpqq@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-11T13:27:10","subject":"Re: [PATCH v10 18/24] 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 Mon, Sep 11, 2017 at 02:38:23PM +0200, Hans Verkuil wrote:\n> On 09/11/2017 02:28 PM, Sakari Ailus wrote:\n> > Hi Hans,\n> > \n> > Thanks for the review.\n> > \n> > On Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:\n> >> Typo in subject: interger -> integer\n> >>\n> >> On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n> >>> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under\n> >>> the device's own fwnode, \n> >>\n> >> Sorry, you lost me here. Which device are we talking about?\n> > \n> > The fwnode related a struct device, in other words what dev_fwnode(dev)\n> > gives you. This is either struct device.fwnode or struct\n> > device.of_node.fwnode, depending on which firmware interface was used to\n> > create the device.\n> > \n> > I'll add a note of this.\n> > \n> >>\n> >>> it will follow child fwnodes with the given\n> >>> property -- value pair and return the resulting fwnode.\n> >>\n> >> property-value pair (easier readable that way).\n> >>\n> >> You only describe v4l2_fwnode_reference_parse_int_prop(), not\n> >> v4l2_fwnode_reference_parse_int_props().\n> > \n> > Yes, I think I changed the naming but forgot to update the commit. I'll do\n> > that now.\n> > \n> >>\n> >>>\n> >>> Signed-off-by: Sakari Ailus \n> >>> ---\n> >>> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++\n> >>> 1 file changed, 93 insertions(+)\n> >>>\n> >>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> index 4821c4989119..56eee5bbd3b5 100644\n> >>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> >>> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(\n> >>> \treturn ret;\n> >>> }\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> >> Need comments describing what this does.\n> > \n> > Yes. I'll also rename it (get -> read) for consistency with the async\n> > changes.\n> \n> Which async changes? Since the fwnode_handle that's returned is refcounted\n> I wonder if 'get' isn't the right name in this case.\n\nRight. True. I'll leave that as-is then.\n\n> \n> > \n> >>\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> >>> +\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 == -EINVAL ? -ENOENT : ret);\n> >>\n> >> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.\n> > \n> > fwnode_property_get_reference_args() returns currently a little bit\n> > different error codes in ACPI / DT. This is worth documenting there and\n> > fixing as well.\n> > \n> >>\n> >>> +\n> >>> +\tfor (fwnode = fwnode_args.fwnode;\n> >>> +\t nprops; nprops--, fwnode = child, props++, args++) {\n> >>\n> >> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...\n> >> It's hard to parse.\n> > \n> > Hmm. I'm not sure if that really helps; the function is just handling each\n> > entry in the array and related array pointers are changed accordingly. The\n> > fwnode = child assignment is there to move to the child node. I.e. what you\n> > need for handling the loop itself.\n> > \n> > I can change this though if you think it really makes a difference for\n> > better.\n> \n> I think so, yes. I noticed you like complex for-loops :-)\n\nI don't really see a difference. The loop increment will just move at the\nend of the block inside the loop.\n\n> \n> > \n> >>\n> >> I would make this a 'while (nprops)' and write out all the other assignments,\n> >> increments and decrements.\n> >>\n> >>> +\t\tu32 val;\n> >>> +\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\tif (val == *args)\n> >>> +\t\t\t\tbreak;\n> >>\n> >> I'm lost. This really needs comments and perhaps even an DT or ACPI example\n> >> so you can see what exactly it is we're doing here.\n> > \n> > I'll add comments to the code. A good example will be ACPI documentation\n> > for LEDs, see 17th patch in v9. That will go through the linux-pm tree so\n> > it won't be available in the same tree for a while.\n> \n> Ideally an ACPI and an equivalent DT example would be nice to have, but I might\n> be asking too much. I'm not that familiar with ACPI, so for me a DT example\n> is easier.\n\nThis won't be useful on DT although you could technically use it. In DT you\ncan directly refer to any node but on ACPI you can just refer to devices,\nhence this.\n\nWould you be happy with the leds.txt example? I think it's a good example\nas it's directly related to this.\n\n> \n> > \n> >>\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tfwnode_handle_put(fwnode);\n> >>> +\n> >>> +\t\tif (!child) {\n> >>> +\t\t\tfwnode = ERR_PTR(-ENOENT);\n> >>> +\t\t\tbreak;\n> >>> +\t\t}\n> >>> +\t}\n> >>> +\n> >>> +\treturn fwnode;\n> >>> +}\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> >> Needs comments describing what this does.\n> > \n> > Will add.\n> > \n> >>\n> >>> +{\n> >>> +\tstruct fwnode_handle *fwnode;\n> >>> +\tunsigned int index = 0;\n> >>> +\tint ret;\n> >>> +\n> >>> +\twhile (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(\n> >>> +\t\t\t\tdev_fwnode(dev), prop, index, props,\n> >>> +\t\t\t\tnprops)))) {\n> >>> +\t\tfwnode_handle_put(fwnode);\n> >>> +\t\tindex++;\n> >>> +\t}\n> >>> +\n> >>> +\tif (PTR_ERR(fwnode) != -ENOENT)\n> >>> +\t\treturn PTR_ERR(fwnode);\n> >>\n> >> Missing 'if (index == 0)'?\n> > \n> > Yes, will add.\n> > \n> >>\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))); ) {\n> >>\n> >> I'd add 'index++' in this for-loop. It's weird that it is missing.\n> > \n> > Agreed, I'll move it there.\n> > \n> >>\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> >>> +\n> >>> +\t\tfwnode_handle_put(fwnode);\n> >>> +\n> >>> +\t\tindex++;\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> \n> Regards,\n> \n> \tHans","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 3xrTHq6FMHz9s83\n\tfor ;\n\tMon, 11 Sep 2017 23:27:35 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752158AbdIKN1P (ORCPT\n\t);\n\tMon, 11 Sep 2017 09:27:15 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:52698 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1750842AbdIKN1O (ORCPT\n\t); Mon, 11 Sep 2017 09:27:14 -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 40D1360100;\n\tMon, 11 Sep 2017 16:27:11 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1drOk2-0003kt-Qj; Mon, 11 Sep 2017 16:27:10 +0300"],"Date":"Mon, 11 Sep 2017 16:27:10 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","Message-ID":"<20170911132710.mcgqn6tbiabzvpqq@valkosipuli.retiisi.org.uk>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-19-sakari.ailus@linux.intel.com>\n\t<11c951eb-0315-0149-829e-ed73d748e783@xs4all.nl>\n\t<20170911122820.fkbd2rnaddiestab@valkosipuli.retiisi.org.uk>\n\t<2e2eba02-39bc-11e1-d9f1-b83a6f580667@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<2e2eba02-39bc-11e1-d9f1-b83a6f580667@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":1766306,"web_url":"http://patchwork.ozlabs.org/comment/1766306/","msgid":"<21d712c2-7608-7153-4421-2c12b90dcb7a@xs4all.nl>","list_archive_url":null,"date":"2017-09-11T13:34:16","subject":"Re: [PATCH v10 18/24] 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":"On 09/11/2017 03:27 PM, Sakari Ailus wrote:\n> Hi Hans,\n> \n> On Mon, Sep 11, 2017 at 02:38:23PM +0200, Hans Verkuil wrote:\n>> On 09/11/2017 02:28 PM, Sakari Ailus wrote:\n>>> Hi Hans,\n>>>\n>>> Thanks for the review.\n>>>\n>>> On Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:\n>>>> Typo in subject: interger -> integer\n>>>>\n>>>> On 09/11/2017 10:00 AM, Sakari Ailus wrote:\n>>>>> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under\n>>>>> the device's own fwnode, \n>>>>\n>>>> Sorry, you lost me here. Which device are we talking about?\n>>>\n>>> The fwnode related a struct device, in other words what dev_fwnode(dev)\n>>> gives you. This is either struct device.fwnode or struct\n>>> device.of_node.fwnode, depending on which firmware interface was used to\n>>> create the device.\n>>>\n>>> I'll add a note of this.\n>>>\n>>>>\n>>>>> it will follow child fwnodes with the given\n>>>>> property -- value pair and return the resulting fwnode.\n>>>>\n>>>> property-value pair (easier readable that way).\n>>>>\n>>>> You only describe v4l2_fwnode_reference_parse_int_prop(), not\n>>>> v4l2_fwnode_reference_parse_int_props().\n>>>\n>>> Yes, I think I changed the naming but forgot to update the commit. I'll do\n>>> that now.\n>>>\n>>>>\n>>>>>\n>>>>> Signed-off-by: Sakari Ailus \n>>>>> ---\n>>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++\n>>>>> 1 file changed, 93 insertions(+)\n>>>>>\n>>>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>>>> index 4821c4989119..56eee5bbd3b5 100644\n>>>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n>>>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n>>>>> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(\n>>>>> \treturn ret;\n>>>>> }\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>>>> Need comments describing what this does.\n>>>\n>>> Yes. I'll also rename it (get -> read) for consistency with the async\n>>> changes.\n>>\n>> Which async changes? Since the fwnode_handle that's returned is refcounted\n>> I wonder if 'get' isn't the right name in this case.\n> \n> Right. True. I'll leave that as-is then.\n> \n>>\n>>>\n>>>>\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>>>>> +\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 == -EINVAL ? -ENOENT : ret);\n>>>>\n>>>> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.\n>>>\n>>> fwnode_property_get_reference_args() returns currently a little bit\n>>> different error codes in ACPI / DT. This is worth documenting there and\n>>> fixing as well.\n>>>\n>>>>\n>>>>> +\n>>>>> +\tfor (fwnode = fwnode_args.fwnode;\n>>>>> +\t nprops; nprops--, fwnode = child, props++, args++) {\n>>>>\n>>>> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...\n>>>> It's hard to parse.\n>>>\n>>> Hmm. I'm not sure if that really helps; the function is just handling each\n>>> entry in the array and related array pointers are changed accordingly. The\n>>> fwnode = child assignment is there to move to the child node. I.e. what you\n>>> need for handling the loop itself.\n>>>\n>>> I can change this though if you think it really makes a difference for\n>>> better.\n>>\n>> I think so, yes. I noticed you like complex for-loops :-)\n> \n> I don't really see a difference. The loop increment will just move at the\n> end of the block inside the loop.\n> \n>>\n>>>\n>>>>\n>>>> I would make this a 'while (nprops)' and write out all the other assignments,\n>>>> increments and decrements.\n>>>>\n>>>>> +\t\tu32 val;\n>>>>> +\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\tif (val == *args)\n>>>>> +\t\t\t\tbreak;\n>>>>\n>>>> I'm lost. This really needs comments and perhaps even an DT or ACPI example\n>>>> so you can see what exactly it is we're doing here.\n>>>\n>>> I'll add comments to the code. A good example will be ACPI documentation\n>>> for LEDs, see 17th patch in v9. That will go through the linux-pm tree so\n>>> it won't be available in the same tree for a while.\n>>\n>> Ideally an ACPI and an equivalent DT example would be nice to have, but I might\n>> be asking too much. I'm not that familiar with ACPI, so for me a DT example\n>> is easier.\n> \n> This won't be useful on DT although you could technically use it. In DT you\n> can directly refer to any node but on ACPI you can just refer to devices,\n> hence this.\n\nSo this function will effectively only be used with acpi? That should be\ndocumented. I think that explains some of my confusion since I was trying\nto map this code to a device tree, without much success.\n\n> Would you be happy with the leds.txt example? I think it's a good example\n> as it's directly related to this.\n\nYes, that will work.\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 3xrTRm1tQkz9sNV\n\tfor ;\n\tMon, 11 Sep 2017 23:34:28 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752152AbdIKNeY (ORCPT\n\t);\n\tMon, 11 Sep 2017 09:34:24 -0400","from lb3-smtp-cloud8.xs4all.net ([194.109.24.29]:46517 \"EHLO\n\tlb3-smtp-cloud8.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1752149AbdIKNeW (ORCPT\n\t); Mon, 11 Sep 2017 09:34:22 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid rOqude08QcQyLrOqydpgHg; Mon, 11 Sep 2017 15:34:21 +0200"],"Subject":"Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","To":"Sakari Ailus ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-19-sakari.ailus@linux.intel.com>\n\t<11c951eb-0315-0149-829e-ed73d748e783@xs4all.nl>\n\t<20170911122820.fkbd2rnaddiestab@valkosipuli.retiisi.org.uk>\n\t<2e2eba02-39bc-11e1-d9f1-b83a6f580667@xs4all.nl>\n\t<20170911132710.mcgqn6tbiabzvpqq@valkosipuli.retiisi.org.uk>","From":"Hans Verkuil ","Message-ID":"<21d712c2-7608-7153-4421-2c12b90dcb7a@xs4all.nl>","Date":"Mon, 11 Sep 2017 15:34:16 +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":"<20170911132710.mcgqn6tbiabzvpqq@valkosipuli.retiisi.org.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfJgAG4DdbdX69QmmezD7XOXdq+1sA2lufjxfyIrFVvdfzWoz0j1NSkCckCpQVkaacVDLugetkje5+7GM7RCkizHM8Wcr9yX22/82LCfK1K51ys3BU0rv\n\t20Z+zSCW3b2rs0tYqOy50HZr+8u5Grtel7sPr4rmVjpFznXnrZlN7ITRvRAZCQnSfIZedKt6e6u/1zHA6GO6sM2LeqEYomMwFDK9IjjHuhuB2NsNdGl748h3\n\ttQU8tlGUOmiSDDCsRArbCTnzQAyZXi1MGV2sSc6Ndl8bja6OIcZ/rt3OZ2Iu9CUQbI6OD3pj8UCY7Ep/tjKrVSKdIFm9d6goLOmpS6179L8mG8nlrxslSYvu\n\tBTb9GCIRLwzzwl37dFhsWf6NLu30+Po3/rBMiwprDH3jZthTWt5DBCRzP4syCaG9jYgxyDMiU20M2ZR/FHjRaqBz8oqrIXT1D4x5IQKgG4BD8Ipp88TIhjRz\n\ti0Rx5n/Nnc68Nu5D","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1766325,"web_url":"http://patchwork.ozlabs.org/comment/1766325/","msgid":"<20170911141200.rxn36p7u77b2rzzs@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-11T14:12:01","subject":"Re: [PATCH v10 18/24] 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":"On Mon, Sep 11, 2017 at 03:34:16PM +0200, Hans Verkuil wrote:\n> >>>>> +\t\tu32 val;\n> >>>>> +\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\tif (val == *args)\n> >>>>> +\t\t\t\tbreak;\n> >>>>\n> >>>> I'm lost. This really needs comments and perhaps even an DT or ACPI example\n> >>>> so you can see what exactly it is we're doing here.\n> >>>\n> >>> I'll add comments to the code. A good example will be ACPI documentation\n> >>> for LEDs, see 17th patch in v9. That will go through the linux-pm tree so\n> >>> it won't be available in the same tree for a while.\n> >>\n> >> Ideally an ACPI and an equivalent DT example would be nice to have, but I might\n> >> be asking too much. I'm not that familiar with ACPI, so for me a DT example\n> >> is easier.\n> > \n> > This won't be useful on DT although you could technically use it. In DT you\n> > can directly refer to any node but on ACPI you can just refer to devices,\n> > hence this.\n> \n> So this function will effectively only be used with acpi? That should be\n> documented. I think that explains some of my confusion since I was trying\n> to map this code to a device tree, without much success.\n\nI'll add to the documentation of the function:\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> \n> > Would you be happy with the leds.txt example? I think it's a good example\n> > as it's directly related to this.\n> \n> Yes, that will work.\n\nI'll add a separate patch that I'll post later on. The ACPI documentation\nshould get merged first.","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 3xrVHD0nhtz9s83\n\tfor ;\n\tTue, 12 Sep 2017 00:12:08 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750926AbdIKOMF (ORCPT\n\t);\n\tMon, 11 Sep 2017 10:12:05 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:53056 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1750715AbdIKOME (ORCPT\n\t); Mon, 11 Sep 2017 10:12:04 -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 E7BDB60100;\n\tMon, 11 Sep 2017 17:12:01 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1drPRR-0003l7-9K; Mon, 11 Sep 2017 17:12:01 +0300"],"Date":"Mon, 11 Sep 2017 17:12:01 +0300","From":"Sakari Ailus ","To":"Hans Verkuil ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tlinux-acpi@vger.kernel.org, mika.westerberg@intel.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain\n\tdevice / interger references","Message-ID":"<20170911141200.rxn36p7u77b2rzzs@valkosipuli.retiisi.org.uk>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-19-sakari.ailus@linux.intel.com>\n\t<11c951eb-0315-0149-829e-ed73d748e783@xs4all.nl>\n\t<20170911122820.fkbd2rnaddiestab@valkosipuli.retiisi.org.uk>\n\t<2e2eba02-39bc-11e1-d9f1-b83a6f580667@xs4all.nl>\n\t<20170911132710.mcgqn6tbiabzvpqq@valkosipuli.retiisi.org.uk>\n\t<21d712c2-7608-7153-4421-2c12b90dcb7a@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<21d712c2-7608-7153-4421-2c12b90dcb7a@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":1766329,"web_url":"http://patchwork.ozlabs.org/comment/1766329/","msgid":"<20170911141543.phkw3heyb2er2ozz@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-11T14:15:44","subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"On Mon, Sep 11, 2017 at 01:17:46PM +0200, Pavel Machek wrote:\n> On Mon 2017-09-11 11:00:03, Sakari Ailus wrote:\n> > Add v4l2_fwnode_parse_reference_sensor_common for parsing common\n> > sensor properties that refer to adjacent devices such as flash or lens\n> > driver chips.\n> > \n> > As this is an association only, there's little a regular driver needs to\n> > know about these devices as such.\n> > \n> > Signed-off-by: Sakari Ailus \n> > Acked-by: Hans Verkuil \n> > ---\n> > drivers/media/v4l2-core/v4l2-fwnode.c | 35 +++++++++++++++++++++++++++++++++++\n> > include/media/v4l2-fwnode.h | 13 +++++++++++++\n> > 2 files changed, 48 insertions(+)\n> > \n> > \n> > +/**\n> > + * v4l2_fwnode_reference_parse_sensor_common - parse common references on\n> > + *\t\t\t\t\t sensors 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> > + *\n> > + * Return: 0 on success\n> > + *\t -ENOMEM if memory allocation failed\n> > + *\t -EINVAL if property parsing failed\n> > + */\n> \n> Returns: would sound more correct to me, but it seems kernel is split\n> on that.\n\nI think in V4L2 there are roughly as many of each instances. I'll keep it\nas it is.\n\n> \n> Acked-by: Pavel Machek \n\nThanks!","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 3xrVMT3JBcz9s2G\n\tfor ;\n\tTue, 12 Sep 2017 00:15:49 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750927AbdIKOPr (ORCPT\n\t);\n\tMon, 11 Sep 2017 10:15:47 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:53120 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1750782AbdIKOPr (ORCPT\n\t); Mon, 11 Sep 2017 10:15:47 -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 04F1360102;\n\tMon, 11 Sep 2017 17:15:44 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1drPV2-0003lH-GS; Mon, 11 Sep 2017 17:15:44 +0300"],"Date":"Mon, 11 Sep 2017 17:15:44 +0300","From":"Sakari Ailus ","To":"Pavel Machek ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, linux-acpi@vger.kernel.org,\n\tmika.westerberg@intel.com, devicetree@vger.kernel.org, sre@kernel.org","Subject":"Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for\n\tparsing common external refs","Message-ID":"<20170911141543.phkw3heyb2er2ozz@valkosipuli.retiisi.org.uk>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>\n\t<20170911080008.21208-20-sakari.ailus@linux.intel.com>\n\t<20170911111746.GC28095@amd>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170911111746.GC28095@amd>","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":1778675,"web_url":"http://patchwork.ozlabs.org/comment/1778675/","msgid":"","list_archive_url":null,"date":"2017-10-03T00:04:50","subject":"Re: [PATCH v10 00/24] Unified fwnode endpoint parser, async\n\tsub-device notifier support, N9 flash DTS","submitter":{"id":64267,"url":"http://patchwork.ozlabs.org/api/people/64267/","name":"Rafael J. Wysocki","email":"rafael@kernel.org"},"content":"Hi,\n\nOn Mon, Sep 11, 2017 at 9:59 AM, Sakari Ailus\n wrote:\n> Hi folks,\n>\n> We have a large influx of new, unmerged, drivers that are now parsing\n> fwnode endpoints and each one of them is doing this a little bit\n> differently. The needs are still exactly the same for the graph data\n> structure is device independent. This is still a non-trivial task and the\n> majority of the driver implementations are buggy, just buggy in different\n> ways.\n>\n> Facilitate parsing endpoints by adding a convenience function for parsing\n> the endpoints, and make the omap3isp and rcar-vin drivers use them as an\n> example.\n>\n> To show where we're getting with this, I've added support for async\n> sub-device notifier support that is notifiers that can be registered by\n> sub-device drivers as well as V4L2 fwnode improvements to make use of them\n> and the DTS changes for the Nokia N9. Some of these patches I've posted\n> previously in this set here:\n>\n> \n>\n> Since that, the complete callback of the master notifier registering the\n> V4L2 device is only called once all sub-notifiers have been completed as\n> well. This way the device node creation can be postponed until all devices\n> have been successfully initialised.\n>\n> With this, the as3645a driver successfully registers a sub-device to the\n> media device created by the omap3isp driver. The kernel also has the\n> information it's related to the sensor driven by the smiapp driver but we\n> don't have a way to expose that information yet.\n\nI don't see core changes in this set, so I'm assuming it to be\ntargeted at the users of endpoints etc.\n\nThanks,\nRafael\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=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"lT9aRpeY\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y5fRg32nyz9sBd\n\tfor ;\n\tTue, 3 Oct 2017 11:05:03 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751131AbdJCAE6 (ORCPT\n\t);\n\tMon, 2 Oct 2017 20:04:58 -0400","from mail-oi0-f68.google.com ([209.85.218.68]:36227 \"EHLO\n\tmail-oi0-f68.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750986AbdJCAEv (ORCPT\n\t); Mon, 2 Oct 2017 20:04:51 -0400","by mail-oi0-f68.google.com with SMTP id p187so1566441oif.3;\n\tMon, 02 Oct 2017 17:04:51 -0700 (PDT)","by 10.157.73.24 with HTTP; Mon, 2 Oct 2017 17:04:50 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=HIXLaH18vwLY6BFnLXJ11VfDj0cqtL6NJJtirm8QQQ8=;\n\tb=lT9aRpeYIPRrRly0Ac6wG7WH7fbKk5HxxkM85FgtACrIOcYMJx7SVLxgA+O5Pi0yYK\n\twYddcdSuZXJ8RZ+GBE5cs8QlB4pCSB+0O/ENzEBsX8noyn2FU17+OSfprz5dFc09sqHJ\n\txhPPVb+ZzfILtABsmVUu/mERbRH+CAlX52rXLVF7f1nXOuckZ4z4Ddnw0oAX8dbl6q8g\n\t//9E/tsb08MtLfE5BCUP43Jx6J7OXyCCL0jAgbtyWSjFhLJqiKd9n5C7tp84pGPH9JTu\n\tiz9C14YpYYHeOrkmzaWSyNiHG+LYVAhNe9GDJqhvFwCrKrjccGfEPiQ1+/fSE2vkJyQa\n\t6B/w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=HIXLaH18vwLY6BFnLXJ11VfDj0cqtL6NJJtirm8QQQ8=;\n\tb=QJ9wA4By1eftBvmv2wKRMEZ6K8rQdDheoxTHvHoJljCr8Skc03wJZIBb8fZnv50UFQ\n\tcHwykyG4KqQAictGqwl+QxEBovXLJ5OK+L4vq3Bc8kLZC2G+zge+7cTq6YkfCzuKBOgF\n\tJZc99g8a4JEGrxa6XbVmtMc7kmwWHDaDJnoXNtXcMxU4ov4yrenkhjmvp/IfPcPOpzEm\n\t6nFELr9wXC6jI55Y2VMVZ/U/wHlQxW1nwuPBmtqG7fGG+ZwWJsX8MNabhHMv1ZoJIJik\n\t38wTMo/tJKXmov5P10ZykApkepQvnsnVTwgP26e97thBhcmp4Nm5oDLOVNzrpBRTHSHD\n\ts7yQ==","X-Gm-Message-State":"AMCzsaXSg+wyGHo08i/6lBUmaEOVUU44uGxQ67dsUrNr5mCMnNMsmUjj\n\thTAtbvLJWCyB4xfAkaxn7jIQn/lFUOagyRhEWb0=","X-Google-Smtp-Source":"AOwi7QBCdwlMnOAGe1XgOmt7Oce2TJZowcH42b0LdKrTSMt/U8vC0ctOa91KKCjJli3stYnppRkeU6XPuP19EyxYZ20=","X-Received":"by 10.157.91.107 with SMTP id e40mr3190862otj.62.1506989090931; \n\tMon, 02 Oct 2017 17:04:50 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>","References":"<20170911080008.21208-1-sakari.ailus@linux.intel.com>","From":"\"Rafael J. Wysocki\" ","Date":"Tue, 3 Oct 2017 02:04:50 +0200","X-Google-Sender-Auth":"pKF81c5Q6umbAQXBlhV9WnWmmMs","Message-ID":"","Subject":"Re: [PATCH v10 00/24] Unified fwnode endpoint parser, async\n\tsub-device notifier support, N9 flash DTS","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tRob Herring , hverkuil@xs4all.nl,\n\tLaurent Pinchart ,\n\tACPI Devel Maling List ,\n\tMika Westerberg ,\n\t\"devicetree@vger.kernel.org\" ,\n\tPavel Machek , Sebastian Reichel ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1779776,"web_url":"http://patchwork.ozlabs.org/comment/1779776/","msgid":"<20171004124500.bm2rt3abfem63svi@kekkonen.localdomain>","list_archive_url":null,"date":"2017-10-04T12:45:01","subject":"Re: [PATCH v10 00/24] Unified fwnode endpoint parser, async\n\tsub-device notifier support, N9 flash DTS","submitter":{"id":65485,"url":"http://patchwork.ozlabs.org/api/people/65485/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Rafael,\n\nOn Tue, Oct 03, 2017 at 02:04:50AM +0200, Rafael J. Wysocki wrote:\n> Hi,\n> \n> On Mon, Sep 11, 2017 at 9:59 AM, Sakari Ailus\n> wrote:\n> > Hi folks,\n> >\n> > We have a large influx of new, unmerged, drivers that are now parsing\n> > fwnode endpoints and each one of them is doing this a little bit\n> > differently. The needs are still exactly the same for the graph data\n> > structure is device independent. This is still a non-trivial task and the\n> > majority of the driver implementations are buggy, just buggy in different\n> > ways.\n> >\n> > Facilitate parsing endpoints by adding a convenience function for parsing\n> > the endpoints, and make the omap3isp and rcar-vin drivers use them as an\n> > example.\n> >\n> > To show where we're getting with this, I've added support for async\n> > sub-device notifier support that is notifiers that can be registered by\n> > sub-device drivers as well as V4L2 fwnode improvements to make use of them\n> > and the DTS changes for the Nokia N9. Some of these patches I've posted\n> > previously in this set here:\n> >\n> > \n> >\n> > Since that, the complete callback of the master notifier registering the\n> > V4L2 device is only called once all sub-notifiers have been completed as\n> > well. This way the device node creation can be postponed until all devices\n> > have been successfully initialised.\n> >\n> > With this, the as3645a driver successfully registers a sub-device to the\n> > media device created by the omap3isp driver. The kernel also has the\n> > information it's related to the sensor driven by the smiapp driver but we\n> > don't have a way to expose that information yet.\n> \n> I don't see core changes in this set, so I'm assuming it to be\n> targeted at the users of endpoints etc.\n\nYes, this is dealing with V4L2 and how information from firmware is used\nmainly but there is also DT binding documentation.","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 3y6bGJ2tWtz9t2h\n\tfor ;\n\tWed, 4 Oct 2017 23:45:12 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751673AbdJDMpK (ORCPT\n\t);\n\tWed, 4 Oct 2017 08:45:10 -0400","from mga09.intel.com ([134.134.136.24]:7372 \"EHLO mga09.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751278AbdJDMpK (ORCPT );\n\tWed, 4 Oct 2017 08:45:10 -0400","from fmsmga006.fm.intel.com ([10.253.24.20])\n\tby orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t04 Oct 2017 05:45:06 -0700","from pgerasim-mobl.ccr.corp.intel.com (HELO kekkonen.fi.intel.com)\n\t([10.249.35.237])\n\tby fmsmga006.fm.intel.com with ESMTP; 04 Oct 2017 05:45:04 -0700","by kekkonen.fi.intel.com (Postfix, from userid 1000)\n\tid 45CF321E17; Wed, 4 Oct 2017 15:45:01 +0300 (EEST)"],"X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos;i=\"5.42,477,1500966000\"; d=\"scan'208\";a=\"159285010\"","Date":"Wed, 4 Oct 2017 15:45:01 +0300","From":"Sakari Ailus ","To":"\"Rafael J. Wysocki\" ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\tRob Herring , hverkuil@xs4all.nl,\n\tLaurent Pinchart ,\n\tACPI Devel Maling List ,\n\tMika Westerberg ,\n\t\"devicetree@vger.kernel.org\" ,\n\tPavel Machek , Sebastian Reichel ","Subject":"Re: [PATCH v10 00/24] Unified fwnode endpoint parser, async\n\tsub-device notifier support, N9 flash DTS","Message-ID":"<20171004124500.bm2rt3abfem63svi@kekkonen.localdomain>","References":"<20170911080008.21208-1-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"}}]