linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver
@ 2021-05-29 16:52 Hector Yuan
  2021-05-29 16:52 ` [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW Hector Yuan
  2021-05-29 16:52 ` [PATCH v12 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW Hector Yuan
  0 siblings, 2 replies; 12+ messages in thread
From: Hector Yuan @ 2021-05-29 16:52 UTC (permalink / raw)
  To: linux-mediatek, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
	Viresh Kumar, Matthias Brugger, Rob Herring, devicetree,
	linux-kernel
  Cc: wsd_upstream, hector.yuan

The CPUfreq HW present in some Mediatek chipsets offloads the steps necessary for changing the frequency of CPUs. 
The driver implements the cpufreq driver interface for this hardware engine. 

From v11 to v12, there are two modifications.
1. Based on patchset[1], align binding with scmi for performance domain(latest version).
2. Shrink binding example wording. 

From v8 to v9, there are three more modifications.
1. Based on patchset[2], align binding with scmi for performance domain.
2. Add the CPUFREQ fast switch function support and define DVFS latency.
3. Based on patchser[3], add energy model API parameter for mW.

From v7 to v8, there are three more patches based on patchset v8[4].
This patchset is about to register power table to Energy model for EAS and thermal usage.
1. EM CPU power table
- Register energy model table for EAS and thermal cooling device usage.
- Read the coresponding LUT for power table.
2. SVS initialization
- The SVS(Smart Voltage Scaling) engine is a hardware which is
  used to calculate optimized voltage values for CPU power domain.
  DVFS driver could apply those optimized voltage values to reduce power consumption.
- Driver will polling if HW engine is done for SVS initialization.
  After that, driver will read power table and register it to EAS.
- CPUs must be in power on state when doing SVS. Use pm_qos to block cpu-idle state for SVS initializing.
3. Cooling device flag
- Add cooling device flag for thermal
[1]  https://lore.kernel.org/linux-devicetree/20210517155458.1016707-1-sudeep.holla@arm.com/
[2]  https://lore.kernel.org/lkml/20201116181356.804590-1-sudeep.holla@arm.com/
[3]  https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=c250d50fe2ce627ca9805d9c8ac11cbbf922a4a6
[4]  https://lkml.org/lkml/2020/9/23/384


Hector.Yuan (2):
  cpufreq: mediatek-hw: Add support for CPUFREQ HW
  dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW

 .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |   71 ++++
 drivers/cpufreq/Kconfig.arm                        |   12 +
 drivers/cpufreq/Makefile                           |    1 +
 drivers/cpufreq/mediatek-cpufreq-hw.c              |  370 ++++++++++++++++++++
 4 files changed, 454 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
 create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c

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

* [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
  2021-05-29 16:52 [PATCH v12] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver Hector Yuan
@ 2021-05-29 16:52 ` Hector Yuan
  2021-06-14 10:40   ` Viresh Kumar
  2021-05-29 16:52 ` [PATCH v12 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW Hector Yuan
  1 sibling, 1 reply; 12+ messages in thread
From: Hector Yuan @ 2021-05-29 16:52 UTC (permalink / raw)
  To: linux-mediatek, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
	Viresh Kumar, Matthias Brugger, Rob Herring, devicetree,
	linux-kernel
  Cc: wsd_upstream, hector.yuan

From: "Hector.Yuan" <hector.yuan@mediatek.com>

Add cpufreq HW support.

Signed-off-by: Hector.Yuan <hector.yuan@mediatek.com>
---
 drivers/cpufreq/Kconfig.arm           |   12 ++
 drivers/cpufreq/Makefile              |    1 +
 drivers/cpufreq/mediatek-cpufreq-hw.c |  370 +++++++++++++++++++++++++++++++++
 3 files changed, 383 insertions(+)
 create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index a5c5f70..954749a 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -133,6 +133,18 @@ config ARM_MEDIATEK_CPUFREQ
 	help
 	  This adds the CPUFreq driver support for MediaTek SoCs.
 
+config ARM_MEDIATEK_CPUFREQ_HW
+	tristate "MediaTek CPUFreq HW driver"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	default m
+	help
+	  Support for the CPUFreq HW driver.
+	  Some MediaTek chipsets have a HW engine to offload the steps
+	  necessary for changing the frequency of the CPUs. Firmware loaded
+	  in this engine exposes a programming interface to the OS.
+	  The driver implements the cpufreq interface for this HW engine.
+	  Say Y if you want to support CPUFreq HW.
+
 config ARM_OMAP2PLUS_CPUFREQ
 	bool "TI OMAP2+"
 	depends on ARCH_OMAP2PLUS
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 27d3bd7..48ee585 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_ARM_IMX6Q_CPUFREQ)		+= imx6q-cpufreq.o
 obj-$(CONFIG_ARM_IMX_CPUFREQ_DT)	+= imx-cpufreq-dt.o
 obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ)	+= kirkwood-cpufreq.o
 obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ)	+= mediatek-cpufreq.o
+obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ_HW)	+= mediatek-cpufreq-hw.o
 obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
new file mode 100644
index 0000000..6f3a461
--- /dev/null
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -0,0 +1,370 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cpufreq.h>
+#include <linux/energy_model.h>
+#include <linux/init.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/pm_qos.h>
+#include <linux/slab.h>
+
+#define LUT_MAX_ENTRIES			32U
+#define LUT_FREQ			GENMASK(11, 0)
+#define LUT_ROW_SIZE			0x4
+#define CPUFREQ_HW_STATUS		BIT(0)
+#define SVS_HW_STATUS			BIT(1)
+#define POLL_USEC			1000
+#define TIMEOUT_USEC			300000
+
+enum {
+	REG_FREQ_LUT_TABLE,
+	REG_FREQ_ENABLE,
+	REG_FREQ_PERF_STATE,
+	REG_FREQ_HW_STATE,
+	REG_EM_POWER_TBL,
+	REG_FREQ_LATENCY,
+
+	REG_ARRAY_SIZE,
+};
+
+struct cpufreq_mtk {
+	struct cpufreq_frequency_table *table;
+	void __iomem *reg_bases[REG_ARRAY_SIZE];
+	int nr_opp;
+	cpumask_t related_cpus;
+};
+
+static const u16 cpufreq_mtk_offsets[REG_ARRAY_SIZE] = {
+	[REG_FREQ_LUT_TABLE]	= 0x0,
+	[REG_FREQ_ENABLE]	= 0x84,
+	[REG_FREQ_PERF_STATE]	= 0x88,
+	[REG_FREQ_HW_STATE]	= 0x8c,
+	[REG_EM_POWER_TBL]	= 0x90,
+	[REG_FREQ_LATENCY]	= 0x110,
+};
+
+static struct cpufreq_mtk *mtk_freq_domain_map[NR_CPUS];
+
+static int __maybe_unused
+mtk_cpufreq_get_cpu_power(unsigned long *mW,
+			  unsigned long *KHz, struct device *cpu_dev)
+{
+	struct cpufreq_mtk *c = mtk_freq_domain_map[cpu_dev->id];
+	int i;
+
+	for (i = 0; i < c->nr_opp; i++) {
+		if (c->table[i].frequency < *KHz)
+			break;
+	}
+	i--;
+
+	*KHz = c->table[i].frequency;
+	*mW = readl_relaxed(c->reg_bases[REG_EM_POWER_TBL] +
+			    i * LUT_ROW_SIZE) / 1000;
+
+	return 0;
+}
+
+static int mtk_cpufreq_hw_target_index(struct cpufreq_policy *policy,
+				       unsigned int index)
+{
+	struct cpufreq_mtk *c = policy->driver_data;
+
+	writel_relaxed(index, c->reg_bases[REG_FREQ_PERF_STATE]);
+
+	return 0;
+}
+
+static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
+{
+	struct cpufreq_mtk *c;
+	unsigned int index;
+
+	c = mtk_freq_domain_map[cpu];
+
+	index = readl_relaxed(c->reg_bases[REG_FREQ_PERF_STATE]);
+	index = min(index, LUT_MAX_ENTRIES - 1);
+
+	return c->table[index].frequency;
+}
+
+static unsigned int mtk_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
+					       unsigned int target_freq)
+{
+	struct cpufreq_mtk *c = policy->driver_data;
+	unsigned int index;
+
+	index = cpufreq_table_find_index_dl(policy, target_freq);
+
+	writel_relaxed(index, c->reg_bases[REG_FREQ_PERF_STATE]);
+
+	return policy->freq_table[index].frequency;
+}
+
+static int mtk_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
+{
+	struct cpufreq_mtk *c;
+	struct device *cpu_dev;
+	struct em_data_callback em_cb = EM_DATA_CB(mtk_cpufreq_get_cpu_power);
+	struct pm_qos_request *qos_request;
+	int sig, pwr_hw = CPUFREQ_HW_STATUS | SVS_HW_STATUS;
+	unsigned int latency;
+
+	qos_request = kzalloc(sizeof(*qos_request), GFP_KERNEL);
+	if (!qos_request)
+		return -ENOMEM;
+
+	cpu_dev = get_cpu_device(policy->cpu);
+	if (!cpu_dev) {
+		pr_err("failed to get cpu%d device\n", policy->cpu);
+		return -ENODEV;
+	}
+
+	c = mtk_freq_domain_map[policy->cpu];
+	if (!c) {
+		pr_err("No scaling support for CPU%d\n", policy->cpu);
+		return -ENODEV;
+	}
+
+	cpumask_copy(policy->cpus, &c->related_cpus);
+
+	policy->freq_table = c->table;
+	policy->driver_data = c;
+
+	latency = readl_relaxed(c->reg_bases[REG_FREQ_LATENCY]);
+	if (!latency)
+		latency = CPUFREQ_ETERNAL;
+
+	/* us convert to ns */
+	policy->cpuinfo.transition_latency = latency * 1000;
+
+	policy->fast_switch_possible = true;
+
+	/* Let CPUs leave idle-off state for SVS CPU initializing */
+	cpu_latency_qos_add_request(qos_request, 0);
+
+	/* HW should be in enabled state to proceed now */
+	writel_relaxed(0x1, c->reg_bases[REG_FREQ_ENABLE]);
+
+	if (readl_poll_timeout(c->reg_bases[REG_FREQ_HW_STATE], sig,
+			       (sig & pwr_hw) == pwr_hw, POLL_USEC,
+			       TIMEOUT_USEC)) {
+		if (!(sig & CPUFREQ_HW_STATUS)) {
+			pr_info("cpufreq hardware of CPU%d is not enabled\n",
+				policy->cpu);
+			return -ENODEV;
+		}
+
+		pr_info("SVS of CPU%d is not enabled\n", policy->cpu);
+	}
+
+	em_dev_register_perf_domain(cpu_dev, c->nr_opp, &em_cb, policy->cpus, true);
+
+	cpu_latency_qos_remove_request(qos_request);
+	kfree(qos_request);
+
+	return 0;
+}
+
+static int mtk_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
+{
+	struct cpufreq_mtk *c;
+
+	c = mtk_freq_domain_map[policy->cpu];
+	if (!c) {
+		pr_err("No scaling support for CPU%d\n", policy->cpu);
+		return -ENODEV;
+	}
+
+	/* HW should be in paused state now */
+	writel_relaxed(0x0, c->reg_bases[REG_FREQ_ENABLE]);
+
+	return 0;
+}
+
+static struct cpufreq_driver cpufreq_mtk_hw_driver = {
+	.flags		= CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
+			  CPUFREQ_IS_COOLING_DEV,
+	.verify		= cpufreq_generic_frequency_table_verify,
+	.target_index	= mtk_cpufreq_hw_target_index,
+	.get		= mtk_cpufreq_hw_get,
+	.init		= mtk_cpufreq_hw_cpu_init,
+	.exit		= mtk_cpufreq_hw_cpu_exit,
+	.fast_switch	= mtk_cpufreq_hw_fast_switch,
+	.name		= "mtk-cpufreq-hw",
+	.attr		= cpufreq_generic_attr,
+};
+
+static int mtk_cpu_create_freq_table(struct platform_device *pdev,
+				     struct cpufreq_mtk *c)
+{
+	struct device *dev = &pdev->dev;
+	void __iomem *base_table;
+	u32 data, i, freq, prev_freq = 0;
+
+	c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
+				sizeof(*c->table), GFP_KERNEL);
+	if (!c->table)
+		return -ENOMEM;
+
+	base_table = c->reg_bases[REG_FREQ_LUT_TABLE];
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		data = readl_relaxed(base_table + (i * LUT_ROW_SIZE));
+		freq = FIELD_GET(LUT_FREQ, data) * 1000;
+
+		if (freq == prev_freq)
+			break;
+
+		c->table[i].frequency = freq;
+
+		dev_dbg(dev, "index=%d freq=%d\n",
+			i, c->table[i].frequency);
+
+		prev_freq = freq;
+	}
+
+	c->table[i].frequency = CPUFREQ_TABLE_END;
+	c->nr_opp = i;
+
+	return 0;
+}
+
+static int mtk_get_related_cpus(int index, struct cpufreq_mtk *c)
+{
+	struct device_node *cpu_np;
+	struct of_phandle_args args;
+	int cpu, ret;
+
+	for_each_possible_cpu(cpu) {
+		cpu_np = of_cpu_device_node_get(cpu);
+		if (!cpu_np)
+			continue;
+
+		ret = of_parse_phandle_with_args(cpu_np, "performance-domains",
+						 "#performance-domain-cells", 0,
+						 &args);
+		of_node_put(cpu_np);
+		if (ret < 0)
+			continue;
+
+		if (index == args.args[0]) {
+			cpumask_set_cpu(cpu, &c->related_cpus);
+			mtk_freq_domain_map[cpu] = c;
+		}
+	}
+
+	return 0;
+}
+
+static int mtk_cpu_resources_init(struct platform_device *pdev,
+				  unsigned int cpu, int index,
+				  const u16 *offsets)
+{
+	struct cpufreq_mtk *c;
+	struct device *dev = &pdev->dev;
+	int ret, i;
+	void __iomem *base;
+
+	if (mtk_freq_domain_map[cpu])
+		return 0;
+
+	c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+	if (!c)
+		return -ENOMEM;
+
+	base = devm_platform_ioremap_resource(pdev, index);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	for (i = REG_FREQ_LUT_TABLE; i < REG_ARRAY_SIZE; i++)
+		c->reg_bases[i] = base + offsets[i];
+
+	ret = mtk_get_related_cpus(index, c);
+	if (ret) {
+		dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
+		return ret;
+	}
+
+	ret = mtk_cpu_create_freq_table(pdev, c);
+	if (ret) {
+		dev_err(dev, "Domain-%d failed to create freq table\n", index);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
+{
+	struct device_node *cpu_np;
+	struct of_phandle_args args;
+	const u16 *offsets;
+	unsigned int cpu;
+	int ret;
+
+	offsets = of_device_get_match_data(&pdev->dev);
+	if (!offsets)
+		return -EINVAL;
+
+	for_each_possible_cpu(cpu) {
+		cpu_np = of_cpu_device_node_get(cpu);
+		if (!cpu_np) {
+			dev_err(&pdev->dev, "Failed to get cpu %d device\n",
+				cpu);
+			return -ENODEV;
+		}
+
+		ret = of_parse_phandle_with_args(cpu_np, "performance-domains",
+						 "#performance-domain-cells", 0,
+						 &args);
+		if (ret < 0)
+			return ret;
+
+		/* Get the bases of cpufreq for domains */
+		ret = mtk_cpu_resources_init(pdev, cpu, args.args[0], offsets);
+		if (ret) {
+			dev_err(&pdev->dev, "CPUFreq resource init failed\n");
+			return ret;
+		}
+	}
+
+	ret = cpufreq_register_driver(&cpufreq_mtk_hw_driver);
+	if (ret) {
+		dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mtk_cpufreq_hw_driver_remove(struct platform_device *pdev)
+{
+	return cpufreq_unregister_driver(&cpufreq_mtk_hw_driver);
+}
+
+static const struct of_device_id mtk_cpufreq_hw_match[] = {
+	{ .compatible = "mediatek,cpufreq-hw", .data = &cpufreq_mtk_offsets },
+	{}
+};
+
+static struct platform_driver mtk_cpufreq_hw_driver = {
+	.probe = mtk_cpufreq_hw_driver_probe,
+	.remove = mtk_cpufreq_hw_driver_remove,
+	.driver = {
+		.name = "mtk-cpufreq-hw",
+		.of_match_table = mtk_cpufreq_hw_match,
+	},
+};
+module_platform_driver(mtk_cpufreq_hw_driver);
+
+MODULE_DESCRIPTION("Mediatek cpufreq-hw driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5


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

* [PATCH v12 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW
  2021-05-29 16:52 [PATCH v12] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver Hector Yuan
  2021-05-29 16:52 ` [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW Hector Yuan
@ 2021-05-29 16:52 ` Hector Yuan
  2021-06-02 16:58   ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Hector Yuan @ 2021-05-29 16:52 UTC (permalink / raw)
  To: linux-mediatek, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
	Viresh Kumar, Matthias Brugger, Rob Herring, devicetree,
	linux-kernel
  Cc: wsd_upstream, hector.yuan

From: "Hector.Yuan" <hector.yuan@mediatek.com>

Add devicetree bindings for MediaTek HW driver.

Signed-off-by: Hector.Yuan <hector.yuan@mediatek.com>
---
 .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |   71 ++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
new file mode 100644
index 0000000..1aa4d54
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cpufreq/cpufreq-mediatek-hw.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek's CPUFREQ Bindings
+
+maintainers:
+  - Hector Yuan <hector.yuan@mediatek.com>
+
+description:
+  CPUFREQ HW is a hardware engine used by MediaTek
+  SoCs to manage frequency in hardware. It is capable of controlling frequency
+  for multiple clusters.
+
+properties:
+  compatible:
+    const: mediatek,cpufreq-hw
+
+  reg:
+    minItems: 1
+    maxItems: 2
+    description: |
+      Addresses and sizes for the memory of the
+      HW bases in each frequency domain.
+
+  "#performance-domain-cells":
+    description:
+      Number of cells in a performance domain specifier. Typically 0 for nodes
+      representing a single performance domain and 1 for nodes providing
+      multiple performance domains (e.g. performance controllers), but can be
+      any value as specified by device tree binding documentation of particular
+      provider.
+    enum: [ 0, 1 ]
+
+required:
+  - compatible
+  - reg
+  - "#performance-domain-cells"
+
+additionalProperties: true
+
+examples:
+  - |
+    cpus {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            cpu0: cpu@0 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a55";
+                enable-method = "psci";
+                performance-domains = <&performance 0>;
+                reg = <0x000>;
+            };
+    };
+
+    /* ... */
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        performance: performance-controller@11bc00 {
+            compatible = "mediatek,cpufreq-hw";
+            reg = <0 0x0011bc10 0 0x120>, <0 0x0011bd30 0 0x120>;
+
+            #performance-domain-cells = <1>;
+        };
+    };
-- 
1.7.9.5


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

* Re: [PATCH v12 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW
  2021-05-29 16:52 ` [PATCH v12 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW Hector Yuan
@ 2021-06-02 16:58   ` Rob Herring
  2021-06-27  7:56     ` Hector Yuan
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2021-06-02 16:58 UTC (permalink / raw)
  To: Hector Yuan
  Cc: linux-mediatek, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
	Viresh Kumar, Matthias Brugger, devicetree, linux-kernel,
	wsd_upstream

On Sun, May 30, 2021 at 12:52:33AM +0800, Hector Yuan wrote:
> From: "Hector.Yuan" <hector.yuan@mediatek.com>
> 
> Add devicetree bindings for MediaTek HW driver.
> 
> Signed-off-by: Hector.Yuan <hector.yuan@mediatek.com>
> ---
>  .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |   71 ++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> new file mode 100644
> index 0000000..1aa4d54
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cpufreq/cpufreq-mediatek-hw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek's CPUFREQ Bindings
> +
> +maintainers:
> +  - Hector Yuan <hector.yuan@mediatek.com>
> +
> +description:
> +  CPUFREQ HW is a hardware engine used by MediaTek
> +  SoCs to manage frequency in hardware. It is capable of controlling frequency
> +  for multiple clusters.
> +
> +properties:
> +  compatible:
> +    const: mediatek,cpufreq-hw
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +    description: |
> +      Addresses and sizes for the memory of the
> +      HW bases in each frequency domain.
> +
> +  "#performance-domain-cells":
> +    description:
> +      Number of cells in a performance domain specifier. Typically 0 for nodes
> +      representing a single performance domain and 1 for nodes providing
> +      multiple performance domains (e.g. performance controllers), but can be
> +      any value as specified by device tree binding documentation of particular
> +      provider.
> +    enum: [ 0, 1 ]

Can't you restrict this to be 1 for Mediatek h/w? Even if you sometimes 
have a single domain, it's probably more simple for the driver if this 
is fixed.

> +
> +required:
> +  - compatible
> +  - reg
> +  - "#performance-domain-cells"
> +
> +additionalProperties: true

Should be false.

> +
> +examples:
> +  - |
> +    cpus {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            cpu0: cpu@0 {
> +                device_type = "cpu";
> +                compatible = "arm,cortex-a55";
> +                enable-method = "psci";
> +                performance-domains = <&performance 0>;
> +                reg = <0x000>;
> +            };
> +    };
> +
> +    /* ... */
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        performance: performance-controller@11bc00 {
> +            compatible = "mediatek,cpufreq-hw";
> +            reg = <0 0x0011bc10 0 0x120>, <0 0x0011bd30 0 0x120>;
> +
> +            #performance-domain-cells = <1>;
> +        };
> +    };
> -- 
> 1.7.9.5

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

* Re: [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
  2021-05-29 16:52 ` [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW Hector Yuan
@ 2021-06-14 10:40   ` Viresh Kumar
       [not found]     ` <1624781848.1958.16.camel@mtkswgap22>
  0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2021-06-14 10:40 UTC (permalink / raw)
  To: Hector Yuan
  Cc: linux-mediatek, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
	Matthias Brugger, Rob Herring, devicetree, linux-kernel,
	wsd_upstream

On 30-05-21, 00:52, Hector Yuan wrote:
> From: "Hector.Yuan" <hector.yuan@mediatek.com>
> 
> Add cpufreq HW support.
> 
> Signed-off-by: Hector.Yuan <hector.yuan@mediatek.com>
> ---
>  drivers/cpufreq/Kconfig.arm           |   12 ++
>  drivers/cpufreq/Makefile              |    1 +
>  drivers/cpufreq/mediatek-cpufreq-hw.c |  370 +++++++++++++++++++++++++++++++++
>  3 files changed, 383 insertions(+)
>  create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c
> 
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index a5c5f70..954749a 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -133,6 +133,18 @@ config ARM_MEDIATEK_CPUFREQ
>  	help
>  	  This adds the CPUFreq driver support for MediaTek SoCs.
>  
> +config ARM_MEDIATEK_CPUFREQ_HW
> +	tristate "MediaTek CPUFreq HW driver"
> +	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	default m
> +	help
> +	  Support for the CPUFreq HW driver.
> +	  Some MediaTek chipsets have a HW engine to offload the steps
> +	  necessary for changing the frequency of the CPUs. Firmware loaded
> +	  in this engine exposes a programming interface to the OS.
> +	  The driver implements the cpufreq interface for this HW engine.
> +	  Say Y if you want to support CPUFreq HW.
> +
>  config ARM_OMAP2PLUS_CPUFREQ
>  	bool "TI OMAP2+"
>  	depends on ARCH_OMAP2PLUS
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 27d3bd7..48ee585 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_ARM_IMX6Q_CPUFREQ)		+= imx6q-cpufreq.o
>  obj-$(CONFIG_ARM_IMX_CPUFREQ_DT)	+= imx-cpufreq-dt.o
>  obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ)	+= kirkwood-cpufreq.o
>  obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ)	+= mediatek-cpufreq.o
> +obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ_HW)	+= mediatek-cpufreq-hw.o
>  obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
>  obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
>  obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
> diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
> new file mode 100644
> index 0000000..6f3a461
> --- /dev/null
> +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
> @@ -0,0 +1,370 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/cpufreq.h>
> +#include <linux/energy_model.h>
> +#include <linux/init.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_qos.h>
> +#include <linux/slab.h>
> +
> +#define LUT_MAX_ENTRIES			32U
> +#define LUT_FREQ			GENMASK(11, 0)
> +#define LUT_ROW_SIZE			0x4
> +#define CPUFREQ_HW_STATUS		BIT(0)
> +#define SVS_HW_STATUS			BIT(1)
> +#define POLL_USEC			1000
> +#define TIMEOUT_USEC			300000
> +
> +enum {
> +	REG_FREQ_LUT_TABLE,
> +	REG_FREQ_ENABLE,
> +	REG_FREQ_PERF_STATE,
> +	REG_FREQ_HW_STATE,
> +	REG_EM_POWER_TBL,
> +	REG_FREQ_LATENCY,
> +
> +	REG_ARRAY_SIZE,
> +};
> +
> +struct cpufreq_mtk {
> +	struct cpufreq_frequency_table *table;
> +	void __iomem *reg_bases[REG_ARRAY_SIZE];
> +	int nr_opp;
> +	cpumask_t related_cpus;
> +};
> +
> +static const u16 cpufreq_mtk_offsets[REG_ARRAY_SIZE] = {
> +	[REG_FREQ_LUT_TABLE]	= 0x0,
> +	[REG_FREQ_ENABLE]	= 0x84,
> +	[REG_FREQ_PERF_STATE]	= 0x88,
> +	[REG_FREQ_HW_STATE]	= 0x8c,
> +	[REG_EM_POWER_TBL]	= 0x90,
> +	[REG_FREQ_LATENCY]	= 0x110,
> +};
> +
> +static struct cpufreq_mtk *mtk_freq_domain_map[NR_CPUS];
> +
> +static int __maybe_unused
> +mtk_cpufreq_get_cpu_power(unsigned long *mW,
> +			  unsigned long *KHz, struct device *cpu_dev)
> +{
> +	struct cpufreq_mtk *c = mtk_freq_domain_map[cpu_dev->id];
> +	int i;
> +
> +	for (i = 0; i < c->nr_opp; i++) {
> +		if (c->table[i].frequency < *KHz)
> +			break;
> +	}
> +	i--;
> +
> +	*KHz = c->table[i].frequency;
> +	*mW = readl_relaxed(c->reg_bases[REG_EM_POWER_TBL] +
> +			    i * LUT_ROW_SIZE) / 1000;
> +
> +	return 0;
> +}
> +
> +static int mtk_cpufreq_hw_target_index(struct cpufreq_policy *policy,
> +				       unsigned int index)
> +{
> +	struct cpufreq_mtk *c = policy->driver_data;
> +
> +	writel_relaxed(index, c->reg_bases[REG_FREQ_PERF_STATE]);
> +
> +	return 0;
> +}
> +
> +static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
> +{
> +	struct cpufreq_mtk *c;
> +	unsigned int index;
> +
> +	c = mtk_freq_domain_map[cpu];
> +
> +	index = readl_relaxed(c->reg_bases[REG_FREQ_PERF_STATE]);
> +	index = min(index, LUT_MAX_ENTRIES - 1);
> +
> +	return c->table[index].frequency;
> +}
> +
> +static unsigned int mtk_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
> +					       unsigned int target_freq)
> +{
> +	struct cpufreq_mtk *c = policy->driver_data;
> +	unsigned int index;
> +
> +	index = cpufreq_table_find_index_dl(policy, target_freq);
> +
> +	writel_relaxed(index, c->reg_bases[REG_FREQ_PERF_STATE]);
> +
> +	return policy->freq_table[index].frequency;
> +}
> +
> +static int mtk_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
> +{
> +	struct cpufreq_mtk *c;
> +	struct device *cpu_dev;
> +	struct em_data_callback em_cb = EM_DATA_CB(mtk_cpufreq_get_cpu_power);
> +	struct pm_qos_request *qos_request;
> +	int sig, pwr_hw = CPUFREQ_HW_STATUS | SVS_HW_STATUS;
> +	unsigned int latency;
> +
> +	qos_request = kzalloc(sizeof(*qos_request), GFP_KERNEL);
> +	if (!qos_request)
> +		return -ENOMEM;

Since this is temporarily required, please allocate it while you are
trying to use it.

> +
> +	cpu_dev = get_cpu_device(policy->cpu);
> +	if (!cpu_dev) {
> +		pr_err("failed to get cpu%d device\n", policy->cpu);
> +		return -ENODEV;
> +	}
> +
> +	c = mtk_freq_domain_map[policy->cpu];
> +	if (!c) {
> +		pr_err("No scaling support for CPU%d\n", policy->cpu);
> +		return -ENODEV;
> +	}
> +
> +	cpumask_copy(policy->cpus, &c->related_cpus);
> +
> +	policy->freq_table = c->table;
> +	policy->driver_data = c;
> +
> +	latency = readl_relaxed(c->reg_bases[REG_FREQ_LATENCY]);
> +	if (!latency)
> +		latency = CPUFREQ_ETERNAL;
> +
> +	/* us convert to ns */
> +	policy->cpuinfo.transition_latency = latency * 1000;
> +
> +	policy->fast_switch_possible = true;
> +
> +	/* Let CPUs leave idle-off state for SVS CPU initializing */
> +	cpu_latency_qos_add_request(qos_request, 0);

I am not sure I understand what's going on here.

> +	/* HW should be in enabled state to proceed now */
> +	writel_relaxed(0x1, c->reg_bases[REG_FREQ_ENABLE]);
> +
> +	if (readl_poll_timeout(c->reg_bases[REG_FREQ_HW_STATE], sig,
> +			       (sig & pwr_hw) == pwr_hw, POLL_USEC,
> +			       TIMEOUT_USEC)) {
> +		if (!(sig & CPUFREQ_HW_STATUS)) {
> +			pr_info("cpufreq hardware of CPU%d is not enabled\n",
> +				policy->cpu);
> +			return -ENODEV;
> +		}
> +
> +		pr_info("SVS of CPU%d is not enabled\n", policy->cpu);
> +	}
> +
> +	em_dev_register_perf_domain(cpu_dev, c->nr_opp, &em_cb, policy->cpus, true);
> +
> +	cpu_latency_qos_remove_request(qos_request);

Can this be done before em_dev_register_perf_domain() ?

> +	kfree(qos_request);
> +
> +	return 0;
> +}
> +
> +static int mtk_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
> +{
> +	struct cpufreq_mtk *c;
> +
> +	c = mtk_freq_domain_map[policy->cpu];
> +	if (!c) {

This can't be NULL here.

> +		pr_err("No scaling support for CPU%d\n", policy->cpu);
> +		return -ENODEV;
> +	}
> +
> +	/* HW should be in paused state now */
> +	writel_relaxed(0x0, c->reg_bases[REG_FREQ_ENABLE]);
> +
> +	return 0;
> +}
> +
> +static struct cpufreq_driver cpufreq_mtk_hw_driver = {
> +	.flags		= CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> +			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> +			  CPUFREQ_IS_COOLING_DEV,
> +	.verify		= cpufreq_generic_frequency_table_verify,
> +	.target_index	= mtk_cpufreq_hw_target_index,
> +	.get		= mtk_cpufreq_hw_get,
> +	.init		= mtk_cpufreq_hw_cpu_init,
> +	.exit		= mtk_cpufreq_hw_cpu_exit,
> +	.fast_switch	= mtk_cpufreq_hw_fast_switch,
> +	.name		= "mtk-cpufreq-hw",
> +	.attr		= cpufreq_generic_attr,
> +};
> +
> +static int mtk_cpu_create_freq_table(struct platform_device *pdev,
> +				     struct cpufreq_mtk *c)
> +{
> +	struct device *dev = &pdev->dev;
> +	void __iomem *base_table;
> +	u32 data, i, freq, prev_freq = 0;
> +
> +	c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
> +				sizeof(*c->table), GFP_KERNEL);
> +	if (!c->table)
> +		return -ENOMEM;
> +
> +	base_table = c->reg_bases[REG_FREQ_LUT_TABLE];
> +
> +	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> +		data = readl_relaxed(base_table + (i * LUT_ROW_SIZE));
> +		freq = FIELD_GET(LUT_FREQ, data) * 1000;
> +
> +		if (freq == prev_freq)
> +			break;
> +
> +		c->table[i].frequency = freq;
> +
> +		dev_dbg(dev, "index=%d freq=%d\n",
> +			i, c->table[i].frequency);
> +
> +		prev_freq = freq;
> +	}
> +
> +	c->table[i].frequency = CPUFREQ_TABLE_END;
> +	c->nr_opp = i;
> +
> +	return 0;
> +}
> +
> +static int mtk_get_related_cpus(int index, struct cpufreq_mtk *c)
> +{
> +	struct device_node *cpu_np;
> +	struct of_phandle_args args;
> +	int cpu, ret;
> +
> +	for_each_possible_cpu(cpu) {
> +		cpu_np = of_cpu_device_node_get(cpu);
> +		if (!cpu_np)
> +			continue;
> +
> +		ret = of_parse_phandle_with_args(cpu_np, "performance-domains",
> +						 "#performance-domain-cells", 0,
> +						 &args);
> +		of_node_put(cpu_np);
> +		if (ret < 0)
> +			continue;
> +
> +		if (index == args.args[0]) {
> +			cpumask_set_cpu(cpu, &c->related_cpus);
> +			mtk_freq_domain_map[cpu] = c;
> +		}
> +	}
> +
> +	return 0;
> +}

I really hope this can be moved to a common place as more than one
drier should be required to parse this thing.

> +static int mtk_cpu_resources_init(struct platform_device *pdev,
> +				  unsigned int cpu, int index,
> +				  const u16 *offsets)
> +{
> +	struct cpufreq_mtk *c;
> +	struct device *dev = &pdev->dev;
> +	int ret, i;
> +	void __iomem *base;
> +
> +	if (mtk_freq_domain_map[cpu])
> +		return 0;
> +
> +	c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> +	if (!c)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, index);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	for (i = REG_FREQ_LUT_TABLE; i < REG_ARRAY_SIZE; i++)
> +		c->reg_bases[i] = base + offsets[i];
> +
> +	ret = mtk_get_related_cpus(index, c);
> +	if (ret) {
> +		dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
> +		return ret;
> +	}
> +
> +	ret = mtk_cpu_create_freq_table(pdev, c);
> +	if (ret) {
> +		dev_err(dev, "Domain-%d failed to create freq table\n", index);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
> +{
> +	struct device_node *cpu_np;
> +	struct of_phandle_args args;
> +	const u16 *offsets;
> +	unsigned int cpu;
> +	int ret;
> +
> +	offsets = of_device_get_match_data(&pdev->dev);
> +	if (!offsets)
> +		return -EINVAL;
> +
> +	for_each_possible_cpu(cpu) {
> +		cpu_np = of_cpu_device_node_get(cpu);
> +		if (!cpu_np) {
> +			dev_err(&pdev->dev, "Failed to get cpu %d device\n",
> +				cpu);
> +			return -ENODEV;
> +		}
> +
> +		ret = of_parse_phandle_with_args(cpu_np, "performance-domains",
> +						 "#performance-domain-cells", 0,
> +						 &args);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Get the bases of cpufreq for domains */
> +		ret = mtk_cpu_resources_init(pdev, cpu, args.args[0], offsets);
> +		if (ret) {
> +			dev_err(&pdev->dev, "CPUFreq resource init failed\n");
> +			return ret;
> +		}
> +	}

Why don't you do above stuff in driver's ->init() ? As this really
isn't required to be done per CPU, but per policy.

> +
> +	ret = cpufreq_register_driver(&cpufreq_mtk_hw_driver);
> +	if (ret) {
> +		dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mtk_cpufreq_hw_driver_remove(struct platform_device *pdev)
> +{
> +	return cpufreq_unregister_driver(&cpufreq_mtk_hw_driver);
> +}
> +
> +static const struct of_device_id mtk_cpufreq_hw_match[] = {
> +	{ .compatible = "mediatek,cpufreq-hw", .data = &cpufreq_mtk_offsets },
> +	{}
> +};
> +
> +static struct platform_driver mtk_cpufreq_hw_driver = {
> +	.probe = mtk_cpufreq_hw_driver_probe,
> +	.remove = mtk_cpufreq_hw_driver_remove,
> +	.driver = {
> +		.name = "mtk-cpufreq-hw",
> +		.of_match_table = mtk_cpufreq_hw_match,
> +	},
> +};
> +module_platform_driver(mtk_cpufreq_hw_driver);
> +
> +MODULE_DESCRIPTION("Mediatek cpufreq-hw driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.7.9.5

-- 
viresh

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

* Re: [PATCH v12 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW
  2021-06-02 16:58   ` Rob Herring
@ 2021-06-27  7:56     ` Hector Yuan
  0 siblings, 0 replies; 12+ messages in thread
From: Hector Yuan @ 2021-06-27  7:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-mediatek, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
	Viresh Kumar, Matthias Brugger, devicetree, linux-kernel,
	wsd_upstream

On Wed, 2021-06-02 at 11:58 -0500, Rob Herring wrote:
> On Sun, May 30, 2021 at 12:52:33AM +0800, Hector Yuan wrote:
> > From: "Hector.Yuan" <hector.yuan@mediatek.com>
> > 
> > Add devicetree bindings for MediaTek HW driver.
> > 
> > Signed-off-by: Hector.Yuan <hector.yuan@mediatek.com>
> > ---
> >  .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |   71 ++++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> > new file mode 100644
> > index 0000000..1aa4d54
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> > @@ -0,0 +1,71 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/cpufreq/cpufreq-mediatek-hw.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek's CPUFREQ Bindings
> > +
> > +maintainers:
> > +  - Hector Yuan <hector.yuan@mediatek.com>
> > +
> > +description:
> > +  CPUFREQ HW is a hardware engine used by MediaTek
> > +  SoCs to manage frequency in hardware. It is capable of controlling frequency
> > +  for multiple clusters.
> > +
> > +properties:
> > +  compatible:
> > +    const: mediatek,cpufreq-hw
> > +
> > +  reg:
> > +    minItems: 1
> > +    maxItems: 2
> > +    description: |
> > +      Addresses and sizes for the memory of the
> > +      HW bases in each frequency domain.
> > +
> > +  "#performance-domain-cells":
> > +    description:
> > +      Number of cells in a performance domain specifier. Typically 0 for nodes
> > +      representing a single performance domain and 1 for nodes providing
> > +      multiple performance domains (e.g. performance controllers), but can be
> > +      any value as specified by device tree binding documentation of particular
> > +      provider.
> > +    enum: [ 0, 1 ]
> 
> Can't you restrict this to be 1 for Mediatek h/w? Even if you sometimes 
> have a single domain, it's probably more simple for the driver if this 
> is fixed.
> 
OK, I will restrict this as 1 in next version.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#performance-domain-cells"
> > +
> > +additionalProperties: true
> 
> Should be false.
> 
OK, thanks
> > +
> > +examples:
> > +  - |
> > +    cpus {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            cpu0: cpu@0 {
> > +                device_type = "cpu";
> > +                compatible = "arm,cortex-a55";
> > +                enable-method = "psci";
> > +                performance-domains = <&performance 0>;
> > +                reg = <0x000>;
> > +            };
> > +    };
> > +
> > +    /* ... */
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        performance: performance-controller@11bc00 {
> > +            compatible = "mediatek,cpufreq-hw";
> > +            reg = <0 0x0011bc10 0 0x120>, <0 0x0011bd30 0 0x120>;
> > +
> > +            #performance-domain-cells = <1>;
> > +        };
> > +    };
> > -- 
> > 1.7.9.5


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

* Re: [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
       [not found]     ` <1624781848.1958.16.camel@mtkswgap22>
@ 2021-06-28  7:26       ` Viresh Kumar
  2021-06-28  9:09         ` Sudeep Holla
  0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2021-06-28  7:26 UTC (permalink / raw)
  To: Hector Yuan, Sudeep.Holla
  Cc: linux-mediatek, linux-arm-kernel, linux-pm, Rafael J. Wysocki,
	Matthias Brugger, Rob Herring, devicetree, linux-kernel,
	wsd_upstream

On 27-06-21, 16:17, Hector Yuan wrote:
> On Mon, 2021-06-14 at 16:10 +0530, Viresh Kumar wrote:
> > On 30-05-21, 00:52, Hector Yuan wrote:
> > > +static int mtk_get_related_cpus(int index, struct cpufreq_mtk *c)
> > > +{
> > > +	struct device_node *cpu_np;
> > > +	struct of_phandle_args args;
> > > +	int cpu, ret;
> > > +
> > > +	for_each_possible_cpu(cpu) {
> > > +		cpu_np = of_cpu_device_node_get(cpu);
> > > +		if (!cpu_np)
> > > +			continue;
> > > +
> > > +		ret = of_parse_phandle_with_args(cpu_np, "performance-domains",
> > > +						 "#performance-domain-cells", 0,
> > > +						 &args);
> > > +		of_node_put(cpu_np);
> > > +		if (ret < 0)
> > > +			continue;
> > > +
> > > +		if (index == args.args[0]) {
> > > +			cpumask_set_cpu(cpu, &c->related_cpus);
> > > +			mtk_freq_domain_map[cpu] = c;
> > > +		}
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > 
> > I really hope this can be moved to a common place as more than one
> > drier should be required to parse this thing.
> > 
> 
> Yes, this can be a common part for all performance domain users. But may
> I know whats your suggestion? Put this API in another file or? Thanks

Rob, Sudeep: You guys have a suggestion on where can we keep a routine for this
?

-- 
viresh

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

* Re: [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
  2021-06-28  7:26       ` Viresh Kumar
@ 2021-06-28  9:09         ` Sudeep Holla
  2021-06-29  2:47           ` Viresh Kumar
  0 siblings, 1 reply; 12+ messages in thread
From: Sudeep Holla @ 2021-06-28  9:09 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Hector Yuan, linux-mediatek, linux-arm-kernel, linux-pm,
	Rafael J. Wysocki, Matthias Brugger, Sudeep Holla, Rob Herring,
	devicetree, linux-kernel, wsd_upstream

On Mon, Jun 28, 2021 at 12:56:41PM +0530, Viresh Kumar wrote:
> On 27-06-21, 16:17, Hector Yuan wrote:
> > On Mon, 2021-06-14 at 16:10 +0530, Viresh Kumar wrote:
> > > On 30-05-21, 00:52, Hector Yuan wrote:
> > > > +static int mtk_get_related_cpus(int index, struct cpufreq_mtk *c)
> > > > +{
> > > > +	struct device_node *cpu_np;
> > > > +	struct of_phandle_args args;
> > > > +	int cpu, ret;
> > > > +
> > > > +	for_each_possible_cpu(cpu) {
> > > > +		cpu_np = of_cpu_device_node_get(cpu);
> > > > +		if (!cpu_np)
> > > > +			continue;
> > > > +
> > > > +		ret = of_parse_phandle_with_args(cpu_np, "performance-domains",
> > > > +						 "#performance-domain-cells", 0,
> > > > +						 &args);
> > > > +		of_node_put(cpu_np);
> > > > +		if (ret < 0)
> > > > +			continue;
> > > > +
> > > > +		if (index == args.args[0]) {
> > > > +			cpumask_set_cpu(cpu, &c->related_cpus);
> > > > +			mtk_freq_domain_map[cpu] = c;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > >
> > > I really hope this can be moved to a common place as more than one
> > > drier should be required to parse this thing.
> > >
> >
> > Yes, this can be a common part for all performance domain users. But may
> > I know whats your suggestion? Put this API in another file or? Thanks
>
> Rob, Sudeep: You guys have a suggestion on where can we keep a routine for this
> ?

Probably in driver/cpufreq or some related headers if it needs to access
related_cpus and is more cpufreq related in that way ?

Orthogonal to that, I prefer to make the generic function take list_name
and cells_name as generic. I see we can reuse that qcom-cpufreq-hw.c
with "qcom,freq-domain" and "#freq-domain-cells".

--
Regards,
Sudeep

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

* Re: [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
  2021-06-28  9:09         ` Sudeep Holla
@ 2021-06-29  2:47           ` Viresh Kumar
  2021-06-29  9:02             ` Sudeep Holla
  0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2021-06-29  2:47 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Hector Yuan, linux-mediatek, linux-arm-kernel, linux-pm,
	Rafael J. Wysocki, Matthias Brugger, Rob Herring, devicetree,
	linux-kernel, wsd_upstream

On 28-06-21, 10:09, Sudeep Holla wrote:
> Probably in driver/cpufreq or some related headers if it needs to access
> related_cpus and is more cpufreq related in that way ?

It just needs to set a mask, so doesn't really depend on cpufreq. I
was wondering if drivers/opp/of.c may be used for this, and I am not
sure.

> Orthogonal to that, I prefer to make the generic function take list_name
> and cells_name as generic. I see we can reuse that qcom-cpufreq-hw.c
> with "qcom,freq-domain" and "#freq-domain-cells".

Yes.

-- 
viresh

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

* Re: [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
  2021-06-29  2:47           ` Viresh Kumar
@ 2021-06-29  9:02             ` Sudeep Holla
  2021-06-29  9:05               ` Viresh Kumar
  0 siblings, 1 reply; 12+ messages in thread
From: Sudeep Holla @ 2021-06-29  9:02 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Hector Yuan, linux-mediatek, linux-arm-kernel, linux-pm,
	Rafael J. Wysocki, Sudeep Holla, Matthias Brugger, Rob Herring,
	devicetree, linux-kernel, wsd_upstream

On Tue, Jun 29, 2021 at 08:17:19AM +0530, Viresh Kumar wrote:
> On 28-06-21, 10:09, Sudeep Holla wrote:
> > Probably in driver/cpufreq or some related headers if it needs to access
> > related_cpus and is more cpufreq related in that way ?
>
> It just needs to set a mask, so doesn't really depend on cpufreq. I
> was wondering if drivers/opp/of.c may be used for this, and I am not
> sure.
>

Sounds good to me.

--
Regards,
Sudeep

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

* Re: [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
  2021-06-29  9:02             ` Sudeep Holla
@ 2021-06-29  9:05               ` Viresh Kumar
       [not found]                 ` <1625418089.1958.48.camel@mtkswgap22>
  0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2021-06-29  9:05 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Hector Yuan, linux-mediatek, linux-arm-kernel, linux-pm,
	Rafael J. Wysocki, Matthias Brugger, Rob Herring, devicetree,
	linux-kernel, wsd_upstream

On 29-06-21, 10:02, Sudeep Holla wrote:
> On Tue, Jun 29, 2021 at 08:17:19AM +0530, Viresh Kumar wrote:
> > On 28-06-21, 10:09, Sudeep Holla wrote:
> > > Probably in driver/cpufreq or some related headers if it needs to access
> > > related_cpus and is more cpufreq related in that way ?
> >
> > It just needs to set a mask, so doesn't really depend on cpufreq. I
> > was wondering if drivers/opp/of.c may be used for this, and I am not
> > sure.
> >
> 
> Sounds good to me.

Okay.

Hector, add the routine to of.c in OPP core.

-- 
viresh

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

* Re: [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW
       [not found]                 ` <1625418089.1958.48.camel@mtkswgap22>
@ 2021-07-05  4:32                   ` Viresh Kumar
  0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2021-07-05  4:32 UTC (permalink / raw)
  To: Hector Yuan
  Cc: Sudeep Holla, linux-mediatek, linux-arm-kernel, linux-pm,
	Rafael J. Wysocki, Matthias Brugger, Rob Herring, devicetree,
	linux-kernel, wsd_upstream

On 05-07-21, 01:01, Hector Yuan wrote:
> Hi, Viresh:
> 
> OK, Thank you.
> 
> I'll add this routine in of.c.
> May I know the below prototype is okay to you?
> 
> static int _of_get_related_cpus(const char *list_name, const char
> *cell_name, int index, struct cpumask *m)

of_perf_domain_get_sharing_cpumask(int cpu, const char *list_name,
        const char *cell_name, struct cpumask *cpumask);

And actually add this in cpufreq.h, this doesn't have anything to do with OPP
and I can assume that a platform not enabling the OPP functionality to end up
using it later.

Rafael, will that be fine ? Or we add it in a completely new header ? The users
are going to be cpufreq drivers only I believe.

> And maybe I have to find somewhere to update my
> mtk_freq_domain_map[cpu]?
> 
> static int mtk_get_related_cpus(int index, struct cpufreq_mtk *c)
> {
> 	for_each_possible_cpu(cpu) {
> 		cpumask_set_cpu(cpu, &c->related_cpus);//related_cpu can copy policy
> cpus
> 		mtk_freq_domain_map[cpu] = c;//need to find somewhere to assign
> 
> 	}
> }

I am not sure I understood this part well.

-- 
viresh

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

end of thread, other threads:[~2021-07-05  4:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-29 16:52 [PATCH v12] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver Hector Yuan
2021-05-29 16:52 ` [PATCH v12 1/2] cpufreq: mediatek-hw: Add support for CPUFREQ HW Hector Yuan
2021-06-14 10:40   ` Viresh Kumar
     [not found]     ` <1624781848.1958.16.camel@mtkswgap22>
2021-06-28  7:26       ` Viresh Kumar
2021-06-28  9:09         ` Sudeep Holla
2021-06-29  2:47           ` Viresh Kumar
2021-06-29  9:02             ` Sudeep Holla
2021-06-29  9:05               ` Viresh Kumar
     [not found]                 ` <1625418089.1958.48.camel@mtkswgap22>
2021-07-05  4:32                   ` Viresh Kumar
2021-05-29 16:52 ` [PATCH v12 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW Hector Yuan
2021-06-02 16:58   ` Rob Herring
2021-06-27  7:56     ` Hector Yuan

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