Re: [PATCH v6 2/4] interconnect: add clk-based icc provider support

[Nick Desaulniers <ndesaulniers@google.com>]

On Fri, May 12, 2023 at 03:13:32AM +0300, Dmitry Baryshkov wrote:
> For some devices it is useful to export clocks as interconnect providers,
> if the clock corresponds to bus bandwidth.
> 
> For example, on MSM8996 the cluster interconnect clock should be scaled
> according to the cluster frequencies. Exporting it as an interconnect
> allows one to properly describe this as the cluster bandwidth
> requirements.
> 
> Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Hi Dmitry,
Apologies if this has already been reported elsewhere, but our CI is red
for linux-next today for allmodconfig builds:

>> ERROR: modpost: missing MODULE_LICENSE() in drivers/interconnect/icc-clk.o

https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/5024096989/jobs/9011763868

Can you PTAL?

> ---
>  drivers/interconnect/Kconfig     |   6 ++
>  drivers/interconnect/Makefile    |   2 +
>  drivers/interconnect/icc-clk.c   | 168 +++++++++++++++++++++++++++++++
>  include/linux/interconnect-clk.h |  22 ++++
>  4 files changed, 198 insertions(+)
>  create mode 100644 drivers/interconnect/icc-clk.c
>  create mode 100644 include/linux/interconnect-clk.h
> 
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index d637a89d4695..5faa8d2aecff 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -15,4 +15,10 @@ source "drivers/interconnect/imx/Kconfig"
>  source "drivers/interconnect/qcom/Kconfig"
>  source "drivers/interconnect/samsung/Kconfig"
>  
> +config INTERCONNECT_CLK
> +	tristate
> +	depends on COMMON_CLK
> +	help
> +	  Support for wrapping clocks into the interconnect nodes.
> +
>  endif
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 97d393fd638d..5604ce351a9f 100644
> --- a/drivers/interconnect/Makefile
> +++ b/drivers/interconnect/Makefile
> @@ -7,3 +7,5 @@ obj-$(CONFIG_INTERCONNECT)		+= icc-core.o
>  obj-$(CONFIG_INTERCONNECT_IMX)		+= imx/
>  obj-$(CONFIG_INTERCONNECT_QCOM)		+= qcom/
>  obj-$(CONFIG_INTERCONNECT_SAMSUNG)	+= samsung/
> +
> +obj-$(CONFIG_INTERCONNECT_CLK)		+= icc-clk.o
> diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
> new file mode 100644
> index 000000000000..0db3b654548b
> --- /dev/null
> +++ b/drivers/interconnect/icc-clk.c
> @@ -0,0 +1,168 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023, Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/interconnect-clk.h>
> +#include <linux/interconnect-provider.h>
> +
> +struct icc_clk_node {
> +	struct clk *clk;
> +	bool enabled;
> +};
> +
> +struct icc_clk_provider {
> +	struct icc_provider provider;
> +	int num_clocks;
> +	struct icc_clk_node clocks[];
> +};
> +
> +#define to_icc_clk_provider(_provider) \
> +	container_of(_provider, struct icc_clk_provider, provider)
> +
> +static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
> +{
> +	struct icc_clk_node *qn = src->data;
> +	int ret;
> +
> +	if (!qn || !qn->clk)
> +		return 0;
> +
> +	if (!src->peak_bw) {
> +		if (qn->enabled)
> +			clk_disable_unprepare(qn->clk);
> +		qn->enabled = false;
> +
> +		return 0;
> +	}
> +
> +	if (!qn->enabled) {
> +		ret = clk_prepare_enable(qn->clk);
> +		if (ret)
> +			return ret;
> +		qn->enabled = true;
> +	}
> +
> +	return clk_set_rate(qn->clk, icc_units_to_bps(src->peak_bw));
> +}
> +
> +static int icc_clk_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
> +{
> +	struct icc_clk_node *qn = node->data;
> +
> +	if (!qn || !qn->clk)
> +		*peak = INT_MAX;
> +	else
> +		*peak = Bps_to_icc(clk_get_rate(qn->clk));
> +
> +	return 0;
> +}
> +
> +/**
> + * icc_clk_register() - register a new clk-based interconnect provider
> + * @dev: device supporting this provider
> + * @first_id: an ID of the first provider's node
> + * @num_clocks: number of instances of struct icc_clk_data
> + * @data: data for the provider
> + *
> + * Registers and returns a clk-based interconnect provider. It is a simple
> + * wrapper around COMMON_CLK framework, allowing other devices to vote on the
> + * clock rate.
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +struct icc_provider *icc_clk_register(struct device *dev,
> +				      unsigned int first_id,
> +				      unsigned int num_clocks,
> +				      const struct icc_clk_data *data)
> +{
> +	struct icc_clk_provider *qp;
> +	struct icc_provider *provider;
> +	struct icc_onecell_data *onecell;
> +	struct icc_node *node;
> +	int ret, i, j;
> +
> +	onecell = devm_kzalloc(dev, struct_size(onecell, nodes, 2 * num_clocks), GFP_KERNEL);
> +	if (!onecell)
> +		return ERR_PTR(-ENOMEM);
> +
> +	qp = devm_kzalloc(dev, struct_size(qp, clocks, num_clocks), GFP_KERNEL);
> +	if (!qp)
> +		return ERR_PTR(-ENOMEM);
> +
> +	qp->num_clocks = num_clocks;
> +
> +	provider = &qp->provider;
> +	provider->dev = dev;
> +	provider->get_bw = icc_clk_get_bw;
> +	provider->set = icc_clk_set;
> +	provider->aggregate = icc_std_aggregate;
> +	provider->xlate = of_icc_xlate_onecell;
> +	INIT_LIST_HEAD(&provider->nodes);
> +	provider->data = onecell;
> +
> +	icc_provider_init(provider);
> +
> +	for (i = 0, j = 0; i < num_clocks; i++) {
> +		qp->clocks[i].clk = data[i].clk;
> +
> +		node = icc_node_create(first_id + j);
> +		if (IS_ERR(node)) {
> +			ret = PTR_ERR(node);
> +			goto err;
> +		}
> +
> +		node->name = devm_kasprintf(dev, GFP_KERNEL, "%s_master", data[i].name);
> +		node->data = &qp->clocks[i];
> +		icc_node_add(node, provider);
> +		/* link to the next node, slave */
> +		icc_link_create(node, first_id + j + 1);
> +		onecell->nodes[j++] = node;
> +
> +		node = icc_node_create(first_id + j);
> +		if (IS_ERR(node)) {
> +			ret = PTR_ERR(node);
> +			goto err;
> +		}
> +
> +		node->name = devm_kasprintf(dev, GFP_KERNEL, "%s_slave", data[i].name);
> +		/* no data for slave node */
> +		icc_node_add(node, provider);
> +		onecell->nodes[j++] = node;
> +	}
> +
> +	onecell->num_nodes = j;
> +
> +	ret = icc_provider_register(provider);
> +	if (ret)
> +		goto err;
> +
> +	return provider;
> +
> +err:
> +	icc_nodes_remove(provider);
> +
> +	return ERR_PTR(ret);
> +}
> +
> +/**
> + * icc_clk_unregister() - unregister a previously registered clk interconnect provider
> + * @provider: provider returned by icc_clk_register()
> + */
> +void icc_clk_unregister(struct icc_provider *provider)
> +{
> +	struct icc_clk_provider *qp = container_of(provider, struct icc_clk_provider, provider);
> +	int i;
> +
> +	icc_provider_deregister(&qp->provider);
> +	icc_nodes_remove(&qp->provider);
> +
> +	for (i = 0; i < qp->num_clocks; i++) {
> +		struct icc_clk_node *qn = &qp->clocks[i];
> +
> +		if (qn->enabled)
> +			clk_disable_unprepare(qn->clk);
> +	}
> +}
> diff --git a/include/linux/interconnect-clk.h b/include/linux/interconnect-clk.h
> new file mode 100644
> index 000000000000..0cd80112bea5
> --- /dev/null
> +++ b/include/linux/interconnect-clk.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023, Linaro Ltd.
> + */
> +
> +#ifndef __LINUX_INTERCONNECT_CLK_H
> +#define __LINUX_INTERCONNECT_CLK_H
> +
> +struct device;
> +
> +struct icc_clk_data {
> +	struct clk *clk;
> +	const char *name;
> +};
> +
> +struct icc_provider *icc_clk_register(struct device *dev,
> +				      unsigned int first_id,
> +				      unsigned int num_clocks,
> +				      const struct icc_clk_data *data);
> +void icc_clk_unregister(struct icc_provider *provider);
> +
> +#endif
> -- 
> 2.39.2
>