[{"id":1759301,"web_url":"http://patchwork.ozlabs.org/comment/1759301/","msgid":"<20170829125041.GE12099@bigcity.dyn.berto.se>","list_archive_url":null,"date":"2017-08-29T12:50:41","subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","submitter":{"id":67773,"url":"http://patchwork.ozlabs.org/api/people/67773/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Sakari,\n\nThanks for your patch.\n\nOn 2017-08-29 14:03:09 +0300, 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> 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 \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 75 --------------------------------\n> include/media/v4l2-fwnode.h | 81 ++++++++++++++++++++++++++++++++++-\n> 2 files changed, 80 insertions(+), 76 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index 40b2fbfe8865..706f9e7b90f1 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -181,25 +181,6 @@ v4l2_fwnode_endpoint_parse_csi1_bus(struct fwnode_handle *fwnode,\n> \t\tvep->bus_type = V4L2_MBUS_CSI1;\n> }\n> \n> -/**\n> - * v4l2_fwnode_endpoint_parse() - parse all fwnode node properties\n> - * @fwnode: pointer to the endpoint's fwnode handle\n> - * @vep: pointer to the V4L2 fwnode data structure\n> - *\n> - * All properties are optional. If none are found, we don't set any flags. This\n> - * means the port has a static configuration and no properties have to be\n> - * specified explicitly. If any properties that identify the bus as parallel\n> - * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if\n> - * we recognise the bus as serial CSI-2 and clock-noncontinuous isn't set, we\n> - * set the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a\n> - * reference to @fwnode.\n> - *\n> - * NOTE: This function does not parse properties the size of which is variable\n> - * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in\n> - * new drivers instead.\n> - *\n> - * Return: 0 on success or a negative error code on failure.\n> - */\n> int v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,\n> \t\t\t struct v4l2_fwnode_endpoint *vep)\n> {\n> @@ -239,14 +220,6 @@ int v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,\n> }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_parse);\n> \n> -/*\n> - * v4l2_fwnode_endpoint_free() - free the V4L2 fwnode acquired by\n> - * v4l2_fwnode_endpoint_alloc_parse()\n> - * @vep - the V4L2 fwnode the resources of which are to be released\n> - *\n> - * It is safe to call this function with NULL argument or on a V4L2 fwnode the\n> - * parsing of which failed.\n> - */\n> void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep)\n> {\n> \tif (IS_ERR_OR_NULL(vep))\n> @@ -257,29 +230,6 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep)\n> }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_free);\n> \n> -/**\n> - * v4l2_fwnode_endpoint_alloc_parse() - parse all fwnode node properties\n> - * @fwnode: pointer to the endpoint's fwnode handle\n> - *\n> - * All properties are optional. If none are found, we don't set any flags. This\n> - * means the port has a static configuration and no properties have to be\n> - * specified explicitly. If any properties that identify the bus as parallel\n> - * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if\n> - * we recognise the bus as serial CSI-2 and clock-noncontinuous isn't set, we\n> - * set the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a\n> - * reference to @fwnode.\n> - *\n> - * v4l2_fwnode_endpoint_alloc_parse() has two important differences to\n> - * v4l2_fwnode_endpoint_parse():\n> - *\n> - * 1. It also parses variable size data.\n> - *\n> - * 2. The memory it has allocated to store the variable size data must be freed\n> - * using v4l2_fwnode_endpoint_free() when no longer needed.\n> - *\n> - * Return: Pointer to v4l2_fwnode_endpoint if successful, on an error pointer\n> - * on error.\n> - */\n> struct v4l2_fwnode_endpoint *v4l2_fwnode_endpoint_alloc_parse(\n> \tstruct fwnode_handle *fwnode)\n> {\n> @@ -322,24 +272,6 @@ struct v4l2_fwnode_endpoint *v4l2_fwnode_endpoint_alloc_parse(\n> }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_alloc_parse);\n> \n> -/**\n> - * v4l2_fwnode_endpoint_parse_link() - parse a link between two endpoints\n> - * @__fwnode: pointer to the endpoint's fwnode at the local end of the link\n> - * @link: pointer to the V4L2 fwnode link data structure\n> - *\n> - * Fill the link structure with the local and remote nodes and port numbers.\n> - * The local_node and remote_node fields are set to point to the local and\n> - * remote port's parent nodes respectively (the port parent node being the\n> - * parent node of the port node if that node isn't a 'ports' node, or the\n> - * grand-parent node of the port node otherwise).\n> - *\n> - * A reference is taken to both the local and remote nodes, the caller must use\n> - * v4l2_fwnode_endpoint_put_link() to drop the references when done with the\n> - * link.\n> - *\n> - * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode can't be\n> - * found.\n> - */\n> int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,\n> \t\t\t struct v4l2_fwnode_link *link)\n> {\n> @@ -374,13 +306,6 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,\n> }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_parse_link);\n> \n> -/**\n> - * v4l2_fwnode_put_link() - drop references to nodes in a link\n> - * @link: pointer to the V4L2 fwnode link data structure\n> - *\n> - * Drop references to the local and remote nodes in the link. This function\n> - * must be called on every link parsed with v4l2_fwnode_parse_link().\n> - */\n> void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)\n> {\n> \tfwnode_handle_put(link->local_node);\n> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> index 7adec9851d9e..68eb22ba571b 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -113,13 +113,92 @@ struct v4l2_fwnode_link {\n> \tunsigned int remote_port;\n> };\n> \n> +/**\n> + * v4l2_fwnode_endpoint_parse() - parse all fwnode node properties\n> + * @fwnode: pointer to the endpoint's fwnode handle\n> + * @vep: pointer to the V4L2 fwnode data structure\n> + *\n> + * All properties are optional. If none are found, we don't set any flags. This\n> + * means the port has a static configuration and no properties have to be\n> + * specified explicitly. If any properties that identify the bus as parallel\n> + * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if\n> + * we recognise the bus as serial CSI-2 and clock-noncontinuous isn't set, we\n> + * set the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a\n> + * reference to @fwnode.\n> + *\n> + * NOTE: This function does not parse properties the size of which is variable\n> + * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in\n> + * new drivers instead.\n> + *\n> + * Return: 0 on success or a negative error code on failure.\n> + */\n> int v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,\n> \t\t\t struct v4l2_fwnode_endpoint *vep);\n> +\n> +/*\n> + * v4l2_fwnode_endpoint_free() - free the V4L2 fwnode acquired by\n> + * v4l2_fwnode_endpoint_alloc_parse()\n> + * @vep - the V4L2 fwnode the resources of which are to be released\n> + *\n> + * It is safe to call this function with NULL argument or on a V4L2 fwnode the\n> + * parsing of which failed.\n> + */\n> +void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);\n> +\n> +/**\n> + * v4l2_fwnode_endpoint_alloc_parse() - parse all fwnode node properties\n> + * @fwnode: pointer to the endpoint's fwnode handle\n> + *\n> + * All properties are optional. If none are found, we don't set any flags. This\n> + * means the port has a static configuration and no properties have to be\n> + * specified explicitly. If any properties that identify the bus as parallel\n> + * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if\n> + * we recognise the bus as serial CSI-2 and clock-noncontinuous isn't set, we\n> + * set the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a\n> + * reference to @fwnode.\n> + *\n> + * v4l2_fwnode_endpoint_alloc_parse() has two important differences to\n> + * v4l2_fwnode_endpoint_parse():\n> + *\n> + * 1. It also parses variable size data.\n> + *\n> + * 2. The memory it has allocated to store the variable size data must be freed\n> + * using v4l2_fwnode_endpoint_free() when no longer needed.\n> + *\n> + * Return: Pointer to v4l2_fwnode_endpoint if successful, on an error pointer\n> + * on error.\n> + */\n> struct v4l2_fwnode_endpoint *v4l2_fwnode_endpoint_alloc_parse(\n> \tstruct fwnode_handle *fwnode);\n> -void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);\n> +\n> +/**\n> + * v4l2_fwnode_parse_link() - parse a link between two endpoints\n> + * @fwnode: pointer to the endpoint's fwnode at the local end of the link\n> + * @link: pointer to the V4L2 fwnode link data structure\n> + *\n> + * Fill the link structure with the local and remote nodes and port numbers.\n> + * The local_node and remote_node fields are set to point to the local and\n> + * remote port's parent nodes respectively (the port parent node being the\n> + * parent node of the port node if that node isn't a 'ports' node, or the\n> + * grand-parent node of the port node otherwise).\n> + *\n> + * A reference is taken to both the local and remote nodes, the caller must use\n> + * v4l2_fwnode_put_link() to drop the references when done with the\n> + * link.\n> + *\n> + * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode can't be\n> + * found.\n> + */\n> int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,\n> \t\t\t struct v4l2_fwnode_link *link);\n> +\n> +/**\n> + * v4l2_fwnode_put_link() - drop references to nodes in a link\n> + * @link: pointer to the V4L2 fwnode link data structure\n> + *\n> + * Drop references to the local and remote nodes in the link. This function\n> + * must be called on every link parsed with v4l2_fwnode_parse_link().\n> + */\n> void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);\n> \n> #endif /* _V4L2_FWNODE_H */\n> -- \n> 2.11.0\n>","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"zHKzlpU8\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhT6N5WdLz9t3J\n\tfor ;\n\tTue, 29 Aug 2017 22:50:47 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752455AbdH2Muq (ORCPT\n\t);\n\tTue, 29 Aug 2017 08:50:46 -0400","from mail-lf0-f42.google.com ([209.85.215.42]:33793 \"EHLO\n\tmail-lf0-f42.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752350AbdH2Mup (ORCPT\n\t); Tue, 29 Aug 2017 08:50:45 -0400","by mail-lf0-f42.google.com with SMTP id d17so12953199lfe.1\n\tfor ;\n\tTue, 29 Aug 2017 05:50:44 -0700 (PDT)","from localhost ([89.233.230.99]) by smtp.gmail.com with ESMTPSA id\n\ts139sm509266lfs.18.2017.08.29.05.50.41\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 29 Aug 2017 05:50:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=mQMm1WagVEv43Di2tzN5TlWPtjGnIrEMPgx2ufnGiGg=;\n\tb=zHKzlpU8df9OHuZVfDOpjKTjEQ5n1HaJE0KfHX9c0AftGnmWYzUXosIEF0UyC32GcB\n\tSUUYi65HVXdk9NTg4Kfl0GXpQruc+uYiBxWACjgmcVSosHFS3svyR7YbCuWEIoAHNx/m\n\t8uGCVC5ecUsVZPeqNsE5jLBnW4eCKHQC/li67toHQdI1nubWzYiBk7nsHutfi0jMoPKE\n\tGqRRfSVU5UpJAcxw64ormnD7j3gqnUzrb/LcimlkUiRlyEIaVO0XBPDN6gph7M2Ma1V6\n\twX8eaYoR312+tebC3w5C37CEFuC7a+jiqjiyJSQDiE+xAkVUYTKf1+cUBAntEpgL1Fa7\n\twi5w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=mQMm1WagVEv43Di2tzN5TlWPtjGnIrEMPgx2ufnGiGg=;\n\tb=sQfREShmeqjIUZpMCbYn76a+YibWKw977ue0tl7nC/vV/F8ash1Snf+Jau6n/lIMJ2\n\t7XtNkXvs/+fIL6a7pX62DSYzvr7Jw/73cIIA3cEzen6VDJaPxRKSUhV6PZKXIR4hEgvB\n\tbNrvlEKGjnqZ3m9WhgMYg3xzKYZ2l3YkIgbExc2g/TE9SGE3AzVGefg1z2AFwFzk0HwJ\n\tUl1sONa5+JXToki+RlpE87Kubw4QZidLoAZw7kT/KayIjIGv2WhUOk4ca6SJU1WSNAdt\n\t6tFOQ5UHFwPJ5V6qEarQrGQ0116B210a+VLkYKDgpOMEaRaX+zPVF2irffD32nhC3bqF\n\tFhMQ==","X-Gm-Message-State":"AHYfb5hhAhc2zw/QRlLvGbvT7jwyDq5Tr7tOFmgiiyQkpbtygIjViWgS\n\t97GV+QTNaA6DaWdo","X-Received":"by 10.46.2.150 with SMTP id y22mr83660lje.153.1504011043341;\n\tTue, 29 Aug 2017 05:50:43 -0700 (PDT)","Date":"Tue, 29 Aug 2017 14:50:41 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, robh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","Message-ID":"<20170829125041.GE12099@bigcity.dyn.berto.se>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-2-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20170829110313.19538-2-sakari.ailus@linux.intel.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759304,"web_url":"http://patchwork.ozlabs.org/comment/1759304/","msgid":"<20170829125235.GF12099@bigcity.dyn.berto.se>","list_archive_url":null,"date":"2017-08-29T12:52:35","subject":"Re: [PATCH v5 2/5] v4l: async: Add V4L2 async documentation to the\n\tdocumentation build","submitter":{"id":67773,"url":"http://patchwork.ozlabs.org/api/people/67773/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Sakari,\n\nThanks for your patch.\n\nOn 2017-08-29 14:03:10 +0300, Sakari Ailus wrote:\n> The V4L2 async wasn't part of the documentation build. Fix this.\n> \n> Signed-off-by: Sakari Ailus \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> Documentation/media/kapi/v4l2-async.rst | 3 +++\n> Documentation/media/kapi/v4l2-core.rst | 1 +\n> 2 files changed, 4 insertions(+)\n> create mode 100644 Documentation/media/kapi/v4l2-async.rst\n> \n> diff --git a/Documentation/media/kapi/v4l2-async.rst b/Documentation/media/kapi/v4l2-async.rst\n> new file mode 100644\n> index 000000000000..523ff9eb09a0\n> --- /dev/null\n> +++ b/Documentation/media/kapi/v4l2-async.rst\n> @@ -0,0 +1,3 @@\n> +V4L2 async kAPI\n> +^^^^^^^^^^^^^^^\n> +.. kernel-doc:: include/media/v4l2-async.h\n> diff --git a/Documentation/media/kapi/v4l2-core.rst b/Documentation/media/kapi/v4l2-core.rst\n> index c7434f38fd9c..5cf292037a48 100644\n> --- a/Documentation/media/kapi/v4l2-core.rst\n> +++ b/Documentation/media/kapi/v4l2-core.rst\n> @@ -19,6 +19,7 @@ Video4Linux devices\n> v4l2-mc\n> v4l2-mediabus\n> v4l2-mem2mem\n> + v4l2-async\n> v4l2-fwnode\n> v4l2-rect\n> v4l2-tuner\n> -- \n> 2.11.0\n>","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"mT66zRej\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhTB12QNlz9t3J\n\tfor ;\n\tTue, 29 Aug 2017 22:54:47 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751700AbdH2Mwj (ORCPT\n\t);\n\tTue, 29 Aug 2017 08:52:39 -0400","from mail-lf0-f41.google.com ([209.85.215.41]:36892 \"EHLO\n\tmail-lf0-f41.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751496AbdH2Mwi (ORCPT\n\t); Tue, 29 Aug 2017 08:52:38 -0400","by mail-lf0-f41.google.com with SMTP id y128so7314255lfd.4\n\tfor ;\n\tTue, 29 Aug 2017 05:52:37 -0700 (PDT)","from localhost ([89.233.230.99]) by smtp.gmail.com with ESMTPSA id\n\tv63sm510535lfg.78.2017.08.29.05.52.35\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 29 Aug 2017 05:52:36 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=yVFuIHx2YThreeHMrTU4M2n6JpfAExwtJEnB2xJPgD8=;\n\tb=mT66zRejR1jfMn/4QOTHtLsV6xA2Un0fc6xMOElBzzx7iTpGQ1M5WWcwqfFDVst3CO\n\tQ8urh/SN171AfspSmHyNWCwvfyrzqjw20qmm/gOYLiQJETM0lOmL0dRMiauQzcLWtGci\n\tBDM2XoB3GUoPT8BLXT5F4q0xE1tzxPNPfoRJMoXoWkQCsVi5jX3VynIMTYG7bpg8QkYr\n\tbKgEiSlzvSC+t1/Jginh9prjuQH+vL+g690rVyM0OgZSJePY+AyknyO4Zq3mpxj+Ku8X\n\t1eSjkYZRpn3ogTgW7yHrUampSCnW8u3NKsKeMqlE8w/8uM47Ijjk8y6sS98jWvH3BUK7\n\tSp1A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=yVFuIHx2YThreeHMrTU4M2n6JpfAExwtJEnB2xJPgD8=;\n\tb=f1yi/IPNr4lIFTUOiCTq61rKvNR3OOoZYc2KU5NEqktqv0sOSFEHhahSZj2rX7IK0j\n\t2lialU1jNsVqRemOrkzJ2uUD59wTk21oYJ3BY/e6YarO5vRGzsgaiWtRHaAuRmRlsqdp\n\tm6ZQEbKv8/STZJTUPVt007QRsnGJ8qGtCMjq3D8qdgIGw9L90xlOHtP9ma2piHmcc615\n\tsWO7+2W6WaHGyv2t1vWe+wcddV4GkxLIKy4Fp+YZ4p836/PngNM08+ZfBY6laEdcdtVx\n\tjHQgGN0hibTomdjhD0wstJEHTRhahWk3F862qDHadEBK6DGTEGguOvYyCc9d1EsWdY1u\n\tye6A==","X-Gm-Message-State":"AHYfb5jKnCq/pTlimboePsFw3RoYB/cyLGYS7/kyzyBxlUh/zuacLCXG\n\twarcK+4O5Gvayv1l","X-Received":"by 10.25.28.139 with SMTP id c133mr53285lfc.112.1504011157024;\n\tTue, 29 Aug 2017 05:52:37 -0700 (PDT)","Date":"Tue, 29 Aug 2017 14:52:35 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, robh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 2/5] v4l: async: Add V4L2 async documentation to the\n\tdocumentation build","Message-ID":"<20170829125235.GF12099@bigcity.dyn.berto.se>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-3-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20170829110313.19538-3-sakari.ailus@linux.intel.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759305,"web_url":"http://patchwork.ozlabs.org/comment/1759305/","msgid":"<20170829125742.GG12099@bigcity.dyn.berto.se>","list_archive_url":null,"date":"2017-08-29T12:57:42","subject":"Re: [PATCH v5 5/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a single port","submitter":{"id":67773,"url":"http://patchwork.ozlabs.org/api/people/67773/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Sakari,\n\nThanks for your patch, I like how this turned out :-)\n\nOn 2017-08-29 14:03:13 +0300, Sakari Ailus wrote:\n> This is the preferred way to parse the endpoints.\n> \n> Comment rcar-vin as an example.\n> \n> Signed-off-by: Sakari Ailus \n\nAcked-by: Niklas Söderlund \n\n> ---\n> drivers/media/platform/rcar-vin/rcar-core.c | 111 ++++++++--------------------\n> drivers/media/platform/rcar-vin/rcar-dma.c | 10 +--\n> drivers/media/platform/rcar-vin/rcar-v4l2.c | 14 ++--\n> drivers/media/platform/rcar-vin/rcar-vin.h | 4 +-\n> drivers/media/v4l2-core/v4l2-fwnode.c | 47 ++++++++++++\n> include/media/v4l2-fwnode.h | 39 ++++++++++\n> 6 files changed, 132 insertions(+), 93 deletions(-)\n> \n> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c\n> index 142de447aaaa..4378806be1d4 100644\n> --- a/drivers/media/platform/rcar-vin/rcar-core.c\n> +++ b/drivers/media/platform/rcar-vin/rcar-core.c\n> @@ -21,6 +21,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \"rcar-vin.h\"\n> @@ -77,14 +78,14 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)\n> \tint ret;\n> \n> \t/* Verify subdevices mbus format */\n> -\tif (!rvin_mbus_supported(&vin->digital)) {\n> +\tif (!rvin_mbus_supported(vin->digital)) {\n> \t\tvin_err(vin, \"Unsupported media bus format for %s\\n\",\n> -\t\t\tvin->digital.subdev->name);\n> +\t\t\tvin->digital->subdev->name);\n> \t\treturn -EINVAL;\n> \t}\n> \n> \tvin_dbg(vin, \"Found media bus format for %s: %d\\n\",\n> -\t\tvin->digital.subdev->name, vin->digital.code);\n> +\t\tvin->digital->subdev->name, vin->digital->code);\n> \n> \tret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);\n> \tif (ret < 0) {\n> @@ -103,7 +104,7 @@ static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,\n> \n> \tvin_dbg(vin, \"unbind digital subdev %s\\n\", subdev->name);\n> \trvin_v4l2_remove(vin);\n> -\tvin->digital.subdev = NULL;\n> +\tvin->digital->subdev = NULL;\n> }\n> \n> static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,\n> @@ -120,117 +121,67 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,\n> \tret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);\n> \tif (ret < 0)\n> \t\treturn ret;\n> -\tvin->digital.source_pad = ret;\n> +\tvin->digital->source_pad = ret;\n> \n> \tret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);\n> -\tvin->digital.sink_pad = ret < 0 ? 0 : ret;\n> +\tvin->digital->sink_pad = ret < 0 ? 0 : ret;\n> \n> -\tvin->digital.subdev = subdev;\n> +\tvin->digital->subdev = subdev;\n> \n> \tvin_dbg(vin, \"bound subdev %s source pad: %u sink pad: %u\\n\",\n> -\t\tsubdev->name, vin->digital.source_pad,\n> -\t\tvin->digital.sink_pad);\n> +\t\tsubdev->name, vin->digital->source_pad,\n> +\t\tvin->digital->sink_pad);\n> \n> \treturn 0;\n> }\n> \n> -static int rvin_digitial_parse_v4l2(struct rvin_dev *vin,\n> -\t\t\t\t struct device_node *ep,\n> -\t\t\t\t struct v4l2_mbus_config *mbus_cfg)\n> +static int rvin_digital_parse_v4l2(struct device *dev,\n> +\t\t\t\t struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t\t struct v4l2_async_subdev *asd)\n> {\n> -\tstruct v4l2_fwnode_endpoint v4l2_ep;\n> -\tint ret;\n> -\n> -\tret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);\n> -\tif (ret) {\n> -\t\tvin_err(vin, \"Could not parse v4l2 endpoint\\n\");\n> -\t\treturn -EINVAL;\n> -\t}\n> +\tstruct rvin_dev *vin = dev_get_drvdata(dev);\n> +\tstruct rvin_graph_entity *rvge =\n> +\t\tcontainer_of(asd, struct rvin_graph_entity, asd);\n> \n> -\tmbus_cfg->type = v4l2_ep.bus_type;\n> +\trvge->mbus_cfg.type = vep->bus_type;\n> \n> -\tswitch (mbus_cfg->type) {\n> +\tswitch (rvge->mbus_cfg.type) {\n> \tcase V4L2_MBUS_PARALLEL:\n> \t\tvin_dbg(vin, \"Found PARALLEL media bus\\n\");\n> -\t\tmbus_cfg->flags = v4l2_ep.bus.parallel.flags;\n> +\t\trvge->mbus_cfg.flags = vep->bus.parallel.flags;\n> \t\tbreak;\n> \tcase V4L2_MBUS_BT656:\n> \t\tvin_dbg(vin, \"Found BT656 media bus\\n\");\n> -\t\tmbus_cfg->flags = 0;\n> +\t\trvge->mbus_cfg.flags = 0;\n> \t\tbreak;\n> \tdefault:\n> \t\tvin_err(vin, \"Unknown media bus type\\n\");\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\treturn 0;\n> -}\n> -\n> -static int rvin_digital_graph_parse(struct rvin_dev *vin)\n> -{\n> -\tstruct device_node *ep, *np;\n> -\tint ret;\n> -\n> -\tvin->digital.asd.match.fwnode.fwnode = NULL;\n> -\tvin->digital.subdev = NULL;\n> -\n> -\t/*\n> -\t * Port 0 id 0 is local digital input, try to get it.\n> -\t * Not all instances can or will have this, that is OK\n> -\t */\n> -\tep = of_graph_get_endpoint_by_regs(vin->dev->of_node, 0, 0);\n> -\tif (!ep)\n> -\t\treturn 0;\n> -\n> -\tnp = of_graph_get_remote_port_parent(ep);\n> -\tif (!np) {\n> -\t\tvin_err(vin, \"No remote parent for digital input\\n\");\n> -\t\tof_node_put(ep);\n> -\t\treturn -EINVAL;\n> -\t}\n> -\tof_node_put(np);\n> -\n> -\tret = rvin_digitial_parse_v4l2(vin, ep, &vin->digital.mbus_cfg);\n> -\tof_node_put(ep);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tvin->digital.asd.match.fwnode.fwnode = of_fwnode_handle(np);\n> -\tvin->digital.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;\n> +\tvin->digital = rvge;\n> \n> \treturn 0;\n> }\n> \n> static int rvin_digital_graph_init(struct rvin_dev *vin)\n> {\n> -\tstruct v4l2_async_subdev **subdevs = NULL;\n> \tint ret;\n> \n> -\tret = rvin_digital_graph_parse(vin);\n> +\tret = v4l2_async_notifier_parse_fwnode_endpoint(\n> +\t\tvin->dev, &vin->notifier, 0, 0,\n> +\t\tsizeof(struct rvin_graph_entity), rvin_digital_parse_v4l2);\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tif (!vin->digital.asd.match.fwnode.fwnode) {\n> -\t\tvin_dbg(vin, \"No digital subdevice found\\n\");\n> -\t\treturn -ENODEV;\n> -\t}\n> -\n> -\t/* Register the subdevices notifier. */\n> -\tsubdevs = devm_kzalloc(vin->dev, sizeof(*subdevs), GFP_KERNEL);\n> -\tif (subdevs == NULL)\n> -\t\treturn -ENOMEM;\n> +\tif (vin->notifier.num_subdevs > 0)\n> +\t\tvin_dbg(vin, \"Found digital subdevice %pOF\\n\",\n> +\t\t\tto_of_node(\n> +\t\t\t\tvin->notifier.subdevs[0]->match.fwnode.fwnode));\n> \n> -\tsubdevs[0] = &vin->digital.asd;\n> -\n> -\tvin_dbg(vin, \"Found digital subdevice %pOF\\n\",\n> -\t\tto_of_node(subdevs[0]->match.fwnode.fwnode));\n> -\n> -\tvin->notifier.num_subdevs = 1;\n> -\tvin->notifier.subdevs = subdevs;\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> -\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> @@ -290,6 +241,8 @@ static int rcar_vin_probe(struct platform_device *pdev)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tplatform_set_drvdata(pdev, vin);\n> +\n> \tret = rvin_digital_graph_init(vin);\n> \tif (ret < 0)\n> \t\tgoto error;\n> @@ -297,11 +250,10 @@ static int rcar_vin_probe(struct platform_device *pdev)\n> \tpm_suspend_ignore_children(&pdev->dev, true);\n> \tpm_runtime_enable(&pdev->dev);\n> \n> -\tplatform_set_drvdata(pdev, vin);\n> -\n> \treturn 0;\n> error:\n> \trvin_dma_remove(vin);\n> +\tv4l2_async_notifier_release(&vin->notifier);\n> \n> \treturn ret;\n> }\n> @@ -313,6 +265,7 @@ static int rcar_vin_remove(struct platform_device *pdev)\n> \tpm_runtime_disable(&pdev->dev);\n> \n> \tv4l2_async_notifier_unregister(&vin->notifier);\n> +\tv4l2_async_notifier_release(&vin->notifier);\n> \n> \trvin_dma_remove(vin);\n> \n> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c\n> index b136844499f6..23fdff7a7370 100644\n> --- a/drivers/media/platform/rcar-vin/rcar-dma.c\n> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c\n> @@ -183,7 +183,7 @@ static int rvin_setup(struct rvin_dev *vin)\n> \t/*\n> \t * Input interface\n> \t */\n> -\tswitch (vin->digital.code) {\n> +\tswitch (vin->digital->code) {\n> \tcase MEDIA_BUS_FMT_YUYV8_1X16:\n> \t\t/* BT.601/BT.1358 16bit YCbCr422 */\n> \t\tvnmc |= VNMC_INF_YUV16;\n> @@ -191,7 +191,7 @@ static int rvin_setup(struct rvin_dev *vin)\n> \t\tbreak;\n> \tcase MEDIA_BUS_FMT_UYVY8_2X8:\n> \t\t/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */\n> -\t\tvnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?\n> +\t\tvnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?\n> \t\t\tVNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;\n> \t\tinput_is_yuv = true;\n> \t\tbreak;\n> @@ -200,7 +200,7 @@ static int rvin_setup(struct rvin_dev *vin)\n> \t\tbreak;\n> \tcase MEDIA_BUS_FMT_UYVY10_2X10:\n> \t\t/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */\n> -\t\tvnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?\n> +\t\tvnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?\n> \t\t\tVNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;\n> \t\tinput_is_yuv = true;\n> \t\tbreak;\n> @@ -212,11 +212,11 @@ static int rvin_setup(struct rvin_dev *vin)\n> \tdmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);\n> \n> \t/* Hsync Signal Polarity Select */\n> -\tif (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))\n> +\tif (!(vin->digital->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))\n> \t\tdmr2 |= VNDMR2_HPS;\n> \n> \t/* Vsync Signal Polarity Select */\n> -\tif (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))\n> +\tif (!(vin->digital->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))\n> \t\tdmr2 |= VNDMR2_VPS;\n> \n> \t/*\n> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c\n> index dd37ea811680..b479b882da12 100644\n> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c\n> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c\n> @@ -111,7 +111,7 @@ static int rvin_reset_format(struct rvin_dev *vin)\n> \tstruct v4l2_mbus_framefmt *mf = &fmt.format;\n> \tint ret;\n> \n> -\tfmt.pad = vin->digital.source_pad;\n> +\tfmt.pad = vin->digital->source_pad;\n> \n> \tret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);\n> \tif (ret)\n> @@ -172,13 +172,13 @@ static int __rvin_try_format_source(struct rvin_dev *vin,\n> \n> \tsd = vin_to_source(vin);\n> \n> -\tv4l2_fill_mbus_format(&format.format, pix, vin->digital.code);\n> +\tv4l2_fill_mbus_format(&format.format, pix, vin->digital->code);\n> \n> \tpad_cfg = v4l2_subdev_alloc_pad_config(sd);\n> \tif (pad_cfg == NULL)\n> \t\treturn -ENOMEM;\n> \n> -\tformat.pad = vin->digital.source_pad;\n> +\tformat.pad = vin->digital->source_pad;\n> \n> \tfield = pix->field;\n> \n> @@ -555,7 +555,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,\n> \tif (timings->pad)\n> \t\treturn -EINVAL;\n> \n> -\ttimings->pad = vin->digital.sink_pad;\n> +\ttimings->pad = vin->digital->sink_pad;\n> \n> \tret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);\n> \n> @@ -607,7 +607,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,\n> \tif (cap->pad)\n> \t\treturn -EINVAL;\n> \n> -\tcap->pad = vin->digital.sink_pad;\n> +\tcap->pad = vin->digital->sink_pad;\n> \n> \tret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);\n> \n> @@ -625,7 +625,7 @@ static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)\n> \tif (edid->pad)\n> \t\treturn -EINVAL;\n> \n> -\tedid->pad = vin->digital.sink_pad;\n> +\tedid->pad = vin->digital->sink_pad;\n> \n> \tret = v4l2_subdev_call(sd, pad, get_edid, edid);\n> \n> @@ -643,7 +643,7 @@ static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)\n> \tif (edid->pad)\n> \t\treturn -EINVAL;\n> \n> -\tedid->pad = vin->digital.sink_pad;\n> +\tedid->pad = vin->digital->sink_pad;\n> \n> \tret = v4l2_subdev_call(sd, pad, set_edid, edid);\n> \n> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h\n> index 9bfb5a7c4dc4..5382078143fb 100644\n> --- a/drivers/media/platform/rcar-vin/rcar-vin.h\n> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h\n> @@ -126,7 +126,7 @@ struct rvin_dev {\n> \tstruct v4l2_device v4l2_dev;\n> \tstruct v4l2_ctrl_handler ctrl_handler;\n> \tstruct v4l2_async_notifier notifier;\n> -\tstruct rvin_graph_entity digital;\n> +\tstruct rvin_graph_entity *digital;\n> \n> \tstruct mutex lock;\n> \tstruct vb2_queue queue;\n> @@ -145,7 +145,7 @@ struct rvin_dev {\n> \tstruct v4l2_rect compose;\n> };\n> \n> -#define vin_to_source(vin)\t\tvin->digital.subdev\n> +#define vin_to_source(vin)\t\t((vin)->digital->subdev)\n> \n> /* Debug */\n> #define vin_dbg(d, fmt, arg...)\t\tdev_dbg(d->dev, fmt, ##arg)\n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c\n> index 39a587c6992a..e1a07916b9ca 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -445,6 +445,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> }\n> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);\n> \n> +int v4l2_async_notifier_parse_fwnode_endpoint(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tunsigned int port_id, unsigned int endpoint_id, size_t asd_struct_size,\n> +\tint (*parse_single)(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> +\tint ret;\n> +\n> +\twhile ((fwnode = fwnode_graph_get_next_endpoint(\n> +\t\t\tdev_fwnode(dev), fwnode))) {\n> +\t\tstruct fwnode_endpoint ep;\n> +\n> +\t\tret = fwnode_graph_parse_endpoint(fwnode, &ep);\n> +\t\tif (ret < 0)\n> +\t\t\tcontinue;\n> +\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 (ep.port == port_id && ep.id == endpoint_id)\n> +\t\t\tbreak;\n> +\t}\n> +\n> +\tif (!fwnode)\n> +\t\treturn -ENOENT;\n> +\n> +\tret = notifier_realloc(notifier, notifier->num_subdevs + 1);\n> +\tif (ret)\n> +\t\tgoto out_err;\n> +\n> +\tret = parse_endpoint(dev, notifier, fwnode, asd_struct_size,\n> +\t\t\t parse_single);\n> +\tif (ret)\n> +\t\tgoto out_err;\n> +\n> +\treturn 0;\n> +\n> +out_err:\n> +\tfwnode_handle_put(fwnode);\n> +\n> +\treturn ret;\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoint);\n> +\n> MODULE_LICENSE(\"GPL\");\n> MODULE_AUTHOR(\"Sakari Ailus \");\n> MODULE_AUTHOR(\"Sylwester Nawrocki \");\n> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> index 46521e8c8872..6bc7b02b2f46 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -240,4 +240,43 @@ int v4l2_async_notifier_parse_fwnode_endpoints(\n> \t\t\t struct v4l2_fwnode_endpoint *vep,\n> \t\t\t struct v4l2_async_subdev *asd));\n> \n> +/**\n> + * v4l2_async_notifier_parse_fwnode_endpoint - Set up async notifier for an\n> + *\t\t\t\t\t fwnode based sub-device\n> + * @dev: @struct device pointer\n> + * @notifier: pointer to &struct v4l2_async_notifier\n> + * @port_id: port number\n> + * @endpoint_id: endpoint number\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_single: driver's callback function called on each V4L2 fwnode endpoint\n> + *\n> + * Find an endpoint node for the given port and endpoint IDs and allocate an\n> + * async sub-device struct for it. Parse the V4L2 fwnode endpoint and call the\n> + * provided callback function.\n> + *\n> + * The function may not be called on a registered notifier.\n> + *\n> + * Once the user has called this function, the resources released by it need to\n> + * be released by callin v4l2_async_notifier_release after the notifier has been\n> + * unregistered and the sub-devices are no longer in use.\n> + *\n> + * A driver supporting fwnode (currently Devicetree and ACPI) should call this\n> + * function as part of its probe function before it registers the notifier.\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_single\n> + */\n> +int v4l2_async_notifier_parse_fwnode_endpoint(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tunsigned int port_id, unsigned int endpoint_id, size_t asd_struct_size,\n> +\tint (*parse_single)(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> 2.11.0\n>","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"KxwiYEuR\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhTFb0n4Fz9t42\n\tfor ;\n\tTue, 29 Aug 2017 22:57:54 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752082AbdH2M5r (ORCPT\n\t);\n\tTue, 29 Aug 2017 08:57:47 -0400","from mail-lf0-f48.google.com ([209.85.215.48]:35198 \"EHLO\n\tmail-lf0-f48.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752002AbdH2M5q (ORCPT\n\t); Tue, 29 Aug 2017 08:57:46 -0400","by mail-lf0-f48.google.com with SMTP id k186so13005659lfe.2\n\tfor ;\n\tTue, 29 Aug 2017 05:57:45 -0700 (PDT)","from localhost ([89.233.230.99]) by smtp.gmail.com with ESMTPSA id\n\tq124sm605962ljb.53.2017.08.29.05.57.43\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 29 Aug 2017 05:57:43 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=a0FiwjWABWZyjxdk+i1WeX6wOW1/mTeqmwfMB7PL/tU=;\n\tb=KxwiYEuRxa7FNPJxlWYUtgaLVswO5cg8I9ptLM6xmnuaso8Dmfw5NNKfKJAd7eaXxw\n\tYCUK6e5zQzgCLkGNo+MFAvfMukRuTLDDE34wN3sQq3q/TrqyVIReTY6fXkVsn38QCsge\n\tYGYgqRuqyv6Uei8XEzYEw4g7Bh2wWew8RRbk996s1xTjtL0vhbrWKE/2xKaeDmxrwWbM\n\tPJkw705klhUR1e0TpIxpE1ychbFpmkbX0k2BHZAPo6JymbT2F0Ig237P7y9zCUc5cnRj\n\t4t7As+ZJBxMws+K77JpBqiz18p/7MwevocF8i3y6qzzLRLv1SJKaAf4EIxK5JFDLUxcg\n\tTyjA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=a0FiwjWABWZyjxdk+i1WeX6wOW1/mTeqmwfMB7PL/tU=;\n\tb=TkGZ2dFGdnlIDQWSr11Udy9w7HFFzt+9ZL4L0aSc5JTBO+KdJIBjNHApvv3aZ5P+Pc\n\tPwmtKdZT0cVHI22oMlJkyVQ6E5sMZheXzOcW7n0vxh0bj0EZN23H8lSjzw7fiwsOeg3J\n\tN2oLGIZM1uCWBDVAqiqMy1GwppexcvlXEbuBPubd8Uiwb2vvBwfiGMChBkuzpq+GxuOl\n\tY873cAhT60mE3HbyDTlJCM3Uu9jAiTOQYFMMLPqLkValyjImmaAOxF1nExh2m16Rrphh\n\t+jZ3fm26gbnVkd67nrEjh6/PicgN5JKJwnJqvvC+xw2nFS7oGP/OUw74c8hr/1D2iQ3m\n\t1mFQ==","X-Gm-Message-State":"AHYfb5gbjdYy2oHGVAdU1D/RfIrsl7kKTCLXEoAy2VGmQ0uSFAEKIzl0\n\tcIknaJEoDSIAleok","X-Received":"by 10.25.31.136 with SMTP id f130mr38127lff.135.1504011464434;\n\tTue, 29 Aug 2017 05:57:44 -0700 (PDT)","Date":"Tue, 29 Aug 2017 14:57:42 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, robh@kernel.org, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 5/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a single port","Message-ID":"<20170829125742.GG12099@bigcity.dyn.berto.se>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-6-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20170829110313.19538-6-sakari.ailus@linux.intel.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759323,"web_url":"http://patchwork.ozlabs.org/comment/1759323/","msgid":"<2676284.rKR023OhTM@avalon>","list_archive_url":null,"date":"2017-08-29T13:15:22","subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","submitter":{"id":11034,"url":"http://patchwork.ozlabs.org/api/people/11034/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sakari,\n\nThank you for the patch.\n\nOn Tuesday, 29 August 2017 14:03:09 EEST 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> 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 \n\nI prefer documenting functions in the C file. Documentation in header files \nwill get out-of-sync with the implementation much more easily.\n\n> ---\n> drivers/media/v4l2-core/v4l2-fwnode.c | 75 --------------------------------\n> include/media/v4l2-fwnode.h | 81 ++++++++++++++++++++++++++++++++-\n> 2 files changed, 80 insertions(+), 76 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c\n> b/drivers/media/v4l2-core/v4l2-fwnode.c index 40b2fbfe8865..706f9e7b90f1\n> 100644\n> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n> @@ -181,25 +181,6 @@ v4l2_fwnode_endpoint_parse_csi1_bus(struct\n> fwnode_handle *fwnode, vep->bus_type = V4L2_MBUS_CSI1;\n> }\n> \n> -/**\n> - * v4l2_fwnode_endpoint_parse() - parse all fwnode node properties\n> - * @fwnode: pointer to the endpoint's fwnode handle\n> - * @vep: pointer to the V4L2 fwnode data structure\n> - *\n> - * All properties are optional. If none are found, we don't set any flags.\n> This - * means the port has a static configuration and no properties have\n> to be - * specified explicitly. If any properties that identify the bus as\n> parallel - * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER.\n> Similarly, if - * we recognise the bus as serial CSI-2 and\n> clock-noncontinuous isn't set, we - * set the\n> V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a - *\n> reference to @fwnode.\n> - *\n> - * NOTE: This function does not parse properties the size of which is\n> variable - * without a low fixed limit. Please use\n> v4l2_fwnode_endpoint_alloc_parse() in - * new drivers instead.\n> - *\n> - * Return: 0 on success or a negative error code on failure.\n> - */\n> int v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,\n> \t\t\t struct v4l2_fwnode_endpoint *vep)\n> {\n> @@ -239,14 +220,6 @@ int v4l2_fwnode_endpoint_parse(struct fwnode_handle\n> *fwnode, }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_parse);\n> \n> -/*\n> - * v4l2_fwnode_endpoint_free() - free the V4L2 fwnode acquired by\n> - * v4l2_fwnode_endpoint_alloc_parse()\n> - * @vep - the V4L2 fwnode the resources of which are to be released\n> - *\n> - * It is safe to call this function with NULL argument or on a V4L2 fwnode\n> the - * parsing of which failed.\n> - */\n> void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep)\n> {\n> \tif (IS_ERR_OR_NULL(vep))\n> @@ -257,29 +230,6 @@ void v4l2_fwnode_endpoint_free(struct\n> v4l2_fwnode_endpoint *vep) }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_free);\n> \n> -/**\n> - * v4l2_fwnode_endpoint_alloc_parse() - parse all fwnode node properties\n> - * @fwnode: pointer to the endpoint's fwnode handle\n> - *\n> - * All properties are optional. If none are found, we don't set any flags.\n> This - * means the port has a static configuration and no properties have\n> to be - * specified explicitly. If any properties that identify the bus as\n> parallel - * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER.\n> Similarly, if - * we recognise the bus as serial CSI-2 and\n> clock-noncontinuous isn't set, we - * set the\n> V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a - *\n> reference to @fwnode.\n> - *\n> - * v4l2_fwnode_endpoint_alloc_parse() has two important differences to\n> - * v4l2_fwnode_endpoint_parse():\n> - *\n> - * 1. It also parses variable size data.\n> - *\n> - * 2. The memory it has allocated to store the variable size data must be\n> freed - * using v4l2_fwnode_endpoint_free() when no longer needed.\n> - *\n> - * Return: Pointer to v4l2_fwnode_endpoint if successful, on an error\n> pointer - * on error.\n> - */\n> struct v4l2_fwnode_endpoint *v4l2_fwnode_endpoint_alloc_parse(\n> \tstruct fwnode_handle *fwnode)\n> {\n> @@ -322,24 +272,6 @@ struct v4l2_fwnode_endpoint\n> *v4l2_fwnode_endpoint_alloc_parse( }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_alloc_parse);\n> \n> -/**\n> - * v4l2_fwnode_endpoint_parse_link() - parse a link between two endpoints\n> - * @__fwnode: pointer to the endpoint's fwnode at the local end of the link\n> - * @link: pointer to the V4L2 fwnode link data structure\n> - *\n> - * Fill the link structure with the local and remote nodes and port\n> numbers. - * The local_node and remote_node fields are set to point to the\n> local and - * remote port's parent nodes respectively (the port parent node\n> being the - * parent node of the port node if that node isn't a 'ports'\n> node, or the - * grand-parent node of the port node otherwise).\n> - *\n> - * A reference is taken to both the local and remote nodes, the caller must\n> use - * v4l2_fwnode_endpoint_put_link() to drop the references when done\n> with the - * link.\n> - *\n> - * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode can't be\n> - * found.\n> - */\n> int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,\n> \t\t\t struct v4l2_fwnode_link *link)\n> {\n> @@ -374,13 +306,6 @@ int v4l2_fwnode_parse_link(struct fwnode_handle\n> *__fwnode, }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_parse_link);\n> \n> -/**\n> - * v4l2_fwnode_put_link() - drop references to nodes in a link\n> - * @link: pointer to the V4L2 fwnode link data structure\n> - *\n> - * Drop references to the local and remote nodes in the link. This function\n> - * must be called on every link parsed with v4l2_fwnode_parse_link(). - */\n> void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)\n> {\n> \tfwnode_handle_put(link->local_node);\n> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> index 7adec9851d9e..68eb22ba571b 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -113,13 +113,92 @@ struct v4l2_fwnode_link {\n> \tunsigned int remote_port;\n> };\n> \n> +/**\n> + * v4l2_fwnode_endpoint_parse() - parse all fwnode node properties\n> + * @fwnode: pointer to the endpoint's fwnode handle\n> + * @vep: pointer to the V4L2 fwnode data structure\n> + *\n> + * All properties are optional. If none are found, we don't set any flags.\n> This + * means the port has a static configuration and no properties have\n> to be + * specified explicitly. If any properties that identify the bus as\n> parallel + * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER.\n> Similarly, if + * we recognise the bus as serial CSI-2 and\n> clock-noncontinuous isn't set, we + * set the\n> V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a + *\n> reference to @fwnode.\n> + *\n> + * NOTE: This function does not parse properties the size of which is\n> variable + * without a low fixed limit. Please use\n> v4l2_fwnode_endpoint_alloc_parse() in + * new drivers instead.\n> + *\n> + * Return: 0 on success or a negative error code on failure.\n> + */\n> int v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,\n> \t\t\t struct v4l2_fwnode_endpoint *vep);\n> +\n> +/*\n> + * v4l2_fwnode_endpoint_free() - free the V4L2 fwnode acquired by\n> + * v4l2_fwnode_endpoint_alloc_parse()\n> + * @vep - the V4L2 fwnode the resources of which are to be released\n> + *\n> + * It is safe to call this function with NULL argument or on a V4L2 fwnode\n> the + * parsing of which failed.\n> + */\n> +void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);\n> +\n> +/**\n> + * v4l2_fwnode_endpoint_alloc_parse() - parse all fwnode node properties\n> + * @fwnode: pointer to the endpoint's fwnode handle\n> + *\n> + * All properties are optional. If none are found, we don't set any flags.\n> This + * means the port has a static configuration and no properties have\n> to be + * specified explicitly. If any properties that identify the bus as\n> parallel + * are found and slave-mode isn't set, we set V4L2_MBUS_MASTER.\n> Similarly, if + * we recognise the bus as serial CSI-2 and\n> clock-noncontinuous isn't set, we + * set the\n> V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag. The caller should hold a + *\n> reference to @fwnode.\n> + *\n> + * v4l2_fwnode_endpoint_alloc_parse() has two important differences to\n> + * v4l2_fwnode_endpoint_parse():\n> + *\n> + * 1. It also parses variable size data.\n> + *\n> + * 2. The memory it has allocated to store the variable size data must be\n> freed + * using v4l2_fwnode_endpoint_free() when no longer needed.\n> + *\n> + * Return: Pointer to v4l2_fwnode_endpoint if successful, on an error\n> pointer + * on error.\n> + */\n> struct v4l2_fwnode_endpoint *v4l2_fwnode_endpoint_alloc_parse(\n> \tstruct fwnode_handle *fwnode);\n> -void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);\n> +\n> +/**\n> + * v4l2_fwnode_parse_link() - parse a link between two endpoints\n> + * @fwnode: pointer to the endpoint's fwnode at the local end of the link\n> + * @link: pointer to the V4L2 fwnode link data structure\n> + *\n> + * Fill the link structure with the local and remote nodes and port\n> numbers. + * The local_node and remote_node fields are set to point to the\n> local and + * remote port's parent nodes respectively (the port parent node\n> being the + * parent node of the port node if that node isn't a 'ports'\n> node, or the + * grand-parent node of the port node otherwise).\n> + *\n> + * A reference is taken to both the local and remote nodes, the caller must\n> use + * v4l2_fwnode_put_link() to drop the references when done with the +\n> * link.\n> + *\n> + * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode can't be\n> + * found.\n> + */\n> int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,\n> \t\t\t struct v4l2_fwnode_link *link);\n> +\n> +/**\n> + * v4l2_fwnode_put_link() - drop references to nodes in a link\n> + * @link: pointer to the V4L2 fwnode link data structure\n> + *\n> + * Drop references to the local and remote nodes in the link. This function\n> + * must be called on every link parsed with v4l2_fwnode_parse_link(). + */\n> void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);\n> \n> #endif /* _V4L2_FWNODE_H */","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kRiaanyE\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhTdB3C1Zz9t1t\n\tfor ;\n\tTue, 29 Aug 2017 23:14:54 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753248AbdH2NOu (ORCPT\n\t);\n\tTue, 29 Aug 2017 09:14:50 -0400","from galahad.ideasonboard.com ([185.26.127.97]:57301 \"EHLO\n\tgalahad.ideasonboard.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752611AbdH2NOt (ORCPT\n\t); Tue, 29 Aug 2017 09:14:49 -0400","from avalon.localnet (unknown [145.15.244.38])\n\tby galahad.ideasonboard.com (Postfix) with ESMTPSA id 69425201C5;\n\tTue, 29 Aug 2017 15:13:04 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1504012384;\n\tbh=HtgFm6TuRIyNeYOe327XvbC0nPb3SpBCoEfJCY3mKlQ=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=kRiaanyE9vXQhXOd7Wi7K4tT12aArPGK9PxpTsekqn87H/Xzr2nL1i9NToHPyXPQH\n\tF7912nuhzGh79R6yQzKMDXiddZeEXckIRa6vJnSvPaui44s1RKza7V0T7g7ZVyJ1tx\n\t/P7o7m11WKeIqncIzGtNZ9T49AMayvXiurbujwco=","From":"Laurent Pinchart ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","Date":"Tue, 29 Aug 2017 16:15:22 +0300","Message-ID":"<2676284.rKR023OhTM@avalon>","In-Reply-To":"<20170829110313.19538-2-sakari.ailus@linux.intel.com>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-2-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759328,"web_url":"http://patchwork.ozlabs.org/comment/1759328/","msgid":"<20170829132010.nfofeofidvpipw4h@paasikivi.fi.intel.com>","list_archive_url":null,"date":"2017-08-29T13:20:10","subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","submitter":{"id":65485,"url":"http://patchwork.ozlabs.org/api/people/65485/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Laurent,\n\nOn Tue, Aug 29, 2017 at 04:15:22PM +0300, Laurent Pinchart wrote:\n> Hi Sakari,\n> \n> Thank you for the patch.\n> \n> On Tuesday, 29 August 2017 14:03:09 EEST 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> > 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 \n> \n> I prefer documenting functions in the C file. Documentation in header files \n> will get out-of-sync with the implementation much more easily.\n\nThe fact is that there's very little KernelDoc documentation left in the .c\nfiles in V4L2. This actually appears to be the only exception. And it seems\nto have been in the Sphinx build; I missed that earlier, so that part of\nthe commit message doesn't apply.","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhTm00cSGz9t2v\n\tfor ;\n\tTue, 29 Aug 2017 23:20:47 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753864AbdH2NUq (ORCPT\n\t);\n\tTue, 29 Aug 2017 09:20:46 -0400","from mga03.intel.com ([134.134.136.65]:57622 \"EHLO mga03.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753825AbdH2NUp (ORCPT );\n\tTue, 29 Aug 2017 09:20:45 -0400","from orsmga003.jf.intel.com ([10.7.209.27])\n\tby orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t29 Aug 2017 06:20:44 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby orsmga003.jf.intel.com with ESMTP; 29 Aug 2017 06:20:41 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid 6BF6C2058E; Tue, 29 Aug 2017 16:20:10 +0300 (EEST)"],"X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos; i=\"5.41,444,1498546800\"; d=\"scan'208\";\n\ta=\"1008835416\"","Date":"Tue, 29 Aug 2017 16:20:10 +0300","From":"Sakari Ailus ","To":"Laurent Pinchart ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","Message-ID":"<20170829132010.nfofeofidvpipw4h@paasikivi.fi.intel.com>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-2-sakari.ailus@linux.intel.com>\n\t<2676284.rKR023OhTM@avalon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<2676284.rKR023OhTM@avalon>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759331,"web_url":"http://patchwork.ozlabs.org/comment/1759331/","msgid":"<20170829132231.7shg3bq4cjt2e6lu@paasikivi.fi.intel.com>","list_archive_url":null,"date":"2017-08-29T13:22:32","subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","submitter":{"id":65485,"url":"http://patchwork.ozlabs.org/api/people/65485/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"On Tue, Aug 29, 2017 at 04:20:10PM +0300, Sakari Ailus wrote:\n> Hi Laurent,\n> \n> On Tue, Aug 29, 2017 at 04:15:22PM +0300, Laurent Pinchart wrote:\n> > Hi Sakari,\n> > \n> > Thank you for the patch.\n> > \n> > On Tuesday, 29 August 2017 14:03:09 EEST 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> > > 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 \n> > \n> > I prefer documenting functions in the C file. Documentation in header files \n> > will get out-of-sync with the implementation much more easily.\n> \n> The fact is that there's very little KernelDoc documentation left in the .c\n> files in V4L2. This actually appears to be the only exception. And it seems\n> to have been in the Sphinx build; I missed that earlier, so that part of\n> the commit message doesn't apply.\n\nOops. That was just a local change. So yes, the commit message is fine.","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhTp50NP7z9t2v\n\tfor ;\n\tTue, 29 Aug 2017 23:22:37 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752459AbdH2NWf (ORCPT\n\t);\n\tTue, 29 Aug 2017 09:22:35 -0400","from mga06.intel.com ([134.134.136.31]:59794 \"EHLO mga06.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752141AbdH2NWf (ORCPT );\n\tTue, 29 Aug 2017 09:22:35 -0400","from orsmga001.jf.intel.com ([10.7.209.18])\n\tby orsmga104.jf.intel.com with ESMTP; 29 Aug 2017 06:22:34 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby orsmga001.jf.intel.com with ESMTP; 29 Aug 2017 06:22:32 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid 1585F2058E; Tue, 29 Aug 2017 16:22:32 +0300 (EEST)"],"X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos; i=\"5.41,444,1498546800\"; d=\"scan'208\";\n\ta=\"1167227778\"","Date":"Tue, 29 Aug 2017 16:22:32 +0300","From":"Sakari Ailus ","To":"Laurent Pinchart ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 1/5] v4l: fwnode: Move KernelDoc documentation to the\n\theader","Message-ID":"<20170829132231.7shg3bq4cjt2e6lu@paasikivi.fi.intel.com>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-2-sakari.ailus@linux.intel.com>\n\t<2676284.rKR023OhTM@avalon>\n\t<20170829132010.nfofeofidvpipw4h@paasikivi.fi.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170829132010.nfofeofidvpipw4h@paasikivi.fi.intel.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759336,"web_url":"http://patchwork.ozlabs.org/comment/1759336/","msgid":"<5454183.OQTL44fLZD@avalon>","list_archive_url":null,"date":"2017-08-29T13:32:18","subject":"Re: [PATCH v5 3/5] docs-rst: v4l: Include Qualcomm CAMSS in\n\tdocumentation build","submitter":{"id":11034,"url":"http://patchwork.ozlabs.org/api/people/11034/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sakari,\n\nThank you for the patch.\n\nOn Tuesday, 29 August 2017 14:03:11 EEST Sakari Ailus wrote:\n> Qualcomm CAMSS was left out from documentation build. Fix this.\n> \n> Signed-off-by: Sakari Ailus \n\nI've tested the qcom_camss documentation build and haven't noticed any new \nwarning or error, so\n\nReviewed-by: Laurent Pinchart \n\n> ---\n> Documentation/media/v4l-drivers/index.rst | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/Documentation/media/v4l-drivers/index.rst\n> b/Documentation/media/v4l-drivers/index.rst index\n> 10f2ce42ece2..5c202e23616b 100644\n> --- a/Documentation/media/v4l-drivers/index.rst\n> +++ b/Documentation/media/v4l-drivers/index.rst\n> @@ -50,6 +50,7 @@ For more details see the file COPYING in the source\n> distribution of Linux. philips\n> \tpvrusb2\n> \tpxa_camera\n> +\tqcom_camss\n> \tradiotrack\n> \trcar-fdp1\n> \tsaa7134","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DjCLKrm/\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhV0f277vz9t33\n\tfor ;\n\tTue, 29 Aug 2017 23:31:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752945AbdH2Nbo (ORCPT\n\t);\n\tTue, 29 Aug 2017 09:31:44 -0400","from galahad.ideasonboard.com ([185.26.127.97]:57384 \"EHLO\n\tgalahad.ideasonboard.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752693AbdH2Nbo (ORCPT\n\t); Tue, 29 Aug 2017 09:31:44 -0400","from avalon.localnet (unknown [145.15.244.38])\n\tby galahad.ideasonboard.com (Postfix) with ESMTPSA id CD762201C5;\n\tTue, 29 Aug 2017 15:29:59 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1504013399;\n\tbh=P9at13Wmw0AotJYE86kwKYNs9E20P7z7NdypMI3QrtM=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=DjCLKrm/ftzUamNEuGCW8G0OHIXNnxO8trlCUNAePjkI9sEPj9RW1yVeVWb8suEdu\n\tkyS7MMyEnYb2a8znZACRQrTJ9MKZwtzt6Gvam5qoEn6AK6/u3v1nQtSzh4NsnmPAmJ\n\tDULnCcR9BXagCl+7py6Rsv0XRfnFFvaA5G3vp4Sk=","From":"Laurent Pinchart ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 3/5] docs-rst: v4l: Include Qualcomm CAMSS in\n\tdocumentation build","Date":"Tue, 29 Aug 2017 16:32:18 +0300","Message-ID":"<5454183.OQTL44fLZD@avalon>","In-Reply-To":"<20170829110313.19538-4-sakari.ailus@linux.intel.com>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-4-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759367,"web_url":"http://patchwork.ozlabs.org/comment/1759367/","msgid":"<2739432.dQ1BSg1MPy@avalon>","list_archive_url":null,"date":"2017-08-29T14:02:54","subject":"Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","submitter":{"id":11034,"url":"http://patchwork.ozlabs.org/api/people/11034/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sakari,\n\nThank you for the patch.\n\nOn Tuesday, 29 August 2017 14:03:12 EEST 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> Convert the omap3isp as an example.\n> \n> Signed-off-by: Sakari Ailus \n> ---\n> drivers/media/platform/omap3isp/isp.c | 115 ++++++++++-------------------\n> drivers/media/platform/omap3isp/isp.h | 5 +-\n> drivers/media/v4l2-core/v4l2-async.c | 16 +++++\n> drivers/media/v4l2-core/v4l2-fwnode.c | 132\n> ++++++++++++++++++++++++++++++++++ include/media/v4l2-async.h | \n> 20 +++++-\n> include/media/v4l2-fwnode.h | 39 ++++++++++\n> 6 files changed, 242 insertions(+), 85 deletions(-)\n> \n> diff --git a/drivers/media/platform/omap3isp/isp.c\n> b/drivers/media/platform/omap3isp/isp.c index 1a428fe9f070..a546cf774d40\n> 100644\n> --- a/drivers/media/platform/omap3isp/isp.c\n> +++ b/drivers/media/platform/omap3isp/isp.c\n> @@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)\n> \t__omap3isp_put(isp, false);\n> \n> \tmedia_entity_enum_cleanup(&isp->crashed);\n> +\tv4l2_async_notifier_release(&isp->notifier);\n> \n> \treturn 0;\n> }\n> @@ -2011,44 +2012,41 @@ enum isp_of_phy {\n> \tISP_OF_PHY_CSIPHY2,\n> };\n> \n> -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle\n> *fwnode, -\t\t\t struct isp_async_subdev *isd)\n> +static int isp_fwnode_parse(struct device *dev,\n> +\t\t\t struct v4l2_fwnode_endpoint *vep,\n> +\t\t\t struct v4l2_async_subdev *asd)\n> {\n> +\tstruct isp_async_subdev *isd =\n> +\t\tcontainer_of(asd, struct isp_async_subdev, asd);\n> \tstruct isp_bus_cfg *buscfg = &isd->bus;\n> -\tstruct v4l2_fwnode_endpoint vep;\n> -\tunsigned int i;\n> -\tint ret;\n> \tbool csi1 = false;\n> -\n> -\tret = v4l2_fwnode_endpoint_parse(fwnode, &vep);\n> -\tif (ret)\n> -\t\treturn ret;\n> +\tunsigned int i;\n> \n> \tdev_dbg(dev, \"parsing endpoint %pOF, interface %u\\n\",\n> -\t\tto_of_node(fwnode), vep.base.port);\n> +\t\tto_of_node(vep->base.local_fwnode), vep->base.port);\n> \n> -\tswitch (vep.base.port) {\n> +\tswitch (vep->base.port) {\n> \tcase ISP_OF_PHY_PARALLEL:\n> \t\tbuscfg->interface = ISP_INTERFACE_PARALLEL;\n> \t\tbuscfg->bus.parallel.data_lane_shift =\n> -\t\t\tvep.bus.parallel.data_shift;\n> +\t\t\tvep->bus.parallel.data_shift;\n> \t\tbuscfg->bus.parallel.clk_pol =\n> -\t\t\t!!(vep.bus.parallel.flags\n> +\t\t\t!!(vep->bus.parallel.flags\n> \t\t\t & V4L2_MBUS_PCLK_SAMPLE_FALLING);\n> \t\tbuscfg->bus.parallel.hs_pol =\n> -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);\n> +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);\n> \t\tbuscfg->bus.parallel.vs_pol =\n> -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);\n> +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);\n> \t\tbuscfg->bus.parallel.fld_pol =\n> -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);\n> +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);\n> \t\tbuscfg->bus.parallel.data_pol =\n> -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);\n> -\t\tbuscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;\n> +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);\n> +\t\tbuscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;\n> \t\tbreak;\n> \n> \tcase ISP_OF_PHY_CSIPHY1:\n> \tcase ISP_OF_PHY_CSIPHY2:\n> -\t\tswitch (vep.bus_type) {\n> +\t\tswitch (vep->bus_type) {\n> \t\tcase V4L2_MBUS_CCP2:\n> \t\tcase V4L2_MBUS_CSI1:\n> \t\t\tdev_dbg(dev, \"CSI-1/CCP-2 configuration\\n\");\n> @@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev,\n> struct fwnode_handle *fwnode, break;\n> \t\tdefault:\n> \t\t\tdev_err(dev, \"unsupported bus type %u\\n\",\n> -\t\t\t\tvep.bus_type);\n> +\t\t\t\tvep->bus_type);\n> \t\t\treturn -EINVAL;\n> \t\t}\n> \n> -\t\tswitch (vep.base.port) {\n> +\t\tswitch (vep->base.port) {\n> \t\tcase ISP_OF_PHY_CSIPHY1:\n> \t\t\tif (csi1)\n> \t\t\t\tbuscfg->interface = ISP_INTERFACE_CCP2B_PHY1;\n> @@ -2080,47 +2078,47 @@ static int isp_fwnode_parse(struct device *dev,\n> struct fwnode_handle *fwnode, }\n> \t\tif (csi1) {\n> \t\t\tbuscfg->bus.ccp2.lanecfg.clk.pos =\n> -\t\t\t\tvep.bus.mipi_csi1.clock_lane;\n> +\t\t\t\tvep->bus.mipi_csi1.clock_lane;\n> \t\t\tbuscfg->bus.ccp2.lanecfg.clk.pol =\n> -\t\t\t\tvep.bus.mipi_csi1.lane_polarity[0];\n> +\t\t\t\tvep->bus.mipi_csi1.lane_polarity[0];\n> \t\t\tdev_dbg(dev, \"clock lane polarity %u, pos %u\\n\",\n> \t\t\t\tbuscfg->bus.ccp2.lanecfg.clk.pol,\n> \t\t\t\tbuscfg->bus.ccp2.lanecfg.clk.pos);\n> \n> \t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pos =\n> -\t\t\t\tvep.bus.mipi_csi1.data_lane;\n> +\t\t\t\tvep->bus.mipi_csi1.data_lane;\n> \t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pol =\n> -\t\t\t\tvep.bus.mipi_csi1.lane_polarity[1];\n> +\t\t\t\tvep->bus.mipi_csi1.lane_polarity[1];\n> \n> \t\t\tdev_dbg(dev, \"data lane polarity %u, pos %u\\n\",\n> \t\t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pol,\n> \t\t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pos);\n> \n> \t\t\tbuscfg->bus.ccp2.strobe_clk_pol =\n> -\t\t\t\tvep.bus.mipi_csi1.clock_inv;\n> -\t\t\tbuscfg->bus.ccp2.phy_layer = vep.bus.mipi_csi1.strobe;\n> +\t\t\t\tvep->bus.mipi_csi1.clock_inv;\n> +\t\t\tbuscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;\n> \t\t\tbuscfg->bus.ccp2.ccp2_mode =\n> -\t\t\t\tvep.bus_type == V4L2_MBUS_CCP2;\n> +\t\t\t\tvep->bus_type == V4L2_MBUS_CCP2;\n> \t\t\tbuscfg->bus.ccp2.vp_clk_pol = 1;\n> \n> \t\t\tbuscfg->bus.ccp2.crc = 1;\n> \t\t} else {\n> \t\t\tbuscfg->bus.csi2.lanecfg.clk.pos =\n> -\t\t\t\tvep.bus.mipi_csi2.clock_lane;\n> +\t\t\t\tvep->bus.mipi_csi2.clock_lane;\n> \t\t\tbuscfg->bus.csi2.lanecfg.clk.pol =\n> -\t\t\t\tvep.bus.mipi_csi2.lane_polarities[0];\n> +\t\t\t\tvep->bus.mipi_csi2.lane_polarities[0];\n> \t\t\tdev_dbg(dev, \"clock lane polarity %u, pos %u\\n\",\n> \t\t\t\tbuscfg->bus.csi2.lanecfg.clk.pol,\n> \t\t\t\tbuscfg->bus.csi2.lanecfg.clk.pos);\n> \n> \t\t\tbuscfg->bus.csi2.num_data_lanes =\n> -\t\t\t\tvep.bus.mipi_csi2.num_data_lanes;\n> +\t\t\t\tvep->bus.mipi_csi2.num_data_lanes;\n> \n> \t\t\tfor (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {\n> \t\t\t\tbuscfg->bus.csi2.lanecfg.data[i].pos =\n> -\t\t\t\t\tvep.bus.mipi_csi2.data_lanes[i];\n> +\t\t\t\t\tvep->bus.mipi_csi2.data_lanes[i];\n> \t\t\t\tbuscfg->bus.csi2.lanecfg.data[i].pol =\n> -\t\t\t\t\tvep.bus.mipi_csi2.lane_polarities[i + 1];\n> +\t\t\t\t\tvep->bus.mipi_csi2.lane_polarities[i + 1];\n> \t\t\t\tdev_dbg(dev,\n> \t\t\t\t\t\"data lane %u polarity %u, pos %u\\n\", i,\n> \t\t\t\t\tbuscfg->bus.csi2.lanecfg.data[i].pol,\n> @@ -2137,57 +2135,13 @@ static int isp_fwnode_parse(struct device *dev,\n> struct fwnode_handle *fwnode,\n> \n> \tdefault:\n> \t\tdev_warn(dev, \"%pOF: invalid interface %u\\n\",\n> -\t\t\t to_of_node(fwnode), vep.base.port);\n> +\t\t\t to_of_node(vep->base.local_fwnode), vep->base.port);\n> \t\treturn -EINVAL;\n> \t}\n> \n> \treturn 0;\n> }\n> \n> -static int isp_fwnodes_parse(struct device *dev,\n> -\t\t\t struct v4l2_async_notifier *notifier)\n> -{\n> -\tstruct fwnode_handle *fwnode = NULL;\n> -\n> -\tnotifier->subdevs = devm_kcalloc(\n> -\t\tdev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);\n> -\tif (!notifier->subdevs)\n> -\t\treturn -ENOMEM;\n> -\n> -\twhile (notifier->num_subdevs < ISP_MAX_SUBDEVS &&\n> -\t (fwnode = fwnode_graph_get_next_endpoint(\n> -\t\t\tof_fwnode_handle(dev->of_node), fwnode))) {\n> -\t\tstruct isp_async_subdev *isd;\n> -\n> -\t\tisd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);\n> -\t\tif (!isd)\n> -\t\t\tgoto error;\n> -\n> -\t\tif (isp_fwnode_parse(dev, fwnode, isd)) {\n> -\t\t\tdevm_kfree(dev, isd);\n> -\t\t\tcontinue;\n> -\t\t}\n> -\n> -\t\tnotifier->subdevs[notifier->num_subdevs] = &isd->asd;\n> -\n> -\t\tisd->asd.match.fwnode.fwnode =\n> -\t\t\tfwnode_graph_get_remote_port_parent(fwnode);\n> -\t\tif (!isd->asd.match.fwnode.fwnode) {\n> -\t\t\tdev_warn(dev, \"bad remote port parent\\n\");\n> -\t\t\tgoto error;\n> -\t\t}\n> -\n> -\t\tisd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;\n> -\t\tnotifier->num_subdevs++;\n> -\t}\n> -\n> -\treturn notifier->num_subdevs;\n> -\n> -error:\n> -\tfwnode_handle_put(fwnode);\n> -\treturn -EINVAL;\n> -}\n> -\n> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)\n> {\n> \tstruct isp_device *isp = container_of(async, struct isp_device,\n> @@ -2256,7 +2210,9 @@ static int isp_probe(struct platform_device *pdev)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tret = isp_fwnodes_parse(&pdev->dev, &isp->notifier);\n> +\tret = v4l2_async_notifier_parse_fwnode_endpoints(\n> +\t\t&pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),\n> +\t\tisp_fwnode_parse);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> @@ -2407,6 +2363,7 @@ static int isp_probe(struct platform_device *pdev)\n> \t__omap3isp_put(isp, false);\n> error:\n> \tmutex_destroy(&isp->isp_mutex);\n> +\tv4l2_async_notifier_release(&isp->notifier);\n> \n> \treturn ret;\n> }\n> diff --git a/drivers/media/platform/omap3isp/isp.h\n> b/drivers/media/platform/omap3isp/isp.h index e528df6efc09..8b9043db94b3\n> 100644\n> --- a/drivers/media/platform/omap3isp/isp.h\n> +++ b/drivers/media/platform/omap3isp/isp.h\n> @@ -220,14 +220,11 @@ struct isp_device {\n> \n> \tunsigned int sbl_resources;\n> \tunsigned int subclk_resources;\n> -\n> -#define ISP_MAX_SUBDEVS\t\t8\n> -\tstruct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];\n> };\n> \n> struct isp_async_subdev {\n> -\tstruct isp_bus_cfg bus;\n> \tstruct v4l2_async_subdev asd;\n> +\tstruct isp_bus_cfg bus;\n> };\n> \n> #define v4l2_subdev_to_bus_cfg(sd) \\\n> diff --git a/drivers/media/v4l2-core/v4l2-async.c\n> b/drivers/media/v4l2-core/v4l2-async.c index 851f128eba22..c490acf5ae82\n> 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 \n> #include \n> +#include \n> #include \n> \n> static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev\n> *asd) @@ -278,6 +279,21 @@ void v4l2_async_notifier_unregister(struct\n> v4l2_async_notifier *notifier) }\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> +\tif (!notifier->max_subdevs)\n> +\t\treturn;\n> +\n> +\tfor (i = 0; i < notifier->num_subdevs; i++)\n> +\t\tkfree(notifier->subdevs[i]);\n> +\n> +\tkvfree(notifier->subdevs);\n> +\tnotifier->max_subdevs = 0;\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\n> b/drivers/media/v4l2-core/v4l2-fwnode.c index 706f9e7b90f1..39a587c6992a\n> 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 \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -26,6 +27,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> enum v4l2_fwnode_bus_type {\n> @@ -313,6 +315,136 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link\n> *link) }\n> EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);\n> \n> +static int notifier_realloc(struct v4l2_async_notifier *notifier,\n> +\t\t\t unsigned int max_subdevs)\n\nI'd prefix static functions with v4l2_async_ to avoid namespace clashes.\n\n> +{\n> +\tstruct v4l2_async_subdev **subdevs;\n> +\tunsigned int i;\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\nShould it be mentioned in the documentation that the address of the subdevs \narray will change during parsing and should not be stored by drivers ? It \nmight be overkill.\n\n> +\tif (!subdevs)\n> +\t\treturn -ENOMEM;\n> +\n> +\tif (notifier->subdevs) {\n> +\t\tfor (i = 0; i < notifier->num_subdevs; i++)\n> +\t\t\tsubdevs[i] = notifier->subdevs[i];\n\nTo answer your previous question, yes, I would find\n\n\tmemcpy(subdevs, notifier->subdevs, sizeof(*subdevs) * num_subdevs);\n\neasier to read :-)\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 parse_endpoint(\n> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> +\tstruct fwnode_handle *endpoint, unsigned int asd_struct_size,\n> +\tint (*parse_single)(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\nYou don't ignore them anymore, the comment should be updated.\n\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_single(dev, vep, asd);\n> +\tv4l2_fwnode_endpoint_free(vep);\n> +\tif (ret) {\n> +\t\tdev_warn(dev, \"driver could not parse endpoint (%d)\\n\", ret);\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_single)(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 notifier->v4l2_dev)\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 = 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> +\t\tret = parse_endpoint(dev, notifier, fwnode, asd_struct_size,\n> +\t\t\t\t parse_single);\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 \");\n> MODULE_AUTHOR(\"Sylwester Nawrocki \");\n> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> index c69d8c8a66d0..4a44ab47ab04 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> @@ -78,7 +77,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 subdevs array\n> + * @max_subdevs: number of subdevices allocated in 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 +90,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 +122,21 @@ int v4l2_async_notifier_register(struct v4l2_device\n> *v4l2_dev, void v4l2_async_notifier_unregister(struct v4l2_async_notifier\n> *notifier);\n> \n> /**\n> + * v4l2_async_notifier_release - release notifier resources\n> + * @notifier: pointer to &struct v4l2_async_notifier\n\nThat's quite obvious given the type of the argument. It would be much more \nuseful to tell which notifier pointer this function expects (although in this \ncase it should be obvious too): \"(pointer to )?the notifier whose resources \nwill be released\".\n\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\nZeroing the memory is pretty much a requirement, as \nv4l2_async_notifier_parse_fwnode_endpoints() won't operate correctly if memory \ncontains random data anyway. Maybe we should introduce \nv4l2_async_notifier_init() and make v4l2_async_notifier_release() mandatory, \nbut that's out of scope for this patch.\n\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..46521e8c8872 100644\n> --- a/include/media/v4l2-fwnode.h\n> +++ b/include/media/v4l2-fwnode.h\n> @@ -25,6 +25,8 @@\n> #include \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,41 @@ int v4l2_fwnode_parse_link(struct fwnode_handle\n> *fwnode, */\n> void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);\n> \n> +/**\n> + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints\n> in a\n> + *\t\t\t\t\t\tdevice node\n> + * @dev: @struct device pointer\n\nSimilarly to my previous comment (and my comments to v3), you should tell \nwhich device the function expects.\n\n> + * @notifier: pointer to &struct v4l2_async_notifier\n> + * @asd_struct_size: size of the driver's async sub-device struct,\n> 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\nShould this be documented in the kerneldoc of the v4l2_async_subdev structure \n?\n\n> + * @parse_single: driver's callback function called on each V4L2 fwnode\n> endpoint\n> + *\n> + * Allocate async sub-device array and sub-devices for each fwnode\n> endpoint,\n> + * parse the related fwnode endpoints and finally call driver's callback\n> + * function to that V4L2 fwnode endpoint.\n\nI'd document this from the notifier point of view.\n\n\"Parse the fwnode endpoints of the @dev device and populate the async sub-\ndevices array of the notifier. The @parse_endpoint callback function is called \nfor each endpoint with the corresponding async sub-device pointer to let the \ncaller initialize the driver-specific part of the async sub-device structure.\"\n\n> + * The function may not be called on a registered notifier.\n\nYou should mention that the function may be called multiple times on an \nunregistered notifier.\n\n\"The function can be called multiple times to populate the same notifier from \nendpoints of different @dev devices before registering the notifier. It can't \nbe called anymore once the notifier has been registered.\"\n\n> + *\n> + * Once the user has called this function, the resources released by it\n> need to\n> + * be released by callin v4l2_async_notifier_release after the notifier has\n> been\n> + * unregistered and the sub-devices are no longer in use.\n\n\"Any notifier populated using this function must be released with a call to \nv4l2_async_notifier_release() after it has been unregistered and the async \nsub-devices are no longer in use.\"\n\n> + *\n> + * A driver supporting fwnode (currently Devicetree and ACPI) should call\n> this\n> + * function as part of its probe function before it registers the notifier.\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_single\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_single)(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 */","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"aAdpK+r5\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhVh22l7cz9t38\n\tfor ;\n\tWed, 30 Aug 2017 00:02:26 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754126AbdH2OCY (ORCPT\n\t);\n\tTue, 29 Aug 2017 10:02:24 -0400","from galahad.ideasonboard.com ([185.26.127.97]:57494 \"EHLO\n\tgalahad.ideasonboard.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754100AbdH2OCV (ORCPT\n\t); Tue, 29 Aug 2017 10:02:21 -0400","from avalon.localnet (unknown [145.15.244.38])\n\tby galahad.ideasonboard.com (Postfix) with ESMTPSA id 5C825208A5;\n\tTue, 29 Aug 2017 16:00:36 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1504015236;\n\tbh=+Gwerx+rrNIvNpvEUeIRlfRCqt6ggEyIpxNQ7/FSz7Y=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=aAdpK+r5n1fY+FiMmqukTS5vSym+yqSKRVkxTWeeXQkK0GHWj9ooFKXgrAjHV+V1T\n\tNDFNTmRdLYi3yL1V4Fx0t5EAAdjlVvf6oSJeSsuw0ZrpVze4ih8SnbGPet3Z3j5DtO\n\tPge6uzIGnG3Ns1u/IG48EO0wl0SRMDZcJ+AV8OtM=","From":"Laurent Pinchart ","To":"Sakari Ailus ","Cc":"linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","Date":"Tue, 29 Aug 2017 17:02:54 +0300","Message-ID":"<2739432.dQ1BSg1MPy@avalon>","In-Reply-To":"<20170829110313.19538-5-sakari.ailus@linux.intel.com>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-5-sakari.ailus@linux.intel.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1759393,"web_url":"http://patchwork.ozlabs.org/comment/1759393/","msgid":"<20170829143121.6sjdx3lgcoxm6mva@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-08-29T14:31:21","subject":"Re: [PATCH v5 4/5] 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 Laurent,\n\nOn Tue, Aug 29, 2017 at 05:02:54PM +0300, Laurent Pinchart wrote:\n> Hi Sakari,\n> \n> Thank you for the patch.\n\nThanks for the review!\n\n> \n> On Tuesday, 29 August 2017 14:03:12 EEST 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> > Convert the omap3isp as an example.\n> > \n> > Signed-off-by: Sakari Ailus \n> > ---\n> > drivers/media/platform/omap3isp/isp.c | 115 ++++++++++-------------------\n> > drivers/media/platform/omap3isp/isp.h | 5 +-\n> > drivers/media/v4l2-core/v4l2-async.c | 16 +++++\n> > drivers/media/v4l2-core/v4l2-fwnode.c | 132\n> > ++++++++++++++++++++++++++++++++++ include/media/v4l2-async.h | \n> > 20 +++++-\n> > include/media/v4l2-fwnode.h | 39 ++++++++++\n> > 6 files changed, 242 insertions(+), 85 deletions(-)\n> > \n> > diff --git a/drivers/media/platform/omap3isp/isp.c\n> > b/drivers/media/platform/omap3isp/isp.c index 1a428fe9f070..a546cf774d40\n> > 100644\n> > --- a/drivers/media/platform/omap3isp/isp.c\n> > +++ b/drivers/media/platform/omap3isp/isp.c\n> > @@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)\n> > \t__omap3isp_put(isp, false);\n> > \n> > \tmedia_entity_enum_cleanup(&isp->crashed);\n> > +\tv4l2_async_notifier_release(&isp->notifier);\n> > \n> > \treturn 0;\n> > }\n> > @@ -2011,44 +2012,41 @@ enum isp_of_phy {\n> > \tISP_OF_PHY_CSIPHY2,\n> > };\n> > \n> > -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle\n> > *fwnode, -\t\t\t struct isp_async_subdev *isd)\n> > +static int isp_fwnode_parse(struct device *dev,\n> > +\t\t\t struct v4l2_fwnode_endpoint *vep,\n> > +\t\t\t struct v4l2_async_subdev *asd)\n> > {\n> > +\tstruct isp_async_subdev *isd =\n> > +\t\tcontainer_of(asd, struct isp_async_subdev, asd);\n> > \tstruct isp_bus_cfg *buscfg = &isd->bus;\n> > -\tstruct v4l2_fwnode_endpoint vep;\n> > -\tunsigned int i;\n> > -\tint ret;\n> > \tbool csi1 = false;\n> > -\n> > -\tret = v4l2_fwnode_endpoint_parse(fwnode, &vep);\n> > -\tif (ret)\n> > -\t\treturn ret;\n> > +\tunsigned int i;\n> > \n> > \tdev_dbg(dev, \"parsing endpoint %pOF, interface %u\\n\",\n> > -\t\tto_of_node(fwnode), vep.base.port);\n> > +\t\tto_of_node(vep->base.local_fwnode), vep->base.port);\n> > \n> > -\tswitch (vep.base.port) {\n> > +\tswitch (vep->base.port) {\n> > \tcase ISP_OF_PHY_PARALLEL:\n> > \t\tbuscfg->interface = ISP_INTERFACE_PARALLEL;\n> > \t\tbuscfg->bus.parallel.data_lane_shift =\n> > -\t\t\tvep.bus.parallel.data_shift;\n> > +\t\t\tvep->bus.parallel.data_shift;\n> > \t\tbuscfg->bus.parallel.clk_pol =\n> > -\t\t\t!!(vep.bus.parallel.flags\n> > +\t\t\t!!(vep->bus.parallel.flags\n> > \t\t\t & V4L2_MBUS_PCLK_SAMPLE_FALLING);\n> > \t\tbuscfg->bus.parallel.hs_pol =\n> > -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);\n> > +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);\n> > \t\tbuscfg->bus.parallel.vs_pol =\n> > -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);\n> > +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);\n> > \t\tbuscfg->bus.parallel.fld_pol =\n> > -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);\n> > +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);\n> > \t\tbuscfg->bus.parallel.data_pol =\n> > -\t\t\t!!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);\n> > -\t\tbuscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;\n> > +\t\t\t!!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);\n> > +\t\tbuscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;\n> > \t\tbreak;\n> > \n> > \tcase ISP_OF_PHY_CSIPHY1:\n> > \tcase ISP_OF_PHY_CSIPHY2:\n> > -\t\tswitch (vep.bus_type) {\n> > +\t\tswitch (vep->bus_type) {\n> > \t\tcase V4L2_MBUS_CCP2:\n> > \t\tcase V4L2_MBUS_CSI1:\n> > \t\t\tdev_dbg(dev, \"CSI-1/CCP-2 configuration\\n\");\n> > @@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev,\n> > struct fwnode_handle *fwnode, break;\n> > \t\tdefault:\n> > \t\t\tdev_err(dev, \"unsupported bus type %u\\n\",\n> > -\t\t\t\tvep.bus_type);\n> > +\t\t\t\tvep->bus_type);\n> > \t\t\treturn -EINVAL;\n> > \t\t}\n> > \n> > -\t\tswitch (vep.base.port) {\n> > +\t\tswitch (vep->base.port) {\n> > \t\tcase ISP_OF_PHY_CSIPHY1:\n> > \t\t\tif (csi1)\n> > \t\t\t\tbuscfg->interface = ISP_INTERFACE_CCP2B_PHY1;\n> > @@ -2080,47 +2078,47 @@ static int isp_fwnode_parse(struct device *dev,\n> > struct fwnode_handle *fwnode, }\n> > \t\tif (csi1) {\n> > \t\t\tbuscfg->bus.ccp2.lanecfg.clk.pos =\n> > -\t\t\t\tvep.bus.mipi_csi1.clock_lane;\n> > +\t\t\t\tvep->bus.mipi_csi1.clock_lane;\n> > \t\t\tbuscfg->bus.ccp2.lanecfg.clk.pol =\n> > -\t\t\t\tvep.bus.mipi_csi1.lane_polarity[0];\n> > +\t\t\t\tvep->bus.mipi_csi1.lane_polarity[0];\n> > \t\t\tdev_dbg(dev, \"clock lane polarity %u, pos %u\\n\",\n> > \t\t\t\tbuscfg->bus.ccp2.lanecfg.clk.pol,\n> > \t\t\t\tbuscfg->bus.ccp2.lanecfg.clk.pos);\n> > \n> > \t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pos =\n> > -\t\t\t\tvep.bus.mipi_csi1.data_lane;\n> > +\t\t\t\tvep->bus.mipi_csi1.data_lane;\n> > \t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pol =\n> > -\t\t\t\tvep.bus.mipi_csi1.lane_polarity[1];\n> > +\t\t\t\tvep->bus.mipi_csi1.lane_polarity[1];\n> > \n> > \t\t\tdev_dbg(dev, \"data lane polarity %u, pos %u\\n\",\n> > \t\t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pol,\n> > \t\t\t\tbuscfg->bus.ccp2.lanecfg.data[0].pos);\n> > \n> > \t\t\tbuscfg->bus.ccp2.strobe_clk_pol =\n> > -\t\t\t\tvep.bus.mipi_csi1.clock_inv;\n> > -\t\t\tbuscfg->bus.ccp2.phy_layer = vep.bus.mipi_csi1.strobe;\n> > +\t\t\t\tvep->bus.mipi_csi1.clock_inv;\n> > +\t\t\tbuscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;\n> > \t\t\tbuscfg->bus.ccp2.ccp2_mode =\n> > -\t\t\t\tvep.bus_type == V4L2_MBUS_CCP2;\n> > +\t\t\t\tvep->bus_type == V4L2_MBUS_CCP2;\n> > \t\t\tbuscfg->bus.ccp2.vp_clk_pol = 1;\n> > \n> > \t\t\tbuscfg->bus.ccp2.crc = 1;\n> > \t\t} else {\n> > \t\t\tbuscfg->bus.csi2.lanecfg.clk.pos =\n> > -\t\t\t\tvep.bus.mipi_csi2.clock_lane;\n> > +\t\t\t\tvep->bus.mipi_csi2.clock_lane;\n> > \t\t\tbuscfg->bus.csi2.lanecfg.clk.pol =\n> > -\t\t\t\tvep.bus.mipi_csi2.lane_polarities[0];\n> > +\t\t\t\tvep->bus.mipi_csi2.lane_polarities[0];\n> > \t\t\tdev_dbg(dev, \"clock lane polarity %u, pos %u\\n\",\n> > \t\t\t\tbuscfg->bus.csi2.lanecfg.clk.pol,\n> > \t\t\t\tbuscfg->bus.csi2.lanecfg.clk.pos);\n> > \n> > \t\t\tbuscfg->bus.csi2.num_data_lanes =\n> > -\t\t\t\tvep.bus.mipi_csi2.num_data_lanes;\n> > +\t\t\t\tvep->bus.mipi_csi2.num_data_lanes;\n> > \n> > \t\t\tfor (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {\n> > \t\t\t\tbuscfg->bus.csi2.lanecfg.data[i].pos =\n> > -\t\t\t\t\tvep.bus.mipi_csi2.data_lanes[i];\n> > +\t\t\t\t\tvep->bus.mipi_csi2.data_lanes[i];\n> > \t\t\t\tbuscfg->bus.csi2.lanecfg.data[i].pol =\n> > -\t\t\t\t\tvep.bus.mipi_csi2.lane_polarities[i + 1];\n> > +\t\t\t\t\tvep->bus.mipi_csi2.lane_polarities[i + 1];\n> > \t\t\t\tdev_dbg(dev,\n> > \t\t\t\t\t\"data lane %u polarity %u, pos %u\\n\", i,\n> > \t\t\t\t\tbuscfg->bus.csi2.lanecfg.data[i].pol,\n> > @@ -2137,57 +2135,13 @@ static int isp_fwnode_parse(struct device *dev,\n> > struct fwnode_handle *fwnode,\n> > \n> > \tdefault:\n> > \t\tdev_warn(dev, \"%pOF: invalid interface %u\\n\",\n> > -\t\t\t to_of_node(fwnode), vep.base.port);\n> > +\t\t\t to_of_node(vep->base.local_fwnode), vep->base.port);\n> > \t\treturn -EINVAL;\n> > \t}\n> > \n> > \treturn 0;\n> > }\n> > \n> > -static int isp_fwnodes_parse(struct device *dev,\n> > -\t\t\t struct v4l2_async_notifier *notifier)\n> > -{\n> > -\tstruct fwnode_handle *fwnode = NULL;\n> > -\n> > -\tnotifier->subdevs = devm_kcalloc(\n> > -\t\tdev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);\n> > -\tif (!notifier->subdevs)\n> > -\t\treturn -ENOMEM;\n> > -\n> > -\twhile (notifier->num_subdevs < ISP_MAX_SUBDEVS &&\n> > -\t (fwnode = fwnode_graph_get_next_endpoint(\n> > -\t\t\tof_fwnode_handle(dev->of_node), fwnode))) {\n> > -\t\tstruct isp_async_subdev *isd;\n> > -\n> > -\t\tisd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);\n> > -\t\tif (!isd)\n> > -\t\t\tgoto error;\n> > -\n> > -\t\tif (isp_fwnode_parse(dev, fwnode, isd)) {\n> > -\t\t\tdevm_kfree(dev, isd);\n> > -\t\t\tcontinue;\n> > -\t\t}\n> > -\n> > -\t\tnotifier->subdevs[notifier->num_subdevs] = &isd->asd;\n> > -\n> > -\t\tisd->asd.match.fwnode.fwnode =\n> > -\t\t\tfwnode_graph_get_remote_port_parent(fwnode);\n> > -\t\tif (!isd->asd.match.fwnode.fwnode) {\n> > -\t\t\tdev_warn(dev, \"bad remote port parent\\n\");\n> > -\t\t\tgoto error;\n> > -\t\t}\n> > -\n> > -\t\tisd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;\n> > -\t\tnotifier->num_subdevs++;\n> > -\t}\n> > -\n> > -\treturn notifier->num_subdevs;\n> > -\n> > -error:\n> > -\tfwnode_handle_put(fwnode);\n> > -\treturn -EINVAL;\n> > -}\n> > -\n> > static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)\n> > {\n> > \tstruct isp_device *isp = container_of(async, struct isp_device,\n> > @@ -2256,7 +2210,9 @@ static int isp_probe(struct platform_device *pdev)\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > -\tret = isp_fwnodes_parse(&pdev->dev, &isp->notifier);\n> > +\tret = v4l2_async_notifier_parse_fwnode_endpoints(\n> > +\t\t&pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),\n> > +\t\tisp_fwnode_parse);\n> > \tif (ret < 0)\n> > \t\treturn ret;\n> > \n> > @@ -2407,6 +2363,7 @@ static int isp_probe(struct platform_device *pdev)\n> > \t__omap3isp_put(isp, false);\n> > error:\n> > \tmutex_destroy(&isp->isp_mutex);\n> > +\tv4l2_async_notifier_release(&isp->notifier);\n> > \n> > \treturn ret;\n> > }\n> > diff --git a/drivers/media/platform/omap3isp/isp.h\n> > b/drivers/media/platform/omap3isp/isp.h index e528df6efc09..8b9043db94b3\n> > 100644\n> > --- a/drivers/media/platform/omap3isp/isp.h\n> > +++ b/drivers/media/platform/omap3isp/isp.h\n> > @@ -220,14 +220,11 @@ struct isp_device {\n> > \n> > \tunsigned int sbl_resources;\n> > \tunsigned int subclk_resources;\n> > -\n> > -#define ISP_MAX_SUBDEVS\t\t8\n> > -\tstruct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];\n> > };\n> > \n> > struct isp_async_subdev {\n> > -\tstruct isp_bus_cfg bus;\n> > \tstruct v4l2_async_subdev asd;\n> > +\tstruct isp_bus_cfg bus;\n> > };\n> > \n> > #define v4l2_subdev_to_bus_cfg(sd) \\\n> > diff --git a/drivers/media/v4l2-core/v4l2-async.c\n> > b/drivers/media/v4l2-core/v4l2-async.c index 851f128eba22..c490acf5ae82\n> > 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 \n> > #include \n> > +#include \n> > #include \n> > \n> > static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev\n> > *asd) @@ -278,6 +279,21 @@ void v4l2_async_notifier_unregister(struct\n> > v4l2_async_notifier *notifier) }\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> > +\tif (!notifier->max_subdevs)\n> > +\t\treturn;\n> > +\n> > +\tfor (i = 0; i < notifier->num_subdevs; i++)\n> > +\t\tkfree(notifier->subdevs[i]);\n> > +\n> > +\tkvfree(notifier->subdevs);\n> > +\tnotifier->max_subdevs = 0;\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\n> > b/drivers/media/v4l2-core/v4l2-fwnode.c index 706f9e7b90f1..39a587c6992a\n> > 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 \n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -26,6 +27,7 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > #include \n> > \n> > enum v4l2_fwnode_bus_type {\n> > @@ -313,6 +315,136 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link\n> > *link) }\n> > EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);\n> > \n> > +static int notifier_realloc(struct v4l2_async_notifier *notifier,\n> > +\t\t\t unsigned int max_subdevs)\n> \n> I'd prefix static functions with v4l2_async_ to avoid namespace clashes.\n\nI can add that.\n\n> \n> > +{\n> > +\tstruct v4l2_async_subdev **subdevs;\n> > +\tunsigned int i;\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> \n> Should it be mentioned in the documentation that the address of the subdevs \n> array will change during parsing and should not be stored by drivers ? It \n> might be overkill.\n\nIt won't hurt. I'll add that.\n\n> \n> > +\tif (!subdevs)\n> > +\t\treturn -ENOMEM;\n> > +\n> > +\tif (notifier->subdevs) {\n> > +\t\tfor (i = 0; i < notifier->num_subdevs; i++)\n> > +\t\t\tsubdevs[i] = notifier->subdevs[i];\n> \n> To answer your previous question, yes, I would find\n> \n> \tmemcpy(subdevs, notifier->subdevs, sizeof(*subdevs) * num_subdevs);\n> \n> easier to read :-)\n\nI don't agree but I don't have a strong opinion about this, I can change\nit.\n\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 parse_endpoint(\n> > +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> > +\tstruct fwnode_handle *endpoint, unsigned int asd_struct_size,\n> > +\tint (*parse_single)(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> \n> You don't ignore them anymore, the comment should be updated.\n\nHmm. I actually intended to do something else about this. :-) As there's a\nsingle error code, handling that would need to be done a little bit\ndifferently right now.\n\nI'd print a warning and proceed. What do you think?\n\nEven if there's a bad DT endpoint, that shouldn't prevent the rest from\nworking, right?\n\n> \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_single(dev, vep, asd);\n> > +\tv4l2_fwnode_endpoint_free(vep);\n> > +\tif (ret) {\n> > +\t\tdev_warn(dev, \"driver could not parse endpoint (%d)\\n\", ret);\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_single)(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 notifier->v4l2_dev)\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 = 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> > +\t\tret = parse_endpoint(dev, notifier, fwnode, asd_struct_size,\n> > +\t\t\t\t parse_single);\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 \");\n> > MODULE_AUTHOR(\"Sylwester Nawrocki \");\n> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> > index c69d8c8a66d0..4a44ab47ab04 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> > @@ -78,7 +77,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 subdevs array\n> > + * @max_subdevs: number of subdevices allocated in 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 +90,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 +122,21 @@ int v4l2_async_notifier_register(struct v4l2_device\n> > *v4l2_dev, void v4l2_async_notifier_unregister(struct v4l2_async_notifier\n> > *notifier);\n> > \n> > /**\n> > + * v4l2_async_notifier_release - release notifier resources\n> > + * @notifier: pointer to &struct v4l2_async_notifier\n> \n> That's quite obvious given the type of the argument. It would be much more \n> useful to tell which notifier pointer this function expects (although in this \n> case it should be obvious too): \"(pointer to )?the notifier whose resources \n> will be released\".\n\nThis fully matches to the documentation elsewhere in the same file. :-)\n\nI'll replace the text with yours.\n\n> \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> Zeroing the memory is pretty much a requirement, as \n> v4l2_async_notifier_parse_fwnode_endpoints() won't operate correctly if memory \n> contains random data anyway. Maybe we should introduce \n> v4l2_async_notifier_init() and make v4l2_async_notifier_release() mandatory, \n> but that's out of scope for this patch.\n\nNotifiers are typically allocated as part of a driver specific struct which\nis zeroed by the driver.\n\nRegistering the notifier won't work either if the rest of the struct wasn't\nzeroed.\n\n> \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..46521e8c8872 100644\n> > --- a/include/media/v4l2-fwnode.h\n> > +++ b/include/media/v4l2-fwnode.h\n> > @@ -25,6 +25,8 @@\n> > #include \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,41 @@ int v4l2_fwnode_parse_link(struct fwnode_handle\n> > *fwnode, */\n> > void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);\n> > \n> > +/**\n> > + * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints\n> > in a\n> > + *\t\t\t\t\t\tdevice node\n> > + * @dev: @struct device pointer\n> \n> Similarly to my previous comment (and my comments to v3), you should tell \n> which device the function expects.\n\nWill fix for the next version.\n\n> \n> > + * @notifier: pointer to &struct v4l2_async_notifier\n> > + * @asd_struct_size: size of the driver's async sub-device struct,\n> > 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> \n> Should this be documented in the kerneldoc of the v4l2_async_subdev structure \n> ?\n\nYes, I'll add that. It won't hurt to make it a requirement even if the\nhelper functions weren't used.\n\n> \n> > + * @parse_single: driver's callback function called on each V4L2 fwnode\n> > endpoint\n> > + *\n> > + * Allocate async sub-device array and sub-devices for each fwnode\n> > endpoint,\n> > + * parse the related fwnode endpoints and finally call driver's callback\n> > + * function to that V4L2 fwnode endpoint.\n> \n> I'd document this from the notifier point of view.\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 called \n> for each endpoint with the corresponding async sub-device pointer to let the \n> caller initialize the driver-specific part of the async sub-device structure.\"\n\nWorks for me.\n\n> \n> > + * The function may not be called on a registered notifier.\n> \n> You should mention that the function may be called multiple times on an \n> unregistered notifier.\n\nThere's no point in calling it more than once as the endpoints wouldn't end\nup being different from the previous time. Actually it'd make sense to\ndocument this.\n\n> \n> \"The function can be called multiple times to populate the same notifier from \n> endpoints of different @dev devices before registering the notifier. It can't \n> be called anymore once the notifier has been registered.\"\n\nI don't think there's really a use case for calling this for more than one\ndevice, is there?\n\n> \n> > + *\n> > + * Once the user has called this function, the resources released by it\n> > need to\n> > + * be released by callin v4l2_async_notifier_release after the notifier has\n> > been\n> > + * unregistered and the sub-devices are no longer in use.\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\nAgreed.\n\n> \n> > + *\n> > + * A driver supporting fwnode (currently Devicetree and ACPI) should call\n> > this\n> > + * function as part of its probe function before it registers the notifier.\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_single\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_single)(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>","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhWKW1wX1z9t16\n\tfor ;\n\tWed, 30 Aug 2017 00:31:27 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752367AbdH2ObZ (ORCPT\n\t);\n\tTue, 29 Aug 2017 10:31:25 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:53232 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1751562AbdH2ObZ (ORCPT\n\t); Tue, 29 Aug 2017 10:31:25 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 83412600CA;\n\tTue, 29 Aug 2017 17:31:22 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dmhY2-0001Jb-37; Tue, 29 Aug 2017 17:31:22 +0300"],"Date":"Tue, 29 Aug 2017 17:31:21 +0300","From":"Sakari Ailus ","To":"Laurent Pinchart ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","Message-ID":"<20170829143121.6sjdx3lgcoxm6mva@valkosipuli.retiisi.org.uk>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<20170829110313.19538-5-sakari.ailus@linux.intel.com>\n\t<2739432.dQ1BSg1MPy@avalon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<2739432.dQ1BSg1MPy@avalon>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761523,"web_url":"http://patchwork.ozlabs.org/comment/1761523/","msgid":"<4764194.MvKE5XMHvq@avalon>","list_archive_url":null,"date":"2017-09-01T08:55:43","subject":"Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","submitter":{"id":11034,"url":"http://patchwork.ozlabs.org/api/people/11034/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sakari,\n\nOn Tuesday, 29 August 2017 17:31:21 EEST Sakari Ailus wrote:\n> On Tue, Aug 29, 2017 at 05:02:54PM +0300, Laurent Pinchart wrote:\n> > On Tuesday, 29 August 2017 14:03:12 EEST 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\n> >> matters can be taken into account in the driver specific callback.\n> >> \n> >> Convert the omap3isp as an example.\n> >> \n> >> Signed-off-by: Sakari Ailus \n> >> ---\n> >> \n> >> drivers/media/platform/omap3isp/isp.c | 115 ++++++++++-----------------\n> >> drivers/media/platform/omap3isp/isp.h | 5 +-\n> >> drivers/media/v4l2-core/v4l2-async.c | 16 +++++\n> >> drivers/media/v4l2-core/v4l2-fwnode.c | 132 ++++++++++++++++++++++++++++\n> >> include/media/v4l2-async.h | > 20 +++++-\n> >> include/media/v4l2-fwnode.h | 39 ++++++++++\n> >> 6 files changed, 242 insertions(+), 85 deletions(-)\n\n[snip]\n\n> >> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c\n> >> b/drivers/media/v4l2-core/v4l2-fwnode.c index 706f9e7b90f1..39a587c6992a\n> >> 100644\n> >> --- a/drivers/media/v4l2-core/v4l2-fwnode.c\n> >> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c\n\n[snip]\n\n> >> +static int parse_endpoint(\n> >> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> >> +\tstruct fwnode_handle *endpoint, unsigned int asd_struct_size,\n> >> +\tint (*parse_single)(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> > \n> > You don't ignore them anymore, the comment should be updated.\n> \n> Hmm. I actually intended to do something else about this. :-) As there's a\n> single error code, handling that would need to be done a little bit\n> differently right now.\n> \n> I'd print a warning and proceed. What do you think?\n> \n> Even if there's a bad DT endpoint, that shouldn't prevent the rest from\n> working, right?\n\nI think it should, to make sure we catch DT issues. We both know how many \nvendors don't care about warnings, so I'd rather fail completely to ensure DT \nwill be fixed (and even ten I wouldn't be surprised if some vendors patched \nthe code to remove the check instead of fixing their DT ;-)).\n\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_single(dev, vep, asd);\n> >> +\tv4l2_fwnode_endpoint_free(vep);\n> >> +\tif (ret) {\n> >> +\t\tdev_warn(dev, \"driver could not parse endpoint (%d)\\n\", ret);\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[snip]\n\n> >> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h\n> >> index c69d8c8a66d0..4a44ab47ab04 100644\n> >> --- a/include/media/v4l2-async.h\n> >> +++ b/include/media/v4l2-async.h\n\n\n> >> @@ -121,6 +122,21 @@ int v4l2_async_notifier_register(struct v4l2_device\n> >> *v4l2_dev, void v4l2_async_notifier_unregister(struct\n> >> v4l2_async_notifier *notifier);\n> >> \n> >> /**\n> >> + * v4l2_async_notifier_release - release notifier resources\n> >> + * @notifier: pointer to &struct v4l2_async_notifier\n> > \n> > That's quite obvious given the type of the argument. It would be much more\n> > useful to tell which notifier pointer this function expects (although in\n> > this case it should be obvious too): \"(pointer to )?the notifier whose\n> > resources will be released\".\n> \n> This fully matches to the documentation elsewhere in the same file. :-)\n\nFeel free to fix the rest of the file :-)\n\n> I'll replace the text with yours.\n> \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> > Zeroing the memory is pretty much a requirement, as\n> > v4l2_async_notifier_parse_fwnode_endpoints() won't operate correctly if\n> > memory contains random data anyway. Maybe we should introduce\n> > v4l2_async_notifier_init() and make v4l2_async_notifier_release()\n> > mandatory, but that's out of scope for this patch.\n> \n> Notifiers are typically allocated as part of a driver specific struct which\n> is zeroed by the driver.\n> \n> Registering the notifier won't work either if the rest of the struct wasn't\n> zeroed.\n> \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\n> >> asynchronous\n> >> * \tsubdevice framework\n> >> *\n> >> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h\n> >> index 68eb22ba571b..46521e8c8872 100644\n> >> --- a/include/media/v4l2-fwnode.h\n> >> +++ b/include/media/v4l2-fwnode.h\n\n[snip]\n\n> >> @@ -201,4 +203,41 @@ int v4l2_fwnode_parse_link(struct fwnode_handle\n> >> *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\n> >> endpoints in a\n> >> + *\t\t\t\t\t\tdevice node\n> >> + * @dev: @struct device pointer\n> > \n> > Similarly to my previous comment (and my comments to v3), you should tell\n> > which device the function expects.\n> \n> Will fix for the next version.\n> \n> >> + * @notifier: pointer to &struct v4l2_async_notifier\n> >> + * @asd_struct_size: size of the driver's async sub-device struct,\n> >> 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> > \n> > Should this be documented in the kerneldoc of the v4l2_async_subdev\n> > structure ?\n> \n> Yes, I'll add that. It won't hurt to make it a requirement even if the\n> helper functions weren't used.\n> \n> >> + * @parse_single: driver's callback function called on each V4L2 fwnode\n> >> endpoint\n> >> + *\n> >> + * Allocate async sub-device array and sub-devices for each fwnode\n> >> endpoint,\n> >> + * parse the related fwnode endpoints and finally call driver's\n> >> callback\n> >> + * function to that V4L2 fwnode endpoint.\n> > \n> > I'd document this from the notifier point of view.\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\n> > to let the caller initialize the driver-specific part of the async\n> > sub-device structure.\"\n> \n> Works for me.\n> \n> > > + * The function may not be called on a registered notifier.\n> > \n> > You should mention that the function may be called multiple times on an\n> > unregistered notifier.\n> \n> There's no point in calling it more than once as the endpoints wouldn't end\n> up being different from the previous time. Actually it'd make sense to\n> document this.\n> \n> > \"The function can be called multiple times to populate the same notifier\n> > from endpoints of different @dev devices before registering the notifier.\n> > It can't be called anymore once the notifier has been registered.\"\n> \n> I don't think there's really a use case for calling this for more than one\n> device, is there?\n\nI don't have one in mind, but I was wondering. If there isn't then you don't \nneed notifier_realloc(), which could be moved to the next patch.\n\n> >> + *\n> >> + * Once the user has called this function, the resources released by it\n> >> need to\n> >> + * be released by callin v4l2_async_notifier_release after the notifier\n> >> has been\n> >> + * unregistered and the sub-devices are no longer in use.\n> > \n> > \"Any notifier populated using this function must be released with a call\n> > to v4l2_async_notifier_release() after it has been unregistered and the\n> > async sub-devices are no longer in use.\"\n> \n> Agreed.\n> \n> >> + *\n> >> + * A driver supporting fwnode (currently Devicetree and ACPI) should\n> >> call this\n> >> + * function as part of its probe function before it registers the\n> >> notifier.\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_single\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_single)(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 */","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RPlRPGAv\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkCkB07xYz9t2c\n\tfor ;\n\tFri, 1 Sep 2017 18:55:14 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751515AbdIAIzM (ORCPT\n\t);\n\tFri, 1 Sep 2017 04:55:12 -0400","from galahad.ideasonboard.com ([185.26.127.97]:41588 \"EHLO\n\tgalahad.ideasonboard.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751503AbdIAIzJ (ORCPT\n\t); Fri, 1 Sep 2017 04:55:09 -0400","from avalon.localnet (84-199-88-196.iFiber.telenet-ops.be\n\t[84.199.88.196])\n\tby galahad.ideasonboard.com (Postfix) with ESMTPSA id 22AF1201F5;\n\tFri, 1 Sep 2017 10:53:18 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1504255998;\n\tbh=FMCSdpUuJ1XoFF8J5aufNR20sQmfIkp0pUwFEWmFGWk=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=RPlRPGAv7pX3XXd4RWOWj4zFx7m+APAqufGn2ix2sUq1dSVeTR2pwjkoQMUUwQvhT\n\tjrvbNZA6CzY1rsJ3L/m2w7WT3Vj+JOOJkSQY56jCuls/pV8KhXH2X7QjXPeiRlkj7K\n\toTlFOaVlB9YjNU0dyMVFu6nQsCRqfOBr7ySCP+pY=","From":"Laurent Pinchart ","To":"Sakari Ailus ","Cc":"Sakari Ailus ,\n\tlinux-media@vger.kernel.org, niklas.soderlund@ragnatech.se,\n\trobh@kernel.org, hverkuil@xs4all.nl, devicetree@vger.kernel.org","Subject":"Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","Date":"Fri, 01 Sep 2017 11:55:43 +0300","Message-ID":"<4764194.MvKE5XMHvq@avalon>","In-Reply-To":"<20170829143121.6sjdx3lgcoxm6mva@valkosipuli.retiisi.org.uk>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<2739432.dQ1BSg1MPy@avalon>\n\t<20170829143121.6sjdx3lgcoxm6mva@valkosipuli.retiisi.org.uk>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1761528,"web_url":"http://patchwork.ozlabs.org/comment/1761528/","msgid":"<20170901090434.5xyjz5ooswbx2kt2@paasikivi.fi.intel.com>","list_archive_url":null,"date":"2017-09-01T09:04:35","subject":"Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","submitter":{"id":65485,"url":"http://patchwork.ozlabs.org/api/people/65485/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Laurent,\n\nOn Fri, Sep 01, 2017 at 11:55:43AM +0300, Laurent Pinchart wrote:\n...\n> > >> +static int parse_endpoint(\n> > >> +\tstruct device *dev, struct v4l2_async_notifier *notifier,\n> > >> +\tstruct fwnode_handle *endpoint, unsigned int asd_struct_size,\n> > >> +\tint (*parse_single)(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> > > \n> > > You don't ignore them anymore, the comment should be updated.\n> > \n> > Hmm. I actually intended to do something else about this. :-) As there's a\n> > single error code, handling that would need to be done a little bit\n> > differently right now.\n> > \n> > I'd print a warning and proceed. What do you think?\n> > \n> > Even if there's a bad DT endpoint, that shouldn't prevent the rest from\n> > working, right?\n> \n> I think it should, to make sure we catch DT issues. We both know how many \n> vendors don't care about warnings, so I'd rather fail completely to ensure DT \n> will be fixed (and even ten I wouldn't be surprised if some vendors patched \n> the code to remove the check instead of fixing their DT ;-)).\n\nIf they test the DTS, they should find out that the device does not work.\n\nIf they do not, some devices will work even if others fail.\n\nTherefore I don't see why everything should fail when a part of faulty.\nExtending that a little, you should also halt the system to make sure the\nproblem will be noticed. :-)\n\n> > >> @@ -121,6 +122,21 @@ int v4l2_async_notifier_register(struct v4l2_device\n> > >> *v4l2_dev, void v4l2_async_notifier_unregister(struct\n> > >> v4l2_async_notifier *notifier);\n> > >> \n> > >> /**\n> > >> + * v4l2_async_notifier_release - release notifier resources\n> > >> + * @notifier: pointer to &struct v4l2_async_notifier\n> > > \n> > > That's quite obvious given the type of the argument. It would be much more\n> > > useful to tell which notifier pointer this function expects (although in\n> > > this case it should be obvious too): \"(pointer to )?the notifier whose\n> > > resources will be released\".\n> > \n> > This fully matches to the documentation elsewhere in the same file. :-)\n> \n> Feel free to fix the rest of the file :-)\n\nThat's out of scope of this patch.\n\n> > > \"The function can be called multiple times to populate the same notifier\n> > > from endpoints of different @dev devices before registering the notifier.\n> > > It can't be called anymore once the notifier has been registered.\"\n> > \n> > I don't think there's really a use case for calling this for more than one\n> > device, is there?\n> \n> I don't have one in mind, but I was wondering. If there isn't then you don't \n> need notifier_realloc(), which could be moved to the next patch.\n\nI don't think there's even benefit from moving it to the next patch, it\njust adds to the reviewable code, nothing else.","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkCxn4pGMz9t2r\n\tfor ;\n\tFri, 1 Sep 2017 19:05:17 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751758AbdIAJFM (ORCPT\n\t);\n\tFri, 1 Sep 2017 05:05:12 -0400","from mga05.intel.com ([192.55.52.43]:33611 \"EHLO mga05.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751752AbdIAJFI (ORCPT );\n\tFri, 1 Sep 2017 05:05:08 -0400","from fmsmga002.fm.intel.com ([10.253.24.26])\n\tby fmsmga105.fm.intel.com with ESMTP; 01 Sep 2017 02:05:08 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby fmsmga002.fm.intel.com with ESMTP; 01 Sep 2017 02:05:05 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid 25E0320642; Fri, 1 Sep 2017 12:04:35 +0300 (EEST)"],"X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos; i=\"5.41,457,1498546800\"; d=\"scan'208\";\n\ta=\"1213378068\"","Date":"Fri, 1 Sep 2017 12:04:35 +0300","From":"Sakari Ailus ","To":"Laurent Pinchart ","Cc":"Sakari Ailus , linux-media@vger.kernel.org,\n\tniklas.soderlund@ragnatech.se, robh@kernel.org, hverkuil@xs4all.nl,\n\tdevicetree@vger.kernel.org","Subject":"Re: [PATCH v5 4/5] v4l: fwnode: Support generic parsing of graph\n\tendpoints in a device","Message-ID":"<20170901090434.5xyjz5ooswbx2kt2@paasikivi.fi.intel.com>","References":"<20170829110313.19538-1-sakari.ailus@linux.intel.com>\n\t<2739432.dQ1BSg1MPy@avalon>\n\t<20170829143121.6sjdx3lgcoxm6mva@valkosipuli.retiisi.org.uk>\n\t<4764194.MvKE5XMHvq@avalon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<4764194.MvKE5XMHvq@avalon>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}}]