Received-SPF: pass (google.com: domain of linux-kernel+bounces-53130-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 5 Feb 2024 17:36:26 +0000
From: Rob Herring <robh@kernel.org>
To: Saravana Kannan <saravanak@google.com>
Cc: Frank Rowand <frowand.list@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kernel-team@android.com, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/2] of: property: Improve finding the consumer of a
 remote-endpoint property
Message-ID: <20240205173626.GA3657127-robh@kernel.org>
References: <20240202101326.876070-1-saravanak@google.com>
 <20240202101326.876070-2-saravanak@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240202101326.876070-2-saravanak@google.com>

On Fri, Feb 02, 2024 at 02:13:24AM -0800, Saravana Kannan wrote:
> We have a more accurate function to find the right consumer of a
> remote-endpoint property instead of searching for a parent with
> compatible string property. So, use that instead. While at it, make the
> code to find the consumer a bit more flexible and based on the property
> being parsed.
> 
> Fixes: f7514a663016 ("of: property: fw_devlink: Add support for remote-endpoint")
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  drivers/of/property.c | 52 +++++++++++++------------------------------
>  1 file changed, 15 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 641a40cf5cf3..ba374a1f2072 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1063,36 +1063,6 @@ of_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
>  	return of_device_get_match_data(dev);
>  }
>  
> -static struct device_node *of_get_compat_node(struct device_node *np)
> -{
> -	of_node_get(np);
> -
> -	while (np) {
> -		if (!of_device_is_available(np)) {
> -			of_node_put(np);
> -			np = NULL;
> -		}
> -
> -		if (of_property_present(np, "compatible"))
> -			break;
> -
> -		np = of_get_next_parent(np);
> -	}
> -
> -	return np;
> -}
> -
> -static struct device_node *of_get_compat_node_parent(struct device_node *np)
> -{
> -	struct device_node *parent, *node;
> -
> -	parent = of_get_parent(np);
> -	node = of_get_compat_node(parent);
> -	of_node_put(parent);
> -
> -	return node;
> -}
> -
>  static void of_link_to_phandle(struct device_node *con_np,
>  			      struct device_node *sup_np)
>  {
> @@ -1222,10 +1192,10 @@ static struct device_node *parse_##fname(struct device_node *np,	     \
>   *  parse_prop.prop_name: Name of property holding a phandle value
>   *  parse_prop.index: For properties holding a list of phandles, this is the
>   *		      index into the list
> + * @get_con_dev: If the consumer node containing the property is never converted
> + *		 to a struct device, implement this ops so fw_devlink can use it
> + *		 to find the true consumer.
>   * @optional: Describes whether a supplier is mandatory or not
> - * @node_not_dev: The consumer node containing the property is never converted
> - *		  to a struct device. Instead, parse ancestor nodes for the
> - *		  compatible property to find a node corresponding to a device.
>   *
>   * Returns:
>   * parse_prop() return values are
> @@ -1236,8 +1206,8 @@ static struct device_node *parse_##fname(struct device_node *np,	     \
>  struct supplier_bindings {
>  	struct device_node *(*parse_prop)(struct device_node *np,
>  					  const char *prop_name, int index);
> +	struct device_node *(*get_con_dev)(struct device_node *np);
>  	bool optional;
> -	bool node_not_dev;
>  };
>  
>  DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells")
> @@ -1328,6 +1298,11 @@ static struct device_node *parse_interrupts(struct device_node *np,
>  	return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np;
>  }
>  
> +static struct device_node *get_remote_endpoint_dev(struct device_node *np)
> +{
> +	return to_of_node(fwnode_graph_get_port_parent(of_fwnode_handle(np)));

DT APIs shouldn't depend on fwnode APIs.

Otherwise, this series looks good.