[PATCH v4 0/4] Add OSM L3 Interconnect Provider

[PATCH v4 0/4] Add OSM L3 Interconnect Provider

[Sibi Sankar]

This patch series aims to add Operating State Manager (OSM) L3
interconnect provider support on SDM845 and SC7180 SoCs to handle
bandwidth requests from CPU to scale L3 caches.

V4:
 * add SC7180 support
 * use icc_std_aggregate
 * use icc_nodes_remove
 * Fixup Evan's review comments.

v3:
 * switched the dt-bindings to dual-license
 * Rebased to linux-next

v2:
 * addressed review comments from Evan
 * dropped unused gpu icc node on SDM845 SoC

Sibi Sankar (4):
  dt-bindings: interconnect: Add OSM L3 DT bindings
  interconnect: qcom: Add OSM L3 interconnect provider support
  dt-bindings: interconnect: Add OSM L3 DT binding on SC7180
  interconnect: qcom: Add OSM L3 support on SC7180

 .../bindings/interconnect/qcom,osm-l3.yaml    |  62 ++++
 drivers/interconnect/qcom/Kconfig             |   7 +
 drivers/interconnect/qcom/Makefile            |   2 +
 drivers/interconnect/qcom/osm-l3.c            | 287 ++++++++++++++++++
 .../dt-bindings/interconnect/qcom,osm-l3.h    |  12 +
 5 files changed, 370 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
 create mode 100644 drivers/interconnect/qcom/osm-l3.c
 create mode 100644 include/dt-bindings/interconnect/qcom,osm-l3.h

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 1/4] dt-bindings: interconnect: Add OSM L3 DT bindings

[Sibi Sankar]

Add bindings for Operating State Manager (OSM) L3 interconnect provider
on SDM845 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 .../bindings/interconnect/qcom,osm-l3.yaml    | 61 +++++++++++++++++++
 .../dt-bindings/interconnect/qcom,osm-l3.h    | 12 ++++
 2 files changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
 create mode 100644 include/dt-bindings/interconnect/qcom,osm-l3.h

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
new file mode 100644
index 0000000000000..b4d46a1e92573
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/qcom,osm-l3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Operating State Manager (OSM) L3 Interconnect Provider
+
+maintainers:
+  - Sibi Sankar <sibis@codeaurora.org>
+
+description:
+  L3 cache bandwidth requirements on Qualcomm SoCs is serviced by the OSM.
+  The OSM L3 interconnect provider aggregates the L3 bandwidth requests
+  from CPU/GPU and relays it to the OSM.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sdm845-osm-l3
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: xo clock
+      - description: alternate clock
+
+  clock-names:
+    items:
+      - const: xo
+      - const: alternate
+
+  '#interconnect-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#interconnect-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #define GPLL0               165
+    #define RPMH_CXO_CLK        0
+
+    osm_l3: interconnect@17d41000 {
+      compatible = "qcom,sdm845-osm-l3";
+      reg = <0x17d41000 0x1400>;
+
+      clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
+      clock-names = "xo", "alternate";
+
+      #interconnect-cells = <1>;
+    };
diff --git a/include/dt-bindings/interconnect/qcom,osm-l3.h b/include/dt-bindings/interconnect/qcom,osm-l3.h
new file mode 100644
index 0000000000000..54858ff7674d7
--- /dev/null
+++ b/include/dt-bindings/interconnect/qcom,osm-l3.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_OSM_L3_H
+#define __DT_BINDINGS_INTERCONNECT_QCOM_OSM_L3_H
+
+#define MASTER_OSM_L3_APPS	0
+#define SLAVE_OSM_L3		1
+
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Sibi Sankar]

On some Qualcomm SoCs, Operating State Manager (OSM) controls the
resources of scaling L3 caches. Add a driver to handle bandwidth
requests to OSM L3 from CPU on SDM845 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/interconnect/qcom/Kconfig  |   7 +
 drivers/interconnect/qcom/Makefile |   2 +
 drivers/interconnect/qcom/osm-l3.c | 267 +++++++++++++++++++++++++++++
 3 files changed, 276 insertions(+)
 create mode 100644 drivers/interconnect/qcom/osm-l3.c

diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
index a9bbbdf7400f9..b94d28e7bf700 100644
--- a/drivers/interconnect/qcom/Kconfig
+++ b/drivers/interconnect/qcom/Kconfig
@@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
 	 This is a driver for the Qualcomm Network-on-Chip on msm8974-based
 	 platforms.
 
+config INTERCONNECT_QCOM_OSM_L3
+	tristate "Qualcomm OSM L3 interconnect driver"
+	depends on INTERCONNECT_QCOM || COMPILE_TEST
+	help
+	  Say y here to support the Operating State Manager (OSM) interconnect
+	  driver which controls the scaling of L3 caches on Qualcomm SoCs.
+
 config INTERCONNECT_QCOM_QCS404
 	tristate "Qualcomm QCS404 interconnect driver"
 	depends on INTERCONNECT_QCOM
diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
index 55ec3c5c89dbd..89fecbd1257c7 100644
--- a/drivers/interconnect/qcom/Makefile
+++ b/drivers/interconnect/qcom/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
+icc-osm-l3-objs				:= osm-l3.o
 qnoc-msm8974-objs			:= msm8974.o
 qnoc-qcs404-objs			:= qcs404.o
 qnoc-sc7180-objs			:= sc7180.o
@@ -12,6 +13,7 @@ icc-smd-rpm-objs			:= smd-rpm.o
 obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
 obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
+obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
 obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
 obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
 obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
new file mode 100644
index 0000000000000..7fde53c70081e
--- /dev/null
+++ b/drivers/interconnect/qcom/osm-l3.c
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, The Linux Foundation. All rights reserved.
+ *
+ */
+
+#include <dt-bindings/interconnect/qcom,osm-l3.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/interconnect-provider.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#define LUT_MAX_ENTRIES			40U
+#define LUT_SRC				GENMASK(31, 30)
+#define LUT_L_VAL			GENMASK(7, 0)
+#define LUT_ROW_SIZE			32
+#define CLK_HW_DIV			2
+
+/* Register offsets */
+#define REG_ENABLE			0x0
+#define REG_FREQ_LUT			0x110
+#define REG_PERF_STATE			0x920
+
+#define OSM_L3_MAX_LINKS		1
+#define SDM845_MAX_RSC_NODES		130
+
+#define to_qcom_provider(_provider) \
+	container_of(_provider, struct qcom_osm_l3_icc_provider, provider)
+
+enum {
+	SDM845_MASTER_OSM_L3_APPS = SDM845_MAX_RSC_NODES + 1,
+	SDM845_SLAVE_OSM_L3,
+};
+
+struct qcom_osm_l3_icc_provider {
+	void __iomem *base;
+	unsigned int max_state;
+	unsigned long lut_tables[LUT_MAX_ENTRIES];
+	struct icc_provider provider;
+};
+
+/**
+ * struct qcom_icc_node - Qualcomm specific interconnect nodes
+ * @name: the node name used in debugfs
+ * @links: an array of nodes where we can go next while traversing
+ * @id: a unique node identifier
+ * @num_links: the total number of @links
+ * @buswidth: width of the interconnect between a node and the bus
+ */
+struct qcom_icc_node {
+	const char *name;
+	u16 links[OSM_L3_MAX_LINKS];
+	u16 id;
+	u16 num_links;
+	u16 buswidth;
+};
+
+struct qcom_icc_desc {
+	struct qcom_icc_node **nodes;
+	size_t num_nodes;
+};
+
+#define DEFINE_QNODE(_name, _id, _buswidth, ...)			\
+		static struct qcom_icc_node _name = {			\
+		.name = #_name,						\
+		.id = _id,						\
+		.buswidth = _buswidth,					\
+		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\
+		.links = { __VA_ARGS__ },				\
+	}
+
+DEFINE_QNODE(sdm845_osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
+DEFINE_QNODE(sdm845_osm_l3, SDM845_SLAVE_OSM_L3, 16);
+
+static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
+	[MASTER_OSM_L3_APPS] = &sdm845_osm_apps_l3,
+	[SLAVE_OSM_L3] = &sdm845_osm_l3,
+};
+
+static struct qcom_icc_desc sdm845_icc_osm_l3 = {
+	.nodes = sdm845_osm_l3_nodes,
+	.num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
+};
+
+static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	struct qcom_osm_l3_icc_provider *qp;
+	struct icc_provider *provider;
+	struct qcom_icc_node *qn;
+	struct icc_node *n;
+	unsigned int index;
+	u32 agg_peak = 0;
+	u32 agg_avg = 0;
+	u64 rate;
+
+	qn = src->data;
+	provider = src->provider;
+	qp = to_qcom_provider(provider);
+
+	list_for_each_entry(n, &provider->nodes, node_list)
+		provider->aggregate(n, 0, n->avg_bw, n->peak_bw,
+				    &agg_avg, &agg_peak);
+
+	rate = max(agg_avg, agg_peak);
+	rate = icc_units_to_bps(rate);
+	do_div(rate, qn->buswidth);
+
+	for (index = 0; index < qp->max_state - 1; index++) {
+		if (qp->lut_tables[index] >= rate)
+			break;
+	}
+
+	writel_relaxed(index, qp->base + REG_PERF_STATE);
+
+	return 0;
+}
+
+static int qcom_osm_l3_remove(struct platform_device *pdev)
+{
+	struct qcom_osm_l3_icc_provider *qp = platform_get_drvdata(pdev);
+
+	icc_nodes_remove(&qp->provider);
+	return icc_provider_del(&qp->provider);
+}
+
+static int qcom_osm_l3_probe(struct platform_device *pdev)
+{
+	u32 info, src, lval, i, prev_freq = 0, freq;
+	static unsigned long hw_rate, xo_rate;
+	struct qcom_osm_l3_icc_provider *qp;
+	const struct qcom_icc_desc *desc;
+	struct icc_onecell_data *data;
+	struct icc_provider *provider;
+	struct qcom_icc_node **qnodes;
+	struct icc_node *node;
+	size_t num_nodes;
+	struct clk *clk;
+	int ret;
+
+	clk = clk_get(&pdev->dev, "xo");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	xo_rate = clk_get_rate(clk);
+	clk_put(clk);
+
+	clk = clk_get(&pdev->dev, "alternate");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
+	clk_put(clk);
+
+	qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
+	if (!qp)
+		return -ENOMEM;
+
+	qp->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(qp->base))
+		return PTR_ERR(qp->base);
+
+	/* HW should be in enabled state to proceed */
+	if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
+		dev_err(&pdev->dev, "error hardware not enabled\n");
+		return -ENODEV;
+	}
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		info = readl_relaxed(qp->base + REG_FREQ_LUT +
+				     i * LUT_ROW_SIZE);
+		src = FIELD_GET(LUT_SRC, info);
+		lval = FIELD_GET(LUT_L_VAL, info);
+		if (src)
+			freq = xo_rate * lval;
+		else
+			freq = hw_rate;
+
+		/* Two of the same frequencies signify end of table */
+		if (i > 0 && prev_freq == freq)
+			break;
+
+		dev_dbg(&pdev->dev, "index=%d freq=%d\n", i, freq);
+
+		qp->lut_tables[i] = freq;
+		prev_freq = freq;
+	}
+	qp->max_state = i;
+
+	desc = of_device_get_match_data(&pdev->dev);
+	if (!desc)
+		return -EINVAL;
+
+	qnodes = desc->nodes;
+	num_nodes = desc->num_nodes;
+
+	data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	provider = &qp->provider;
+	provider->dev = &pdev->dev;
+	provider->set = qcom_icc_set;
+	provider->aggregate = icc_std_aggregate;
+	provider->xlate = of_icc_xlate_onecell;
+	INIT_LIST_HEAD(&provider->nodes);
+	provider->data = data;
+
+	ret = icc_provider_add(provider);
+	if (ret) {
+		dev_err(&pdev->dev, "error adding interconnect provider\n");
+		return ret;
+	}
+
+	for (i = 0; i < num_nodes; i++) {
+		size_t j;
+
+		node = icc_node_create(qnodes[i]->id);
+		if (IS_ERR(node)) {
+			ret = PTR_ERR(node);
+			goto err;
+		}
+
+		node->name = qnodes[i]->name;
+		node->data = qnodes[i];
+		icc_node_add(node, provider);
+
+		dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
+			qnodes[i]->name, node->id);
+
+		for (j = 0; j < qnodes[i]->num_links; j++)
+			icc_link_create(node, qnodes[i]->links[j]);
+
+		data->nodes[i] = node;
+	}
+	data->num_nodes = num_nodes;
+
+	platform_set_drvdata(pdev, qp);
+
+	return ret;
+err:
+	qcom_osm_l3_remove(pdev);
+	return ret;
+}
+
+static const struct of_device_id osm_l3_of_match[] = {
+	{ .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_icc_osm_l3 },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, osm_l3_of_match);
+
+static struct platform_driver osm_l3_driver = {
+	.probe = qcom_osm_l3_probe,
+	.remove = qcom_osm_l3_remove,
+	.driver = {
+		.name = "osm-l3",
+		.of_match_table = osm_l3_of_match,
+	},
+};
+module_platform_driver(osm_l3_driver);
+
+MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 3/4] dt-bindings: interconnect: Add OSM L3 DT binding on SC7180

[Sibi Sankar]

Add OSM L3 interconnect provider binding on SC7180 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
index b4d46a1e92573..91f70c9067d12 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
@@ -17,6 +17,7 @@ description:
 properties:
   compatible:
     enum:
+      - qcom,sc7180-osm-l3
       - qcom,sdm845-osm-l3
 
   reg:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 4/4] interconnect: qcom: Add OSM L3 support on SC7180

[Sibi Sankar]

Add Operating State Manager (OSM) L3 interconnect provider support on
SC7180 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/interconnect/qcom/osm-l3.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
index 7fde53c70081e..df2cec3fa2913 100644
--- a/drivers/interconnect/qcom/osm-l3.c
+++ b/drivers/interconnect/qcom/osm-l3.c
@@ -28,6 +28,7 @@
 
 #define OSM_L3_MAX_LINKS		1
 #define SDM845_MAX_RSC_NODES		130
+#define SC7180_MAX_RSC_NODES		137
 
 #define to_qcom_provider(_provider) \
 	container_of(_provider, struct qcom_osm_l3_icc_provider, provider)
@@ -37,6 +38,11 @@ enum {
 	SDM845_SLAVE_OSM_L3,
 };
 
+enum {
+	SC7180_MASTER_OSM_L3_APPS = SC7180_MAX_RSC_NODES + 1,
+	SC7180_SLAVE_OSM_L3,
+};
+
 struct qcom_osm_l3_icc_provider {
 	void __iomem *base;
 	unsigned int max_state;
@@ -87,6 +93,19 @@ static struct qcom_icc_desc sdm845_icc_osm_l3 = {
 	.num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
 };
 
+DEFINE_QNODE(sc7180_osm_apps_l3, SC7180_MASTER_OSM_L3_APPS, 16, SC7180_SLAVE_OSM_L3);
+DEFINE_QNODE(sc7180_osm_l3, SC7180_SLAVE_OSM_L3, 16);
+
+static struct qcom_icc_node *sc7180_osm_l3_nodes[] = {
+	[MASTER_OSM_L3_APPS] = &sc7180_osm_apps_l3,
+	[SLAVE_OSM_L3] = &sc7180_osm_l3,
+};
+
+static struct qcom_icc_desc sc7180_icc_osm_l3 = {
+	.nodes = sc7180_osm_l3_nodes,
+	.num_nodes = ARRAY_SIZE(sc7180_osm_l3_nodes),
+};
+
 static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
 {
 	struct qcom_osm_l3_icc_provider *qp;
@@ -248,6 +267,7 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id osm_l3_of_match[] = {
+	{ .compatible = "qcom,sc7180-osm-l3", .data = &sc7180_icc_osm_l3 },
 	{ .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_icc_osm_l3 },
 	{ },
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 1/4] dt-bindings: interconnect: Add OSM L3 DT bindings

[Rob Herring]

On Fri, 10 Jan 2020 02:42:12 +0530, Sibi Sankar wrote:
> Add bindings for Operating State Manager (OSM) L3 interconnect provider
> on SDM845 SoCs.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  .../bindings/interconnect/qcom,osm-l3.yaml    | 61 +++++++++++++++++++
>  .../dt-bindings/interconnect/qcom,osm-l3.h    | 12 ++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
>  create mode 100644 include/dt-bindings/interconnect/qcom,osm-l3.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 3/4] dt-bindings: interconnect: Add OSM L3 DT binding on SC7180

[Rob Herring]

On Fri, 10 Jan 2020 02:42:14 +0530, Sibi Sankar wrote:
> Add OSM L3 interconnect provider binding on SC7180 SoCs.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Evan Green]

On Thu, Jan 9, 2020 at 1:12 PM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> resources of scaling L3 caches. Add a driver to handle bandwidth
> requests to OSM L3 from CPU on SDM845 SoCs.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/interconnect/qcom/Kconfig  |   7 +
>  drivers/interconnect/qcom/Makefile |   2 +
>  drivers/interconnect/qcom/osm-l3.c | 267 +++++++++++++++++++++++++++++
>  3 files changed, 276 insertions(+)
>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
>
> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> index a9bbbdf7400f9..b94d28e7bf700 100644
> --- a/drivers/interconnect/qcom/Kconfig
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
>          This is a driver for the Qualcomm Network-on-Chip on msm8974-based
>          platforms.
>
> +config INTERCONNECT_QCOM_OSM_L3
> +       tristate "Qualcomm OSM L3 interconnect driver"
> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
> +       help
> +         Say y here to support the Operating State Manager (OSM) interconnect
> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
> +
>  config INTERCONNECT_QCOM_QCS404
>         tristate "Qualcomm QCS404 interconnect driver"
>         depends on INTERCONNECT_QCOM
> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> index 55ec3c5c89dbd..89fecbd1257c7 100644
> --- a/drivers/interconnect/qcom/Makefile
> +++ b/drivers/interconnect/qcom/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> +icc-osm-l3-objs                                := osm-l3.o
>  qnoc-msm8974-objs                      := msm8974.o
>  qnoc-qcs404-objs                       := qcs404.o
>  qnoc-sc7180-objs                       := sc7180.o
> @@ -12,6 +13,7 @@ icc-smd-rpm-objs                      := smd-rpm.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> new file mode 100644
> index 0000000000000..7fde53c70081e
> --- /dev/null
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> + *
> + */
> +
> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#define LUT_MAX_ENTRIES                        40U
> +#define LUT_SRC                                GENMASK(31, 30)
> +#define LUT_L_VAL                      GENMASK(7, 0)
> +#define LUT_ROW_SIZE                   32
> +#define CLK_HW_DIV                     2
> +
> +/* Register offsets */
> +#define REG_ENABLE                     0x0
> +#define REG_FREQ_LUT                   0x110
> +#define REG_PERF_STATE                 0x920
> +
> +#define OSM_L3_MAX_LINKS               1
> +#define SDM845_MAX_RSC_NODES           130

I'm nervous this define is going to fall out of date with
qcom,sdm845.h. I'm worried someone will end up adding a few more nodes
that were always there but previously hidden from Linux. Can we put
this define in include/dt-bindings/interconnect/qcom,sdm845.h, so at
least when that happens they'll come face to face with this define?
The same comment goes for the SC7180 define in patch 4.

On second thought, this trick only works once. Are we sure there
aren't going to be other drivers that might want to tag on
interconnect nodes as well? How about instead we just add the enum
values below in qcom,sdm845.h as defines?

-Evan

Re: [PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Sibi Sankar]

Hey Evan,

Thanks for the review!

On 2020-01-22 03:03, Evan Green wrote:
> On Thu, Jan 9, 2020 at 1:12 PM Sibi Sankar <sibis@codeaurora.org> 
> wrote:
>> 
>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
>> resources of scaling L3 caches. Add a driver to handle bandwidth
>> requests to OSM L3 from CPU on SDM845 SoCs.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>  drivers/interconnect/qcom/Kconfig  |   7 +
>>  drivers/interconnect/qcom/Makefile |   2 +
>>  drivers/interconnect/qcom/osm-l3.c | 267 
>> +++++++++++++++++++++++++++++
>>  3 files changed, 276 insertions(+)
>>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
>> 
>> diff --git a/drivers/interconnect/qcom/Kconfig 
>> b/drivers/interconnect/qcom/Kconfig
>> index a9bbbdf7400f9..b94d28e7bf700 100644
>> --- a/drivers/interconnect/qcom/Kconfig
>> +++ b/drivers/interconnect/qcom/Kconfig
>> @@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
>>          This is a driver for the Qualcomm Network-on-Chip on 
>> msm8974-based
>>          platforms.
>> 
>> +config INTERCONNECT_QCOM_OSM_L3
>> +       tristate "Qualcomm OSM L3 interconnect driver"
>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
>> +       help
>> +         Say y here to support the Operating State Manager (OSM) 
>> interconnect
>> +         driver which controls the scaling of L3 caches on Qualcomm 
>> SoCs.
>> +
>>  config INTERCONNECT_QCOM_QCS404
>>         tristate "Qualcomm QCS404 interconnect driver"
>>         depends on INTERCONNECT_QCOM
>> diff --git a/drivers/interconnect/qcom/Makefile 
>> b/drivers/interconnect/qcom/Makefile
>> index 55ec3c5c89dbd..89fecbd1257c7 100644
>> --- a/drivers/interconnect/qcom/Makefile
>> +++ b/drivers/interconnect/qcom/Makefile
>> @@ -1,5 +1,6 @@
>>  # SPDX-License-Identifier: GPL-2.0
>> 
>> +icc-osm-l3-objs                                := osm-l3.o
>>  qnoc-msm8974-objs                      := msm8974.o
>>  qnoc-qcs404-objs                       := qcs404.o
>>  qnoc-sc7180-objs                       := sc7180.o
>> @@ -12,6 +13,7 @@ icc-smd-rpm-objs                      := smd-rpm.o
>>  obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>>  obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
>>  obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
>>  obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
>> diff --git a/drivers/interconnect/qcom/osm-l3.c 
>> b/drivers/interconnect/qcom/osm-l3.c
>> new file mode 100644
>> index 0000000000000..7fde53c70081e
>> --- /dev/null
>> +++ b/drivers/interconnect/qcom/osm-l3.c
>> @@ -0,0 +1,267 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
>> + *
>> + */
>> +
>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/interconnect-provider.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define LUT_MAX_ENTRIES                        40U
>> +#define LUT_SRC                                GENMASK(31, 30)
>> +#define LUT_L_VAL                      GENMASK(7, 0)
>> +#define LUT_ROW_SIZE                   32
>> +#define CLK_HW_DIV                     2
>> +
>> +/* Register offsets */
>> +#define REG_ENABLE                     0x0
>> +#define REG_FREQ_LUT                   0x110
>> +#define REG_PERF_STATE                 0x920
>> +
>> +#define OSM_L3_MAX_LINKS               1
>> +#define SDM845_MAX_RSC_NODES           130
> 
> I'm nervous this define is going to fall out of date with
> qcom,sdm845.h. I'm worried someone will end up adding a few more nodes
> that were always there but previously hidden from Linux. Can we put
> this define in include/dt-bindings/interconnect/qcom,sdm845.h, so at
> least when that happens they'll come face to face with this define?
> The same comment goes for the SC7180 define in patch 4.

Yeah both solution require manual
intervention how about we just go
with what I proposed below.

> 
> On second thought, this trick only works once. Are we sure there
> aren't going to be other drivers that might want to tag on
> interconnect nodes as well? How about instead we just add the enum
> values below in qcom,sdm845.h as defines?

Georgi/Evan,
Since qcom,sdm845.h is specific to
bindings shouldn't I just create a
.h file with all the enums so that
it can used across all icc providers
on SDM845?

> 
> -Evan

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Georgi Djakov]

Hi Sibi,

Thanks for working on this!

On 1/9/20 23:12, Sibi Sankar wrote:
> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> resources of scaling L3 caches. Add a driver to handle bandwidth
> requests to OSM L3 from CPU on SDM845 SoCs.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/interconnect/qcom/Kconfig  |   7 +
>  drivers/interconnect/qcom/Makefile |   2 +
>  drivers/interconnect/qcom/osm-l3.c | 267 +++++++++++++++++++++++++++++
>  3 files changed, 276 insertions(+)
>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
> 
> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> index a9bbbdf7400f9..b94d28e7bf700 100644
> --- a/drivers/interconnect/qcom/Kconfig
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
>  	 This is a driver for the Qualcomm Network-on-Chip on msm8974-based
>  	 platforms.
>  
> +config INTERCONNECT_QCOM_OSM_L3
> +	tristate "Qualcomm OSM L3 interconnect driver"
> +	depends on INTERCONNECT_QCOM || COMPILE_TEST
> +	help
> +	  Say y here to support the Operating State Manager (OSM) interconnect
> +	  driver which controls the scaling of L3 caches on Qualcomm SoCs.
> +
>  config INTERCONNECT_QCOM_QCS404
>  	tristate "Qualcomm QCS404 interconnect driver"
>  	depends on INTERCONNECT_QCOM
> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> index 55ec3c5c89dbd..89fecbd1257c7 100644
> --- a/drivers/interconnect/qcom/Makefile
> +++ b/drivers/interconnect/qcom/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> +icc-osm-l3-objs				:= osm-l3.o
>  qnoc-msm8974-objs			:= msm8974.o
>  qnoc-qcs404-objs			:= qcs404.o
>  qnoc-sc7180-objs			:= sc7180.o
> @@ -12,6 +13,7 @@ icc-smd-rpm-objs			:= smd-rpm.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> new file mode 100644
> index 0000000000000..7fde53c70081e
> --- /dev/null
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.

You will need to update the year.

> + *
> + */
> +
> +#include <dt-bindings/interconnect/qcom,osm-l3.h>

Please move this below the other linux/ headers.

> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>

Is this used?

> +#include <linux/platform_device.h>
> +
> +#define LUT_MAX_ENTRIES			40U
> +#define LUT_SRC				GENMASK(31, 30)
> +#define LUT_L_VAL			GENMASK(7, 0)
> +#define LUT_ROW_SIZE			32
> +#define CLK_HW_DIV			2
> +
> +/* Register offsets */
> +#define REG_ENABLE			0x0
> +#define REG_FREQ_LUT			0x110
> +#define REG_PERF_STATE			0x920
> +
> +#define OSM_L3_MAX_LINKS		1
> +#define SDM845_MAX_RSC_NODES		130

This looks fragile.

> +
> +#define to_qcom_provider(_provider) \
> +	container_of(_provider, struct qcom_osm_l3_icc_provider, provider)
> +
> +enum {
> +	SDM845_MASTER_OSM_L3_APPS = SDM845_MAX_RSC_NODES + 1,
> +	SDM845_SLAVE_OSM_L3,
> +};
> +
> +struct qcom_osm_l3_icc_provider {
> +	void __iomem *base;
> +	unsigned int max_state;
> +	unsigned long lut_tables[LUT_MAX_ENTRIES];
> +	struct icc_provider provider;
> +};
> +
> +/**
> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
> + * @name: the node name used in debugfs
> + * @links: an array of nodes where we can go next while traversing
> + * @id: a unique node identifier
> + * @num_links: the total number of @links
> + * @buswidth: width of the interconnect between a node and the bus
> + */
> +struct qcom_icc_node {
> +	const char *name;
> +	u16 links[OSM_L3_MAX_LINKS];
> +	u16 id;
> +	u16 num_links;
> +	u16 buswidth;
> +};
> +
> +struct qcom_icc_desc {
> +	struct qcom_icc_node **nodes;
> +	size_t num_nodes;
> +};
> +
> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)			\
> +		static struct qcom_icc_node _name = {			\
> +		.name = #_name,						\
> +		.id = _id,						\
> +		.buswidth = _buswidth,					\
> +		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\
> +		.links = { __VA_ARGS__ },				\
> +	}
> +
> +DEFINE_QNODE(sdm845_osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
> +DEFINE_QNODE(sdm845_osm_l3, SDM845_SLAVE_OSM_L3, 16);
> +
> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
> +	[MASTER_OSM_L3_APPS] = &sdm845_osm_apps_l3,
> +	[SLAVE_OSM_L3] = &sdm845_osm_l3,
> +};
> +
> +static struct qcom_icc_desc sdm845_icc_osm_l3 = {
> +	.nodes = sdm845_osm_l3_nodes,
> +	.num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
> +};
> +
> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> +	struct qcom_osm_l3_icc_provider *qp;
> +	struct icc_provider *provider;
> +	struct qcom_icc_node *qn;
> +	struct icc_node *n;
> +	unsigned int index;
> +	u32 agg_peak = 0;
> +	u32 agg_avg = 0;
> +	u64 rate;
> +
> +	qn = src->data;
> +	provider = src->provider;
> +	qp = to_qcom_provider(provider);
> +
> +	list_for_each_entry(n, &provider->nodes, node_list)
> +		provider->aggregate(n, 0, n->avg_bw, n->peak_bw,
> +				    &agg_avg, &agg_peak);
> +
> +	rate = max(agg_avg, agg_peak);
> +	rate = icc_units_to_bps(rate);
> +	do_div(rate, qn->buswidth);
> +
> +	for (index = 0; index < qp->max_state - 1; index++) {
> +		if (qp->lut_tables[index] >= rate)
> +			break;
> +	}
> +
> +	writel_relaxed(index, qp->base + REG_PERF_STATE);
> +
> +	return 0;
> +}
> +
> +static int qcom_osm_l3_remove(struct platform_device *pdev)
> +{
> +	struct qcom_osm_l3_icc_provider *qp = platform_get_drvdata(pdev);
> +
> +	icc_nodes_remove(&qp->provider);
> +	return icc_provider_del(&qp->provider);
> +}
> +
> +static int qcom_osm_l3_probe(struct platform_device *pdev)
> +{
> +	u32 info, src, lval, i, prev_freq = 0, freq;
> +	static unsigned long hw_rate, xo_rate;
> +	struct qcom_osm_l3_icc_provider *qp;
> +	const struct qcom_icc_desc *desc;
> +	struct icc_onecell_data *data;
> +	struct icc_provider *provider;
> +	struct qcom_icc_node **qnodes;
> +	struct icc_node *node;
> +	size_t num_nodes;
> +	struct clk *clk;
> +	int ret;
> +
> +	clk = clk_get(&pdev->dev, "xo");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	xo_rate = clk_get_rate(clk);
> +	clk_put(clk);
> +
> +	clk = clk_get(&pdev->dev, "alternate");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> +	clk_put(clk);
> +
> +	qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> +	if (!qp)
> +		return -ENOMEM;
> +
> +	qp->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(qp->base))
> +		return PTR_ERR(qp->base);
> +
> +	/* HW should be in enabled state to proceed */
> +	if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
> +		dev_err(&pdev->dev, "error hardware not enabled\n");
> +		return -ENODEV;
> +	}
> +
> +	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> +		info = readl_relaxed(qp->base + REG_FREQ_LUT +
> +				     i * LUT_ROW_SIZE);
> +		src = FIELD_GET(LUT_SRC, info);
> +		lval = FIELD_GET(LUT_L_VAL, info);
> +		if (src)
> +			freq = xo_rate * lval;
> +		else
> +			freq = hw_rate;
> +
> +		/* Two of the same frequencies signify end of table */
> +		if (i > 0 && prev_freq == freq)
> +			break;
> +
> +		dev_dbg(&pdev->dev, "index=%d freq=%d\n", i, freq);
> +
> +		qp->lut_tables[i] = freq;
> +		prev_freq = freq;
> +	}
> +	qp->max_state = i;
> +
> +	desc = of_device_get_match_data(&pdev->dev);
> +	if (!desc)
> +		return -EINVAL;
> +
> +	qnodes = desc->nodes;
> +	num_nodes = desc->num_nodes;
> +
> +	data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	provider = &qp->provider;
> +	provider->dev = &pdev->dev;
> +	provider->set = qcom_icc_set;
> +	provider->aggregate = icc_std_aggregate;
> +	provider->xlate = of_icc_xlate_onecell;
> +	INIT_LIST_HEAD(&provider->nodes);
> +	provider->data = data;
> +
> +	ret = icc_provider_add(provider);
> +	if (ret) {
> +		dev_err(&pdev->dev, "error adding interconnect provider\n");
> +		return ret;
> +	}
> +
> +	for (i = 0; i < num_nodes; i++) {
> +		size_t j;
> +
> +		node = icc_node_create(qnodes[i]->id);
> +		if (IS_ERR(node)) {
> +			ret = PTR_ERR(node);
> +			goto err;
> +		}
> +
> +		node->name = qnodes[i]->name;
> +		node->data = qnodes[i];
> +		icc_node_add(node, provider);
> +
> +		dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
> +			qnodes[i]->name, node->id);

Not sure how useful is this, but maybe it would be more appropriate to move it
to the framework instead of duplicating it in all the drivers. Please remove it.

> +
> +		for (j = 0; j < qnodes[i]->num_links; j++)
> +			icc_link_create(node, qnodes[i]->links[j]);
> +
> +		data->nodes[i] = node;
> +	}
> +	data->num_nodes = num_nodes;
> +
> +	platform_set_drvdata(pdev, qp);
> +
> +	return ret;

Just return 0.

> +err:
> +	qcom_osm_l3_remove(pdev);

I am afraid that this will not work. This function is using
platform_get_drvdata(), but the data is set after all the nodes
and links are created.

> +	return ret;
> +}
> +
> +static const struct of_device_id osm_l3_of_match[] = {
> +	{ .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_icc_osm_l3 },
> +	{ },

The comma is not needed.

> +};
> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
> +
> +static struct platform_driver osm_l3_driver = {
> +	.probe = qcom_osm_l3_probe,
> +	.remove = qcom_osm_l3_remove,
> +	.driver = {
> +		.name = "osm-l3",
> +		.of_match_table = osm_l3_of_match,
> +	},
> +};
> +module_platform_driver(osm_l3_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
> +MODULE_LICENSE("GPL v2");
> 

Thanks,
Georgi

Re: [PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Georgi Djakov]

On 1/22/20 08:45, Sibi Sankar wrote:
> Hey Evan,
> 
> Thanks for the review!
> 
> On 2020-01-22 03:03, Evan Green wrote:
>> On Thu, Jan 9, 2020 at 1:12 PM Sibi Sankar <sibis@codeaurora.org> wrote:
>>>
>>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
>>> resources of scaling L3 caches. Add a driver to handle bandwidth
>>> requests to OSM L3 from CPU on SDM845 SoCs.
>>>
>>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>>> ---
>>>  drivers/interconnect/qcom/Kconfig  |   7 +
>>>  drivers/interconnect/qcom/Makefile |   2 +
>>>  drivers/interconnect/qcom/osm-l3.c | 267 +++++++++++++++++++++++++++++
>>>  3 files changed, 276 insertions(+)
>>>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
>>>
>>> diff --git a/drivers/interconnect/qcom/Kconfig
>>> b/drivers/interconnect/qcom/Kconfig
>>> index a9bbbdf7400f9..b94d28e7bf700 100644
>>> --- a/drivers/interconnect/qcom/Kconfig
>>> +++ b/drivers/interconnect/qcom/Kconfig
>>> @@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
>>>          This is a driver for the Qualcomm Network-on-Chip on msm8974-based
>>>          platforms.
>>>
>>> +config INTERCONNECT_QCOM_OSM_L3
>>> +       tristate "Qualcomm OSM L3 interconnect driver"
>>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
>>> +       help
>>> +         Say y here to support the Operating State Manager (OSM) interconnect
>>> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
>>> +
>>>  config INTERCONNECT_QCOM_QCS404
>>>         tristate "Qualcomm QCS404 interconnect driver"
>>>         depends on INTERCONNECT_QCOM
>>> diff --git a/drivers/interconnect/qcom/Makefile
>>> b/drivers/interconnect/qcom/Makefile
>>> index 55ec3c5c89dbd..89fecbd1257c7 100644
>>> --- a/drivers/interconnect/qcom/Makefile
>>> +++ b/drivers/interconnect/qcom/Makefile
>>> @@ -1,5 +1,6 @@
>>>  # SPDX-License-Identifier: GPL-2.0
>>>
>>> +icc-osm-l3-objs                                := osm-l3.o
>>>  qnoc-msm8974-objs                      := msm8974.o
>>>  qnoc-qcs404-objs                       := qcs404.o
>>>  qnoc-sc7180-objs                       := sc7180.o
>>> @@ -12,6 +13,7 @@ icc-smd-rpm-objs                      := smd-rpm.o
>>>  obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
>>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
>>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
>>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>>>  obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
>>>  obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
>>>  obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
>>> diff --git a/drivers/interconnect/qcom/osm-l3.c
>>> b/drivers/interconnect/qcom/osm-l3.c
>>> new file mode 100644
>>> index 0000000000000..7fde53c70081e
>>> --- /dev/null
>>> +++ b/drivers/interconnect/qcom/osm-l3.c
>>> @@ -0,0 +1,267 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
>>> + *
>>> + */
>>> +
>>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
>>> +#include <linux/bitfield.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/interconnect-provider.h>
>>> +#include <linux/io.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define LUT_MAX_ENTRIES                        40U
>>> +#define LUT_SRC                                GENMASK(31, 30)
>>> +#define LUT_L_VAL                      GENMASK(7, 0)
>>> +#define LUT_ROW_SIZE                   32
>>> +#define CLK_HW_DIV                     2
>>> +
>>> +/* Register offsets */
>>> +#define REG_ENABLE                     0x0
>>> +#define REG_FREQ_LUT                   0x110
>>> +#define REG_PERF_STATE                 0x920
>>> +
>>> +#define OSM_L3_MAX_LINKS               1
>>> +#define SDM845_MAX_RSC_NODES           130
>>
>> I'm nervous this define is going to fall out of date with
>> qcom,sdm845.h. I'm worried someone will end up adding a few more nodes
>> that were always there but previously hidden from Linux. Can we put
>> this define in include/dt-bindings/interconnect/qcom,sdm845.h, so at
>> least when that happens they'll come face to face with this define?
>> The same comment goes for the SC7180 define in patch 4.
> 
> Yeah both solution require manual
> intervention how about we just go
> with what I proposed below.
> 
>>
>> On second thought, this trick only works once. Are we sure there
>> aren't going to be other drivers that might want to tag on
>> interconnect nodes as well? How about instead we just add the enum
>> values below in qcom,sdm845.h as defines?
> 
> Georgi/Evan,
> Since qcom,sdm845.h is specific to
> bindings shouldn't I just create a
> .h file with all the enums so that
> it can used across all icc providers
> on SDM845?

This sounds good to me, unless Evan has any objections.

Thanks,
Georgi

Re: [PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Evan Green]

On Wed, Jan 22, 2020 at 12:20 AM Georgi Djakov <georgi.djakov@linaro.org> wrote:
>
> On 1/22/20 08:45, Sibi Sankar wrote:
> > Hey Evan,
> >
> > Thanks for the review!
> >
> > On 2020-01-22 03:03, Evan Green wrote:
> >> On Thu, Jan 9, 2020 at 1:12 PM Sibi Sankar <sibis@codeaurora.org> wrote:
> >>>
> >>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> >>> resources of scaling L3 caches. Add a driver to handle bandwidth
> >>> requests to OSM L3 from CPU on SDM845 SoCs.
> >>>
> >>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >>> ---
> >>>  drivers/interconnect/qcom/Kconfig  |   7 +
> >>>  drivers/interconnect/qcom/Makefile |   2 +
> >>>  drivers/interconnect/qcom/osm-l3.c | 267 +++++++++++++++++++++++++++++
> >>>  3 files changed, 276 insertions(+)
> >>>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
> >>>
> >>> diff --git a/drivers/interconnect/qcom/Kconfig
> >>> b/drivers/interconnect/qcom/Kconfig
> >>> index a9bbbdf7400f9..b94d28e7bf700 100644
> >>> --- a/drivers/interconnect/qcom/Kconfig
> >>> +++ b/drivers/interconnect/qcom/Kconfig
> >>> @@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
> >>>          This is a driver for the Qualcomm Network-on-Chip on msm8974-based
> >>>          platforms.
> >>>
> >>> +config INTERCONNECT_QCOM_OSM_L3
> >>> +       tristate "Qualcomm OSM L3 interconnect driver"
> >>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
> >>> +       help
> >>> +         Say y here to support the Operating State Manager (OSM) interconnect
> >>> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
> >>> +
> >>>  config INTERCONNECT_QCOM_QCS404
> >>>         tristate "Qualcomm QCS404 interconnect driver"
> >>>         depends on INTERCONNECT_QCOM
> >>> diff --git a/drivers/interconnect/qcom/Makefile
> >>> b/drivers/interconnect/qcom/Makefile
> >>> index 55ec3c5c89dbd..89fecbd1257c7 100644
> >>> --- a/drivers/interconnect/qcom/Makefile
> >>> +++ b/drivers/interconnect/qcom/Makefile
> >>> @@ -1,5 +1,6 @@
> >>>  # SPDX-License-Identifier: GPL-2.0
> >>>
> >>> +icc-osm-l3-objs                                := osm-l3.o
> >>>  qnoc-msm8974-objs                      := msm8974.o
> >>>  qnoc-qcs404-objs                       := qcs404.o
> >>>  qnoc-sc7180-objs                       := sc7180.o
> >>> @@ -12,6 +13,7 @@ icc-smd-rpm-objs                      := smd-rpm.o
> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
> >>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
> >>> diff --git a/drivers/interconnect/qcom/osm-l3.c
> >>> b/drivers/interconnect/qcom/osm-l3.c
> >>> new file mode 100644
> >>> index 0000000000000..7fde53c70081e
> >>> --- /dev/null
> >>> +++ b/drivers/interconnect/qcom/osm-l3.c
> >>> @@ -0,0 +1,267 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> >>> + *
> >>> + */
> >>> +
> >>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> >>> +#include <linux/bitfield.h>
> >>> +#include <linux/clk.h>
> >>> +#include <linux/interconnect-provider.h>
> >>> +#include <linux/io.h>
> >>> +#include <linux/kernel.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/of_device.h>
> >>> +#include <linux/of_platform.h>
> >>> +#include <linux/platform_device.h>
> >>> +
> >>> +#define LUT_MAX_ENTRIES                        40U
> >>> +#define LUT_SRC                                GENMASK(31, 30)
> >>> +#define LUT_L_VAL                      GENMASK(7, 0)
> >>> +#define LUT_ROW_SIZE                   32
> >>> +#define CLK_HW_DIV                     2
> >>> +
> >>> +/* Register offsets */
> >>> +#define REG_ENABLE                     0x0
> >>> +#define REG_FREQ_LUT                   0x110
> >>> +#define REG_PERF_STATE                 0x920
> >>> +
> >>> +#define OSM_L3_MAX_LINKS               1
> >>> +#define SDM845_MAX_RSC_NODES           130
> >>
> >> I'm nervous this define is going to fall out of date with
> >> qcom,sdm845.h. I'm worried someone will end up adding a few more nodes
> >> that were always there but previously hidden from Linux. Can we put
> >> this define in include/dt-bindings/interconnect/qcom,sdm845.h, so at
> >> least when that happens they'll come face to face with this define?
> >> The same comment goes for the SC7180 define in patch 4.
> >
> > Yeah both solution require manual
> > intervention how about we just go
> > with what I proposed below.
> >
> >>
> >> On second thought, this trick only works once. Are we sure there
> >> aren't going to be other drivers that might want to tag on
> >> interconnect nodes as well? How about instead we just add the enum
> >> values below in qcom,sdm845.h as defines?
> >
> > Georgi/Evan,
> > Since qcom,sdm845.h is specific to
> > bindings shouldn't I just create a
> > .h file with all the enums so that
> > it can used across all icc providers
> > on SDM845?
>
> This sounds good to me, unless Evan has any objections.

So is this a new .h file with all the node numbers from qcom,sdm845.h
and your new couple of nodes here? That would be fine with me.

Or is it a .h file with only your two new node numbers? My worry there
is when there are two or three other drivers like this one, it will be
difficult to follow the total order of nodes as "base provider', "L3
driver", "new driver 1", "new driver 2".... any thoughts on how we
might address that?
-Evan

Re: [PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Sibi Sankar]

On 2020-01-22 22:18, Evan Green wrote:
> On Wed, Jan 22, 2020 at 12:20 AM Georgi Djakov 
> <georgi.djakov@linaro.org> wrote:
>> 
>> On 1/22/20 08:45, Sibi Sankar wrote:
>> > Hey Evan,
>> >
>> > Thanks for the review!
>> >
>> > On 2020-01-22 03:03, Evan Green wrote:
>> >> On Thu, Jan 9, 2020 at 1:12 PM Sibi Sankar <sibis@codeaurora.org> wrote:
>> >>>
>> >>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
>> >>> resources of scaling L3 caches. Add a driver to handle bandwidth
>> >>> requests to OSM L3 from CPU on SDM845 SoCs.
>> >>>
>> >>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> >>> ---
>> >>>  drivers/interconnect/qcom/Kconfig  |   7 +
>> >>>  drivers/interconnect/qcom/Makefile |   2 +
>> >>>  drivers/interconnect/qcom/osm-l3.c | 267 +++++++++++++++++++++++++++++
>> >>>  3 files changed, 276 insertions(+)
>> >>>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
>> >>>
>> >>> diff --git a/drivers/interconnect/qcom/Kconfig
>> >>> b/drivers/interconnect/qcom/Kconfig
>> >>> index a9bbbdf7400f9..b94d28e7bf700 100644
>> >>> --- a/drivers/interconnect/qcom/Kconfig
>> >>> +++ b/drivers/interconnect/qcom/Kconfig
>> >>> @@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
>> >>>          This is a driver for the Qualcomm Network-on-Chip on msm8974-based
>> >>>          platforms.
>> >>>
>> >>> +config INTERCONNECT_QCOM_OSM_L3
>> >>> +       tristate "Qualcomm OSM L3 interconnect driver"
>> >>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
>> >>> +       help
>> >>> +         Say y here to support the Operating State Manager (OSM) interconnect
>> >>> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
>> >>> +
>> >>>  config INTERCONNECT_QCOM_QCS404
>> >>>         tristate "Qualcomm QCS404 interconnect driver"
>> >>>         depends on INTERCONNECT_QCOM
>> >>> diff --git a/drivers/interconnect/qcom/Makefile
>> >>> b/drivers/interconnect/qcom/Makefile
>> >>> index 55ec3c5c89dbd..89fecbd1257c7 100644
>> >>> --- a/drivers/interconnect/qcom/Makefile
>> >>> +++ b/drivers/interconnect/qcom/Makefile
>> >>> @@ -1,5 +1,6 @@
>> >>>  # SPDX-License-Identifier: GPL-2.0
>> >>>
>> >>> +icc-osm-l3-objs                                := osm-l3.o
>> >>>  qnoc-msm8974-objs                      := msm8974.o
>> >>>  qnoc-qcs404-objs                       := qcs404.o
>> >>>  qnoc-sc7180-objs                       := sc7180.o
>> >>> @@ -12,6 +13,7 @@ icc-smd-rpm-objs                      := smd-rpm.o
>> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
>> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
>> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
>> >>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
>> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
>> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
>> >>> diff --git a/drivers/interconnect/qcom/osm-l3.c
>> >>> b/drivers/interconnect/qcom/osm-l3.c
>> >>> new file mode 100644
>> >>> index 0000000000000..7fde53c70081e
>> >>> --- /dev/null
>> >>> +++ b/drivers/interconnect/qcom/osm-l3.c
>> >>> @@ -0,0 +1,267 @@
>> >>> +// SPDX-License-Identifier: GPL-2.0
>> >>> +/*
>> >>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
>> >>> + *
>> >>> + */
>> >>> +
>> >>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
>> >>> +#include <linux/bitfield.h>
>> >>> +#include <linux/clk.h>
>> >>> +#include <linux/interconnect-provider.h>
>> >>> +#include <linux/io.h>
>> >>> +#include <linux/kernel.h>
>> >>> +#include <linux/module.h>
>> >>> +#include <linux/of_device.h>
>> >>> +#include <linux/of_platform.h>
>> >>> +#include <linux/platform_device.h>
>> >>> +
>> >>> +#define LUT_MAX_ENTRIES                        40U
>> >>> +#define LUT_SRC                                GENMASK(31, 30)
>> >>> +#define LUT_L_VAL                      GENMASK(7, 0)
>> >>> +#define LUT_ROW_SIZE                   32
>> >>> +#define CLK_HW_DIV                     2
>> >>> +
>> >>> +/* Register offsets */
>> >>> +#define REG_ENABLE                     0x0
>> >>> +#define REG_FREQ_LUT                   0x110
>> >>> +#define REG_PERF_STATE                 0x920
>> >>> +
>> >>> +#define OSM_L3_MAX_LINKS               1
>> >>> +#define SDM845_MAX_RSC_NODES           130
>> >>
>> >> I'm nervous this define is going to fall out of date with
>> >> qcom,sdm845.h. I'm worried someone will end up adding a few more nodes
>> >> that were always there but previously hidden from Linux. Can we put
>> >> this define in include/dt-bindings/interconnect/qcom,sdm845.h, so at
>> >> least when that happens they'll come face to face with this define?
>> >> The same comment goes for the SC7180 define in patch 4.
>> >
>> > Yeah both solution require manual
>> > intervention how about we just go
>> > with what I proposed below.
>> >
>> >>
>> >> On second thought, this trick only works once. Are we sure there
>> >> aren't going to be other drivers that might want to tag on
>> >> interconnect nodes as well? How about instead we just add the enum
>> >> values below in qcom,sdm845.h as defines?
>> >
>> > Georgi/Evan,
>> > Since qcom,sdm845.h is specific to
>> > bindings shouldn't I just create a
>> > .h file with all the enums so that
>> > it can used across all icc providers
>> > on SDM845?
>> 
>> This sounds good to me, unless Evan has any objections.
> 
> So is this a new .h file with all the node numbers from qcom,sdm845.h
> and your new couple of nodes here? That would be fine with me.
> 
> Or is it a .h file with only your two new node numbers? My worry there
> is when there are two or three other drivers like this one, it will be
> difficult to follow the total order of nodes as "base provider', "L3
> driver", "new driver 1", "new driver 2".... any thoughts on how we
> might address that?

the relative provider numbers from
qcom,sdm845.h have no useful meaning
for other icc providers. However the
enum defined in the sdm845.c which are
the node ids are needed and should be
sufficient to add/link to any icc node
across icc providers. So introducing a
sdm845.h with all the enumbs global node
ids is what I am proposing to do.

> -Evan

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH v4 2/4] interconnect: qcom: Add OSM L3 interconnect provider support

[Evan Green]

On Wed, Jan 22, 2020 at 10:05 AM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> On 2020-01-22 22:18, Evan Green wrote:
> > On Wed, Jan 22, 2020 at 12:20 AM Georgi Djakov
> > <georgi.djakov@linaro.org> wrote:
> >>
> >> On 1/22/20 08:45, Sibi Sankar wrote:
> >> > Hey Evan,
> >> >
> >> > Thanks for the review!
> >> >
> >> > On 2020-01-22 03:03, Evan Green wrote:
> >> >> On Thu, Jan 9, 2020 at 1:12 PM Sibi Sankar <sibis@codeaurora.org> wrote:
> >> >>>
> >> >>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> >> >>> resources of scaling L3 caches. Add a driver to handle bandwidth
> >> >>> requests to OSM L3 from CPU on SDM845 SoCs.
> >> >>>
> >> >>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >> >>> ---
> >> >>>  drivers/interconnect/qcom/Kconfig  |   7 +
> >> >>>  drivers/interconnect/qcom/Makefile |   2 +
> >> >>>  drivers/interconnect/qcom/osm-l3.c | 267 +++++++++++++++++++++++++++++
> >> >>>  3 files changed, 276 insertions(+)
> >> >>>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
> >> >>>
> >> >>> diff --git a/drivers/interconnect/qcom/Kconfig
> >> >>> b/drivers/interconnect/qcom/Kconfig
> >> >>> index a9bbbdf7400f9..b94d28e7bf700 100644
> >> >>> --- a/drivers/interconnect/qcom/Kconfig
> >> >>> +++ b/drivers/interconnect/qcom/Kconfig
> >> >>> @@ -14,6 +14,13 @@ config INTERCONNECT_QCOM_MSM8974
> >> >>>          This is a driver for the Qualcomm Network-on-Chip on msm8974-based
> >> >>>          platforms.
> >> >>>
> >> >>> +config INTERCONNECT_QCOM_OSM_L3
> >> >>> +       tristate "Qualcomm OSM L3 interconnect driver"
> >> >>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
> >> >>> +       help
> >> >>> +         Say y here to support the Operating State Manager (OSM) interconnect
> >> >>> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
> >> >>> +
> >> >>>  config INTERCONNECT_QCOM_QCS404
> >> >>>         tristate "Qualcomm QCS404 interconnect driver"
> >> >>>         depends on INTERCONNECT_QCOM
> >> >>> diff --git a/drivers/interconnect/qcom/Makefile
> >> >>> b/drivers/interconnect/qcom/Makefile
> >> >>> index 55ec3c5c89dbd..89fecbd1257c7 100644
> >> >>> --- a/drivers/interconnect/qcom/Makefile
> >> >>> +++ b/drivers/interconnect/qcom/Makefile
> >> >>> @@ -1,5 +1,6 @@
> >> >>>  # SPDX-License-Identifier: GPL-2.0
> >> >>>
> >> >>> +icc-osm-l3-objs                                := osm-l3.o
> >> >>>  qnoc-msm8974-objs                      := msm8974.o
> >> >>>  qnoc-qcs404-objs                       := qcs404.o
> >> >>>  qnoc-sc7180-objs                       := sc7180.o
> >> >>> @@ -12,6 +13,7 @@ icc-smd-rpm-objs                      := smd-rpm.o
> >> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
> >> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
> >> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
> >> >>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
> >> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
> >> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_RPMH) += icc-rpmh.o
> >> >>>  obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
> >> >>> diff --git a/drivers/interconnect/qcom/osm-l3.c
> >> >>> b/drivers/interconnect/qcom/osm-l3.c
> >> >>> new file mode 100644
> >> >>> index 0000000000000..7fde53c70081e
> >> >>> --- /dev/null
> >> >>> +++ b/drivers/interconnect/qcom/osm-l3.c
> >> >>> @@ -0,0 +1,267 @@
> >> >>> +// SPDX-License-Identifier: GPL-2.0
> >> >>> +/*
> >> >>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> >> >>> + *
> >> >>> + */
> >> >>> +
> >> >>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> >> >>> +#include <linux/bitfield.h>
> >> >>> +#include <linux/clk.h>
> >> >>> +#include <linux/interconnect-provider.h>
> >> >>> +#include <linux/io.h>
> >> >>> +#include <linux/kernel.h>
> >> >>> +#include <linux/module.h>
> >> >>> +#include <linux/of_device.h>
> >> >>> +#include <linux/of_platform.h>
> >> >>> +#include <linux/platform_device.h>
> >> >>> +
> >> >>> +#define LUT_MAX_ENTRIES                        40U
> >> >>> +#define LUT_SRC                                GENMASK(31, 30)
> >> >>> +#define LUT_L_VAL                      GENMASK(7, 0)
> >> >>> +#define LUT_ROW_SIZE                   32
> >> >>> +#define CLK_HW_DIV                     2
> >> >>> +
> >> >>> +/* Register offsets */
> >> >>> +#define REG_ENABLE                     0x0
> >> >>> +#define REG_FREQ_LUT                   0x110
> >> >>> +#define REG_PERF_STATE                 0x920
> >> >>> +
> >> >>> +#define OSM_L3_MAX_LINKS               1
> >> >>> +#define SDM845_MAX_RSC_NODES           130
> >> >>
> >> >> I'm nervous this define is going to fall out of date with
> >> >> qcom,sdm845.h. I'm worried someone will end up adding a few more nodes
> >> >> that were always there but previously hidden from Linux. Can we put
> >> >> this define in include/dt-bindings/interconnect/qcom,sdm845.h, so at
> >> >> least when that happens they'll come face to face with this define?
> >> >> The same comment goes for the SC7180 define in patch 4.
> >> >
> >> > Yeah both solution require manual
> >> > intervention how about we just go
> >> > with what I proposed below.
> >> >
> >> >>
> >> >> On second thought, this trick only works once. Are we sure there
> >> >> aren't going to be other drivers that might want to tag on
> >> >> interconnect nodes as well? How about instead we just add the enum
> >> >> values below in qcom,sdm845.h as defines?
> >> >
> >> > Georgi/Evan,
> >> > Since qcom,sdm845.h is specific to
> >> > bindings shouldn't I just create a
> >> > .h file with all the enums so that
> >> > it can used across all icc providers
> >> > on SDM845?
> >>
> >> This sounds good to me, unless Evan has any objections.
> >
> > So is this a new .h file with all the node numbers from qcom,sdm845.h
> > and your new couple of nodes here? That would be fine with me.
> >
> > Or is it a .h file with only your two new node numbers? My worry there
> > is when there are two or three other drivers like this one, it will be
> > difficult to follow the total order of nodes as "base provider', "L3
> > driver", "new driver 1", "new driver 2".... any thoughts on how we
> > might address that?
>
> the relative provider numbers from
> qcom,sdm845.h have no useful meaning
> for other icc providers. However the
> enum defined in the sdm845.c which are
> the node ids are needed and should be
> sufficient to add/link to any icc node
> across icc providers. So introducing a
> sdm845.h with all the enumbs global node
> ids is what I am proposing to do.

Sibi and
I chatted
offline. I
am on
board!