linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock
@ 2023-05-12  0:13 Dmitry Baryshkov
  2023-05-12  0:13 ` [PATCH v6 1/4] dt-bindings: interconnect/msm8996-cbf: add defines to be used by CBF Dmitry Baryshkov
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-05-12  0:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov
  Cc: linux-arm-msm, linux-clk, devicetree, Yassine Oudjana, linux-pm

On MSM8996 two CPU clusters are interconnected using the Core Bus
Fabric (CBF). In order for the CPU clusters to function properly, it
should be clocked following the core's frequencies to provide adequate
bandwidth.

Register CBF as a clock (required for CPU to boot) and add a tiny
interconnect layer on top of it to let cpufreq/opp scale the CBF clock.

Changes since v5:
- Fixed !INTERCONNECT warning (Konrad)
- Fixed the sync_state wrapper for !INTERCONNECT case

Changes since v4:
- Fixed typos in commit messages

Changes since v3:
- Dropped merged patches
- Moved interconnect shim to drivers/interconnect/icc-clk.c

Changes since v2:
- Added interconnect-related bindings
- Switched CPU and CBF clocks to RPM_SMD_XO_A_CLK_SRC

Changes since v1:
- Relicensed schema to GPL-2.0 + BSD-2-Clause (Krzysztof)
- Changed clock driver to use parent_hws (Konrad)
- Fixed indentation in CBF clock driver (Konrad)
- Changed MODULE_LICENSE of CBF clock driver to GPL from GPL-v2
- Switched CBF to use RPM_SMD_XO_CLK_SRC as one of the parents
- Enabled RPM_SMD_XO_CLK_SRC on msm8996 platform and switch to it from
  RPM_SMD_BB_CLK1 clock

Dmitry Baryshkov (4):
  dt-bindings: interconnect/msm8996-cbf: add defines to be used by CBF
  interconnect: add clk-based icc provider support
  clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq
  arm64: dts: qcom: msm8996: scale CBF clock according to the CPUfreq

 arch/arm64/boot/dts/qcom/msm8996.dtsi         |  51 ++++++
 drivers/clk/qcom/Kconfig                      |   1 +
 drivers/clk/qcom/clk-cbf-8996.c               |  60 ++++++-
 drivers/interconnect/Kconfig                  |   6 +
 drivers/interconnect/Makefile                 |   2 +
 drivers/interconnect/icc-clk.c                | 168 ++++++++++++++++++
 .../interconnect/qcom,msm8996-cbf.h           |  12 ++
 include/linux/interconnect-clk.h              |  22 +++
 8 files changed, 321 insertions(+), 1 deletion(-)
 create mode 100644 drivers/interconnect/icc-clk.c
 create mode 100644 include/dt-bindings/interconnect/qcom,msm8996-cbf.h
 create mode 100644 include/linux/interconnect-clk.h

-- 
2.39.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v6 1/4] dt-bindings: interconnect/msm8996-cbf: add defines to be used by CBF
  2023-05-12  0:13 [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Dmitry Baryshkov
@ 2023-05-12  0:13 ` Dmitry Baryshkov
  2023-05-12  0:13 ` [PATCH v6 2/4] interconnect: add clk-based icc provider support Dmitry Baryshkov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-05-12  0:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov
  Cc: linux-arm-msm, linux-clk, devicetree, Yassine Oudjana, linux-pm,
	Krzysztof Kozlowski

On msm8996 CBF interconnects power and performance CPU clusters. Add
corresponding interconnect defines to be used in device trees.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 include/dt-bindings/interconnect/qcom,msm8996-cbf.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 include/dt-bindings/interconnect/qcom,msm8996-cbf.h

diff --git a/include/dt-bindings/interconnect/qcom,msm8996-cbf.h b/include/dt-bindings/interconnect/qcom,msm8996-cbf.h
new file mode 100644
index 000000000000..aac5e69f6bd5
--- /dev/null
+++ b/include/dt-bindings/interconnect/qcom,msm8996-cbf.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) 2023 Linaro Ltd. All rights reserved.
+ */
+
+#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_MSM8996_CBF_H
+#define __DT_BINDINGS_INTERCONNECT_QCOM_MSM8996_CBF_H
+
+#define MASTER_CBF_M4M		0
+#define SLAVE_CBF_M4M		1
+
+#endif
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 2/4] interconnect: add clk-based icc provider support
  2023-05-12  0:13 [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Dmitry Baryshkov
  2023-05-12  0:13 ` [PATCH v6 1/4] dt-bindings: interconnect/msm8996-cbf: add defines to be used by CBF Dmitry Baryshkov
@ 2023-05-12  0:13 ` Dmitry Baryshkov
  2023-05-13  8:59   ` Konrad Dybcio
  2023-05-19 17:30   ` Nick Desaulniers
  2023-05-12  0:13 ` [PATCH v6 3/4] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq Dmitry Baryshkov
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-05-12  0:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov
  Cc: linux-arm-msm, linux-clk, devicetree, Yassine Oudjana, linux-pm

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>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 3/4] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq
  2023-05-12  0:13 [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Dmitry Baryshkov
  2023-05-12  0:13 ` [PATCH v6 1/4] dt-bindings: interconnect/msm8996-cbf: add defines to be used by CBF Dmitry Baryshkov
  2023-05-12  0:13 ` [PATCH v6 2/4] interconnect: add clk-based icc provider support Dmitry Baryshkov
@ 2023-05-12  0:13 ` Dmitry Baryshkov
  2023-06-10  0:18   ` Bjorn Andersson
  2023-05-12  0:13 ` [PATCH v6 4/4] arm64: dts: qcom: msm8996: " Dmitry Baryshkov
  2023-07-14  5:33 ` (subset) [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Bjorn Andersson
  4 siblings, 1 reply; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-05-12  0:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov
  Cc: linux-arm-msm, linux-clk, devicetree, Yassine Oudjana, linux-pm

Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth)
according to CPU frequencies.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/Kconfig        |  1 +
 drivers/clk/qcom/clk-cbf-8996.c | 60 ++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 12be3e2371b3..85869e7a9f16 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -48,6 +48,7 @@ config QCOM_CLK_APCS_MSM8916
 config QCOM_CLK_APCC_MSM8996
 	tristate "MSM8996 CPU Clock Controller"
 	select QCOM_KRYO_L2_ACCESSORS
+	select INTERCONNECT_CLK if INTERCONNECT
 	depends on ARM64
 	help
 	  Support for the CPU clock controller on msm8996 devices.
diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c
index cfd567636f4e..1e23b734abb3 100644
--- a/drivers/clk/qcom/clk-cbf-8996.c
+++ b/drivers/clk/qcom/clk-cbf-8996.c
@@ -5,11 +5,15 @@
 #include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/interconnect-clk.h>
+#include <linux/interconnect-provider.h>
 #include <linux/of.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
+#include <dt-bindings/interconnect/qcom,msm8996-cbf.h>
+
 #include "clk-alpha-pll.h"
 #include "clk-regmap.h"
 
@@ -223,6 +227,49 @@ static const struct regmap_config cbf_msm8996_regmap_config = {
 	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
 };
 
+#ifdef CONFIG_INTERCONNECT
+
+/* Random ID that doesn't clash with main qnoc and OSM */
+#define CBF_MASTER_NODE 2000
+
+static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev, struct clk_hw *cbf_hw)
+{
+	struct device *dev = &pdev->dev;
+	struct clk *clk = devm_clk_hw_get_clk(dev, cbf_hw, "cbf");
+	const struct icc_clk_data data[] = {
+		{ .clk = clk, .name = "cbf", },
+	};
+	struct icc_provider *provider;
+
+	provider = icc_clk_register(dev, CBF_MASTER_NODE, ARRAY_SIZE(data), data);
+	if (IS_ERR(provider))
+		return PTR_ERR(provider);
+
+	platform_set_drvdata(pdev, provider);
+
+	return 0;
+}
+
+static int qcom_msm8996_cbf_icc_remove(struct platform_device *pdev)
+{
+	struct icc_provider *provider = platform_get_drvdata(pdev);
+
+	icc_clk_unregister(provider);
+
+	return 0;
+}
+#define qcom_msm8996_cbf_icc_sync_state icc_sync_state
+#else
+static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev,  struct clk_hw *cbf_hw)
+{
+	dev_warn(&pdev->dev, "CONFIG_INTERCONNECT is disabled, CBF clock is fixed\n");
+
+	return 0;
+}
+#define qcom_msm8996_cbf_icc_remove(pdev) (0)
+#define qcom_msm8996_cbf_icc_sync_state NULL
+#endif
+
 static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
 {
 	void __iomem *base;
@@ -281,7 +328,16 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
+	if (ret)
+		return ret;
+
+	return qcom_msm8996_cbf_icc_register(pdev, &cbf_mux.clkr.hw);
+}
+
+static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
+{
+	return qcom_msm8996_cbf_icc_remove(pdev);
 }
 
 static const struct of_device_id qcom_msm8996_cbf_match_table[] = {
@@ -292,9 +348,11 @@ MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table);
 
 static struct platform_driver qcom_msm8996_cbf_driver = {
 	.probe = qcom_msm8996_cbf_probe,
+	.remove = qcom_msm8996_cbf_remove,
 	.driver = {
 		.name = "qcom-msm8996-cbf",
 		.of_match_table = qcom_msm8996_cbf_match_table,
+		.sync_state = qcom_msm8996_cbf_icc_sync_state,
 	},
 };
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 4/4] arm64: dts: qcom: msm8996: scale CBF clock according to the CPUfreq
  2023-05-12  0:13 [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Dmitry Baryshkov
                   ` (2 preceding siblings ...)
  2023-05-12  0:13 ` [PATCH v6 3/4] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq Dmitry Baryshkov
@ 2023-05-12  0:13 ` Dmitry Baryshkov
  2023-07-14  5:33   ` (subset) " Bjorn Andersson
  2023-07-14  5:33 ` (subset) [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Bjorn Andersson
  4 siblings, 1 reply; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-05-12  0:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov
  Cc: linux-arm-msm, linux-clk, devicetree, Yassine Oudjana, linux-pm

Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth)
according to CPU frequencies.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 51 +++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 2b35cb3f5292..5b006cebe47c 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -8,6 +8,7 @@
 #include <dt-bindings/clock/qcom,mmcc-msm8996.h>
 #include <dt-bindings/clock/qcom,rpmcc.h>
 #include <dt-bindings/interconnect/qcom,msm8996.h>
+#include <dt-bindings/interconnect/qcom,msm8996-cbf.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/soc/qcom,apr.h>
@@ -49,6 +50,7 @@ CPU0: cpu@0 {
 			cpu-idle-states = <&CPU_SLEEP_0>;
 			capacity-dmips-mhz = <1024>;
 			clocks = <&kryocc 0>;
+			interconnects = <&cbf MASTER_CBF_M4M &cbf SLAVE_CBF_M4M>;
 			operating-points-v2 = <&cluster0_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_0>;
@@ -66,6 +68,7 @@ CPU1: cpu@1 {
 			cpu-idle-states = <&CPU_SLEEP_0>;
 			capacity-dmips-mhz = <1024>;
 			clocks = <&kryocc 0>;
+			interconnects = <&cbf MASTER_CBF_M4M &cbf SLAVE_CBF_M4M>;
 			operating-points-v2 = <&cluster0_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_0>;
@@ -79,6 +82,7 @@ CPU2: cpu@100 {
 			cpu-idle-states = <&CPU_SLEEP_0>;
 			capacity-dmips-mhz = <1024>;
 			clocks = <&kryocc 1>;
+			interconnects = <&cbf MASTER_CBF_M4M &cbf SLAVE_CBF_M4M>;
 			operating-points-v2 = <&cluster1_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_1>;
@@ -96,6 +100,7 @@ CPU3: cpu@101 {
 			cpu-idle-states = <&CPU_SLEEP_0>;
 			capacity-dmips-mhz = <1024>;
 			clocks = <&kryocc 1>;
+			interconnects = <&cbf MASTER_CBF_M4M &cbf SLAVE_CBF_M4M>;
 			operating-points-v2 = <&cluster1_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_1>;
@@ -147,91 +152,109 @@ opp-307200000 {
 			opp-hz = /bits/ 64 <307200000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-422400000 {
 			opp-hz = /bits/ 64 <422400000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-480000000 {
 			opp-hz = /bits/ 64 <480000000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-556800000 {
 			opp-hz = /bits/ 64 <556800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-652800000 {
 			opp-hz = /bits/ 64 <652800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <384000>;
 		};
 		opp-729600000 {
 			opp-hz = /bits/ 64 <729600000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <460800>;
 		};
 		opp-844800000 {
 			opp-hz = /bits/ 64 <844800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <537600>;
 		};
 		opp-960000000 {
 			opp-hz = /bits/ 64 <960000000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <672000>;
 		};
 		opp-1036800000 {
 			opp-hz = /bits/ 64 <1036800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <672000>;
 		};
 		opp-1113600000 {
 			opp-hz = /bits/ 64 <1113600000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <825600>;
 		};
 		opp-1190400000 {
 			opp-hz = /bits/ 64 <1190400000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <825600>;
 		};
 		opp-1228800000 {
 			opp-hz = /bits/ 64 <1228800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <902400>;
 		};
 		opp-1324800000 {
 			opp-hz = /bits/ 64 <1324800000>;
 			opp-supported-hw = <0xd>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1056000>;
 		};
 		opp-1363200000 {
 			opp-hz = /bits/ 64 <1363200000>;
 			opp-supported-hw = <0x2>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1132800>;
 		};
 		opp-1401600000 {
 			opp-hz = /bits/ 64 <1401600000>;
 			opp-supported-hw = <0xd>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1132800>;
 		};
 		opp-1478400000 {
 			opp-hz = /bits/ 64 <1478400000>;
 			opp-supported-hw = <0x9>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1190400>;
 		};
 		opp-1497600000 {
 			opp-hz = /bits/ 64 <1497600000>;
 			opp-supported-hw = <0x04>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1305600>;
 		};
 		opp-1593600000 {
 			opp-hz = /bits/ 64 <1593600000>;
 			opp-supported-hw = <0x9>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1382400>;
 		};
 	};
 
@@ -245,136 +268,163 @@ opp-307200000 {
 			opp-hz = /bits/ 64 <307200000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-403200000 {
 			opp-hz = /bits/ 64 <403200000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-480000000 {
 			opp-hz = /bits/ 64 <480000000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-556800000 {
 			opp-hz = /bits/ 64 <556800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-652800000 {
 			opp-hz = /bits/ 64 <652800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-729600000 {
 			opp-hz = /bits/ 64 <729600000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <307200>;
 		};
 		opp-806400000 {
 			opp-hz = /bits/ 64 <806400000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <384000>;
 		};
 		opp-883200000 {
 			opp-hz = /bits/ 64 <883200000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <460800>;
 		};
 		opp-940800000 {
 			opp-hz = /bits/ 64 <940800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <537600>;
 		};
 		opp-1036800000 {
 			opp-hz = /bits/ 64 <1036800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <595200>;
 		};
 		opp-1113600000 {
 			opp-hz = /bits/ 64 <1113600000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <672000>;
 		};
 		opp-1190400000 {
 			opp-hz = /bits/ 64 <1190400000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <672000>;
 		};
 		opp-1248000000 {
 			opp-hz = /bits/ 64 <1248000000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <748800>;
 		};
 		opp-1324800000 {
 			opp-hz = /bits/ 64 <1324800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <825600>;
 		};
 		opp-1401600000 {
 			opp-hz = /bits/ 64 <1401600000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <902400>;
 		};
 		opp-1478400000 {
 			opp-hz = /bits/ 64 <1478400000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <979200>;
 		};
 		opp-1555200000 {
 			opp-hz = /bits/ 64 <1555200000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1056000>;
 		};
 		opp-1632000000 {
 			opp-hz = /bits/ 64 <1632000000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1190400>;
 		};
 		opp-1708800000 {
 			opp-hz = /bits/ 64 <1708800000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1228800>;
 		};
 		opp-1785600000 {
 			opp-hz = /bits/ 64 <1785600000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1305600>;
 		};
 		opp-1804800000 {
 			opp-hz = /bits/ 64 <1804800000>;
 			opp-supported-hw = <0xe>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1305600>;
 		};
 		opp-1824000000 {
 			opp-hz = /bits/ 64 <1824000000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1382400>;
 		};
 		opp-1900800000 {
 			opp-hz = /bits/ 64 <1900800000>;
 			opp-supported-hw = <0x4>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1305600>;
 		};
 		opp-1920000000 {
 			opp-hz = /bits/ 64 <1920000000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1459200>;
 		};
 		opp-1996800000 {
 			opp-hz = /bits/ 64 <1996800000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1593600>;
 		};
 		opp-2073600000 {
 			opp-hz = /bits/ 64 <2073600000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1593600>;
 		};
 		opp-2150400000 {
 			opp-hz = /bits/ 64 <2150400000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
+			opp-peak-kBps = <1593600>;
 		};
 	};
 
@@ -3549,6 +3599,7 @@ cbf: clock-controller@9a11000 {
 			reg = <0x09a11000 0x10000>;
 			clocks = <&rpmcc RPM_SMD_XO_A_CLK_SRC>, <&apcs_glb>;
 			#clock-cells = <0>;
+			#interconnect-cells = <1>;
 		};
 
 		intc: interrupt-controller@9bc0000 {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/4] interconnect: add clk-based icc provider support
  2023-05-12  0:13 ` [PATCH v6 2/4] interconnect: add clk-based icc provider support Dmitry Baryshkov
@ 2023-05-13  8:59   ` Konrad Dybcio
  2023-05-19 17:30   ` Nick Desaulniers
  1 sibling, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2023-05-13  8:59 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov
  Cc: linux-arm-msm, linux-clk, devicetree, Yassine Oudjana, linux-pm



On 12.05.2023 02:13, 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>
> ---
[...]

> +
> +	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;
I'm still not very into using 2 iterators and modifying one
on the flight, but I don't think I have any other issues with
this driver..

Some sort of a Mostly-Acked-by tag would be helpful here!

Konrad
> +	}
> +
> +	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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/4] interconnect: add clk-based icc provider support
  2023-05-12  0:13 ` [PATCH v6 2/4] interconnect: add clk-based icc provider support Dmitry Baryshkov
  2023-05-13  8:59   ` Konrad Dybcio
@ 2023-05-19 17:30   ` Nick Desaulniers
  2023-05-19 17:32     ` Nick Desaulniers
  1 sibling, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2023-05-19 17:30 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov, linux-arm-msm, linux-clk, devicetree,
	Yassine Oudjana, linux-pm, llvm

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
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/4] interconnect: add clk-based icc provider support
  2023-05-19 17:30   ` Nick Desaulniers
@ 2023-05-19 17:32     ` Nick Desaulniers
  2023-05-19 19:26       ` Dmitry Baryshkov
  0 siblings, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2023-05-19 17:32 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov, linux-arm-msm, linux-clk, devicetree,
	Yassine Oudjana, linux-pm, llvm

On Fri, May 19, 2023 at 10:30 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> 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

I also suspect these 2 additional errors are related to this series?
>> Error: ERROR: modpost: "icc_clk_register" [drivers/clk/qcom/clk-cbf-8996.ko] undefined!
>> Error: ERROR: modpost: "icc_clk_unregister" [drivers/clk/qcom/clk-cbf-8996.ko] undefined!

>
> 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
> >



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/4] interconnect: add clk-based icc provider support
  2023-05-19 17:32     ` Nick Desaulniers
@ 2023-05-19 19:26       ` Dmitry Baryshkov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 19:26 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Stephen Boyd,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	Georgi Djakov, linux-arm-msm, linux-clk, devicetree,
	Yassine Oudjana, linux-pm, llvm

On 19/05/2023 20:32, Nick Desaulniers wrote:
> On Fri, May 19, 2023 at 10:30 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
>>
>> 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
> 
> I also suspect these 2 additional errors are related to this series?
>>> Error: ERROR: modpost: "icc_clk_register" [drivers/clk/qcom/clk-cbf-8996.ko] undefined!
>>> Error: ERROR: modpost: "icc_clk_unregister" [drivers/clk/qcom/clk-cbf-8996.ko] undefined!

Ack, thanks for the report. I will send a fix in a few hours.

> 
>>
>> 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
>>>
> 
> 
> 

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 3/4] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq
  2023-05-12  0:13 ` [PATCH v6 3/4] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq Dmitry Baryshkov
@ 2023-06-10  0:18   ` Bjorn Andersson
  2023-06-10  7:26     ` Georgi Djakov
  0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Andersson @ 2023-06-10  0:18 UTC (permalink / raw)
  To: Dmitry Baryshkov, Georgi Djakov
  Cc: Andy Gross, Konrad Dybcio, Stephen Boyd, linux-arm-msm,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	linux-clk, devicetree, Yassine Oudjana, linux-pm

On Fri, May 12, 2023 at 03:13:33AM +0300, Dmitry Baryshkov wrote:
> Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth)
> according to CPU frequencies.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Georgi,

Dmitry tells me that you picked up the interconnect patches, I don't see
an immutable branch in your tree with them, but this patch has a build
time dependency on them. Could you please pick this through your tree as
well?

Acked-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> ---
>  drivers/clk/qcom/Kconfig        |  1 +
>  drivers/clk/qcom/clk-cbf-8996.c | 60 ++++++++++++++++++++++++++++++++-
>  2 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 12be3e2371b3..85869e7a9f16 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -48,6 +48,7 @@ config QCOM_CLK_APCS_MSM8916
>  config QCOM_CLK_APCC_MSM8996
>  	tristate "MSM8996 CPU Clock Controller"
>  	select QCOM_KRYO_L2_ACCESSORS
> +	select INTERCONNECT_CLK if INTERCONNECT
>  	depends on ARM64
>  	help
>  	  Support for the CPU clock controller on msm8996 devices.
> diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c
> index cfd567636f4e..1e23b734abb3 100644
> --- a/drivers/clk/qcom/clk-cbf-8996.c
> +++ b/drivers/clk/qcom/clk-cbf-8996.c
> @@ -5,11 +5,15 @@
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
> +#include <linux/interconnect-clk.h>
> +#include <linux/interconnect-provider.h>
>  #include <linux/of.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  
> +#include <dt-bindings/interconnect/qcom,msm8996-cbf.h>
> +
>  #include "clk-alpha-pll.h"
>  #include "clk-regmap.h"
>  
> @@ -223,6 +227,49 @@ static const struct regmap_config cbf_msm8996_regmap_config = {
>  	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
>  };
>  
> +#ifdef CONFIG_INTERCONNECT
> +
> +/* Random ID that doesn't clash with main qnoc and OSM */
> +#define CBF_MASTER_NODE 2000
> +
> +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev, struct clk_hw *cbf_hw)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct clk *clk = devm_clk_hw_get_clk(dev, cbf_hw, "cbf");
> +	const struct icc_clk_data data[] = {
> +		{ .clk = clk, .name = "cbf", },
> +	};
> +	struct icc_provider *provider;
> +
> +	provider = icc_clk_register(dev, CBF_MASTER_NODE, ARRAY_SIZE(data), data);
> +	if (IS_ERR(provider))
> +		return PTR_ERR(provider);
> +
> +	platform_set_drvdata(pdev, provider);
> +
> +	return 0;
> +}
> +
> +static int qcom_msm8996_cbf_icc_remove(struct platform_device *pdev)
> +{
> +	struct icc_provider *provider = platform_get_drvdata(pdev);
> +
> +	icc_clk_unregister(provider);
> +
> +	return 0;
> +}
> +#define qcom_msm8996_cbf_icc_sync_state icc_sync_state
> +#else
> +static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev,  struct clk_hw *cbf_hw)
> +{
> +	dev_warn(&pdev->dev, "CONFIG_INTERCONNECT is disabled, CBF clock is fixed\n");
> +
> +	return 0;
> +}
> +#define qcom_msm8996_cbf_icc_remove(pdev) (0)
> +#define qcom_msm8996_cbf_icc_sync_state NULL
> +#endif
> +
>  static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
>  {
>  	void __iomem *base;
> @@ -281,7 +328,16 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
> +	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
> +	if (ret)
> +		return ret;
> +
> +	return qcom_msm8996_cbf_icc_register(pdev, &cbf_mux.clkr.hw);
> +}
> +
> +static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
> +{
> +	return qcom_msm8996_cbf_icc_remove(pdev);
>  }
>  
>  static const struct of_device_id qcom_msm8996_cbf_match_table[] = {
> @@ -292,9 +348,11 @@ MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table);
>  
>  static struct platform_driver qcom_msm8996_cbf_driver = {
>  	.probe = qcom_msm8996_cbf_probe,
> +	.remove = qcom_msm8996_cbf_remove,
>  	.driver = {
>  		.name = "qcom-msm8996-cbf",
>  		.of_match_table = qcom_msm8996_cbf_match_table,
> +		.sync_state = qcom_msm8996_cbf_icc_sync_state,
>  	},
>  };
>  
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 3/4] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq
  2023-06-10  0:18   ` Bjorn Andersson
@ 2023-06-10  7:26     ` Georgi Djakov
  0 siblings, 0 replies; 13+ messages in thread
From: Georgi Djakov @ 2023-06-10  7:26 UTC (permalink / raw)
  To: Bjorn Andersson, Dmitry Baryshkov
  Cc: Andy Gross, Konrad Dybcio, Stephen Boyd, linux-arm-msm,
	Michael Turquette, Rob Herring, Krzysztof Kozlowski, Taniya Das,
	linux-clk, devicetree, Yassine Oudjana, linux-pm

On 10.06.23 3:18, Bjorn Andersson wrote:
> On Fri, May 12, 2023 at 03:13:33AM +0300, Dmitry Baryshkov wrote:
>> Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth)
>> according to CPU frequencies.
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> Georgi,
> 
> Dmitry tells me that you picked up the interconnect patches, I don't see
> an immutable branch in your tree with them, but this patch has a build
> time dependency on them. Could you please pick this through your tree as
> well?

It's done. Thanks a lot Bjorn!

BR,
Georgi

> Acked-by: Bjorn Andersson <andersson@kernel.org>
> 
> Regards,
> Bjorn
> 
>> ---
>>   drivers/clk/qcom/Kconfig        |  1 +
>>   drivers/clk/qcom/clk-cbf-8996.c | 60 ++++++++++++++++++++++++++++++++-
>>   2 files changed, 60 insertions(+), 1 deletion(-)
>>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: (subset) [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock
  2023-05-12  0:13 [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Dmitry Baryshkov
                   ` (3 preceding siblings ...)
  2023-05-12  0:13 ` [PATCH v6 4/4] arm64: dts: qcom: msm8996: " Dmitry Baryshkov
@ 2023-07-14  5:33 ` Bjorn Andersson
  4 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2023-07-14  5:33 UTC (permalink / raw)
  To: Andy Gross, Taniya Das, Michael Turquette, Georgi Djakov,
	Konrad Dybcio, Stephen Boyd, Krzysztof Kozlowski, Rob Herring,
	Dmitry Baryshkov
  Cc: linux-clk, linux-arm-msm, Yassine Oudjana, devicetree, linux-pm


On Fri, 12 May 2023 03:13:30 +0300, Dmitry Baryshkov wrote:
> On MSM8996 two CPU clusters are interconnected using the Core Bus
> Fabric (CBF). In order for the CPU clusters to function properly, it
> should be clocked following the core's frequencies to provide adequate
> bandwidth.
> 
> Register CBF as a clock (required for CPU to boot) and add a tiny
> interconnect layer on top of it to let cpufreq/opp scale the CBF clock.
> 
> [...]

Applied, thanks!

[4/4] arm64: dts: qcom: msm8996: scale CBF clock according to the CPUfreq
      commit: 8bb8688c1d73f21f413e4ea2a37fbbb90997f2bd

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: (subset) [PATCH v6 4/4] arm64: dts: qcom: msm8996: scale CBF clock according to the CPUfreq
  2023-05-12  0:13 ` [PATCH v6 4/4] arm64: dts: qcom: msm8996: " Dmitry Baryshkov
@ 2023-07-14  5:33   ` Bjorn Andersson
  0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2023-07-14  5:33 UTC (permalink / raw)
  To: Andy Gross, Konrad Dybcio, Stephen Boyd, Michael Turquette,
	Rob Herring, Krzysztof Kozlowski, Taniya Das, Georgi Djakov,
	Dmitry Baryshkov
  Cc: linux-arm-msm, linux-clk, devicetree, Yassine Oudjana, linux-pm


On Fri, 12 May 2023 03:13:34 +0300, Dmitry Baryshkov wrote:
> Turn CBF into the interconnect provider. Scale CBF frequency (bandwidth)
> according to CPU frequencies.
> 
> 

Applied, thanks!

[4/4] arm64: dts: qcom: msm8996: scale CBF clock according to the CPUfreq
      commit: 8bb8688c1d73f21f413e4ea2a37fbbb90997f2bd

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-07-14  5:30 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-12  0:13 [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Dmitry Baryshkov
2023-05-12  0:13 ` [PATCH v6 1/4] dt-bindings: interconnect/msm8996-cbf: add defines to be used by CBF Dmitry Baryshkov
2023-05-12  0:13 ` [PATCH v6 2/4] interconnect: add clk-based icc provider support Dmitry Baryshkov
2023-05-13  8:59   ` Konrad Dybcio
2023-05-19 17:30   ` Nick Desaulniers
2023-05-19 17:32     ` Nick Desaulniers
2023-05-19 19:26       ` Dmitry Baryshkov
2023-05-12  0:13 ` [PATCH v6 3/4] clk: qcom: cbf-msm8996: scale CBF clock according to the CPUfreq Dmitry Baryshkov
2023-06-10  0:18   ` Bjorn Andersson
2023-06-10  7:26     ` Georgi Djakov
2023-05-12  0:13 ` [PATCH v6 4/4] arm64: dts: qcom: msm8996: " Dmitry Baryshkov
2023-07-14  5:33   ` (subset) " Bjorn Andersson
2023-07-14  5:33 ` (subset) [PATCH v6 0/4] clk: qcom: msm8996: add support for the CBF clock Bjorn Andersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).