Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/2] of: property: Get rid of code duplication in port
 getting
To:     Sam Protsenko <semen.protsenko@linaro.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        John Stultz <john.stultz@linaro.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Sam Ravnborg <sam@ravnborg.org>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>
References: <20201210202944.6747-1-semen.protsenko@linaro.org>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <f18dea6e-5806-370c-0d83-e56f3dc118b4@gmail.com>
Date:   Thu, 10 Dec 2020 23:59:21 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.2
MIME-Version: 1.0
In-Reply-To: <20201210202944.6747-1-semen.protsenko@linaro.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

10.12.2020 23:29, Sam Protsenko пишет:
> Both of_graph_is_present() and of_graph_get_next_endpoint() functions
> share common piece of code for obtaining the graph port. Extract it into
> separate static function to get rid of code duplication and avoid
> possible coding errors in future.
> 
> Fixes: 4ec0a44ba8d7 ("of_graph: add of_graph_is_present()")

The "fixes" tag should be used only for bug-fixes and there is no bug
fixed in this patch.

> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  drivers/of/property.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 408a7b5f06a9..da111fcf37ac 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -30,13 +30,13 @@
>  #include "of_private.h"
>  
>  /**
> - * of_graph_is_present() - check graph's presence
> + * of_graph_get_port - find the "port" node in a given node
>   * @node: pointer to device_node containing graph port
>   *
> - * Return: True if @node has a port or ports (with a port) sub-node,
> - * false otherwise.
> + * Return: A 'port' node pointer with refcount incremented if found or NULL
> + * otherwise. The caller has to use of_node_put() on it when done.
>   */
> -bool of_graph_is_present(const struct device_node *node)
> +static struct device_node *of_graph_get_port(const struct device_node *node)
>  {
>  	struct device_node *ports, *port;
>  
> @@ -46,8 +46,22 @@ bool of_graph_is_present(const struct device_node *node)
>  
>  	port = of_get_child_by_name(node, "port");
>  	of_node_put(ports);
> -	of_node_put(port);
>  
> +	return port;
> +}
> +
> +/**
> + * of_graph_is_present() - check graph's presence
> + * @node: pointer to device_node containing graph port
> + *
> + * Return: True if @node has a port or ports (with a port) sub-node,
> + * false otherwise.
> + */
> +bool of_graph_is_present(const struct device_node *node)
> +{
> +	struct device_node *port = of_graph_get_port(node);
> +
> +	of_node_put(port);
>  	return !!port;
>  }
>  EXPORT_SYMBOL(of_graph_is_present);
> @@ -631,15 +645,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
>  	 * parent port node.
>  	 */
>  	if (!prev) {
> -		struct device_node *node;
> -
> -		node = of_get_child_by_name(parent, "ports");
> -		if (node)
> -			parent = node;
> -
> -		port = of_get_child_by_name(parent, "port");
> -		of_node_put(node);
> -
> +		port = of_graph_get_port(parent);
>  		if (!port) {
>  			pr_err("graph: no port node found in %pOF\n", parent);
>  			return NULL;
> 

This repeats the problem which was made once before:

https://lore.kernel.org/patchwork/patch/1266028/#1461493