Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 25 Jul 2023 11:18:58 +0530
From:   Pavan Kondeti <quic_pkondeti@quicinc.com>
To:     Mike Tipton <quic_mdtipton@quicinc.com>
CC:     <djakov@kernel.org>, <gregkh@linuxfoundation.org>,
        <rafael@kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-pm@vger.kernel.org>, <quic_okukatla@quicinc.com>,
        <quic_viveka@quicinc.com>
Subject: Re: [PATCH 2/3] interconnect: Reintroduce icc_get()
Message-ID: <c68630ee-9f94-40ec-adb1-a899dc22ca1b@quicinc.com>
References: <20230725012859.18474-1-quic_mdtipton@quicinc.com>
 <20230725012859.18474-3-quic_mdtipton@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230725012859.18474-3-quic_mdtipton@quicinc.com>
Precedence: bulk

On Mon, Jul 24, 2023 at 06:28:58PM -0700, Mike Tipton wrote:
> The original icc_get() that took integer node IDs was removed due to
> lack of users. Reintroduce a new version that takes string node names,
> which is needed for the debugfs client.
> 
> Signed-off-by: Mike Tipton <quic_mdtipton@quicinc.com>
> ---
>  drivers/interconnect/core.c  | 64 ++++++++++++++++++++++++++++++++++++
>  include/linux/interconnect.h |  6 ++++
>  2 files changed, 70 insertions(+)

[...]

> +/**
> + * icc_get() - get a path handle between two endpoints
> + * @dev: device pointer for the consumer device
> + * @src: source node name
> + * @dst: destination node name
> + *
> + * This function will search for a path between two endpoints and return an
> + * icc_path handle on success. Use icc_put() to release constraints when they
> + * are not needed anymore.
> + *
> + * Return: icc_path pointer on success or ERR_PTR() on error. NULL is returned
> + * when the API is disabled.
> + */
> +struct icc_path *icc_get(struct device *dev, const char *src, const char *dst)
> +{
> +	struct icc_node *src_node, *dst_node;
> +	struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> +
> +	mutex_lock(&icc_lock);
> +
> +	src_node = node_find_by_name(src);
> +	if (!src_node) {
> +		dev_err(dev, "%s: invalid src=%s\n", __func__, src);
> +		goto out;
> +	}
> +
> +	dst_node = node_find_by_name(dst);
> +	if (!dst_node) {
> +		dev_err(dev, "%s: invalid dst=%s\n", __func__, dst);
> +		goto out;
> +	}
> +
> +	path = path_find(dev, src_node, dst_node);
> +	if (IS_ERR(path)) {
> +		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
> +		goto out;
> +	}
> +
> +	path->name = kasprintf(GFP_KERNEL, "%s-%s", src_node->name, dst_node->name);
> +	if (!path->name) {
> +		kfree(path);
> +		path = ERR_PTR(-ENOMEM);
> +	}
> +out:
> +	mutex_unlock(&icc_lock);
> +	return path;
> +}
> +EXPORT_SYMBOL_GPL(icc_get);
> +
>  /**
>   * icc_set_tag() - set an optional tag on a path
>   * @path: the path we want to tag
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> index 97ac253df62c..39a98ddfdfd4 100644
> --- a/include/linux/interconnect.h
> +++ b/include/linux/interconnect.h
> @@ -40,6 +40,7 @@ struct icc_bulk_data {
>  
>  #if IS_ENABLED(CONFIG_INTERCONNECT)
>  
> +struct icc_path *icc_get(struct device *dev, const char *src, const char *dst);
>  struct icc_path *of_icc_get(struct device *dev, const char *name);
>  struct icc_path *devm_of_icc_get(struct device *dev, const char *name);
>  int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths);
> @@ -59,6 +60,11 @@ void icc_bulk_disable(int num_paths, const struct icc_bulk_data *paths);
>  
>  #else
>  
> +struct icc_path *icc_get(struct device *dev, const char *src, const char *dst)
> +{
> +	return NULL;
> +}
> +

Is this API meant to be public()? As you pointed out, it was removed
recently as there were no users. Since debugfs client is part of ICC
frarmwork, should it be made private?

Thanks,
Pavan