| Message ID | 20230725012859.18474-3-quic_mdtipton@quicinc.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Add interconnect debugfs client | expand |
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
Hi Mike,
kernel test robot noticed the following build warnings:
[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.5-rc3 next-20230725]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mike-Tipton/debugfs-Add-write-support-to-debugfs_create_str/20230725-093242
base: driver-core/driver-core-testing
patch link: https://lore.kernel.org/r/20230725012859.18474-3-quic_mdtipton%40quicinc.com
patch subject: [PATCH 2/3] interconnect: Reintroduce icc_get()
config: um-randconfig-m031-20230725 (https://download.01.org/0day-ci/archive/20230725/202307251508.Z4w38USw-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230725/202307251508.Z4w38USw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307251508.Z4w38USw-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/opp/opp.h:15,
from drivers/opp/core.c:23:
>> include/linux/interconnect.h:63:18: warning: no previous prototype for 'icc_get' [-Wmissing-prototypes]
63 | struct icc_path *icc_get(struct device *dev, const char *src, const char *dst)
| ^~~~~~~
vim +/icc_get +63 include/linux/interconnect.h
62
> 63 struct icc_path *icc_get(struct device *dev, const char *src, const char *dst)
64 {
65 return NULL;
66 }
67
Hi Mike,
kernel test robot noticed the following build errors:
[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.5-rc3 next-20230725]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mike-Tipton/debugfs-Add-write-support-to-debugfs_create_str/20230725-093242
base: driver-core/driver-core-testing
patch link: https://lore.kernel.org/r/20230725012859.18474-3-quic_mdtipton%40quicinc.com
patch subject: [PATCH 2/3] interconnect: Reintroduce icc_get()
config: arm-integrator_defconfig (https://download.01.org/0day-ci/archive/20230725/202307251608.svPOM4UC-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230725/202307251608.svPOM4UC-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307251608.svPOM4UC-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from drivers/opp/opp.h:15,
from drivers/opp/core.c:23:
>> include/linux/interconnect.h:63:18: warning: no previous prototype for 'icc_get' [-Wmissing-prototypes]
63 | struct icc_path *icc_get(struct device *dev, const char *src, const char *dst)
| ^~~~~~~
--
arm-linux-gnueabi-ld: drivers/opp/cpu.o: in function `icc_get':
>> cpu.c:(.text+0x2c8): multiple definition of `icc_get'; drivers/opp/core.o:core.c:(.text+0x2260): first defined here
arm-linux-gnueabi-ld: drivers/opp/of.o: in function `icc_get':
of.c:(.text+0x1b20): multiple definition of `icc_get'; drivers/opp/core.o:core.c:(.text+0x2260): first defined here
On Tue, Jul 25, 2023 at 11:18:58AM +0530, Pavan Kondeti wrote: > 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. > > [..] > > 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? With debugfs as the only user, it could technically be made private. Generally speaking, people should use of_icc_get() instead of this. We have some other downstream users of icc_get(), but we'll likely push them to switch to of_icc_get() instead. There are some potential future use cases that would require exporting this, but none immediate.
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 5fac448c28fd..511152398790 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -147,6 +147,21 @@ static struct icc_node *node_find(const int id) return idr_find(&icc_idr, id); } +static struct icc_node *node_find_by_name(const char *name) +{ + struct icc_provider *provider; + struct icc_node *n; + + list_for_each_entry(provider, &icc_providers, provider_list) { + list_for_each_entry(n, &provider->nodes, node_list) { + if (!strcmp(n->name, name)) + return n; + } + } + + return NULL; +} + static struct icc_path *path_init(struct device *dev, struct icc_node *dst, ssize_t num_nodes) { @@ -561,6 +576,55 @@ struct icc_path *of_icc_get(struct device *dev, const char *name) } EXPORT_SYMBOL_GPL(of_icc_get); +/** + * 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; +} + static inline struct icc_path *of_icc_get(struct device *dev, const char *name) {
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(+)