[{"id":1762487,"web_url":"http://patchwork.ozlabs.org/comment/1762487/","msgid":"<20170904074739.uo2yycszt6luoldd@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-04T07:47:39","subject":"Re: [PATCH v7 00/18] Unified fwnode endpoint parser, async\n\tsub-device notifier support, N9 flash DTS","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Oh, and this set depends on this patch, on its way to 4.14-rc1 I believe:\n\n<URL:https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=device-properties&id=3e3119d3088f41106f3581d39e7694a50ca3fc02>","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xm24w0Zthz9s06\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tMon,  4 Sep 2017 17:47:44 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753261AbdIDHrm (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 03:47:42 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:55016 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751949AbdIDHrm (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 03:47:42 -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 28AC8600E6;\n\tMon,  4 Sep 2017 10:47:40 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1dom6d-0002Ky-LI; Mon, 04 Sep 2017 10:47:39 +0300"],"Date":"Mon, 4 Sep 2017 10:47:39 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Sakari Ailus <sakari.ailus@linux.intel.com>","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v7 00/18] Unified fwnode endpoint parser, async\n\tsub-device notifier support, N9 flash DTS","Message-ID":"<20170904074739.uo2yycszt6luoldd@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762630,"web_url":"http://patchwork.ozlabs.org/comment/1762630/","msgid":"<e07f9b4d-e8dc-5598-98ee-3c69e3100b81@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T13:19:10","subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> The current practice is that drivers iterate over their endpoints and\n> parse each endpoint separately. This is very similar in a number of\n> drivers, implement a generic function for the job. Driver specific matters\n> can be taken into account in the driver specific callback.\n> \n> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> ---\n>  drivers/media/v4l2-core/v4l2-async.c  |  16 ++++\n>  drivers/media/v4l2-core/v4l2-fwnode.c | 136 ++++++++++++++++++++++++++++++++++\n>  include/media/v4l2-async.h            |  24 +++++-\n>  include/media/v4l2-fwnode.h           |  53 +++++++++++++\n>  4 files changed, 227 insertions(+), 2 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> index 851f128eba22..6740a241d4a1 100644\n> --- a/drivers/media/v4l2-core/v4l2-async.c\n> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> @@ -22,6 +22,7 @@\n>  \n>  #include <media/v4l2-async.h>\n>  #include <media/v4l2-device.h>\n> +#include <media/v4l2-fwnode.h>\n>  #include <media/v4l2-subdev.h>\n>  \n>  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)\n> @@ -278,6 +279,21 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n>  }\n>  EXPORT_SYMBOL(v4l2_async_notifier_unregister);\n>  \n> +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)\n> +{\n> +\tunsigned int i;\n> +\n> +\tfor (i = 0; i < notifier->num_subdevs; i++)\n> +\t\tkfree(notifier->subdevs[i]);\n> +\n> +\tnotifier->max_subdevs = 0;\n> +\tnotifier->num_subdevs = 0;\n> +\n> +\tkvfree(notifier->subdevs);\n> +\tnotifier->subdevs = NULL;\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);\n> +\n>  int v4l2_async_register_subdev(struct v4l2_subdev *sd)\n>  {\n>  \tstruct v4l2_async_notifier *notifier;\n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index 706f9e7b90f1..f8c7a9bc6a58 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -19,6 +19,7 @@\n>   */\n>  #include <linux/acpi.h>\n>  #include <linux/kernel.h>\n> +#include <linux/mm.h>\n>  #include <linux/module.h>\n>  #include <linux/of.h>\n>  #include <linux/property.h>\n> @@ -26,6 +27,7 @@\n>  #include <linux/string.h>\n>  #include <linux/types.h>\n>  \n> +#include <media/v4l2-async.h>\n>  #include <media/v4l2-fwnode.h>\n>  \n>  enum v4l2_fwnode_bus_type {\n> @@ -313,6 +315,140 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)\n>  }\n>  EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);\n>  \n> +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier *notifier,\n> +\t\t\t\t       unsigned int max_subdevs)\n> +{\n> +\tstruct v4l2_async_subdev **subdevs;\n> +\n> +\tif (max_subdevs <= notifier->max_subdevs)\n> +\t\treturn 0;\n> +\n> +\tsubdevs = kvmalloc_array(\n> +\t\tmax_subdevs, sizeof(*notifier->subdevs),\n> +\t\tGFP_KERNEL | __GFP_ZERO);\n> +\tif (!subdevs)\n> +\t\treturn -ENOMEM;\n> +\n> +\tif (notifier->subdevs) {\n> +\t\tmemcpy(subdevs, notifier->subdevs,\n> +\t\t       sizeof(*subdevs) * notifier->num_subdevs);\n> +\n> +\t\tkvfree(notifier->subdevs);\n> +\t}\n> +\n> +\tnotifier->subdevs = subdevs;\n> +\tnotifier->max_subdevs = max_subdevs;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int v4l2_async_notifier_fwnode_parse_endpoint(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tstruct fwnode_handle *endpoint, unsigned int asd_struct_size,\n> +\tint (*parse_endpoint)(struct device *dev,\n> +\t\t\t    struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t    struct v4l2_async_subdev *asd))\n> +{\n> +\tstruct v4l2_async_subdev *asd;\n> +\tstruct v4l2_fwnode_endpoint *vep;\n> +\tint ret = 0;\n> +\n> +\tasd = kzalloc(asd_struct_size, GFP_KERNEL);\n> +\tif (!asd)\n> +\t\treturn -ENOMEM;\n> +\n> +\tasd->match.fwnode.fwnode =\n> +\t\tfwnode_graph_get_remote_port_parent(endpoint);\n> +\tif (!asd->match.fwnode.fwnode) {\n> +\t\tdev_warn(dev, \"bad remote port parent\\n\");\n> +\t\tret = -EINVAL;\n> +\t\tgoto out_err;\n> +\t}\n> +\n> +\t/* Ignore endpoints the parsing of which failed. */\n> +\tvep = v4l2_fwnode_endpoint_alloc_parse(endpoint);\n> +\tif (IS_ERR(vep)) {\n> +\t\tret = PTR_ERR(vep);\n> +\t\tdev_warn(dev, \"unable to parse V4L2 fwnode endpoint (%d)\\n\",\n> +\t\t\t ret);\n> +\t\tgoto out_err;\n> +\t}\n> +\n> +\tret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;\n\nHow likely is it that parse_endpoint will be NULL? I ask because if it is\ncommon, then you could put everything from v4l2_fwnode_endpoint_alloc_parse\nuntil just before 'asd->match_type = V4L2_ASYNC_MATCH_FWNODE;' under\n'if (parse_endpoint) {'.\n\nThe disadvantage is that you won't see parse errors, but on the other hand\nnobody apparently needs it, so why bother. I'm not sure what is the right thing\nto do here.\n\n> +\tv4l2_fwnode_endpoint_free(vep);\n\nHere vep is freed,\n\n> +\tif (ret == -ENOTCONN) {\n> +\t\tdev_dbg(dev, \"ignoring endpoint %u,%u\\n\", vep->base.port,\n> +\t\t\tvep->base.id);\n\nbut still used here\n\n> +\t\tkfree(asd);\n> +\t\treturn 0;\n> +\t} else if (ret < 0) {\n> +\t\tdev_warn(dev, \"driver could not parse endpoint %u,%u (%d)\\n\",\n> +\t\t\t vep->base.port, vep->base.id, ret);\n\nand here.\n\n> +\t\tgoto out_err;\n> +\t}\n> +\n> +\tasd->match_type = V4L2_ASYNC_MATCH_FWNODE;\n> +\tnotifier->subdevs[notifier->num_subdevs] = asd;\n> +\tnotifier->num_subdevs++;\n> +\n> +\treturn 0;\n> +\n> +out_err:\n> +\tfwnode_handle_put(asd->match.fwnode.fwnode);\n> +\tkfree(asd);\n> +\n> +\treturn ret;\n> +}\n> +\n> +int v4l2_async_notifier_parse_fwnode_endpoints(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tsize_t asd_struct_size,\n> +\tint (*parse_endpoint)(struct device *dev,\n> +\t\t\t    struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t    struct v4l2_async_subdev *asd))\n> +{\n> +\tstruct fwnode_handle *fwnode = NULL;\n> +\tunsigned int max_subdevs = notifier->max_subdevs;\n> +\tint ret;\n> +\n> +\tif (asd_struct_size < sizeof(struct v4l2_async_subdev))\n> +\t\treturn -EINVAL;\n> +\n> +\tfor (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(\n> +\t\t\t\t     dev_fwnode(dev), fwnode)); )\n> +\t\tif (fwnode_device_is_available(\n> +\t\t\t    fwnode_graph_get_port_parent(fwnode)))\n> +\t\t\tmax_subdevs++;\n> +\n> +\t/* No subdevs to add? Return here. */\n> +\tif (max_subdevs == notifier->max_subdevs)\n> +\t\treturn 0;\n> +\n> +\tret = v4l2_async_notifier_realloc(notifier, max_subdevs);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tfor (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(\n> +\t\t\t\t     dev_fwnode(dev), fwnode)); ) {\n> +\t\tif (!fwnode_device_is_available(\n> +\t\t\t    fwnode_graph_get_port_parent(fwnode)))\n> +\t\t\tcontinue;\n> +\n> +\t\tif (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs))\n> +\t\t\tbreak;\n\nI think you should return an error here. I feel that if this happens, then\nsomething very strange is going on and you are better off returning -EINVAL\nor something like that.\n\n> +\n> +\t\tret = v4l2_async_notifier_fwnode_parse_endpoint(\n> +\t\t\tdev, notifier, fwnode, asd_struct_size, parse_endpoint);\n> +\t\tif (ret < 0)\n> +\t\t\tbreak;\n> +\t}\n> +\n> +\tfwnode_handle_put(fwnode);\n> +\n> +\treturn ret;\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\n> +\n>  MODULE_LICENSE(\"GPL\");\n>  MODULE_AUTHOR(\"Sakari Ailus <sakari.ailus@linux.intel.com>\");\n>  MODULE_AUTHOR(\"Sylwester Nawrocki <s.nawrocki@samsung.com>\");\n> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> index c69d8c8a66d0..96fa1afc00dd 100644\n> --- a/include/media/v4l2-async.h\n> +++ b/include/media/v4l2-async.h\n> @@ -18,7 +18,6 @@ struct device;\n>  struct device_node;\n>  struct v4l2_device;\n>  struct v4l2_subdev;\n> -struct v4l2_async_notifier;\n>  \n>  /* A random max subdevice number, used to allocate an array on stack */\n>  #define V4L2_MAX_SUBDEVS 128U\n> @@ -50,6 +49,10 @@ enum v4l2_async_match_type {\n>   * @match:\tunion of per-bus type matching data sets\n>   * @list:\tused to link struct v4l2_async_subdev objects, waiting to be\n>   *\t\tprobed, to a notifier->waiting list\n> + *\n> + * When this struct is used as a member in a driver specific struct,\n> + * the driver specific struct shall contain the @struct\n> + * v4l2_async_subdev as its first member.\n>   */\n>  struct v4l2_async_subdev {\n>  \tenum v4l2_async_match_type match_type;\n> @@ -78,7 +81,8 @@ struct v4l2_async_subdev {\n>  /**\n>   * struct v4l2_async_notifier - v4l2_device notifier data\n>   *\n> - * @num_subdevs: number of subdevices\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>   * @waiting:\tlist of struct v4l2_async_subdev, waiting for their drivers\n> @@ -90,6 +94,7 @@ struct v4l2_async_subdev {\n>   */\n>  struct v4l2_async_notifier {\n>  \tunsigned int num_subdevs;\n> +\tunsigned int max_subdevs;\n>  \tstruct v4l2_async_subdev **subdevs;\n>  \tstruct v4l2_device *v4l2_dev;\n>  \tstruct list_head waiting;\n> @@ -121,6 +126,21 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n>  void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);\n>  \n>  /**\n> + * v4l2_async_notifier_release - release notifier resources\n> + * @notifier: the notifier the resources of which are to be released\n> + *\n> + * Release memory resources related to a notifier, including the async\n> + * sub-devices allocated for the purposes of the notifier. The user is\n> + * responsible for releasing the notifier's resources after calling\n> + * @v4l2_async_notifier_parse_fwnode_endpoints.\n> + *\n> + * There is no harm from calling v4l2_async_notifier_release in other\n> + * cases as long as its memory has been zeroed after it has been\n> + * allocated.\n> + */\n> +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier);\n> +\n> +/**\n>   * v4l2_async_register_subdev - registers a sub-device to the asynchronous\n>   * \tsubdevice framework\n>   *\n> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> index 68eb22ba571b..6d125f26ec84 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -25,6 +25,8 @@\n>  #include <media/v4l2-mediabus.h>\n>  \n>  struct fwnode_handle;\n> +struct v4l2_async_notifier;\n> +struct v4l2_async_subdev;\n>  \n>  #define V4L2_FWNODE_CSI2_MAX_DATA_LANES\t4\n>  \n> @@ -201,4 +203,55 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,\n>   */\n>  void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);\n>  \n> +/**\n> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints in a\n> + *\t\t\t\t\t\tdevice node\n> + * @dev: the device the endpoints of which are to be parsed\n> + * @notifier: notifier for @dev\n> + * @asd_struct_size: size of the driver's async sub-device struct, including\n> + *\t\t     sizeof(struct v4l2_async_subdev). The &struct\n> + *\t\t     v4l2_async_subdev shall be the first member of\n> + *\t\t     the driver's async sub-device struct, i.e. both\n> + *\t\t     begin at the same memory address.\n> + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode\n> + *\t\t    endpoint. Optional.\n> + *\t\t    Return: %0 on success\n> + *\t\t\t    %-ENOTCONN if the endpoint is to be skipped but this\n> + *\t\t\t\t       should not be considered as an error\n> + *\t\t\t    %-EINVAL if the endpoint configuration is invalid\n> + *\n> + * Parse the fwnode endpoints of the @dev device and populate the async sub-\n> + * devices array of the notifier. The @parse_endpoint callback function is\n> + * called for each endpoint with the corresponding async sub-device pointer to\n> + * let the caller initialize the driver-specific part of the async sub-device\n> + * structure.\n> + *\n> + * The notifier memory shall be zeroed before this function is called on the\n> + * notifier.\n> + *\n> + * This function may not be called on a registered notifier and may be called on\n> + * a notifier only once. When using this function, the user may not access the\n> + * notifier's subdevs array nor change notifier's num_subdevs field, these are\n> + * reserved for the framework's internal use only.\n\nI don't think the sentence \"When using...only\" makes any sense. How would you\neven be able to access any of the notifier fields? You don't have access to it\nfrom the parse_endpoint callback.\n\nI think it can just be dropped.\n\n> + *\n> + * The @struct v4l2_fwnode_endpoint passed to the callback function\n> + * @parse_endpoint is released once the function is finished. If there is a need\n> + * to retain that configuration, the user needs to allocate memory for it.\n> + *\n> + * Any notifier populated using this function must be released with a call to\n> + * v4l2_async_notifier_release() after it has been unregistered and the async\n> + * sub-devices are no longer in use.\n> + *\n> + * Return: %0 on success, including when no async sub-devices are found\n> + *\t   %-ENOMEM if memory allocation failed\n> + *\t   %-EINVAL if graph or endpoint parsing failed\n> + *\t   Other error codes as returned by @parse_endpoint\n> + */\n> +int v4l2_async_notifier_parse_fwnode_endpoints(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tsize_t asd_struct_size,\n> +\tint (*parse_endpoint)(struct device *dev,\n> +\t\t\t      struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t      struct v4l2_async_subdev *asd));\n> +\n>  #endif /* _V4L2_FWNODE_H */\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xm9RX4017z9sR9\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tMon,  4 Sep 2017 23:19:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753557AbdIDNTS (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 09:19:18 -0400","from lb1-smtp-cloud9.xs4all.net ([194.109.24.22]:53712 \"EHLO\n\tlb1-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1752835AbdIDNTS (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 09:19:18 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid orHSd2DSEdRLjorHWdVy7U; Mon, 04 Sep 2017 15:19:16 +0200"],"Subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-5-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<e07f9b4d-e8dc-5598-98ee-3c69e3100b81@xs4all.nl>","Date":"Mon, 4 Sep 2017 15:19:10 +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":"<20170903174958.27058-5-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfExUmDI29U/nVauB8xs4yKmi82ROCtNieAaos7Rj+QnYkTuECD5nhih0WqWrSdsKSWgugc7NPu++WkNAxZYjjF7qf/rtglzKGyGLFzUE80JhC7uR8RNS\n\tjMAkN9ne+6f01BQA0ak1gmk57BcwvYgzG7mQlb8l4e4RDRbBFyiMXTe+QJcTxbx1so/e/z+kQkXRFo5eyDR5P+29tkU4viFewt9M6JSKB0FhOGc4XR+GEYXI\n\thuefcfm2eLbvjJQlfVZInDAWDDivvHi3xHUoBUUvBYwJ3TdL0wedUCSBd7FiApLf3NvyfN870pMUkBW4Zqxr+HqyOd5mqT3DZOH1Zu3KdehVK/J92k2FVBLK\n\ta8d8i18LOB/ULlaSljBI3oKX0AjuB5+Dp3dRvcQKZcwEmpKyaldBMtEDh2T+I0Y7qBPQVfyRnbEywFr32sxD0GQccyj4Qo9g5I55wMJD1emQ1f/3X1Wj2oAc\n\tY/aPWXHRoC/CRAJ3","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762634,"web_url":"http://patchwork.ozlabs.org/comment/1762634/","msgid":"<f8a4ba8c-f749-26ab-0897-c929b3f23e9f@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T13:28:04","subject":"Re: [PATCH v7 07/18] omap3isp: Fix check for our own sub-devices","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> We only want to link sub-devices that were bound to the async notifier the\n> isp driver registered but there may be other sub-devices in the\n> v4l2_device as well. Check for the correct async notifier.\n\nJust to be sure I understand this correctly: the original code wasn't wrong as such,\nbut this new test is just more precise.\n\nRight?\n\n\tHans\n\n> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> ---\n>  drivers/media/platform/omap3isp/isp.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c\n> index a546cf774d40..3b1a9cd0e591 100644\n> --- a/drivers/media/platform/omap3isp/isp.c\n> +++ b/drivers/media/platform/omap3isp/isp.c\n> @@ -2155,7 +2155,7 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)\n>  \t\treturn ret;\n>  \n>  \tlist_for_each_entry(sd, &v4l2_dev->subdevs, list) {\n> -\t\tif (!sd->asd)\n> +\t\tif (sd->notifier != &isp->notifier)\n>  \t\t\tcontinue;\n>  \n>  \t\tret = isp_link_entity(isp, &sd->entity,\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xm9dl61x9z9t2c\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tMon,  4 Sep 2017 23:28:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753697AbdIDN2K (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 09:28:10 -0400","from lb3-smtp-cloud9.xs4all.net ([194.109.24.30]:49055 \"EHLO\n\tlb3-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753694AbdIDN2J (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 09:28:09 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid orQ4d2KMIdRLjorQ8dW3X8; Mon, 04 Sep 2017 15:28:08 +0200"],"Subject":"Re: [PATCH v7 07/18] omap3isp: Fix check for our own sub-devices","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-8-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<f8a4ba8c-f749-26ab-0897-c929b3f23e9f@xs4all.nl>","Date":"Mon, 4 Sep 2017 15:28:04 +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":"<20170903174958.27058-8-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfM13aBVVifFkc/1DWB2GYWGmHgeqR/ppivDFhVoFQMj0EMXLRu2ROvEbbgisDIG+xmDUetAqA6bI/lm1WpOo29TRgNOeJpr+lpDRIR37OvD+rslmce4x\n\tvzkBG1R5dz6tiZMzKpxq9namGGgIXm7ePw+M4FD3WGnKq6qfxyuJAl035R/euJNXWd7ZHU87B35Us8VVoubahW+b4CU/dPcD/gcNzE3RzyXWqxEowt2c5njZ\n\titpbD3SE+uoebCOorYdd4cP3f9fnoKGVHISa7eumjUDrPs3ATABBZJbgj+5lIWFhR3A9EaibdbfoRzvDGNAAFCEGudWvEBXLyqq3QnkORKTDJPOQlzlgJ33B\n\tJ8dQDobhnQlSKAOrgoMc2rRYREKw9fLRJdhnklKWAMgqLcSlX+lcOWVZbyfpfTfPv58yVzAnkq1tsFZDU2NPXgQqvOO2Bn2CHY3GfjW0uMumPxQ7MODYaLgU\n\tYBrooD0UkatZN1yZ","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762649,"web_url":"http://patchwork.ozlabs.org/comment/1762649/","msgid":"<742ff031-6c2b-8ff2-f3db-f621325dc664@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T13:48:01","subject":"Re: [PATCH v7 09/18] v4l: async: Move async subdev notifier\n\toperations to a separate structure","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> The async subdev notifier .bound(), .unbind() and .complete() operations\n> are function pointers stored directly in the v4l2_async_subdev\n> structure. As the structure isn't immutable, this creates a potential\n> security risk as the function pointers are mutable.\n> \n> To fix this, move the function pointers to a new\n> v4l2_async_subdev_operations structure that can be made const in\n> drivers.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nAcked-by: Hans Verkuil <hans.verkuil@cisco.com>\n\nRegards,\n\n\tHans\n\n> ---\n>  drivers/media/platform/am437x/am437x-vpfe.c    |  8 +++++--\n>  drivers/media/platform/atmel/atmel-isc.c       | 10 ++++++---\n>  drivers/media/platform/atmel/atmel-isi.c       | 10 ++++++---\n>  drivers/media/platform/davinci/vpif_capture.c  |  8 +++++--\n>  drivers/media/platform/davinci/vpif_display.c  |  8 +++++--\n>  drivers/media/platform/exynos4-is/media-dev.c  |  8 +++++--\n>  drivers/media/platform/omap3isp/isp.c          |  6 +++++-\n>  drivers/media/platform/pxa_camera.c            |  8 +++++--\n>  drivers/media/platform/qcom/camss-8x16/camss.c |  8 +++++--\n>  drivers/media/platform/rcar-vin/rcar-core.c    | 10 ++++++---\n>  drivers/media/platform/rcar_drif.c             | 10 ++++++---\n>  drivers/media/platform/soc_camera/soc_camera.c | 14 +++++++------\n>  drivers/media/platform/stm32/stm32-dcmi.c      | 10 ++++++---\n>  drivers/media/platform/ti-vpe/cal.c            |  8 +++++--\n>  drivers/media/platform/xilinx/xilinx-vipp.c    |  8 +++++--\n>  drivers/media/v4l2-core/v4l2-async.c           | 20 +++++++++---------\n>  drivers/staging/media/imx/imx-media-dev.c      |  8 +++++--\n>  include/media/v4l2-async.h                     | 29 +++++++++++++++++---------\n>  18 files changed, 131 insertions(+), 60 deletions(-)\n> \n> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c\n> index dfcc484cab89..0997c640191d 100644\n> --- a/drivers/media/platform/am437x/am437x-vpfe.c\n> +++ b/drivers/media/platform/am437x/am437x-vpfe.c\n> @@ -2417,6 +2417,11 @@ static int vpfe_async_complete(struct v4l2_async_notifier *notifier)\n>  \treturn vpfe_probe_complete(vpfe);\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations vpfe_async_ops = {\n> +\t.bound = vpfe_async_bound,\n> +\t.complete = vpfe_async_complete,\n> +};\n> +\n>  static struct vpfe_config *\n>  vpfe_get_pdata(struct platform_device *pdev)\n>  {\n> @@ -2590,8 +2595,7 @@ static int vpfe_probe(struct platform_device *pdev)\n>  \n>  \tvpfe->notifier.subdevs = vpfe->cfg->asd;\n>  \tvpfe->notifier.num_subdevs = ARRAY_SIZE(vpfe->cfg->asd);\n> -\tvpfe->notifier.bound = vpfe_async_bound;\n> -\tvpfe->notifier.complete = vpfe_async_complete;\n> +\tvpfe->notifier.ops = &vpfe_async_ops;\n>  \tret = v4l2_async_notifier_register(&vpfe->v4l2_dev,\n>  \t\t\t\t\t\t&vpfe->notifier);\n>  \tif (ret) {\n> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c\n> index d7103c5f92c3..48544c4137cb 100644\n> --- a/drivers/media/platform/atmel/atmel-isc.c\n> +++ b/drivers/media/platform/atmel/atmel-isc.c\n> @@ -1639,6 +1639,12 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)\n>  \treturn 0;\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations isc_async_ops = {\n> +\t.bound = isc_async_bound,\n> +\t.unbind = isc_async_unbind,\n> +\t.complete = isc_async_complete,\n> +};\n> +\n>  static void isc_subdev_cleanup(struct isc_device *isc)\n>  {\n>  \tstruct isc_subdev_entity *subdev_entity;\n> @@ -1851,9 +1857,7 @@ static int atmel_isc_probe(struct platform_device *pdev)\n>  \tlist_for_each_entry(subdev_entity, &isc->subdev_entities, list) {\n>  \t\tsubdev_entity->notifier.subdevs = &subdev_entity->asd;\n>  \t\tsubdev_entity->notifier.num_subdevs = 1;\n> -\t\tsubdev_entity->notifier.bound = isc_async_bound;\n> -\t\tsubdev_entity->notifier.unbind = isc_async_unbind;\n> -\t\tsubdev_entity->notifier.complete = isc_async_complete;\n> +\t\tsubdev_entity->notifier.ops = &isc_async_ops;\n>  \n>  \t\tret = v4l2_async_notifier_register(&isc->v4l2_dev,\n>  \t\t\t\t\t\t   &subdev_entity->notifier);\n> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c\n> index 891fa2505efa..eadbf9def358 100644\n> --- a/drivers/media/platform/atmel/atmel-isi.c\n> +++ b/drivers/media/platform/atmel/atmel-isi.c\n> @@ -1105,6 +1105,12 @@ static int isi_graph_notify_bound(struct v4l2_async_notifier *notifier,\n>  \treturn 0;\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations isi_graph_notify_ops = {\n> +\t.bound = isi_graph_notify_bound,\n> +\t.unbind = isi_graph_notify_unbind,\n> +\t.complete = isi_graph_notify_complete,\n> +};\n> +\n>  static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)\n>  {\n>  \tstruct device_node *ep = NULL;\n> @@ -1152,9 +1158,7 @@ static int isi_graph_init(struct atmel_isi *isi)\n>  \n>  \tisi->notifier.subdevs = subdevs;\n>  \tisi->notifier.num_subdevs = 1;\n> -\tisi->notifier.bound = isi_graph_notify_bound;\n> -\tisi->notifier.unbind = isi_graph_notify_unbind;\n> -\tisi->notifier.complete = isi_graph_notify_complete;\n> +\tisi->notifier.ops = &isi_graph_notify_ops;\n>  \n>  \tret = v4l2_async_notifier_register(&isi->v4l2_dev, &isi->notifier);\n>  \tif (ret < 0) {\n> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c\n> index 0ef36cec21d1..a89367ab1e06 100644\n> --- a/drivers/media/platform/davinci/vpif_capture.c\n> +++ b/drivers/media/platform/davinci/vpif_capture.c\n> @@ -1500,6 +1500,11 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)\n>  \treturn vpif_probe_complete();\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations vpif_async_ops = {\n> +\t.bound = vpif_async_bound,\n> +\t.complete = vpif_async_complete,\n> +};\n> +\n>  static struct vpif_capture_config *\n>  vpif_capture_get_pdata(struct platform_device *pdev)\n>  {\n> @@ -1691,8 +1696,7 @@ static __init int vpif_probe(struct platform_device *pdev)\n>  \t} else {\n>  \t\tvpif_obj.notifier.subdevs = vpif_obj.config->asd;\n>  \t\tvpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];\n> -\t\tvpif_obj.notifier.bound = vpif_async_bound;\n> -\t\tvpif_obj.notifier.complete = vpif_async_complete;\n> +\t\tvpif_obj.notifier.ops = &vpif_async_ops;\n>  \t\terr = v4l2_async_notifier_register(&vpif_obj.v4l2_dev,\n>  \t\t\t\t\t\t   &vpif_obj.notifier);\n>  \t\tif (err) {\n> diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c\n> index 56fe4e5b396e..ff2f75a328c9 100644\n> --- a/drivers/media/platform/davinci/vpif_display.c\n> +++ b/drivers/media/platform/davinci/vpif_display.c\n> @@ -1232,6 +1232,11 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)\n>  \treturn vpif_probe_complete();\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations vpif_async_ops = {\n> +\t.bound = vpif_async_bound,\n> +\t.complete = vpif_async_complete,\n> +};\n> +\n>  /*\n>   * vpif_probe: This function creates device entries by register itself to the\n>   * V4L2 driver and initializes fields of each channel objects\n> @@ -1313,8 +1318,7 @@ static __init int vpif_probe(struct platform_device *pdev)\n>  \t} else {\n>  \t\tvpif_obj.notifier.subdevs = vpif_obj.config->asd;\n>  \t\tvpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];\n> -\t\tvpif_obj.notifier.bound = vpif_async_bound;\n> -\t\tvpif_obj.notifier.complete = vpif_async_complete;\n> +\t\tvpif_obj.notifier.ops = &vpif_async_ops;\n>  \t\terr = v4l2_async_notifier_register(&vpif_obj.v4l2_dev,\n>  \t\t\t\t\t\t   &vpif_obj.notifier);\n>  \t\tif (err) {\n> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c\n> index d4656d5175d7..c15596b56dc9 100644\n> --- a/drivers/media/platform/exynos4-is/media-dev.c\n> +++ b/drivers/media/platform/exynos4-is/media-dev.c\n> @@ -1405,6 +1405,11 @@ static int subdev_notifier_complete(struct v4l2_async_notifier *notifier)\n>  \treturn media_device_register(&fmd->media_dev);\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations subdev_notifier_ops = {\n> +\t.bound = subdev_notifier_bound,\n> +\t.complete = subdev_notifier_complete,\n> +};\n> +\n>  static int fimc_md_probe(struct platform_device *pdev)\n>  {\n>  \tstruct device *dev = &pdev->dev;\n> @@ -1479,8 +1484,7 @@ static int fimc_md_probe(struct platform_device *pdev)\n>  \tif (fmd->num_sensors > 0) {\n>  \t\tfmd->subdev_notifier.subdevs = fmd->async_subdevs;\n>  \t\tfmd->subdev_notifier.num_subdevs = fmd->num_sensors;\n> -\t\tfmd->subdev_notifier.bound = subdev_notifier_bound;\n> -\t\tfmd->subdev_notifier.complete = subdev_notifier_complete;\n> +\t\tfmd->subdev_notifier.ops = &subdev_notifier_ops;\n>  \t\tfmd->num_sensors = 0;\n>  \n>  \t\tret = v4l2_async_notifier_register(&fmd->v4l2_dev,\n> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c\n> index 9a694924e46e..0f08d602f756 100644\n> --- a/drivers/media/platform/omap3isp/isp.c\n> +++ b/drivers/media/platform/omap3isp/isp.c\n> @@ -2171,6 +2171,10 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)\n>  \treturn media_device_register(&isp->media_dev);\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations isp_subdev_notifier_ops = {\n> +\t.complete = isp_subdev_notifier_complete,\n> +};\n> +\n>  /*\n>   * isp_probe - Probe ISP platform device\n>   * @pdev: Pointer to ISP platform device\n> @@ -2341,7 +2345,7 @@ static int isp_probe(struct platform_device *pdev)\n>  \tif (ret < 0)\n>  \t\tgoto error_register_entities;\n>  \n> -\tisp->notifier.complete = isp_subdev_notifier_complete;\n> +\tisp->notifier.ops = &isp_subdev_notifier_ops;\n>  \n>  \tret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);\n>  \tif (ret)\n> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c\n> index edca993c2b1f..9d3f0cb1d95a 100644\n> --- a/drivers/media/platform/pxa_camera.c\n> +++ b/drivers/media/platform/pxa_camera.c\n> @@ -2221,6 +2221,11 @@ static void pxa_camera_sensor_unbind(struct v4l2_async_notifier *notifier,\n>  \tmutex_unlock(&pcdev->mlock);\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations pxa_camera_sensor_ops = {\n> +\t.bound = pxa_camera_sensor_bound,\n> +\t.unbind = pxa_camera_sensor_unbind,\n> +};\n> +\n>  /*\n>   * Driver probe, remove, suspend and resume operations\n>   */\n> @@ -2489,8 +2494,7 @@ static int pxa_camera_probe(struct platform_device *pdev)\n>  \tpcdev->asds[0] = &pcdev->asd;\n>  \tpcdev->notifier.subdevs = pcdev->asds;\n>  \tpcdev->notifier.num_subdevs = 1;\n> -\tpcdev->notifier.bound = pxa_camera_sensor_bound;\n> -\tpcdev->notifier.unbind = pxa_camera_sensor_unbind;\n> +\tpcdev->notifier.ops = &pxa_camera_sensor_ops;\n>  \n>  \tif (!of_have_populated_dt())\n>  \t\tpcdev->asd.match_type = V4L2_ASYNC_MATCH_I2C;\n> diff --git a/drivers/media/platform/qcom/camss-8x16/camss.c b/drivers/media/platform/qcom/camss-8x16/camss.c\n> index a3760b5dd1d1..390a42c17b66 100644\n> --- a/drivers/media/platform/qcom/camss-8x16/camss.c\n> +++ b/drivers/media/platform/qcom/camss-8x16/camss.c\n> @@ -601,6 +601,11 @@ static int camss_subdev_notifier_complete(struct v4l2_async_notifier *async)\n>  \treturn media_device_register(&camss->media_dev);\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations camss_subdev_notifier_ops = {\n> +\t.bound = camss_subdev_notifier_bound,\n> +\t.complete = camss_subdev_notifier_complete,\n> +};\n> +\n>  static const struct media_device_ops camss_media_ops = {\n>  \t.link_notify = v4l2_pipeline_link_notify,\n>  };\n> @@ -655,8 +660,7 @@ static int camss_probe(struct platform_device *pdev)\n>  \t\tgoto err_register_entities;\n>  \n>  \tif (camss->notifier.num_subdevs) {\n> -\t\tcamss->notifier.bound = camss_subdev_notifier_bound;\n> -\t\tcamss->notifier.complete = camss_subdev_notifier_complete;\n> +\t\tcamss->notifier.ops = &camss_subdev_notifier_ops;\n>  \n>  \t\tret = v4l2_async_notifier_register(&camss->v4l2_dev,\n>  \t\t\t\t\t\t   &camss->notifier);\n> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c\n> index bd551f0be213..f55b0ef29a56 100644\n> --- a/drivers/media/platform/rcar-vin/rcar-core.c\n> +++ b/drivers/media/platform/rcar-vin/rcar-core.c\n> @@ -134,6 +134,12 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,\n>  \n>  \treturn 0;\n>  }\n> +static const struct v4l2_async_notifier_operations rvin_digital_notify_ops = {\n> +\t.bound = rvin_digital_notify_bound,\n> +\t.unbind = rvin_digital_notify_unbind,\n> +\t.complete = rvin_digital_notify_complete,\n> +};\n> +\n>  \n>  static int rvin_digital_parse_v4l2(struct device *dev,\n>  \t\t\t\t   struct v4l2_fwnode_endpoint *vep,\n> @@ -182,9 +188,7 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)\n>  \t\t\tto_of_node(\n>  \t\t\t\tvin->notifier.subdevs[0]->match.fwnode.fwnode));\n>  \n> -\tvin->notifier.bound = rvin_digital_notify_bound;\n> -\tvin->notifier.unbind = rvin_digital_notify_unbind;\n> -\tvin->notifier.complete = rvin_digital_notify_complete;\n> +\tvin->notifier.ops = &rvin_digital_notify_ops;\n>  \tret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);\n>  \tif (ret < 0) {\n>  \t\tvin_err(vin, \"Notifier registration failed\\n\");\n> diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c\n> index 522364ff0d5d..0b2214d6d621 100644\n> --- a/drivers/media/platform/rcar_drif.c\n> +++ b/drivers/media/platform/rcar_drif.c\n> @@ -1185,6 +1185,12 @@ static int rcar_drif_notify_complete(struct v4l2_async_notifier *notifier)\n>  \treturn ret;\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations rcar_drif_notify_ops = {\n> +\t.bound = rcar_drif_notify_bound,\n> +\t.unbind = rcar_drif_notify_unbind,\n> +\t.complete = rcar_drif_notify_complete,\n> +};\n> +\n>  /* Read endpoint properties */\n>  static void rcar_drif_get_ep_properties(struct rcar_drif_sdr *sdr,\n>  \t\t\t\t\tstruct fwnode_handle *fwnode)\n> @@ -1347,9 +1353,7 @@ static int rcar_drif_sdr_probe(struct rcar_drif_sdr *sdr)\n>  \tif (ret)\n>  \t\tgoto error;\n>  \n> -\tsdr->notifier.bound = rcar_drif_notify_bound;\n> -\tsdr->notifier.unbind = rcar_drif_notify_unbind;\n> -\tsdr->notifier.complete = rcar_drif_notify_complete;\n> +\tsdr->notifier.ops = &rcar_drif_notify_ops;\n>  \n>  \t/* Register notifier */\n>  \tret = v4l2_async_notifier_register(&sdr->v4l2_dev, &sdr->notifier);\n> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c\n> index 1f3c450c7a69..916ff68b73d4 100644\n> --- a/drivers/media/platform/soc_camera/soc_camera.c\n> +++ b/drivers/media/platform/soc_camera/soc_camera.c\n> @@ -1391,6 +1391,12 @@ static int soc_camera_async_complete(struct v4l2_async_notifier *notifier)\n>  \treturn 0;\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations soc_camera_async_ops = {\n> +\t.bound = soc_camera_async_bound,\n> +\t.unbind = soc_camera_async_unbind,\n> +\t.complete = soc_camera_async_complete,\n> +};\n> +\n>  static int scan_async_group(struct soc_camera_host *ici,\n>  \t\t\t    struct v4l2_async_subdev **asd, unsigned int size)\n>  {\n> @@ -1437,9 +1443,7 @@ static int scan_async_group(struct soc_camera_host *ici,\n>  \n>  \tsasc->notifier.subdevs = asd;\n>  \tsasc->notifier.num_subdevs = size;\n> -\tsasc->notifier.bound = soc_camera_async_bound;\n> -\tsasc->notifier.unbind = soc_camera_async_unbind;\n> -\tsasc->notifier.complete = soc_camera_async_complete;\n> +\tsasc->notifier.ops = &soc_camera_async_ops;\n>  \n>  \ticd->sasc = sasc;\n>  \ticd->parent = ici->v4l2_dev.dev;\n> @@ -1537,9 +1541,7 @@ static int soc_of_bind(struct soc_camera_host *ici,\n>  \n>  \tsasc->notifier.subdevs = &info->subdev;\n>  \tsasc->notifier.num_subdevs = 1;\n> -\tsasc->notifier.bound = soc_camera_async_bound;\n> -\tsasc->notifier.unbind = soc_camera_async_unbind;\n> -\tsasc->notifier.complete = soc_camera_async_complete;\n> +\tsasc->notifier.ops = &soc_camera_async_ops;\n>  \n>  \ticd->sasc = sasc;\n>  \ticd->parent = ici->v4l2_dev.dev;\n> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c\n> index 35ba6f211b79..ac4c450a6c7d 100644\n> --- a/drivers/media/platform/stm32/stm32-dcmi.c\n> +++ b/drivers/media/platform/stm32/stm32-dcmi.c\n> @@ -1495,6 +1495,12 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,\n>  \treturn 0;\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = {\n> +\t.bound = dcmi_graph_notify_bound,\n> +\t.unbind = dcmi_graph_notify_unbind,\n> +\t.complete = dcmi_graph_notify_complete,\n> +};\n> +\n>  static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)\n>  {\n>  \tstruct device_node *ep = NULL;\n> @@ -1542,9 +1548,7 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi)\n>  \n>  \tdcmi->notifier.subdevs = subdevs;\n>  \tdcmi->notifier.num_subdevs = 1;\n> -\tdcmi->notifier.bound = dcmi_graph_notify_bound;\n> -\tdcmi->notifier.unbind = dcmi_graph_notify_unbind;\n> -\tdcmi->notifier.complete = dcmi_graph_notify_complete;\n> +\tdcmi->notifier.ops = &dcmi_graph_notify_ops;\n>  \n>  \tret = v4l2_async_notifier_register(&dcmi->v4l2_dev, &dcmi->notifier);\n>  \tif (ret < 0) {\n> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c\n> index 42e383a48ffe..8b586c864524 100644\n> --- a/drivers/media/platform/ti-vpe/cal.c\n> +++ b/drivers/media/platform/ti-vpe/cal.c\n> @@ -1522,6 +1522,11 @@ static int cal_async_complete(struct v4l2_async_notifier *notifier)\n>  \treturn 0;\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations cal_async_ops = {\n> +\t.bound = cal_async_bound,\n> +\t.complete = cal_async_complete,\n> +};\n> +\n>  static int cal_complete_ctx(struct cal_ctx *ctx)\n>  {\n>  \tstruct video_device *vfd;\n> @@ -1736,8 +1741,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)\n>  \tctx->asd_list[0] = asd;\n>  \tctx->notifier.subdevs = ctx->asd_list;\n>  \tctx->notifier.num_subdevs = 1;\n> -\tctx->notifier.bound = cal_async_bound;\n> -\tctx->notifier.complete = cal_async_complete;\n> +\tctx->notifier.ops = &cal_async_ops;\n>  \tret = v4l2_async_notifier_register(&ctx->v4l2_dev,\n>  \t\t\t\t\t   &ctx->notifier);\n>  \tif (ret) {\n> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c\n> index ebfdf334d99c..d881cf09876d 100644\n> --- a/drivers/media/platform/xilinx/xilinx-vipp.c\n> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c\n> @@ -351,6 +351,11 @@ static int xvip_graph_notify_bound(struct v4l2_async_notifier *notifier,\n>  \treturn -EINVAL;\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations xvip_graph_notify_ops = {\n> +\t.bound = xvip_graph_notify_bound,\n> +\t.complete = xvip_graph_notify_complete,\n> +};\n> +\n>  static int xvip_graph_parse_one(struct xvip_composite_device *xdev,\n>  \t\t\t\tstruct device_node *node)\n>  {\n> @@ -548,8 +553,7 @@ static int xvip_graph_init(struct xvip_composite_device *xdev)\n>  \n>  \txdev->notifier.subdevs = subdevs;\n>  \txdev->notifier.num_subdevs = num_subdevs;\n> -\txdev->notifier.bound = xvip_graph_notify_bound;\n> -\txdev->notifier.complete = xvip_graph_notify_complete;\n> +\txdev->notifier.ops = &xvip_graph_notify_ops;\n>  \n>  \tret = v4l2_async_notifier_register(&xdev->v4l2_dev, &xdev->notifier);\n>  \tif (ret < 0) {\n> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> index 6740a241d4a1..810f5e0273dc 100644\n> --- a/drivers/media/v4l2-core/v4l2-async.c\n> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> @@ -107,16 +107,16 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,\n>  {\n>  \tint ret;\n>  \n> -\tif (notifier->bound) {\n> -\t\tret = notifier->bound(notifier, sd, asd);\n> +\tif (notifier->ops->bound) {\n> +\t\tret = notifier->ops->bound(notifier, sd, asd);\n>  \t\tif (ret < 0)\n>  \t\t\treturn ret;\n>  \t}\n>  \n>  \tret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);\n>  \tif (ret < 0) {\n> -\t\tif (notifier->unbind)\n> -\t\t\tnotifier->unbind(notifier, sd, asd);\n> +\t\tif (notifier->ops->unbind)\n> +\t\t\tnotifier->ops->unbind(notifier, sd, asd);\n>  \t\treturn ret;\n>  \t}\n>  \n> @@ -128,8 +128,8 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,\n>  \t/* Move from the global subdevice list to notifier's done */\n>  \tlist_move(&sd->async_list, &notifier->done);\n>  \n> -\tif (list_empty(&notifier->waiting) && notifier->complete)\n> -\t\treturn notifier->complete(notifier);\n> +\tif (list_empty(&notifier->waiting) && notifier->ops->complete)\n> +\t\treturn notifier->ops->complete(notifier);\n>  \n>  \treturn 0;\n>  }\n> @@ -232,8 +232,8 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n>  \t\t/* If we handled USB devices, we'd have to lock the parent too */\n>  \t\tdevice_release_driver(d);\n>  \n> -\t\tif (notifier->unbind)\n> -\t\t\tnotifier->unbind(notifier, sd, sd->asd);\n> +\t\tif (notifier->ops->unbind)\n> +\t\t\tnotifier->ops->unbind(notifier, sd, sd->asd);\n>  \n>  \t\t/*\n>  \t\t * Store device at the device cache, in order to call\n> @@ -344,8 +344,8 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)\n>  \n>  \tv4l2_async_cleanup(sd);\n>  \n> -\tif (notifier->unbind)\n> -\t\tnotifier->unbind(notifier, sd, sd->asd);\n> +\tif (notifier->ops->unbind)\n> +\t\tnotifier->ops->unbind(notifier, sd, sd->asd);\n>  \n>  \tmutex_unlock(&list_lock);\n>  }\n> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c\n> index d96f4512224f..e8dc2c93658a 100644\n> --- a/drivers/staging/media/imx/imx-media-dev.c\n> +++ b/drivers/staging/media/imx/imx-media-dev.c\n> @@ -440,6 +440,11 @@ static int imx_media_probe_complete(struct v4l2_async_notifier *notifier)\n>  \treturn media_device_register(&imxmd->md);\n>  }\n>  \n> +static const struct v4l2_async_notifier_operations imx_media_subdev_ops = {\n> +\t.bound = imx_media_subdev_bound,\n> +\t.complete = imx_media_probe_complete,\n> +};\n> +\n>  /*\n>   * adds controls to a video device from an entity subdevice.\n>   * Continues upstream from the entity's sink pads.\n> @@ -608,8 +613,7 @@ static int imx_media_probe(struct platform_device *pdev)\n>  \n>  \t/* prepare the async subdev notifier and register it */\n>  \timxmd->subdev_notifier.subdevs = imxmd->async_ptrs;\n> -\timxmd->subdev_notifier.bound = imx_media_subdev_bound;\n> -\timxmd->subdev_notifier.complete = imx_media_probe_complete;\n> +\timxmd->subdev_notifier.ops = &imx_media_subdev_ops;\n>  \tret = v4l2_async_notifier_register(&imxmd->v4l2_dev,\n>  \t\t\t\t\t   &imxmd->subdev_notifier);\n>  \tif (ret) {\n> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> index 96fa1afc00dd..3c48f8b66d12 100644\n> --- a/include/media/v4l2-async.h\n> +++ b/include/media/v4l2-async.h\n> @@ -18,6 +18,7 @@ struct device;\n>  struct device_node;\n>  struct v4l2_device;\n>  struct v4l2_subdev;\n> +struct v4l2_async_notifier;\n>  \n>  /* A random max subdevice number, used to allocate an array on stack */\n>  #define V4L2_MAX_SUBDEVS 128U\n> @@ -79,8 +80,25 @@ struct v4l2_async_subdev {\n>  };\n>  \n>  /**\n> + * struct v4l2_async_notifier_operations - Asynchronous V4L2 notifier operations\n> + * @bound:\ta subdevice driver has successfully probed one of the subdevices\n> + * @complete:\tall subdevices have been probed successfully\n> + * @unbind:\ta subdevice is leaving\n> + */\n> +struct v4l2_async_notifier_operations {\n> +\tint (*bound)(struct v4l2_async_notifier *notifier,\n> +\t\t     struct v4l2_subdev *subdev,\n> +\t\t     struct v4l2_async_subdev *asd);\n> +\tint (*complete)(struct v4l2_async_notifier *notifier);\n> +\tvoid (*unbind)(struct v4l2_async_notifier *notifier,\n> +\t\t       struct v4l2_subdev *subdev,\n> +\t\t       struct v4l2_async_subdev *asd);\n> +};\n> +\n> +/**\n>   * struct v4l2_async_notifier - v4l2_device notifier data\n>   *\n> + * @ops:\tnotifier 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> @@ -88,11 +106,9 @@ struct v4l2_async_subdev {\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> - * @bound:\ta subdevice driver has successfully probed one of subdevices\n> - * @complete:\tall subdevices have been probed successfully\n> - * @unbind:\ta subdevice is leaving\n>   */\n>  struct v4l2_async_notifier {\n> +\tconst struct v4l2_async_notifier_operations *ops;\n>  \tunsigned int num_subdevs;\n>  \tunsigned int max_subdevs;\n>  \tstruct v4l2_async_subdev **subdevs;\n> @@ -100,13 +116,6 @@ struct v4l2_async_notifier {\n>  \tstruct list_head waiting;\n>  \tstruct list_head done;\n>  \tstruct list_head list;\n> -\tint (*bound)(struct v4l2_async_notifier *notifier,\n> -\t\t     struct v4l2_subdev *subdev,\n> -\t\t     struct v4l2_async_subdev *asd);\n> -\tint (*complete)(struct v4l2_async_notifier *notifier);\n> -\tvoid (*unbind)(struct v4l2_async_notifier *notifier,\n> -\t\t       struct v4l2_subdev *subdev,\n> -\t\t       struct v4l2_async_subdev *asd);\n>  };\n>  \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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmB4m5Rqrz9t2c\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tMon,  4 Sep 2017 23:48:08 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753775AbdIDNsH (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 09:48:07 -0400","from lb3-smtp-cloud9.xs4all.net ([194.109.24.30]:37145 \"EHLO\n\tlb3-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753701AbdIDNsH (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 09:48:07 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid orjNd2b8YdRLjorjRdWH0L; Mon, 04 Sep 2017 15:48:05 +0200"],"Subject":"Re: [PATCH v7 09/18] v4l: async: Move async subdev notifier\n\toperations to a separate structure","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-10-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<742ff031-6c2b-8ff2-f3db-f621325dc664@xs4all.nl>","Date":"Mon, 4 Sep 2017 15:48:01 +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":"<20170903174958.27058-10-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfLk5dn2xhNeFEt3AoeLLAGmSlcZdHU7x59+3/6zVU85laWwbIMfY+FfObG6o9fmC0uUmrVoDkWTLHH4uq+5mYc6UseVbvVIQMT9CklYGZOGAwCh+1E1S\n\t6RcXT2S+xgRUI6/SDXcxGt2GveRu2H3+wrHZjImKvUsqT6CUXBAuxdA5XTHY0afi6qLVkG15iNTWLPUwpFGpaP7hkJn/5jLIndT/k4AIwiIM8/9TegYwRMkp\n\tuGfXNMBpnD0+A1APlPTNVjesWmjjKGAiWD0m4o4W5UnaWnnDmLqcqzcSGhoO0VtLh7ONsuiQFQ2OUF4Gk6A8DkrZgX1LWXx19DWbxL4WtcpxwlRR195zKTjI\n\tT8h/s2vq83ZRjyyHfyVpyYmz/mcw7LwyyM3j/0eDmuFm05RwFJlFnbEk1UzYhSP5DVok1ECgZJ+A4XhJHrZvmeRVcylQDYlrB2ORKy9ikIlcUDWRvkH+gthy\n\tQH5bUI9wOEAWjus/","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762652,"web_url":"http://patchwork.ozlabs.org/comment/1762652/","msgid":"<96c4a9cf-1231-b6c4-bc2b-a431f4bea7a4@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T13:50:52","subject":"Re: [PATCH v7 10/18] v4l: async: Introduce macros for calling async\n\tops callbacks","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> Add two macros to call async operations callbacks. Besides simplifying\n> callbacks, this allows async notifiers to have no ops set, i.e. it can be\n> left NULL.\n> \n> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> ---\n>  drivers/media/v4l2-core/v4l2-async.c | 19 +++++++------------\n>  include/media/v4l2-async.h           |  8 ++++++++\n>  2 files changed, 15 insertions(+), 12 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> index 810f5e0273dc..91d04f00b4e4 100644\n> --- a/drivers/media/v4l2-core/v4l2-async.c\n> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> @@ -107,16 +107,13 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,\n>  {\n>  \tint ret;\n>  \n> -\tif (notifier->ops->bound) {\n> -\t\tret = notifier->ops->bound(notifier, sd, asd);\n> -\t\tif (ret < 0)\n> -\t\t\treturn ret;\n> -\t}\n> +\tret = v4l2_async_notifier_call_int_op(notifier, bound, sd, asd);\n\nHmm, I think this is rather ugly. We only have three ops, so why not make\nthree macros:\n\n\tv4l2_async_notifier_call_bound/unbind/complete?\n\nMuch cleaner than _int_op(...bound...).\n\nRegards,\n\n\tHans\n\n> +\tif (ret < 0)\n> +\t\treturn ret;\n>  \n>  \tret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);\n>  \tif (ret < 0) {\n> -\t\tif (notifier->ops->unbind)\n> -\t\t\tnotifier->ops->unbind(notifier, sd, asd);\n> +\t\tv4l2_async_notifier_call_void_op(notifier, unbind, sd, asd);\n>  \t\treturn ret;\n>  \t}\n>  \n> @@ -129,7 +126,7 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,\n>  \tlist_move(&sd->async_list, &notifier->done);\n>  \n>  \tif (list_empty(&notifier->waiting) && notifier->ops->complete)\n> -\t\treturn notifier->ops->complete(notifier);\n> +\t\treturn v4l2_async_notifier_call_int_op(notifier, complete);\n>  \n>  \treturn 0;\n>  }\n> @@ -232,8 +229,7 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n>  \t\t/* If we handled USB devices, we'd have to lock the parent too */\n>  \t\tdevice_release_driver(d);\n>  \n> -\t\tif (notifier->ops->unbind)\n> -\t\t\tnotifier->ops->unbind(notifier, sd, sd->asd);\n> +\t\tv4l2_async_notifier_call_void_op(notifier, unbind, sd, sd->asd);\n>  \n>  \t\t/*\n>  \t\t * Store device at the device cache, in order to call\n> @@ -344,8 +340,7 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)\n>  \n>  \tv4l2_async_cleanup(sd);\n>  \n> -\tif (notifier->ops->unbind)\n> -\t\tnotifier->ops->unbind(notifier, sd, sd->asd);\n> +\tv4l2_async_notifier_call_void_op(notifier, unbind, sd, sd->asd);\n>  \n>  \tmutex_unlock(&list_lock);\n>  }\n> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> index 3c48f8b66d12..c3e001e0d1f1 100644\n> --- a/include/media/v4l2-async.h\n> +++ b/include/media/v4l2-async.h\n> @@ -95,6 +95,14 @@ struct v4l2_async_notifier_operations {\n>  \t\t       struct v4l2_async_subdev *asd);\n>  };\n>  \n> +#define v4l2_async_notifier_call_int_op(n, op, ...)\t\t\t\\\n> +\t(((n)->ops && (n)->ops->op) ? (n)->ops->op(n, ## __VA_ARGS__) : 0)\n> +#define v4l2_async_notifier_call_void_op(n, op, ...)\t \\\n> +\tdo {\t\t\t\t\t\t \\\n> +\t\tif ((n)->ops && (n)->ops->op)\t\t \\\n> +\t\t\t(n)->ops->op(n, ## __VA_ARGS__); \\\n> +\t} while (false)\n> +\n>  /**\n>   * struct v4l2_async_notifier - v4l2_device notifier data\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmB883HBcz9t32\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tMon,  4 Sep 2017 23:51:04 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753711AbdIDNu6 (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 09:50:58 -0400","from lb3-smtp-cloud9.xs4all.net ([194.109.24.30]:51829 \"EHLO\n\tlb3-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753618AbdIDNu4 (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 09:50:56 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid orm8d2dJUdRLjormBdWIhy; Mon, 04 Sep 2017 15:50:55 +0200"],"Subject":"Re: [PATCH v7 10/18] v4l: async: Introduce macros for calling async\n\tops callbacks","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-11-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<96c4a9cf-1231-b6c4-bc2b-a431f4bea7a4@xs4all.nl>","Date":"Mon, 4 Sep 2017 15:50:52 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170903174958.27058-11-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfMer8JNnSfgjR/4UBz6R4AQNAO4QfVbFIyLra5beyKcPLZsfterpUJdA/i5YWuVGf+2i4YjJLC0VURxPeKMbt+y1q+vdzZwZUowIKanIly7/qkT9WEHL\n\tCO5ZjWEeIhnV/HLCU8VuJtANqbNdJG9bsx14qN77N7qvVD3cvhGGa6PuM2KoEZ8wuDy137xiSu1k1zdd1QECd7vvxerd8hemBpnLi2E0kJhOk7fmQF8K4lw8\n\tmj+qSVQihCqCROzohScD7TEmWndgC6Px5X4Kt3xVYRT2gLPcg4iYsG2jbh+WfafLxYyo+ldt68/twwBmVtUMU1NvfLBOBY/fsndZvvZOQbu1WX6XUimB6z+H\n\tZ3aV6l6OWv+92UQfCTWW6HhPfNhG3cU6zyRXPwx1iBYmvkMNMbmdVNAX393TRgwd137jtdevBFY5eQe6zm7prNBmc6L1f19YGCjDQeaJsESIcBtVi50FkQu3\n\tOdrSeLKA/VEy3PdS","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762699,"web_url":"http://patchwork.ozlabs.org/comment/1762699/","msgid":"<0bb75f81-cc81-a4bf-f2af-41862c1d777a@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T14:44:48","subject":"Re: [PATCH v7 15/18] v4l2-fwnode: Add convenience function for\n\tparsing generic references","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/03/2017 07:49 PM, 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> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> ---\n>  drivers/media/v4l2-core/v4l2-fwnode.c | 81 +++++++++++++++++++++++++++++++++++\n>  include/media/v4l2-fwnode.h           | 28 ++++++++++++\n>  2 files changed, 109 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index f8c7a9bc6a58..24f8020caf28 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -449,6 +449,87 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n>  }\n>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\n>  \n> +static void v4l2_fwnode_print_args(struct fwnode_reference_args *args)\n> +{\n> +\tunsigned int i;\n> +\n> +\tfor (i = 0; i < args->nargs; i++) {\n> +\t\tpr_cont(\" %u\", args->args[i]);\n> +\t\tif (i + 1 < args->nargs)\n> +\t\t\tpr_cont(\",\");\n> +\t}\n> +}\n> +\n> +int v4l2_fwnode_reference_parse(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tconst char *prop, const char *nargs_prop, unsigned int nargs,\n> +\tsize_t asd_struct_size,\n> +\tint (*parse_single)(struct device *dev,\n> +\t\t\t    struct fwnode_reference_args *args,\n> +\t\t\t    struct v4l2_async_subdev *asd))\n> +{\n> +\tstruct fwnode_reference_args args;\n> +\tunsigned int index = 0;\n> +\tint ret = -ENOENT;\n> +\n> +\tif (asd_struct_size < sizeof(struct v4l2_async_subdev))\n> +\t\treturn -EINVAL;\n> +\n> +\tfor (; !fwnode_property_get_reference_args(\n> +\t\t     dev_fwnode(dev), prop, nargs_prop, nargs,\n> +\t\t     index, &args); index++)\n> +\t\tfwnode_handle_put(args.fwnode);\n> +\n> +\tret = v4l2_async_notifier_realloc(notifier,\n> +\t\t\t\t\t  notifier->num_subdevs + index);\n> +\tif (ret)\n> +\t\treturn -ENOMEM;\n> +\n> +\tfor (ret = -ENOENT, index = 0; !fwnode_property_get_reference_args(\n> +\t\t     dev_fwnode(dev), prop, nargs_prop, nargs,\n> +\t\t     index, &args); index++) {\n> +\t\tstruct v4l2_async_subdev *asd;\n> +\n> +\t\tif (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs))\n> +\t\t\tbreak;\n\nAs mentioned elsewhere: I think this should return an error.\n\n> +\n> +\t\tasd = kzalloc(asd_struct_size, GFP_KERNEL);\n> +\t\tif (!asd) {\n> +\t\t\tret = -ENOMEM;\n> +\t\t\tgoto error;\n> +\t\t}\n> +\n> +\t\tret = parse_single ? parse_single(dev, &args, asd) : 0;\n> +\t\tif (ret == -ENOTCONN) {\n> +\t\t\tdev_dbg(dev,\n> +\t\t\t\t\"ignoring reference prop \\\"%s\\\", nargs_prop \\\"%s\\\", nargs %u, index %u\",\n> +\t\t\t\tprop, nargs_prop, nargs, index);\n> +\t\t\tv4l2_fwnode_print_args(&args);\n> +\t\t\tpr_cont(\"\\n\");\n\nasd isn't freed.\n\n> +\t\t\tcontinue;\n> +\t\t} else if (ret < 0) {\n> +\t\t\tdev_warn(dev,\n> +\t\t\t\t \"driver could not parse reference prop \\\"%s\\\", nargs_prop \\\"%s\\\", nargs %u, index %u\",\n> +\t\t\t\t prop, nargs_prop, nargs, index);\n> +\t\t\tv4l2_fwnode_print_args(&args);\n> +\t\t\tpr_cont(\"\\n\");\n\nDitto.\n\n> +\t\t\tgoto error;\n> +\t\t}\n> +\n> +\t\tnotifier->subdevs[notifier->num_subdevs] = asd;\n> +\t\tasd->match.fwnode.fwnode = args.fwnode;\n> +\t\tasd->match_type = V4L2_ASYNC_MATCH_FWNODE;\n> +\t\tnotifier->num_subdevs++;\n> +\t}\n> +\n> +\treturn 0;\n> +\n> +error:\n> +\tfwnode_handle_put(args.fwnode);\n> +\treturn ret;\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_fwnode_reference_parse);\n> +\n>  MODULE_LICENSE(\"GPL\");\n>  MODULE_AUTHOR(\"Sakari Ailus <sakari.ailus@linux.intel.com>\");\n>  MODULE_AUTHOR(\"Sylwester Nawrocki <s.nawrocki@samsung.com>\");\n> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> index 6d125f26ec84..52f528162818 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -254,4 +254,32 @@ 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 - parse references for async sub-devices\n> + * @dev: the device node the properties of which are parsed for references\n\nthe device node whose properties are...\n\n> + * @notifier: the async notifier where the async subdevs will be added\n> + * @prop: the name of the property\n> + * @nargs_prop: the name of the property in the remote node that specifies the\n> + *\t\tnumber of integer arguments (may be NULL, in that case nargs\n> + *\t\twill be used).\n> + * @nargs: the number of integer arguments after the reference\n> + * @asd_struct_size: the size of the driver's async sub-device struct, including\n> + *\t\t     @struct v4l2_async_subdev\n> + * @parse_single: Driver's callback function for parsing a reference. Optional.\n\nDriver's -> driver's\n\n> + *\t\t  Return: 0 on success\n> + *\t\t\t  %-ENOTCONN if the reference is to be skipped but this\n> + *\t\t\t\t     should not be considered as an error\n\nskipped but this -> skipped. This\n\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(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tconst char *prop, const char *nargs_prop, unsigned int nargs,\n> +\tsize_t asd_struct_size,\n> +\tint (*parse_single)(struct device *dev,\n> +\t\t\t    struct fwnode_reference_args *args,\n> +\t\t\t    struct v4l2_async_subdev *asd));\n> +\n>  #endif /* _V4L2_FWNODE_H */\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmCLL1Wsbz9t2R\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 00:44:58 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753910AbdIDOoy (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 10:44:54 -0400","from lb3-smtp-cloud9.xs4all.net ([194.109.24.30]:59470 \"EHLO\n\tlb3-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753746AbdIDOox (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 10:44:53 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid oscKd3KepdRLjoscNdWo3k; Mon, 04 Sep 2017 16:44:51 +0200"],"Subject":"Re: [PATCH v7 15/18] v4l2-fwnode: Add convenience function for\n\tparsing generic references","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-16-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<0bb75f81-cc81-a4bf-f2af-41862c1d777a@xs4all.nl>","Date":"Mon, 4 Sep 2017 16:44:48 +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":"<20170903174958.27058-16-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfBSsyDhNKnmBzm02QVtOpzfR5fc4N6j/996i1/TLgjX0SbugJYFilFSYi2YHkDVoZbl2a2OnSjTH9Cd7UJN4Aqu+IMstYoCRkdPX6OZ/HqckgeAhTh6O\n\trkjr25itdDY5xlX5YjDe75DJrRvCHnzQUt6r21Ld4DQexYG7EZBw5QD0aarpB0WqWJmw2CbxJdm4szHO2dGDg3hoHgbdL4HcM5OZX/VPOc8mmZ5BNp0nQi/d\n\tfMepJeSmDRuxYg2hTYn1wDvhAt1hQb8CNbeoorFM8d2GKQ3BP+sbhrnSu/scAuYuHvOMDgbaSUAllOTFwOvBnjq+m/z9EqLzKNYmsw+aQgFIO37mMxbPNfeJ\n\tMnOJbN70yuTzz9xb3L5IYuHHjQy1fmhxsL1F6c1yihnRiMGwQj83pP8GMibLCWbQkrVHqzOHde79W5KgHFguB1MRn4NGMTTMPdgI/T2tUK7tWY3NHPtypEJ3\n\tdLj5AQIPe1PrRzzD","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762713,"web_url":"http://patchwork.ozlabs.org/comment/1762713/","msgid":"<60db0a4b-0560-9591-f41f-1d055a50ba12@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T14:56:29","subject":"Re: [PATCH v7 12/18] 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/03/2017 07:49 PM, Sakari Ailus wrote:\n> Registering a notifier has required the knowledge of struct v4l2_device\n> for the reason that sub-devices generally are registered to the\n> v4l2_device (as well as the media device, also available through\n> v4l2_device).\n> \n> This information is not available for sub-device drivers at probe time.\n> \n> What this patch does is that it allows registering notifiers without\n> having v4l2_device around. Instead the sub-device pointer is stored to 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 master notifier's complete callback is only called when all sub-device\n> notifiers are completed.\n> \n> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> ---\n>  drivers/media/v4l2-core/v4l2-async.c | 153 +++++++++++++++++++++++++++++------\n>  include/media/v4l2-async.h           |  19 ++++-\n>  2 files changed, 146 insertions(+), 26 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> index 70d02378b48f..55d7886103d2 100644\n> --- a/drivers/media/v4l2-core/v4l2-async.c\n> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> @@ -25,6 +25,10 @@\n>  #include <media/v4l2-fwnode.h>\n>  #include <media/v4l2-subdev.h>\n>  \n> +static int v4l2_async_test_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> @@ -101,14 +105,69 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *\n>  \treturn NULL;\n>  }\n>  \n> +static bool v4l2_async_subdev_notifiers_complete(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tstruct v4l2_async_notifier *n;\n> +\n> +\tlist_for_each_entry(n, &notifier->notifiers, notifiers) {\n> +\t\tif (!n->master)\n> +\t\t\treturn false;\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +#define notifier_v4l2_dev(n) \\\n> +\t(!!(n)->v4l2_dev ? (n)->v4l2_dev : \\\n> +\t !!(n)->master ? (n)->master->v4l2_dev : NULL)\n\nWhy '!!'?\n\n> +\n> +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(\n> +\tstruct v4l2_async_notifier *notifier, struct v4l2_subdev *sd)\n> +{\n> +\tstruct v4l2_async_notifier *n;\n> +\n> +\tlist_for_each_entry(n, &notifier_list, list) {\n> +\t\tif (n->sd == sd)\n> +\t\t\treturn n;\n> +\t}\n> +\n> +\treturn NULL;\n> +}\n> +\n> +static int v4l2_async_notifier_test_all_subdevs(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tstruct v4l2_subdev *sd, *tmp;\n> +\n> +\tif (!notifier_v4l2_dev(notifier))\n> +\t\treturn 0;\n> +\n> +\tlist_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {\n> +\t\tstruct v4l2_async_subdev *asd;\n> +\t\tint ret;\n> +\n> +\t\tasd = v4l2_async_belongs(notifier, sd);\n> +\t\tif (!asd)\n> +\t\t\tcontinue;\n> +\n> +\t\tret = v4l2_async_test_notify(notifier, sd, asd);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n>  static int v4l2_async_test_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\nA general note (not specific to this patch series): I really don't like\nthis function name. v4l2_async_match_notify is a much better name.\n\nWith 'test' I get association with 'testing something' and not that it is\na match.\n\nI have a similar problem with v4l2_async_belongs: v4l2_async_find_match\nmakes a lot more sense.\n\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(notifier_v4l2_dev(notifier), sd);\n> +\tif (ret)\n>  \t\treturn ret;\n>  \n>  \tret = v4l2_async_notifier_call_int_op(notifier, bound, sd, asd);\n> @@ -125,8 +184,26 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,\n>  \t/* Move from the global subdevice list to notifier's done */\n>  \tlist_move(&sd->async_list, &notifier->done);\n>  \n> -\tif (list_empty(&notifier->waiting) && notifier->ops->complete)\n> -\t\treturn v4l2_async_notifier_call_int_op(notifier, complete);\n> +\tsubdev_notifier = v4l2_async_get_subdev_notifier(notifier, sd);\n> +\tif (subdev_notifier && !subdev_notifier->master) {\n> +\t\tsubdev_notifier->master = notifier;\n> +\t\tlist_add(&subdev_notifier->notifiers, &notifier->notifiers);\n> +\t\tret = v4l2_async_notifier_test_all_subdevs(subdev_notifier);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\tif (list_empty(&notifier->waiting) &&\n> +\t    v4l2_async_subdev_notifiers_complete(notifier)) {\n> +\t\tret = v4l2_async_notifier_call_int_op(notifier, complete);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\tif (notifier->master && list_empty(&notifier->master->waiting) &&\n> +\t    v4l2_async_subdev_notifiers_complete(notifier->master))\n> +\t\treturn v4l2_async_notifier_call_int_op(notifier->master,\n> +\t\t\t\t\t\t       complete);\n>  \n>  \treturn 0;\n>  }\n> @@ -140,18 +217,17 @@ 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 ||\n> +\tif (!!notifier->v4l2_dev == !!notifier->sd || !notifier->num_subdevs ||\n\n'!!notifier->v4l2_dev == !!notifier->sd'???\n\nThis is '(notifier->v4l2_dev && notifier->sd) || (!notifier->v4l2_dev && !notifier->sd)'\nbut it took me a bit of time to parse that.\n\nA 10 for creativity, but not so much for readability :-)\n\nI suspect it can be simplified as well, or some of these tests can be moved to\nthe two functions that call this one. I think that might be best, in fact.\n\n>  \t    notifier->num_subdevs > V4L2_MAX_SUBDEVS)\n>  \t\treturn -EINVAL;\n>  \n> -\tnotifier->v4l2_dev = v4l2_dev;\n> +\tINIT_LIST_HEAD(&notifier->notifiers);\n>  \tINIT_LIST_HEAD(&notifier->waiting);\n>  \tINIT_LIST_HEAD(&notifier->done);\n>  \n> @@ -175,18 +251,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_belongs(notifier, sd);\n> -\t\tif (!asd)\n> -\t\t\tcontinue;\n> -\n> -\t\tret = v4l2_async_test_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_test_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> @@ -196,27 +264,62 @@ 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> +\tnotifier->v4l2_dev = v4l2_dev;\n> +\n> +\treturn __v4l2_async_notifier_register(notifier);\n> +}\n>  EXPORT_SYMBOL(v4l2_async_notifier_register);\n>  \n> +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,\n> +\t\t\t\t\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tnotifier->sd = sd;\n> +\n> +\treturn __v4l2_async_notifier_register(notifier);\n> +}\n> +EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);\n> +\n>  void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n>  {\n> +\tstruct v4l2_async_notifier *notifier_iter, *notifier_tmp;\n>  \tstruct v4l2_subdev *sd, *tmp;\n>  \tunsigned int notif_n_subdev = notifier->num_subdevs;\n>  \tunsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);\n>  \tstruct device **dev;\n>  \tint i = 0;\n>  \n> -\tif (!notifier->v4l2_dev)\n> +\tif (!notifier->v4l2_dev && !notifier->sd)\n>  \t\treturn;\n>  \n>  \tdev = kvmalloc_array(n_subdev, sizeof(*dev), GFP_KERNEL);\n>  \tif (!dev) {\n> -\t\tdev_err(notifier->v4l2_dev->dev,\n> +\t\tdev_err(notifier->v4l2_dev ?\n> +\t\t\tnotifier->v4l2_dev->dev : notifier->sd->dev,\n>  \t\t\t\"Failed to allocate device cache!\\n\");\n>  \t}\n>  \n>  \tmutex_lock(&list_lock);\n>  \n> +\tif (notifier->v4l2_dev) {\n> +\t\t/* Remove all subdev notifiers from the master's list */\n> +\t\tlist_for_each_entry_safe(notifier_iter, notifier_tmp,\n> +\t\t\t\t\t &notifier->notifiers, notifiers) {\n> +\t\t\tlist_del_init(&notifier_iter->notifiers);\n> +\t\t\tWARN_ON(notifier_iter->master != notifier);\n> +\t\t\tnotifier_iter->master = NULL;\n> +\t\t}\n> +\t\tnotifier->v4l2_dev = NULL;\n> +\t} else {\n> +\t\t/* Remove subdev notifier from the master's list */\n> +\t\tlist_del_init(&notifier->notifiers);\n> +\t\tnotifier->master = NULL;\n> +\t\tnotifier->sd = NULL;\n> +\t}\n> +\n>  \tlist_del(&notifier->list);\n>  \n>  \tlist_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {\n> @@ -266,8 +369,6 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n>  \t}\n>  \tkvfree(dev);\n>  \n> -\tnotifier->v4l2_dev = NULL;\n> -\n>  \t/*\n>  \t * Don't care about the waiting list, it is initialised and populated\n>  \t * upon notifier registration.\n> @@ -287,6 +388,8 @@ void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)\n>  \n>  \tkvfree(notifier->subdevs);\n>  \tnotifier->subdevs = NULL;\n> +\n> +\tWARN_ON(!list_empty(&notifier->notifiers));\n>  }\n>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);\n>  \n> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> index c3e001e0d1f1..a5c123ac2873 100644\n> --- a/include/media/v4l2-async.h\n> +++ b/include/media/v4l2-async.h\n> @@ -110,7 +110,11 @@ 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 master, for subdev notifiers NULL\n> + * @sd:\t\tsub-device that registered the notifier, NULL otherwise\n> + * @notifiers:\tlist of struct v4l2_async_notifier, notifiers linked to this\n> + *\t\tnotifier\n> + * @master:\tmaster notifier carrying @v4l2_dev\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> @@ -121,6 +125,9 @@ 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 list_head notifiers;\n> +\tstruct v4l2_async_notifier *master;\n>  \tstruct list_head waiting;\n>  \tstruct list_head done;\n>  \tstruct list_head list;\n> @@ -136,6 +143,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\nDo you have a git tree with this patch series? I think I need to look at this\nin the final version, not just the patch.\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmCbm0zYvz9ryT\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 00:56:36 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753829AbdIDO4f (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 10:56:35 -0400","from lb3-smtp-cloud9.xs4all.net ([194.109.24.30]:58098 \"EHLO\n\tlb3-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753827AbdIDO4e (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 10:56:34 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid osndd3U52dRLjosngdWuXc; Mon, 04 Sep 2017 16:56:33 +0200"],"Subject":"Re: [PATCH v7 12/18] v4l: async: Allow binding notifiers to\n\tsub-devices","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-13-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<60db0a4b-0560-9591-f41f-1d055a50ba12@xs4all.nl>","Date":"Mon, 4 Sep 2017 16:56: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":"<20170903174958.27058-13-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfFA0aVsDKmrmDPNaX+DzZ+XpGtkm5tsIcDZNa5AoZfL8S9D5j2j/L8pp3JpjuVUG1mRTW8KaGmvN2kSnTl7lxpvz5nHI/plSk/FI8OKUq2DPn9aG5WWZ\n\txlbkX2cEuApOk+CAVxKdy53afz/TVzhsEeSHCr+6qDnqFymmmA8qoOU0CM2upJ7NZb8pAFdRmUPK1uavY2FDkqG4N+KCJJt/VigoNF+doaxIZblC3BeXMvO1\n\t2nxuTFH3nOaPw2/V8KYt7phUjpDPOJhY1t9LUo5/TvRyWcbwOnBHC+es5p1UhY7nznx/BoCgIFkkNsgflmgpWpsrl06gGDlwrqkNeYeytzLGe6YslIQUbv8+\n\tdgmh3HKO4Sbi8zQN90LdGlUSgQkT9AH7Y/BAu8ZzexiDYH75lmAcBkRUu8WsfI6KSU55ASB6szg1UkBaV2KDD4k6KmKChYVBcrSnJqMYi6sTe26Fn9BdjTHq\n\tP+Vuxp0vqrm3VRyX","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762714,"web_url":"http://patchwork.ozlabs.org/comment/1762714/","msgid":"<4900fc41-1b57-deb6-2041-26a6333f2033@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T14:58:13","subject":"Re: [PATCH v7 16/18] v4l2-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/03/2017 07:49 PM, 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 <sakari.ailus@linux.intel.com>\n\nAcked-by: Hans Verkuil <hans.verkuil@cisco.com>\n\nRegards,\n\n\tHans\n\n> ---\n>  drivers/media/v4l2-core/v4l2-fwnode.c | 27 +++++++++++++++++++++++++++\n>  include/media/v4l2-fwnode.h           |  3 +++\n>  2 files changed, 30 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index 24f8020caf28..f28be3b751d3 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -530,6 +530,33 @@ int v4l2_fwnode_reference_parse(\n>  }\n>  EXPORT_SYMBOL_GPL(v4l2_fwnode_reference_parse);\n>  \n> +int v4l2_fwnode_reference_parse_sensor_common(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier)\n> +{\n> +\tstatic const struct {\n> +\t\tchar *name;\n> +\t\tchar *cells;\n> +\t\tunsigned int nr_cells;\n> +\t} props[] = {\n> +\t\t{ \"flash\", NULL, 0 },\n> +\t\t{ \"lens-focus\", NULL, 0 },\n> +\t};\n> +\tunsigned int i;\n> +\tint rval;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(props); i++) {\n> +\t\trval = v4l2_fwnode_reference_parse(\n> +\t\t\tdev, notifier, props[i].name, props[i].cells,\n> +\t\t\tprops[i].nr_cells, sizeof(struct v4l2_async_subdev),\n> +\t\t\tNULL);\n> +\t\tif (rval < 0 && rval != -ENOENT)\n> +\t\t\treturn rval;\n> +\t}\n> +\n> +\treturn rval;\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_fwnode_reference_parse_sensor_common);\n> +\n>  MODULE_LICENSE(\"GPL\");\n>  MODULE_AUTHOR(\"Sakari Ailus <sakari.ailus@linux.intel.com>\");\n>  MODULE_AUTHOR(\"Sylwester Nawrocki <s.nawrocki@samsung.com>\");\n> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> index 52f528162818..fd14f1d38966 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -282,4 +282,7 @@ int v4l2_fwnode_reference_parse(\n>  \t\t\t    struct fwnode_reference_args *args,\n>  \t\t\t    struct v4l2_async_subdev *asd));\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmCdt0Pxzz9t2W\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 00:58:26 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753829AbdIDO6T (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 10:58:19 -0400","from lb1-smtp-cloud9.xs4all.net ([194.109.24.22]:42055 \"EHLO\n\tlb1-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753740AbdIDO6S (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 10:58:18 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid ospJd3Va5dRLjospMdWve8; Mon, 04 Sep 2017 16:58:16 +0200"],"Subject":"Re: [PATCH v7 16/18] v4l2-fwnode: Add convenience function for\n\tparsing common external refs","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-17-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<4900fc41-1b57-deb6-2041-26a6333f2033@xs4all.nl>","Date":"Mon, 4 Sep 2017 16:58:13 +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":"<20170903174958.27058-17-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfN5RIiqlm886NIbsBmTApW5/OG6mLWnKrB8FLK4LiMlyrHhtsXGrBksHU4O5nGRtXCT1tJ8Q7FqzdfVL4pMxkoKwe5BepKDmJHIyxtZTLLeVCpb1euRt\n\tPuporD0oRM9Sy7J3LoL2Egw8uTAwUWaqwV2yVOlBgVY7mzqUBFqIi2zr5JPjt+ye6qImcWX40AmLlgFg2f5c/6QeKuX6nhLx1HTeL+f9KydlbdtvKVZya6+E\n\tet8Jn3t1kGZIgwaF3v0nXXu0GlaFIaYhs6iCYJYUUWpoNL6CCraMVib608X3vEZtXDvo/TWs3qgbpHEJcyGoX3z0GDvIHDiaRcGt8Fr9k0kC7Cx44yuwNSyR\n\t0NjJ32BlyHarnnC+n8HgKsWhXSkk9eQU5OJdD724wllSr4JDOc8llvfCawkrsiSiJ9dU5BfZAF+d20HHYcy1a1c9HGQnK836pTlCJKMEgHXqqfGv11jPyi0/\n\tKZww8DLWSyAA7s4U","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762736,"web_url":"http://patchwork.ozlabs.org/comment/1762736/","msgid":"<20170904154940.GA19484@amd>","list_archive_url":null,"date":"2017-09-04T15:49:40","subject":"Re: [PATCH v7 01/18] v4l: fwnode: Move KernelDoc documentation to\n\tthe header","submitter":{"id":2109,"url":"http://patchwork.ozlabs.org/api/people/2109/","name":"Pavel Machek","email":"pavel@ucw.cz"},"content":"On Sun 2017-09-03 20:49:41, Sakari Ailus wrote:\n> In V4L2 the practice is to have the KernelDoc documentation in the header\n> and not in .c source code files. This consequientally makes the V4L2\n\nconsequientally: spelling?\n\n> fwnode function documentation part of the Media documentation build.\n> \n> Also correct the link related function and argument naming in\n> documentation.\n> \n> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> Reviewed-by: Niklas SÃ¶derlund <niklas.soderlund+renesas@ragnatech.se>\n\nSomething funny going on with utf-8 here.\n\nAcked-by: Pavel Machek <pavel@ucw.cz>\n\t\t\t\t\t\t\t\t\tPavel","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmDn45rtDz9sNq\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 01:49:44 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753842AbdIDPtn (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 11:49:43 -0400","from atrey.karlin.mff.cuni.cz ([195.113.26.193]:46040 \"EHLO\n\tatrey.karlin.mff.cuni.cz\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753780AbdIDPtm (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 11:49:42 -0400","by atrey.karlin.mff.cuni.cz (Postfix, from userid 512)\n\tid 60EF082479; Mon,  4 Sep 2017 17:49:41 +0200 (CEST)"],"Date":"Mon, 4 Sep 2017 17:49:40 +0200","From":"Pavel Machek <pavel@ucw.cz>","To":"Sakari Ailus <sakari.ailus@linux.intel.com>","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tsre@kernel.org","Subject":"Re: [PATCH v7 01/18] v4l: fwnode: Move KernelDoc documentation to\n\tthe header","Message-ID":"<20170904154940.GA19484@amd>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-2-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"xHFwDpU9dbj6ez1V\"","Content-Disposition":"inline","In-Reply-To":"<20170903174958.27058-2-sakari.ailus@linux.intel.com>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762737,"web_url":"http://patchwork.ozlabs.org/comment/1762737/","msgid":"<20170904155415.nak4dofd2k6ytupa@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-04T15:54:16","subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","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 04, 2017 at 03:19:10PM +0200, Hans Verkuil wrote:\n> On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> > The current practice is that drivers iterate over their endpoints and\n> > parse each endpoint separately. This is very similar in a number of\n> > drivers, implement a generic function for the job. Driver specific matters\n> > can be taken into account in the driver specific callback.\n> > \n> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> > ---\n> >  drivers/media/v4l2-core/v4l2-async.c  |  16 ++++\n> >  drivers/media/v4l2-core/v4l2-fwnode.c | 136 ++++++++++++++++++++++++++++++++++\n> >  include/media/v4l2-async.h            |  24 +++++-\n> >  include/media/v4l2-fwnode.h           |  53 +++++++++++++\n> >  4 files changed, 227 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> > index 851f128eba22..6740a241d4a1 100644\n> > --- a/drivers/media/v4l2-core/v4l2-async.c\n> > +++ b/drivers/media/v4l2-core/v4l2-async.c\n> > @@ -22,6 +22,7 @@\n> >  \n> >  #include <media/v4l2-async.h>\n> >  #include <media/v4l2-device.h>\n> > +#include <media/v4l2-fwnode.h>\n> >  #include <media/v4l2-subdev.h>\n> >  \n> >  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)\n> > @@ -278,6 +279,21 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n> >  }\n> >  EXPORT_SYMBOL(v4l2_async_notifier_unregister);\n> >  \n> > +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)\n> > +{\n> > +\tunsigned int i;\n> > +\n> > +\tfor (i = 0; i < notifier->num_subdevs; i++)\n> > +\t\tkfree(notifier->subdevs[i]);\n> > +\n> > +\tnotifier->max_subdevs = 0;\n> > +\tnotifier->num_subdevs = 0;\n> > +\n> > +\tkvfree(notifier->subdevs);\n> > +\tnotifier->subdevs = NULL;\n> > +}\n> > +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);\n> > +\n> >  int v4l2_async_register_subdev(struct v4l2_subdev *sd)\n> >  {\n> >  \tstruct v4l2_async_notifier *notifier;\n> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > index 706f9e7b90f1..f8c7a9bc6a58 100644\n> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > @@ -19,6 +19,7 @@\n> >   */\n> >  #include <linux/acpi.h>\n> >  #include <linux/kernel.h>\n> > +#include <linux/mm.h>\n> >  #include <linux/module.h>\n> >  #include <linux/of.h>\n> >  #include <linux/property.h>\n> > @@ -26,6 +27,7 @@\n> >  #include <linux/string.h>\n> >  #include <linux/types.h>\n> >  \n> > +#include <media/v4l2-async.h>\n> >  #include <media/v4l2-fwnode.h>\n> >  \n> >  enum v4l2_fwnode_bus_type {\n> > @@ -313,6 +315,140 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)\n> >  }\n> >  EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);\n> >  \n> > +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier *notifier,\n> > +\t\t\t\t       unsigned int max_subdevs)\n> > +{\n> > +\tstruct v4l2_async_subdev **subdevs;\n> > +\n> > +\tif (max_subdevs <= notifier->max_subdevs)\n> > +\t\treturn 0;\n> > +\n> > +\tsubdevs = kvmalloc_array(\n> > +\t\tmax_subdevs, sizeof(*notifier->subdevs),\n> > +\t\tGFP_KERNEL | __GFP_ZERO);\n> > +\tif (!subdevs)\n> > +\t\treturn -ENOMEM;\n> > +\n> > +\tif (notifier->subdevs) {\n> > +\t\tmemcpy(subdevs, notifier->subdevs,\n> > +\t\t       sizeof(*subdevs) * notifier->num_subdevs);\n> > +\n> > +\t\tkvfree(notifier->subdevs);\n> > +\t}\n> > +\n> > +\tnotifier->subdevs = subdevs;\n> > +\tnotifier->max_subdevs = max_subdevs;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static int v4l2_async_notifier_fwnode_parse_endpoint(\n> > +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> > +\tstruct fwnode_handle *endpoint, unsigned int asd_struct_size,\n> > +\tint (*parse_endpoint)(struct device *dev,\n> > +\t\t\t    struct v4l2_fwnode_endpoint *vep,\n> > +\t\t\t    struct v4l2_async_subdev *asd))\n> > +{\n> > +\tstruct v4l2_async_subdev *asd;\n> > +\tstruct v4l2_fwnode_endpoint *vep;\n> > +\tint ret = 0;\n> > +\n> > +\tasd = kzalloc(asd_struct_size, GFP_KERNEL);\n> > +\tif (!asd)\n> > +\t\treturn -ENOMEM;\n> > +\n> > +\tasd->match.fwnode.fwnode =\n> > +\t\tfwnode_graph_get_remote_port_parent(endpoint);\n> > +\tif (!asd->match.fwnode.fwnode) {\n> > +\t\tdev_warn(dev, \"bad remote port parent\\n\");\n> > +\t\tret = -EINVAL;\n> > +\t\tgoto out_err;\n> > +\t}\n> > +\n> > +\t/* Ignore endpoints the parsing of which failed. */\n> > +\tvep = v4l2_fwnode_endpoint_alloc_parse(endpoint);\n> > +\tif (IS_ERR(vep)) {\n> > +\t\tret = PTR_ERR(vep);\n> > +\t\tdev_warn(dev, \"unable to parse V4L2 fwnode endpoint (%d)\\n\",\n> > +\t\t\t ret);\n> > +\t\tgoto out_err;\n> > +\t}\n> > +\n> > +\tret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;\n> \n> How likely is it that parse_endpoint will be NULL? I ask because if it is\n> common, then you could put everything from v4l2_fwnode_endpoint_alloc_parse\n> until just before 'asd->match_type = V4L2_ASYNC_MATCH_FWNODE;' under\n> 'if (parse_endpoint) {'.\n> \n> The disadvantage is that you won't see parse errors, but on the other hand\n> nobody apparently needs it, so why bother. I'm not sure what is the right thing\n> to do here.\n\nIf a driver later on is amended with that parse_endpoint callback for a\nreason or another, then a range of boards could suddenly stop working. I'd\nkeep it for validation purposes at least.\n\nI can't think this could ever a performance concern.\n\n> \n> > +\tv4l2_fwnode_endpoint_free(vep);\n> \n> Here vep is freed,\n\nOops. I still thought it actually did release only the variable length\nfields. Of course it no longer did. Will fix.\n\n> \n> > +\tif (ret == -ENOTCONN) {\n> > +\t\tdev_dbg(dev, \"ignoring endpoint %u,%u\\n\", vep->base.port,\n> > +\t\t\tvep->base.id);\n> \n> but still used here\n> \n> > +\t\tkfree(asd);\n> > +\t\treturn 0;\n> > +\t} else if (ret < 0) {\n> > +\t\tdev_warn(dev, \"driver could not parse endpoint %u,%u (%d)\\n\",\n> > +\t\t\t vep->base.port, vep->base.id, ret);\n> \n> and here.\n> \n> > +\t\tgoto out_err;\n> > +\t}\n> > +\n> > +\tasd->match_type = V4L2_ASYNC_MATCH_FWNODE;\n> > +\tnotifier->subdevs[notifier->num_subdevs] = asd;\n> > +\tnotifier->num_subdevs++;\n> > +\n> > +\treturn 0;\n> > +\n> > +out_err:\n> > +\tfwnode_handle_put(asd->match.fwnode.fwnode);\n> > +\tkfree(asd);\n> > +\n> > +\treturn ret;\n> > +}\n> > +\n> > +int v4l2_async_notifier_parse_fwnode_endpoints(\n> > +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> > +\tsize_t asd_struct_size,\n> > +\tint (*parse_endpoint)(struct device *dev,\n> > +\t\t\t    struct v4l2_fwnode_endpoint *vep,\n> > +\t\t\t    struct v4l2_async_subdev *asd))\n> > +{\n> > +\tstruct fwnode_handle *fwnode = NULL;\n> > +\tunsigned int max_subdevs = notifier->max_subdevs;\n> > +\tint ret;\n> > +\n> > +\tif (asd_struct_size < sizeof(struct v4l2_async_subdev))\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tfor (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(\n> > +\t\t\t\t     dev_fwnode(dev), fwnode)); )\n> > +\t\tif (fwnode_device_is_available(\n> > +\t\t\t    fwnode_graph_get_port_parent(fwnode)))\n> > +\t\t\tmax_subdevs++;\n> > +\n> > +\t/* No subdevs to add? Return here. */\n> > +\tif (max_subdevs == notifier->max_subdevs)\n> > +\t\treturn 0;\n> > +\n> > +\tret = v4l2_async_notifier_realloc(notifier, max_subdevs);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tfor (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(\n> > +\t\t\t\t     dev_fwnode(dev), fwnode)); ) {\n> > +\t\tif (!fwnode_device_is_available(\n> > +\t\t\t    fwnode_graph_get_port_parent(fwnode)))\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tif (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs))\n> > +\t\t\tbreak;\n> \n> I think you should return an error here. I feel that if this happens, then\n> something very strange is going on and you are better off returning -EINVAL\n> or something like that.\n\nYeah, I'll change it.\n\n> \n> > +\n> > +\t\tret = v4l2_async_notifier_fwnode_parse_endpoint(\n> > +\t\t\tdev, notifier, fwnode, asd_struct_size, parse_endpoint);\n> > +\t\tif (ret < 0)\n> > +\t\t\tbreak;\n> > +\t}\n> > +\n> > +\tfwnode_handle_put(fwnode);\n> > +\n> > +\treturn ret;\n> > +}\n> > +EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\n> > +\n> >  MODULE_LICENSE(\"GPL\");\n> >  MODULE_AUTHOR(\"Sakari Ailus <sakari.ailus@linux.intel.com>\");\n> >  MODULE_AUTHOR(\"Sylwester Nawrocki <s.nawrocki@samsung.com>\");\n> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> > index c69d8c8a66d0..96fa1afc00dd 100644\n> > --- a/include/media/v4l2-async.h\n> > +++ b/include/media/v4l2-async.h\n> > @@ -18,7 +18,6 @@ struct device;\n> >  struct device_node;\n> >  struct v4l2_device;\n> >  struct v4l2_subdev;\n> > -struct v4l2_async_notifier;\n> >  \n> >  /* A random max subdevice number, used to allocate an array on stack */\n> >  #define V4L2_MAX_SUBDEVS 128U\n> > @@ -50,6 +49,10 @@ enum v4l2_async_match_type {\n> >   * @match:\tunion of per-bus type matching data sets\n> >   * @list:\tused to link struct v4l2_async_subdev objects, waiting to be\n> >   *\t\tprobed, to a notifier->waiting list\n> > + *\n> > + * When this struct is used as a member in a driver specific struct,\n> > + * the driver specific struct shall contain the @struct\n> > + * v4l2_async_subdev as its first member.\n> >   */\n> >  struct v4l2_async_subdev {\n> >  \tenum v4l2_async_match_type match_type;\n> > @@ -78,7 +81,8 @@ struct v4l2_async_subdev {\n> >  /**\n> >   * struct v4l2_async_notifier - v4l2_device notifier data\n> >   *\n> > - * @num_subdevs: number of subdevices\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> >   * @waiting:\tlist of struct v4l2_async_subdev, waiting for their drivers\n> > @@ -90,6 +94,7 @@ struct v4l2_async_subdev {\n> >   */\n> >  struct v4l2_async_notifier {\n> >  \tunsigned int num_subdevs;\n> > +\tunsigned int max_subdevs;\n> >  \tstruct v4l2_async_subdev **subdevs;\n> >  \tstruct v4l2_device *v4l2_dev;\n> >  \tstruct list_head waiting;\n> > @@ -121,6 +126,21 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n> >  void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);\n> >  \n> >  /**\n> > + * v4l2_async_notifier_release - release notifier resources\n> > + * @notifier: the notifier the resources of which are to be released\n> > + *\n> > + * Release memory resources related to a notifier, including the async\n> > + * sub-devices allocated for the purposes of the notifier. The user is\n> > + * responsible for releasing the notifier's resources after calling\n> > + * @v4l2_async_notifier_parse_fwnode_endpoints.\n> > + *\n> > + * There is no harm from calling v4l2_async_notifier_release in other\n> > + * cases as long as its memory has been zeroed after it has been\n> > + * allocated.\n> > + */\n> > +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier);\n> > +\n> > +/**\n> >   * v4l2_async_register_subdev - registers a sub-device to the asynchronous\n> >   * \tsubdevice framework\n> >   *\n> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> > index 68eb22ba571b..6d125f26ec84 100644\n> > --- a/include/media/v4l2-fwnode.h\n> > +++ b/include/media/v4l2-fwnode.h\n> > @@ -25,6 +25,8 @@\n> >  #include <media/v4l2-mediabus.h>\n> >  \n> >  struct fwnode_handle;\n> > +struct v4l2_async_notifier;\n> > +struct v4l2_async_subdev;\n> >  \n> >  #define V4L2_FWNODE_CSI2_MAX_DATA_LANES\t4\n> >  \n> > @@ -201,4 +203,55 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,\n> >   */\n> >  void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);\n> >  \n> > +/**\n> > + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints in a\n> > + *\t\t\t\t\t\tdevice node\n> > + * @dev: the device the endpoints of which are to be parsed\n> > + * @notifier: notifier for @dev\n> > + * @asd_struct_size: size of the driver's async sub-device struct, including\n> > + *\t\t     sizeof(struct v4l2_async_subdev). The &struct\n> > + *\t\t     v4l2_async_subdev shall be the first member of\n> > + *\t\t     the driver's async sub-device struct, i.e. both\n> > + *\t\t     begin at the same memory address.\n> > + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode\n> > + *\t\t    endpoint. Optional.\n> > + *\t\t    Return: %0 on success\n> > + *\t\t\t    %-ENOTCONN if the endpoint is to be skipped but this\n> > + *\t\t\t\t       should not be considered as an error\n> > + *\t\t\t    %-EINVAL if the endpoint configuration is invalid\n> > + *\n> > + * Parse the fwnode endpoints of the @dev device and populate the async sub-\n> > + * devices array of the notifier. The @parse_endpoint callback function is\n> > + * called for each endpoint with the corresponding async sub-device pointer to\n> > + * let the caller initialize the driver-specific part of the async sub-device\n> > + * structure.\n> > + *\n> > + * The notifier memory shall be zeroed before this function is called on the\n> > + * notifier.\n> > + *\n> > + * This function may not be called on a registered notifier and may be called on\n> > + * a notifier only once. When using this function, the user may not access the\n> > + * notifier's subdevs array nor change notifier's num_subdevs field, these are\n> > + * reserved for the framework's internal use only.\n> \n> I don't think the sentence \"When using...only\" makes any sense. How would you\n> even be able to access any of the notifier fields? You don't have access to it\n> from the parse_endpoint callback.\n\nNot from the parse_endpoint callback. The notifier is first set up by the\ndriver, and this text applies to that --- whether or not parse_endpoint is\ngiven.\n\n> \n> I think it can just be dropped.\n> \n> > + *\n> > + * The @struct v4l2_fwnode_endpoint passed to the callback function\n> > + * @parse_endpoint is released once the function is finished. If there is a need\n> > + * to retain that configuration, the user needs to allocate memory for it.\n> > + *\n> > + * Any notifier populated using this function must be released with a call to\n> > + * v4l2_async_notifier_release() after it has been unregistered and the async\n> > + * sub-devices are no longer in use.\n> > + *\n> > + * Return: %0 on success, including when no async sub-devices are found\n> > + *\t   %-ENOMEM if memory allocation failed\n> > + *\t   %-EINVAL if graph or endpoint parsing failed\n> > + *\t   Other error codes as returned by @parse_endpoint\n> > + */\n> > +int v4l2_async_notifier_parse_fwnode_endpoints(\n> > +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> > +\tsize_t asd_struct_size,\n> > +\tint (*parse_endpoint)(struct device *dev,\n> > +\t\t\t      struct v4l2_fwnode_endpoint *vep,\n> > +\t\t\t      struct v4l2_async_subdev *asd));\n> > +\n> >  #endif /* _V4L2_FWNODE_H */\n> >","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmDtQ2cqSz9t2c\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 01:54:22 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753825AbdIDPyU (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 11:54:20 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:59380 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1753780AbdIDPyU (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 11:54:20 -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 6417D600D7;\n\tMon,  4 Sep 2017 18:54:17 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1dothY-0002NM-QJ; Mon, 04 Sep 2017 18:54:16 +0300"],"Date":"Mon, 4 Sep 2017 18:54:16 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Hans Verkuil <hverkuil@xs4all.nl>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","Message-ID":"<20170904155415.nak4dofd2k6ytupa@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-5-sakari.ailus@linux.intel.com>\n\t<e07f9b4d-e8dc-5598-98ee-3c69e3100b81@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<e07f9b4d-e8dc-5598-98ee-3c69e3100b81@xs4all.nl>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762743,"web_url":"http://patchwork.ozlabs.org/comment/1762743/","msgid":"<20170904160712.ha6n3k52swgzlbkm@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-04T16:07:13","subject":"Re: [PATCH v7 07/18] omap3isp: Fix check for our own sub-devices","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"On Mon, Sep 04, 2017 at 03:28:04PM +0200, Hans Verkuil wrote:\n> On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> > We only want to link sub-devices that were bound to the async notifier the\n> > isp driver registered but there may be other sub-devices in the\n> > v4l2_device as well. Check for the correct async notifier.\n> \n> Just to be sure I understand this correctly: the original code wasn't wrong as such,\n> but this new test is just more precise.\n\nWell, it would be wrong very soon. :-)\n\nSo yes. As long as there's just a single user of the async notifiers in for\na V4L2 device, what used to be there works. The other drivers don't seem to\nbe affected.","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmF9L271Wz9t2c\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 02:07:18 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753814AbdIDQHQ (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 12:07:16 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:59478 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1753764AbdIDQHP (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 12:07:15 -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 D5D9A6008F;\n\tMon,  4 Sep 2017 19:07:13 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1dotu5-0002NS-G0; Mon, 04 Sep 2017 19:07:13 +0300"],"Date":"Mon, 4 Sep 2017 19:07:13 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Hans Verkuil <hverkuil@xs4all.nl>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v7 07/18] omap3isp: Fix check for our own sub-devices","Message-ID":"<20170904160712.ha6n3k52swgzlbkm@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-8-sakari.ailus@linux.intel.com>\n\t<f8a4ba8c-f749-26ab-0897-c929b3f23e9f@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<f8a4ba8c-f749-26ab-0897-c929b3f23e9f@xs4all.nl>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762746,"web_url":"http://patchwork.ozlabs.org/comment/1762746/","msgid":"<20170904160900.n7itc23b4xydrdn5@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-04T16:09:00","subject":"Re: [PATCH v7 10/18] v4l: async: Introduce macros for calling async\n\tops callbacks","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 04, 2017 at 03:50:52PM +0200, Hans Verkuil wrote:\n> On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> > Add two macros to call async operations callbacks. Besides simplifying\n> > callbacks, this allows async notifiers to have no ops set, i.e. it can be\n> > left NULL.\n> > \n> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> > ---\n> >  drivers/media/v4l2-core/v4l2-async.c | 19 +++++++------------\n> >  include/media/v4l2-async.h           |  8 ++++++++\n> >  2 files changed, 15 insertions(+), 12 deletions(-)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> > index 810f5e0273dc..91d04f00b4e4 100644\n> > --- a/drivers/media/v4l2-core/v4l2-async.c\n> > +++ b/drivers/media/v4l2-core/v4l2-async.c\n> > @@ -107,16 +107,13 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,\n> >  {\n> >  \tint ret;\n> >  \n> > -\tif (notifier->ops->bound) {\n> > -\t\tret = notifier->ops->bound(notifier, sd, asd);\n> > -\t\tif (ret < 0)\n> > -\t\t\treturn ret;\n> > -\t}\n> > +\tret = v4l2_async_notifier_call_int_op(notifier, bound, sd, asd);\n> \n> Hmm, I think this is rather ugly. We only have three ops, so why not make\n> three macros:\n> \n> \tv4l2_async_notifier_call_bound/unbind/complete?\n> \n> Much cleaner than _int_op(...bound...).\n\nWorks for me.","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmFCQ13c1z9sNr\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 02:09:06 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753876AbdIDQJE (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 12:09:04 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:59518 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1753764AbdIDQJD (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 12:09:03 -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 BA71B600D7;\n\tMon,  4 Sep 2017 19:09:01 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1dotvp-0002NZ-6h; Mon, 04 Sep 2017 19:09:01 +0300"],"Date":"Mon, 4 Sep 2017 19:09:00 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Hans Verkuil <hverkuil@xs4all.nl>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v7 10/18] v4l: async: Introduce macros for calling async\n\tops callbacks","Message-ID":"<20170904160900.n7itc23b4xydrdn5@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-11-sakari.ailus@linux.intel.com>\n\t<96c4a9cf-1231-b6c4-bc2b-a431f4bea7a4@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<96c4a9cf-1231-b6c4-bc2b-a431f4bea7a4@xs4all.nl>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762771,"web_url":"http://patchwork.ozlabs.org/comment/1762771/","msgid":"<20170904163400.z26qmxuejhgdcmrw@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-04T16:34:00","subject":"Re: [PATCH v7 15/18] v4l2-fwnode: Add convenience function for\n\tparsing generic 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 04, 2017 at 04:44:48PM +0200, Hans Verkuil wrote:\n> On 09/03/2017 07:49 PM, 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> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> > ---\n> >  drivers/media/v4l2-core/v4l2-fwnode.c | 81 +++++++++++++++++++++++++++++++++++\n> >  include/media/v4l2-fwnode.h           | 28 ++++++++++++\n> >  2 files changed, 109 insertions(+)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > index f8c7a9bc6a58..24f8020caf28 100644\n> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> > @@ -449,6 +449,87 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> >  }\n> >  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\n> >  \n> > +static void v4l2_fwnode_print_args(struct fwnode_reference_args *args)\n> > +{\n> > +\tunsigned int i;\n> > +\n> > +\tfor (i = 0; i < args->nargs; i++) {\n> > +\t\tpr_cont(\" %u\", args->args[i]);\n> > +\t\tif (i + 1 < args->nargs)\n> > +\t\t\tpr_cont(\",\");\n> > +\t}\n> > +}\n> > +\n> > +int v4l2_fwnode_reference_parse(\n> > +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> > +\tconst char *prop, const char *nargs_prop, unsigned int nargs,\n> > +\tsize_t asd_struct_size,\n> > +\tint (*parse_single)(struct device *dev,\n> > +\t\t\t    struct fwnode_reference_args *args,\n> > +\t\t\t    struct v4l2_async_subdev *asd))\n> > +{\n> > +\tstruct fwnode_reference_args args;\n> > +\tunsigned int index = 0;\n> > +\tint ret = -ENOENT;\n> > +\n> > +\tif (asd_struct_size < sizeof(struct v4l2_async_subdev))\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tfor (; !fwnode_property_get_reference_args(\n> > +\t\t     dev_fwnode(dev), prop, nargs_prop, nargs,\n> > +\t\t     index, &args); index++)\n> > +\t\tfwnode_handle_put(args.fwnode);\n> > +\n> > +\tret = v4l2_async_notifier_realloc(notifier,\n> > +\t\t\t\t\t  notifier->num_subdevs + index);\n> > +\tif (ret)\n> > +\t\treturn -ENOMEM;\n> > +\n> > +\tfor (ret = -ENOENT, index = 0; !fwnode_property_get_reference_args(\n> > +\t\t     dev_fwnode(dev), prop, nargs_prop, nargs,\n> > +\t\t     index, &args); index++) {\n> > +\t\tstruct v4l2_async_subdev *asd;\n> > +\n> > +\t\tif (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs))\n> > +\t\t\tbreak;\n> \n> As mentioned elsewhere: I think this should return an error.\n\nI'll use -EINVAL and put the fwnode as well, i.e. goto error.\n\n> \n> > +\n> > +\t\tasd = kzalloc(asd_struct_size, GFP_KERNEL);\n> > +\t\tif (!asd) {\n> > +\t\t\tret = -ENOMEM;\n> > +\t\t\tgoto error;\n> > +\t\t}\n> > +\n> > +\t\tret = parse_single ? parse_single(dev, &args, asd) : 0;\n> > +\t\tif (ret == -ENOTCONN) {\n> > +\t\t\tdev_dbg(dev,\n> > +\t\t\t\t\"ignoring reference prop \\\"%s\\\", nargs_prop \\\"%s\\\", nargs %u, index %u\",\n> > +\t\t\t\tprop, nargs_prop, nargs, index);\n> > +\t\t\tv4l2_fwnode_print_args(&args);\n> > +\t\t\tpr_cont(\"\\n\");\n> \n> asd isn't freed.\n\nWill fix both.\n\n> \n> > +\t\t\tcontinue;\n> > +\t\t} else if (ret < 0) {\n> > +\t\t\tdev_warn(dev,\n> > +\t\t\t\t \"driver could not parse reference prop \\\"%s\\\", nargs_prop \\\"%s\\\", nargs %u, index %u\",\n> > +\t\t\t\t prop, nargs_prop, nargs, index);\n> > +\t\t\tv4l2_fwnode_print_args(&args);\n> > +\t\t\tpr_cont(\"\\n\");\n> \n> Ditto.\n> \n> > +\t\t\tgoto error;\n> > +\t\t}\n> > +\n> > +\t\tnotifier->subdevs[notifier->num_subdevs] = asd;\n> > +\t\tasd->match.fwnode.fwnode = args.fwnode;\n> > +\t\tasd->match_type = V4L2_ASYNC_MATCH_FWNODE;\n> > +\t\tnotifier->num_subdevs++;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +\n> > +error:\n> > +\tfwnode_handle_put(args.fwnode);\n> > +\treturn ret;\n> > +}\n> > +EXPORT_SYMBOL_GPL(v4l2_fwnode_reference_parse);\n> > +\n> >  MODULE_LICENSE(\"GPL\");\n> >  MODULE_AUTHOR(\"Sakari Ailus <sakari.ailus@linux.intel.com>\");\n> >  MODULE_AUTHOR(\"Sylwester Nawrocki <s.nawrocki@samsung.com>\");\n> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> > index 6d125f26ec84..52f528162818 100644\n> > --- a/include/media/v4l2-fwnode.h\n> > +++ b/include/media/v4l2-fwnode.h\n> > @@ -254,4 +254,32 @@ 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 - parse references for async sub-devices\n> > + * @dev: the device node the properties of which are parsed for references\n> \n> the device node whose properties are...\n\nBoth mean the same thing.\n\n> \n> > + * @notifier: the async notifier where the async subdevs will be added\n> > + * @prop: the name of the property\n> > + * @nargs_prop: the name of the property in the remote node that specifies the\n> > + *\t\tnumber of integer arguments (may be NULL, in that case nargs\n> > + *\t\twill be used).\n> > + * @nargs: the number of integer arguments after the reference\n> > + * @asd_struct_size: the size of the driver's async sub-device struct, including\n> > + *\t\t     @struct v4l2_async_subdev\n> > + * @parse_single: Driver's callback function for parsing a reference. Optional.\n> \n> Driver's -> driver's\n\nFixed.\n\n> \n> > + *\t\t  Return: 0 on success\n> > + *\t\t\t  %-ENOTCONN if the reference is to be skipped but this\n> > + *\t\t\t\t     should not be considered as an error\n> \n> skipped but this -> skipped. This\n\nAlso should -> will.\n\n> \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(\n> > +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> > +\tconst char *prop, const char *nargs_prop, unsigned int nargs,\n> > +\tsize_t asd_struct_size,\n> > +\tint (*parse_single)(struct device *dev,\n> > +\t\t\t    struct fwnode_reference_args *args,\n> > +\t\t\t    struct v4l2_async_subdev *asd));\n> > +\n> >  #endif /* _V4L2_FWNODE_H */\n> >","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmFmF6s0yz9t32\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 02:34:05 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753861AbdIDQeE (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 12:34:04 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:59824 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1752687AbdIDQeD (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 12:34:03 -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 7EB0B600E4;\n\tMon,  4 Sep 2017 19:34:01 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1douK1-0002O3-1k; Mon, 04 Sep 2017 19:34:01 +0300"],"Date":"Mon, 4 Sep 2017 19:34:00 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Hans Verkuil <hverkuil@xs4all.nl>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v7 15/18] v4l2-fwnode: Add convenience function for\n\tparsing generic references","Message-ID":"<20170904163400.z26qmxuejhgdcmrw@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-16-sakari.ailus@linux.intel.com>\n\t<0bb75f81-cc81-a4bf-f2af-41862c1d777a@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<0bb75f81-cc81-a4bf-f2af-41862c1d777a@xs4all.nl>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762777,"web_url":"http://patchwork.ozlabs.org/comment/1762777/","msgid":"<20170904164201.7rycyycvrukiusjz@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-04T16:42:02","subject":"Re: [PATCH v7 12/18] v4l: async: Allow binding notifiers to\n\tsub-devices","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Hans,\n\nOn Mon, Sep 04, 2017 at 04:56:29PM +0200, Hans Verkuil wrote:\n> On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n> > Registering a notifier has required the knowledge of struct v4l2_device\n> > for the reason that sub-devices generally are registered to the\n> > v4l2_device (as well as the media device, also available through\n> > v4l2_device).\n> > \n> > This information is not available for sub-device drivers at probe time.\n> > \n> > What this patch does is that it allows registering notifiers without\n> > having v4l2_device around. Instead the sub-device pointer is stored to 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 master notifier's complete callback is only called when all sub-device\n> > notifiers are completed.\n> > \n> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> > ---\n> >  drivers/media/v4l2-core/v4l2-async.c | 153 +++++++++++++++++++++++++++++------\n> >  include/media/v4l2-async.h           |  19 ++++-\n> >  2 files changed, 146 insertions(+), 26 deletions(-)\n> > \n> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> > index 70d02378b48f..55d7886103d2 100644\n> > --- a/drivers/media/v4l2-core/v4l2-async.c\n> > +++ b/drivers/media/v4l2-core/v4l2-async.c\n> > @@ -25,6 +25,10 @@\n> >  #include <media/v4l2-fwnode.h>\n> >  #include <media/v4l2-subdev.h>\n> >  \n> > +static int v4l2_async_test_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> > @@ -101,14 +105,69 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *\n> >  \treturn NULL;\n> >  }\n> >  \n> > +static bool v4l2_async_subdev_notifiers_complete(\n> > +\tstruct v4l2_async_notifier *notifier)\n> > +{\n> > +\tstruct v4l2_async_notifier *n;\n> > +\n> > +\tlist_for_each_entry(n, &notifier->notifiers, notifiers) {\n> > +\t\tif (!n->master)\n> > +\t\t\treturn false;\n> > +\t}\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> > +#define notifier_v4l2_dev(n) \\\n> > +\t(!!(n)->v4l2_dev ? (n)->v4l2_dev : \\\n> > +\t !!(n)->master ? (n)->master->v4l2_dev : NULL)\n> \n> Why '!!'?\n\nI've since replaced this by a function.\n\nMy understanding is GCC 7 doesn't like a ? x : y construct where a is a\nnon-boolean. This will be effectively the same, but a boolean.\n\nSee e.g.\n\ncommit da48c948c263c9d87dfc64566b3373a858cc8aa2\nAuthor: Arnd Bergmann <arnd@arndb.de>\nDate:   Wed Jul 19 15:23:27 2017 -0400\n\n    media: fix warning on v4l2_subdev_call() result interpreted as bool\n    \n    v4l2_subdev_call is a macro returning whatever the callback return\n    type is, usually 'int'. With gcc-7 and ccache, this can lead to\n    many wanings like:\n    \n    media/platform/pxa_camera.c: In function 'pxa_mbus_build_fmts_xlate':\n    media/platform/pxa_camera.c:766:27: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context]\n      while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {\n    media/atomisp/pci/atomisp2/atomisp_cmd.c: In function 'atomisp_s_ae_window':\n    media/atomisp/pci/atomisp2/atomisp_cmd.c:6414:52: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context]\n      if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,\n    \n    The problem here is that after preprocessing, we the compiler\n    sees a variation of\n    \n            if (a ? 0 : 2)\n    \n    that it thinks is suspicious.\n    \n    This replaces the ?: operator with an different expression that\n    does the same thing in a more easily readable way that cannot\n    tigger the warning\n    \n    Link: https://lkml.org/lkml/2017/7/14/156\n    \n    Signed-off-by: Arnd Bergmann <arnd@arndb.de>\n    Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>\n\n> \n> > +\n> > +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(\n> > +\tstruct v4l2_async_notifier *notifier, struct v4l2_subdev *sd)\n> > +{\n> > +\tstruct v4l2_async_notifier *n;\n> > +\n> > +\tlist_for_each_entry(n, &notifier_list, list) {\n> > +\t\tif (n->sd == sd)\n> > +\t\t\treturn n;\n> > +\t}\n> > +\n> > +\treturn NULL;\n> > +}\n> > +\n> > +static int v4l2_async_notifier_test_all_subdevs(\n> > +\tstruct v4l2_async_notifier *notifier)\n> > +{\n> > +\tstruct v4l2_subdev *sd, *tmp;\n> > +\n> > +\tif (!notifier_v4l2_dev(notifier))\n> > +\t\treturn 0;\n> > +\n> > +\tlist_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {\n> > +\t\tstruct v4l2_async_subdev *asd;\n> > +\t\tint ret;\n> > +\n> > +\t\tasd = v4l2_async_belongs(notifier, sd);\n> > +\t\tif (!asd)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tret = v4l2_async_test_notify(notifier, sd, asd);\n> > +\t\tif (ret < 0)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> >  static int v4l2_async_test_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> A general note (not specific to this patch series): I really don't like\n> this function name. v4l2_async_match_notify is a much better name.\n> \n> With 'test' I get association with 'testing something' and not that it is\n> a match.\n> \n> I have a similar problem with v4l2_async_belongs: v4l2_async_find_match\n> makes a lot more sense.\n\nI can prepend the set with a patch renaming them.\n\n> \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(notifier_v4l2_dev(notifier), sd);\n> > +\tif (ret)\n> >  \t\treturn ret;\n> >  \n> >  \tret = v4l2_async_notifier_call_int_op(notifier, bound, sd, asd);\n> > @@ -125,8 +184,26 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,\n> >  \t/* Move from the global subdevice list to notifier's done */\n> >  \tlist_move(&sd->async_list, &notifier->done);\n> >  \n> > -\tif (list_empty(&notifier->waiting) && notifier->ops->complete)\n> > -\t\treturn v4l2_async_notifier_call_int_op(notifier, complete);\n> > +\tsubdev_notifier = v4l2_async_get_subdev_notifier(notifier, sd);\n> > +\tif (subdev_notifier && !subdev_notifier->master) {\n> > +\t\tsubdev_notifier->master = notifier;\n> > +\t\tlist_add(&subdev_notifier->notifiers, &notifier->notifiers);\n> > +\t\tret = v4l2_async_notifier_test_all_subdevs(subdev_notifier);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (list_empty(&notifier->waiting) &&\n> > +\t    v4l2_async_subdev_notifiers_complete(notifier)) {\n> > +\t\tret = v4l2_async_notifier_call_int_op(notifier, complete);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (notifier->master && list_empty(&notifier->master->waiting) &&\n> > +\t    v4l2_async_subdev_notifiers_complete(notifier->master))\n> > +\t\treturn v4l2_async_notifier_call_int_op(notifier->master,\n> > +\t\t\t\t\t\t       complete);\n> >  \n> >  \treturn 0;\n> >  }\n> > @@ -140,18 +217,17 @@ 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 ||\n> > +\tif (!!notifier->v4l2_dev == !!notifier->sd || !notifier->num_subdevs ||\n> \n> '!!notifier->v4l2_dev == !!notifier->sd'???\n> \n> This is '(notifier->v4l2_dev && notifier->sd) || (!notifier->v4l2_dev && !notifier->sd)'\n> but it took me a bit of time to parse that.\n> \n> A 10 for creativity, but not so much for readability :-)\n\n:-D\n\n> \n> I suspect it can be simplified as well, or some of these tests can be moved to\n> the two functions that call this one. I think that might be best, in fact.\n\nA single ! would be actually enough. What would you think of that? A bit\nless too loud? :-) It should be easies to grasp: both cannot be NULL or\nnon-NULL.\n\nI've moved the check for v4l2_dev or sd to the appropriate functions. Still\nit could be worth checking for this just in case.\n\n> \n> >  \t    notifier->num_subdevs > V4L2_MAX_SUBDEVS)\n> >  \t\treturn -EINVAL;\n> >  \n> > -\tnotifier->v4l2_dev = v4l2_dev;\n> > +\tINIT_LIST_HEAD(&notifier->notifiers);\n> >  \tINIT_LIST_HEAD(&notifier->waiting);\n> >  \tINIT_LIST_HEAD(&notifier->done);\n> >  \n> > @@ -175,18 +251,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_belongs(notifier, sd);\n> > -\t\tif (!asd)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tret = v4l2_async_test_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_test_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> > @@ -196,27 +264,62 @@ 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> > +\tnotifier->v4l2_dev = v4l2_dev;\n> > +\n> > +\treturn __v4l2_async_notifier_register(notifier);\n> > +}\n> >  EXPORT_SYMBOL(v4l2_async_notifier_register);\n> >  \n> > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,\n> > +\t\t\t\t\tstruct v4l2_async_notifier *notifier)\n> > +{\n> > +\tnotifier->sd = sd;\n> > +\n> > +\treturn __v4l2_async_notifier_register(notifier);\n> > +}\n> > +EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);\n> > +\n> >  void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n> >  {\n> > +\tstruct v4l2_async_notifier *notifier_iter, *notifier_tmp;\n> >  \tstruct v4l2_subdev *sd, *tmp;\n> >  \tunsigned int notif_n_subdev = notifier->num_subdevs;\n> >  \tunsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);\n> >  \tstruct device **dev;\n> >  \tint i = 0;\n> >  \n> > -\tif (!notifier->v4l2_dev)\n> > +\tif (!notifier->v4l2_dev && !notifier->sd)\n> >  \t\treturn;\n> >  \n> >  \tdev = kvmalloc_array(n_subdev, sizeof(*dev), GFP_KERNEL);\n> >  \tif (!dev) {\n> > -\t\tdev_err(notifier->v4l2_dev->dev,\n> > +\t\tdev_err(notifier->v4l2_dev ?\n> > +\t\t\tnotifier->v4l2_dev->dev : notifier->sd->dev,\n> >  \t\t\t\"Failed to allocate device cache!\\n\");\n> >  \t}\n> >  \n> >  \tmutex_lock(&list_lock);\n> >  \n> > +\tif (notifier->v4l2_dev) {\n> > +\t\t/* Remove all subdev notifiers from the master's list */\n> > +\t\tlist_for_each_entry_safe(notifier_iter, notifier_tmp,\n> > +\t\t\t\t\t &notifier->notifiers, notifiers) {\n> > +\t\t\tlist_del_init(&notifier_iter->notifiers);\n> > +\t\t\tWARN_ON(notifier_iter->master != notifier);\n> > +\t\t\tnotifier_iter->master = NULL;\n> > +\t\t}\n> > +\t\tnotifier->v4l2_dev = NULL;\n> > +\t} else {\n> > +\t\t/* Remove subdev notifier from the master's list */\n> > +\t\tlist_del_init(&notifier->notifiers);\n> > +\t\tnotifier->master = NULL;\n> > +\t\tnotifier->sd = NULL;\n> > +\t}\n> > +\n> >  \tlist_del(&notifier->list);\n> >  \n> >  \tlist_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {\n> > @@ -266,8 +369,6 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)\n> >  \t}\n> >  \tkvfree(dev);\n> >  \n> > -\tnotifier->v4l2_dev = NULL;\n> > -\n> >  \t/*\n> >  \t * Don't care about the waiting list, it is initialised and populated\n> >  \t * upon notifier registration.\n> > @@ -287,6 +388,8 @@ void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)\n> >  \n> >  \tkvfree(notifier->subdevs);\n> >  \tnotifier->subdevs = NULL;\n> > +\n> > +\tWARN_ON(!list_empty(&notifier->notifiers));\n> >  }\n> >  EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);\n> >  \n> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> > index c3e001e0d1f1..a5c123ac2873 100644\n> > --- a/include/media/v4l2-async.h\n> > +++ b/include/media/v4l2-async.h\n> > @@ -110,7 +110,11 @@ 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 master, for subdev notifiers NULL\n> > + * @sd:\t\tsub-device that registered the notifier, NULL otherwise\n> > + * @notifiers:\tlist of struct v4l2_async_notifier, notifiers linked to this\n> > + *\t\tnotifier\n> > + * @master:\tmaster notifier carrying @v4l2_dev\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> > @@ -121,6 +125,9 @@ 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 list_head notifiers;\n> > +\tstruct v4l2_async_notifier *master;\n> >  \tstruct list_head waiting;\n> >  \tstruct list_head done;\n> >  \tstruct list_head list;\n> > @@ -136,6 +143,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,\n> >  \t\t\t\t struct v4l2_async_notifier *notifier);\n> >  \n> >  /**\n> > + * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous\n> > + *\t\t\t\t\t notifier for a sub-device\n> > + *\n> > + * @sd: pointer to &struct v4l2_subdev\n> > + * @notifier: pointer to &struct v4l2_async_notifier\n> > + */\n> > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,\n> > +\t\t\t\t\tstruct v4l2_async_notifier *notifier);\n> > +\n> > +/**\n> >   * v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier\n> >   *\n> >   * @notifier: pointer to &struct v4l2_async_notifier\n> > \n> \n> Do you have a git tree with this patch series? I think I need to look at this\n> in the final version, not just the patch.\n\nI'll upload the patches after making the latest changes.","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmFxc2mDzz9t3J\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 02:42:12 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752687AbdIDQmI (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 12:42:08 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:59894 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1753962AbdIDQmF (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 12:42:05 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id EB3B0600E4;\n\tMon,  4 Sep 2017 19:42:02 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1douRm-0002O9-GE; Mon, 04 Sep 2017 19:42:02 +0300"],"Date":"Mon, 4 Sep 2017 19:42:02 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Hans Verkuil <hverkuil@xs4all.nl>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v7 12/18] v4l: async: Allow binding notifiers to\n\tsub-devices","Message-ID":"<20170904164201.7rycyycvrukiusjz@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-13-sakari.ailus@linux.intel.com>\n\t<60db0a4b-0560-9591-f41f-1d055a50ba12@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<60db0a4b-0560-9591-f41f-1d055a50ba12@xs4all.nl>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762809,"web_url":"http://patchwork.ozlabs.org/comment/1762809/","msgid":"<cad608c6-93b6-d791-a14a-a5b38fe6e1bf@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T17:37:09","subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/04/2017 05:54 PM, Sakari Ailus wrote:\n>>> +/**\n>>> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints in a\n>>> + *\t\t\t\t\t\tdevice node\n>>> + * @dev: the device the endpoints of which are to be parsed\n>>> + * @notifier: notifier for @dev\n>>> + * @asd_struct_size: size of the driver's async sub-device struct, including\n>>> + *\t\t     sizeof(struct v4l2_async_subdev). The &struct\n>>> + *\t\t     v4l2_async_subdev shall be the first member of\n>>> + *\t\t     the driver's async sub-device struct, i.e. both\n>>> + *\t\t     begin at the same memory address.\n>>> + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode\n>>> + *\t\t    endpoint. Optional.\n>>> + *\t\t    Return: %0 on success\n>>> + *\t\t\t    %-ENOTCONN if the endpoint is to be skipped but this\n>>> + *\t\t\t\t       should not be considered as an error\n>>> + *\t\t\t    %-EINVAL if the endpoint configuration is invalid\n>>> + *\n>>> + * Parse the fwnode endpoints of the @dev device and populate the async sub-\n>>> + * devices array of the notifier. The @parse_endpoint callback function is\n>>> + * called for each endpoint with the corresponding async sub-device pointer to\n>>> + * let the caller initialize the driver-specific part of the async sub-device\n>>> + * structure.\n>>> + *\n>>> + * The notifier memory shall be zeroed before this function is called on the\n>>> + * notifier.\n>>> + *\n>>> + * This function may not be called on a registered notifier and may be called on\n>>> + * a notifier only once. When using this function, the user may not access the\n>>> + * notifier's subdevs array nor change notifier's num_subdevs field, these are\n>>> + * reserved for the framework's internal use only.\n>>\n>> I don't think the sentence \"When using...only\" makes any sense. How would you\n>> even be able to access any of the notifier fields? You don't have access to it\n>> from the parse_endpoint callback.\n> \n> Not from the parse_endpoint callback. The notifier is first set up by the\n> driver, and this text applies to that --- whether or not parse_endpoint is\n> given.\n\nWhat you mean is \"After calling this function...\" since v4l2_async_notifier_release()\nneeds this to release all the memory.\n\nI'll take another look at this text when I see v8.\n\nRegards,\n\n\tHans\n\n> \n>>\n>> I think it can just be dropped.\n>>\n>>> + *\n>>> + * The @struct v4l2_fwnode_endpoint passed to the callback function\n>>> + * @parse_endpoint is released once the function is finished. If there is a need\n>>> + * to retain that configuration, the user needs to allocate memory for it.\n>>> + *\n>>> + * Any notifier populated using this function must be released with a call to\n>>> + * v4l2_async_notifier_release() after it has been unregistered and the async\n>>> + * sub-devices are no longer in use.\n>>> + *\n>>> + * Return: %0 on success, including when no async sub-devices are found\n>>> + *\t   %-ENOMEM if memory allocation failed\n>>> + *\t   %-EINVAL if graph or endpoint parsing failed\n>>> + *\t   Other error codes as returned by @parse_endpoint\n>>> + */\n>>> +int v4l2_async_notifier_parse_fwnode_endpoints(\n>>> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n>>> +\tsize_t asd_struct_size,\n>>> +\tint (*parse_endpoint)(struct device *dev,\n>>> +\t\t\t      struct v4l2_fwnode_endpoint *vep,\n>>> +\t\t\t      struct v4l2_async_subdev *asd));\n>>> +\n>>>  #endif /* _V4L2_FWNODE_H */\n>>>\n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmH9872p6z9t1t\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 03:37:16 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753979AbdIDRhP (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 13:37:15 -0400","from lb3-smtp-cloud9.xs4all.net ([194.109.24.30]:44745 \"EHLO\n\tlb3-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753957AbdIDRhO (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 13:37:14 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid ovJ7d4sXzdRLjovJBdXk47; Mon, 04 Sep 2017 19:37:13 +0200"],"Subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","To":"Sakari Ailus <sakari.ailus@iki.fi>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-5-sakari.ailus@linux.intel.com>\n\t<e07f9b4d-e8dc-5598-98ee-3c69e3100b81@xs4all.nl>\n\t<20170904155415.nak4dofd2k6ytupa@valkosipuli.retiisi.org.uk>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<cad608c6-93b6-d791-a14a-a5b38fe6e1bf@xs4all.nl>","Date":"Mon, 4 Sep 2017 19:37:09 +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":"<20170904155415.nak4dofd2k6ytupa@valkosipuli.retiisi.org.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfMLfIcBpAeKguyuZKFL4dU6dvSHOwtYSrpAmV/o5AYNQlS4oKonn2WlUVIMp/t+90pcwTM2nhVgu8AtpAZo8c35/GAZnaYpADVJlLHf/VWHOtpkFQEQL\n\tdi7NeUpdfKiP0k9tdXBWyoA61YYxRAiTuA0j6vzHiBCcIIAs3SzdO/0nQ+ia/eh7eZwJosrAGIVt6yxDIZ8XEWRRJdVo9lnkOpF1ZoFtDuWBuB278Pk7EWOV\n\tltVKACGeHkHTEouZuITGAWPnzskahT/4WlsAgjH8ihduVekQaViCIUrhP1DKC6iCb8ojBuziwoqdRz64GeAN5rAxr+IcL8cJCJYzDBajSDOUbVE6CqmmG8HQ\n\tK+hJM+cWsARRYV4UdEWWFBJFjMA1bMT0RVAG4yf/+CoadKWtu1jM+NBDt+wAwsQmmfcLFKu0","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762813,"web_url":"http://patchwork.ozlabs.org/comment/1762813/","msgid":"<3921fde8-6192-87d7-9c5d-5dd2035a9565@xs4all.nl>","list_archive_url":null,"date":"2017-09-04T17:38:42","subject":"Re: [PATCH v7 15/18] v4l2-fwnode: Add convenience function for\n\tparsing generic references","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 09/04/2017 06:34 PM, Sakari Ailus wrote:\n>>>  MODULE_LICENSE(\"GPL\");\n>>>  MODULE_AUTHOR(\"Sakari Ailus <sakari.ailus@linux.intel.com>\");\n>>>  MODULE_AUTHOR(\"Sylwester Nawrocki <s.nawrocki@samsung.com>\");\n>>> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n>>> index 6d125f26ec84..52f528162818 100644\n>>> --- a/include/media/v4l2-fwnode.h\n>>> +++ b/include/media/v4l2-fwnode.h\n>>> @@ -254,4 +254,32 @@ 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 - parse references for async sub-devices\n>>> + * @dev: the device node the properties of which are parsed for references\n>>\n>> the device node whose properties are...\n> \n> Both mean the same thing.\n\nYes, but I think your sentence is a bit awkward. Just my opinion, though.\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmHBy0H45z9t32\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 03:38:49 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753913AbdIDRis (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 13:38:48 -0400","from lb2-smtp-cloud9.xs4all.net ([194.109.24.26]:42809 \"EHLO\n\tlb2-smtp-cloud9.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753885AbdIDRir (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 13:38:47 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid ovKcd4tOIdRLjovKfdXkZF; Mon, 04 Sep 2017 19:38:46 +0200"],"Subject":"Re: [PATCH v7 15/18] v4l2-fwnode: Add convenience function for\n\tparsing generic references","To":"Sakari Ailus <sakari.ailus@iki.fi>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-16-sakari.ailus@linux.intel.com>\n\t<0bb75f81-cc81-a4bf-f2af-41862c1d777a@xs4all.nl>\n\t<20170904163400.z26qmxuejhgdcmrw@valkosipuli.retiisi.org.uk>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<3921fde8-6192-87d7-9c5d-5dd2035a9565@xs4all.nl>","Date":"Mon, 4 Sep 2017 19:38:42 +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":"<20170904163400.z26qmxuejhgdcmrw@valkosipuli.retiisi.org.uk>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfODTZ15UW09LS1tGfav2oNChVean8FGS2d2hVMXc8PvlyPBWh1PR7yVGhzgzYMbwsw7BfbL56wx9uT2diMRXbop+m4A6LQk7jwfW3kloHDcGS6h85X6f\n\tQ9fJXwQxEe6rQ/jSsdEFcTJYrmSYwC8PEozN6ZsUI0ze38B4+nqahL8IpLo0jy4iT/JVjp6fK7EfsgVoXFXij+0nRr6lZBCy+AlVBdfDtNMKKMMunkjWGWxB\n\t+qLuK+llidALDG6HGwEFj9/RWC1K7YKAnbSRYjowrfUA2Kd4RjCsYqCJtcBXlRnwFJwhdu1CEG68ENZThVI+wUJ/wGl5biRQRlkEWGq3uaOwwbu+EknXQ+4e\n\tk5tSMxkCOyXsEkxlSxZaLv3HRrq0QZp+ynpyJ1d5SsB3kkNd78qfEpCwDoP08IgRPDB4OhVP","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762864,"web_url":"http://patchwork.ozlabs.org/comment/1762864/","msgid":"<20170904203036.aeyz335w7eypxj4m@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-04T20:30:37","subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"On Mon, Sep 04, 2017 at 07:37:09PM +0200, Hans Verkuil wrote:\n> On 09/04/2017 05:54 PM, Sakari Ailus wrote:\n> >>> +/**\n> >>> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints in a\n> >>> + *\t\t\t\t\t\tdevice node\n> >>> + * @dev: the device the endpoints of which are to be parsed\n> >>> + * @notifier: notifier for @dev\n> >>> + * @asd_struct_size: size of the driver's async sub-device struct, including\n> >>> + *\t\t     sizeof(struct v4l2_async_subdev). The &struct\n> >>> + *\t\t     v4l2_async_subdev shall be the first member of\n> >>> + *\t\t     the driver's async sub-device struct, i.e. both\n> >>> + *\t\t     begin at the same memory address.\n> >>> + * @parse_endpoint: Driver's callback function called on each V4L2 fwnode\n> >>> + *\t\t    endpoint. Optional.\n> >>> + *\t\t    Return: %0 on success\n> >>> + *\t\t\t    %-ENOTCONN if the endpoint is to be skipped but this\n> >>> + *\t\t\t\t       should not be considered as an error\n> >>> + *\t\t\t    %-EINVAL if the endpoint configuration is invalid\n> >>> + *\n> >>> + * Parse the fwnode endpoints of the @dev device and populate the async sub-\n> >>> + * devices array of the notifier. The @parse_endpoint callback function is\n> >>> + * called for each endpoint with the corresponding async sub-device pointer to\n> >>> + * let the caller initialize the driver-specific part of the async sub-device\n> >>> + * structure.\n> >>> + *\n> >>> + * The notifier memory shall be zeroed before this function is called on the\n> >>> + * notifier.\n> >>> + *\n> >>> + * This function may not be called on a registered notifier and may be called on\n> >>> + * a notifier only once. When using this function, the user may not access the\n> >>> + * notifier's subdevs array nor change notifier's num_subdevs field, these are\n> >>> + * reserved for the framework's internal use only.\n> >>\n> >> I don't think the sentence \"When using...only\" makes any sense. How would you\n> >> even be able to access any of the notifier fields? You don't have access to it\n> >> from the parse_endpoint callback.\n> > \n> > Not from the parse_endpoint callback. The notifier is first set up by the\n> > driver, and this text applies to that --- whether or not parse_endpoint is\n> > given.\n> \n> What you mean is \"After calling this function...\" since v4l2_async_notifier_release()\n> needs this to release all the memory.\n\nRight, that's a good point.\n\nnotifier->subdevs may be allocated by the driver as well, so\nv4l2_async_notifier_release() must take that into account.\nnotifier->max_subdevs should be good for that.","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmM1G32pYz9t2R\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 06:30:42 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753994AbdIDUak (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tMon, 4 Sep 2017 16:30:40 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:33264 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1753968AbdIDUak (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Mon, 4 Sep 2017 16:30:40 -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 027D2600EB;\n\tMon,  4 Sep 2017 23:30:37 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1doy0z-0002PR-Bl; Mon, 04 Sep 2017 23:30:37 +0300"],"Date":"Mon, 4 Sep 2017 23:30:37 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Hans Verkuil <hverkuil@xs4all.nl>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, laurent.pinchart@ideasonboard.com,\n\tdevicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org","Subject":"Re: [PATCH v7 04/18] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","Message-ID":"<20170904203036.aeyz335w7eypxj4m@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-5-sakari.ailus@linux.intel.com>\n\t<e07f9b4d-e8dc-5598-98ee-3c69e3100b81@xs4all.nl>\n\t<20170904155415.nak4dofd2k6ytupa@valkosipuli.retiisi.org.uk>\n\t<cad608c6-93b6-d791-a14a-a5b38fe6e1bf@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<cad608c6-93b6-d791-a14a-a5b38fe6e1bf@xs4all.nl>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1762998,"web_url":"http://patchwork.ozlabs.org/comment/1762998/","msgid":"<6ad1c25a-e2a7-b73f-4d7c-6a5c071e6366@xs4all.nl>","list_archive_url":null,"date":"2017-09-05T06:49:43","subject":"Re: [PATCH v7 12/18] 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/03/2017 07:49 PM, Sakari Ailus wrote:\n> Registering a notifier has required the knowledge of struct v4l2_device\n> for the reason that sub-devices generally are registered to the\n> v4l2_device (as well as the media device, also available through\n> v4l2_device).\n> \n> This information is not available for sub-device drivers at probe time.\n> \n> What this patch does is that it allows registering notifiers without\n> having v4l2_device around. Instead the sub-device pointer is stored to 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 master notifier's complete callback is only called when all sub-device\n> notifiers are completed.\n> \n> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> ---\n>  drivers/media/v4l2-core/v4l2-async.c | 153 +++++++++++++++++++++++++++++------\n>  include/media/v4l2-async.h           |  19 ++++-\n>  2 files changed, 146 insertions(+), 26 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> index 70d02378b48f..55d7886103d2 100644\n> --- a/drivers/media/v4l2-core/v4l2-async.c\n> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> @@ -25,6 +25,10 @@\n>  #include <media/v4l2-fwnode.h>\n>  #include <media/v4l2-subdev.h>\n>  \n> +static int v4l2_async_test_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> @@ -101,14 +105,69 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *\n>  \treturn NULL;\n>  }\n>  \n> +static bool v4l2_async_subdev_notifiers_complete(\n> +\tstruct v4l2_async_notifier *notifier)\n> +{\n> +\tstruct v4l2_async_notifier *n;\n> +\n> +\tlist_for_each_entry(n, &notifier->notifiers, notifiers) {\n> +\t\tif (!n->master)\n> +\t\t\treturn false;\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +#define notifier_v4l2_dev(n) \\\n> +\t(!!(n)->v4l2_dev ? (n)->v4l2_dev : \\\n> +\t !!(n)->master ? (n)->master->v4l2_dev : NULL)\n> +\n> +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(\n> +\tstruct v4l2_async_notifier *notifier, struct v4l2_subdev *sd)\n\nWhy pass the notifier argument when it is not actually used in the function?\n\nIs this function needed at all? As far as I can see the sd always belongs to\nthe given notifier, otherwise the v4l2_async_belongs() call would fail.\nAnd v4l2_async_belongs() is always called before v4l2_async_test_notify().\n\nThis could all do with some more code comments. I'm having a difficult time\nunderstanding it all.\n\nI'll wait for v8 before continuing this.\n\nRegards,\n\n\tHans\n\n> +{\n> +\tstruct v4l2_async_notifier *n;\n> +\n> +\tlist_for_each_entry(n, &notifier_list, list) {\n> +\t\tif (n->sd == sd)\n> +\t\t\treturn n;\n> +\t}\n> +\n> +\treturn NULL;\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":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmcll0l07z9sP3\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 16:49:54 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750729AbdIEGtx (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tTue, 5 Sep 2017 02:49:53 -0400","from lb2-smtp-cloud7.xs4all.net ([194.109.24.28]:59819 \"EHLO\n\tlb2-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1750704AbdIEGtw (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Tue, 5 Sep 2017 02:49:52 -0400","from [192.168.2.10] ([212.251.195.8])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid p7g8dXYxwb2snp7gBdkVoz; Tue, 05 Sep 2017 08:49:51 +0200"],"Subject":"Re: [PATCH v7 12/18] v4l: async: Allow binding notifiers to\n\tsub-devices","To":"Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-13-sakari.ailus@linux.intel.com>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<6ad1c25a-e2a7-b73f-4d7c-6a5c071e6366@xs4all.nl>","Date":"Tue, 5 Sep 2017 08:49:43 +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":"<20170903174958.27058-13-sakari.ailus@linux.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfAbOktCxPcznZY6sP4fdZ6BKUYQgRG+PuoYugRM+QVICw5Uk6Xdbv4fMh4LY0+bWuLa5odEuoJVvSb37Ny8OueRL0HYflqqJJU+tyrt6mUYGonfJrWAY\n\tSA4/CIBXyMZDx4C8j6wp0iIfl9KTRkud2cBXIZzhCLCdkbyYaq31k1Tam9XzhuIXIAZJDtGItScbkALG9+qIXVQMOAGCGmRzfvcWpQ2ugtX4KQ6HelwFhDrF\n\tJ9YHEWl8YjCaj01c6wBRN9tTjA3216Vmt08CI7ZvjI5yhZLlzNh+Rjq9+V4Lp4TSCKAkIUojbRhsaoUdi7RRRcWi6i2aSIGIFYXamQErYriowuln5m3q5P6d\n\tpxrohK/WjjqPirbZVRRB3vgEKbuQ5k5APJM6PVEUy7Xof4oSTyAUGEudur6cGjffaNGn5Jh62q6tPUZ6pIK2K4w8/7NujxRWWAO2VospU1tCMGd129nNFGbO\n\tS72xh16vmBZnfLZ3","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1763068,"web_url":"http://patchwork.ozlabs.org/comment/1763068/","msgid":"<31d63a76-adab-2a04-12dc-4717b1512eaa@linux.intel.com>","list_archive_url":null,"date":"2017-09-05T08:36:54","subject":"Re: [PATCH v7 12/18] v4l: async: Allow binding notifiers to\n\tsub-devices","submitter":{"id":65485,"url":"http://patchwork.ozlabs.org/api/people/65485/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Hans,\n\nThanks for the review!\n\nOn 09/05/17 09:49, Hans Verkuil wrote:\n> On 09/03/2017 07:49 PM, Sakari Ailus wrote:\n>> Registering a notifier has required the knowledge of struct v4l2_device\n>> for the reason that sub-devices generally are registered to the\n>> v4l2_device (as well as the media device, also available through\n>> v4l2_device).\n>>\n>> This information is not available for sub-device drivers at probe time.\n>>\n>> What this patch does is that it allows registering notifiers without\n>> having v4l2_device around. Instead the sub-device pointer is stored to 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 master notifier's complete callback is only called when all sub-device\n>> notifiers are completed.\n>>\n>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n>> ---\n>>  drivers/media/v4l2-core/v4l2-async.c | 153 +++++++++++++++++++++++++++++------\n>>  include/media/v4l2-async.h           |  19 ++++-\n>>  2 files changed, 146 insertions(+), 26 deletions(-)\n>>\n>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n>> index 70d02378b48f..55d7886103d2 100644\n>> --- a/drivers/media/v4l2-core/v4l2-async.c\n>> +++ b/drivers/media/v4l2-core/v4l2-async.c\n>> @@ -25,6 +25,10 @@\n>>  #include <media/v4l2-fwnode.h>\n>>  #include <media/v4l2-subdev.h>\n>>  \n>> +static int v4l2_async_test_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>> @@ -101,14 +105,69 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *\n>>  \treturn NULL;\n>>  }\n>>  \n>> +static bool v4l2_async_subdev_notifiers_complete(\n>> +\tstruct v4l2_async_notifier *notifier)\n>> +{\n>> +\tstruct v4l2_async_notifier *n;\n>> +\n>> +\tlist_for_each_entry(n, &notifier->notifiers, notifiers) {\n>> +\t\tif (!n->master)\n>> +\t\t\treturn false;\n>> +\t}\n>> +\n>> +\treturn true;\n>> +}\n>> +\n>> +#define notifier_v4l2_dev(n) \\\n>> +\t(!!(n)->v4l2_dev ? (n)->v4l2_dev : \\\n>> +\t !!(n)->master ? (n)->master->v4l2_dev : NULL)\n>> +\n>> +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(\n>> +\tstruct v4l2_async_notifier *notifier, struct v4l2_subdev *sd)\n> \n> Why pass the notifier argument when it is not actually used in the function?\n> \n> Is this function needed at all? As far as I can see the sd always belongs to\n> the given notifier, otherwise the v4l2_async_belongs() call would fail.\n> And v4l2_async_belongs() is always called before v4l2_async_test_notify().\n\nThe function gets a notifier which the sub-device may have registered,\nit's not the same notifier that was used in registering the sub-device\nitself.\n\nI'll remove the other argument as well.\n\n> \n> This could all do with some more code comments. I'm having a difficult time\n> understanding it all.\n\nYes, I'm adding comments to v8.","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmg7N1DWgz9sNq\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue,  5 Sep 2017 18:37:04 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750738AbdIEIhC (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tTue, 5 Sep 2017 04:37:02 -0400","from mga14.intel.com ([192.55.52.115]:24842 \"EHLO mga14.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750762AbdIEIhB (ORCPT <rfc822;devicetree@vger.kernel.org>);\n\tTue, 5 Sep 2017 04:37:01 -0400","from orsmga001.jf.intel.com ([10.7.209.18])\n\tby fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t05 Sep 2017 01:36:58 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby orsmga001.jf.intel.com with ESMTP; 05 Sep 2017 01:36:55 -0700","from [127.0.0.1] (localhost [127.0.0.1])\n\tby paasikivi.fi.intel.com (Postfix) with ESMTP id 1164B2036B;\n\tTue,  5 Sep 2017 11:36:55 +0300 (EEST)"],"X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos; i=\"5.41,479,1498546800\"; d=\"scan'208\";\n\ta=\"1169193777\"","Subject":"Re: [PATCH v7 12/18] v4l: async: Allow binding notifiers to\n\tsub-devices","To":"Hans Verkuil <hverkuil@xs4all.nl>, linux-media@vger.kernel.org","Cc":"niklas.soderlund@ragnatech.se, robh@kernel.org,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tpavel@ucw.cz, sre@kernel.org","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-13-sakari.ailus@linux.intel.com>\n\t<6ad1c25a-e2a7-b73f-4d7c-6a5c071e6366@xs4all.nl>","From":"Sakari Ailus <sakari.ailus@linux.intel.com>","Message-ID":"<31d63a76-adab-2a04-12dc-4717b1512eaa@linux.intel.com>","Date":"Tue, 5 Sep 2017 11:36:54 +0300","User-Agent":"Mozilla/5.0 (X11; Linux i686 on x86_64; rv:51.0) Gecko/20100101\n\tSeaMonkey/2.48","MIME-Version":"1.0","In-Reply-To":"<6ad1c25a-e2a7-b73f-4d7c-6a5c071e6366@xs4all.nl>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1763420,"web_url":"http://patchwork.ozlabs.org/comment/1763420/","msgid":"<20170905145337.h6qy42k3hv6ha73y@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-05T14:53:37","subject":"Re: [PATCH v7 01/18] v4l: fwnode: Move KernelDoc documentation to\n\tthe header","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"On Mon, Sep 04, 2017 at 05:49:40PM +0200, Pavel Machek wrote:\n> On Sun 2017-09-03 20:49:41, Sakari Ailus wrote:\n> > In V4L2 the practice is to have the KernelDoc documentation in the header\n> > and not in .c source code files. This consequientally makes the V4L2\n> \n> consequientally: spelling?\n> \n> > fwnode function documentation part of the Media documentation build.\n> > \n> > Also correct the link related function and argument naming in\n> > documentation.\n> > \n> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>\n> > Reviewed-by: Niklas SÃ¶derlund <niklas.soderlund+renesas@ragnatech.se>\n> \n> Something funny going on with utf-8 here.\n> \n> Acked-by: Pavel Machek <pavel@ucw.cz>\n\nThanks! Will fix for v9.\n\nI had bad commit message encoding in .gitconfig, that's now fixed.","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","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=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmqTy0kMZz9t2m\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 00:53:42 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750858AbdIEOxk (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tTue, 5 Sep 2017 10:53:40 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:42050 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751345AbdIEOxk (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Tue, 5 Sep 2017 10:53:40 -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 2D242600E7;\n\tTue,  5 Sep 2017 17:53:38 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakke@valkosipuli.retiisi.org.uk>)\n\tid 1dpFEP-0002Y5-NF; Tue, 05 Sep 2017 17:53:37 +0300"],"Date":"Tue, 5 Sep 2017 17:53:37 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Pavel Machek <pavel@ucw.cz>","Cc":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org,\n\tsre@kernel.org","Subject":"Re: [PATCH v7 01/18] v4l: fwnode: Move KernelDoc documentation to\n\tthe header","Message-ID":"<20170905145337.h6qy42k3hv6ha73y@valkosipuli.retiisi.org.uk>","References":"<20170903174958.27058-1-sakari.ailus@linux.intel.com>\n\t<20170903174958.27058-2-sakari.ailus@linux.intel.com>\n\t<20170904154940.GA19484@amd>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20170904154940.GA19484@amd>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}}]