Re: [PATCH 1/2] of: property: Get rid of code duplication in port getting

[Dmitry Osipenko <digetx@gmail.com>]

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