linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver
@ 2020-09-09  9:51 Hector Yuan
  2020-09-09  9:51 ` [PATCH v5 1/2] " Hector Yuan
  2020-09-09  9:51 ` [PATCH v5 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW Hector Yuan
  0 siblings, 2 replies; 5+ messages in thread
From: Hector Yuan @ 2020-09-09  9:51 UTC (permalink / raw)
  To: linux-mediatek, linux-arm-kernel, linux-pm, devicetree,
	Rafael J. Wysocki, Viresh Kumar, Rob Herring
  Cc: linux-kernel, 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. 

This patch depends on the MT6799 DTS patch submitted by Hanks Chen
 https://lkml.org/lkml/2020/8/4/1094
 

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

 .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |  141 ++++++++++
 drivers/cpufreq/Kconfig.arm                        |   12 +
 drivers/cpufreq/Makefile                           |    1 +
 drivers/cpufreq/mediatek-cpufreq-hw.c              |  289 ++++++++++++++++++++
 4 files changed, 443 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] 5+ messages in thread

* [PATCH v5 1/2] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver
  2020-09-09  9:51 [PATCH v5] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver Hector Yuan
@ 2020-09-09  9:51 ` Hector Yuan
  2020-09-09 11:29   ` Viresh Kumar
  2020-09-09  9:51 ` [PATCH v5 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW Hector Yuan
  1 sibling, 1 reply; 5+ messages in thread
From: Hector Yuan @ 2020-09-09  9:51 UTC (permalink / raw)
  To: linux-mediatek, linux-arm-kernel, linux-pm, devicetree,
	Rafael J. Wysocki, Viresh Kumar, Rob Herring
  Cc: linux-kernel, wsd_upstream, hector.yuan

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

Add MT6779 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 |  289 +++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index c6cbfc8..8e58c12 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -121,6 +121,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 f6670c4..dc1f371 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -57,6 +57,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..ae4b38b
--- /dev/null
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cpufreq.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+#define LUT_MAX_ENTRIES			32U
+#define LUT_FREQ			GENMASK(11, 0)
+#define LUT_ROW_SIZE			0x4
+
+enum {
+	REG_LUT_TABLE,
+	REG_ENABLE,
+	REG_PERF_STATE,
+
+	REG_ARRAY_SIZE,
+};
+
+struct cpufreq_mtk {
+	struct cpufreq_frequency_table *table;
+	void __iomem *reg_bases[REG_ARRAY_SIZE];
+	cpumask_t related_cpus;
+};
+
+static const u16 cpufreq_mtk_offsets[REG_ARRAY_SIZE] = {
+	[REG_LUT_TABLE]		= 0x0,
+	[REG_ENABLE]		= 0x84,
+	[REG_PERF_STATE]	= 0x88,
+};
+
+static struct cpufreq_mtk *mtk_freq_domain_map[NR_CPUS];
+
+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_PERF_STATE]);
+	arch_set_freq_scale(policy->related_cpus,
+			    policy->freq_table[index].frequency,
+			    policy->cpuinfo.max_freq);
+
+	return 0;
+}
+
+static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
+{
+	struct cpufreq_mtk *c;
+	struct cpufreq_policy *policy;
+	unsigned int index;
+
+	policy = cpufreq_cpu_get_raw(cpu);
+	if (!policy)
+		return 0;
+
+	c = policy->driver_data;
+
+	index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
+	index = min(index, LUT_MAX_ENTRIES - 1);
+
+	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;
+
+	cpu_dev = get_cpu_device(policy->cpu);
+	if (!cpu_dev) {
+		pr_err("%s: failed to get cpu%d device\n", __func__,
+		       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;
+
+	/* HW should be in enabled state to proceed now */
+	writel_relaxed(0x1, c->reg_bases[REG_ENABLE]);
+
+	return 0;
+}
+
+static struct cpufreq_driver cpufreq_mtk_hw_driver = {
+	.flags		= CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+	.verify		= cpufreq_generic_frequency_table_verify,
+	.target_index	= mtk_cpufreq_hw_target_index,
+	.get		= mtk_cpufreq_hw_get,
+	.init		= mtk_cpufreq_hw_cpu_init,
+	.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_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;
+
+	return 0;
+}
+
+static int mtk_get_related_cpus(int index, struct cpumask *m)
+{
+	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, "mtk-freq-domain",
+						 "#freq-domain-cells", 0,
+						 &args);
+		of_node_put(cpu_np);
+		if (ret < 0)
+			continue;
+
+		if (index == args.args[0])
+			cpumask_set_cpu(cpu, m);
+	}
+
+	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 resource *res;
+	struct device *dev = &pdev->dev;
+	int ret, i, cpu_r;
+	void __iomem *base;
+
+	if (mtk_freq_domain_map[cpu])
+		return 0;
+
+	c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+	if (!c)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	for (i = REG_LUT_TABLE; i < REG_ARRAY_SIZE; i++)
+		c->reg_bases[i] = base + offsets[i];
+
+	ret = mtk_get_related_cpus(index, &c->related_cpus);
+	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 OPP\n", index);
+		return ret;
+	}
+
+	for_each_cpu(cpu_r, &c->related_cpus)
+		mtk_freq_domain_map[cpu_r] = c;
+
+	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_dbg(&pdev->dev, "Failed to get cpu %d device\n",
+				cpu);
+			continue;
+		}
+
+		ret = of_parse_phandle_with_args(cpu_np, "mtk-freq-domain",
+						 "#freq-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,
+	},
+};
+
+static int __init mtk_cpufreq_hw_init(void)
+{
+	return platform_driver_register(&mtk_cpufreq_hw_driver);
+}
+module_init(mtk_cpufreq_hw_init);
+
+static void __exit mtk_cpufreq_hw_exit(void)
+{
+	platform_driver_unregister(&mtk_cpufreq_hw_driver);
+}
+module_exit(mtk_cpufreq_hw_exit);
+
+MODULE_DESCRIPTION("mtk CPUFREQ HW Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

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

* [PATCH v5 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW
  2020-09-09  9:51 [PATCH v5] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver Hector Yuan
  2020-09-09  9:51 ` [PATCH v5 1/2] " Hector Yuan
@ 2020-09-09  9:51 ` Hector Yuan
  1 sibling, 0 replies; 5+ messages in thread
From: Hector Yuan @ 2020-09-09  9:51 UTC (permalink / raw)
  To: linux-mediatek, linux-arm-kernel, linux-pm, devicetree,
	Rafael J. Wysocki, Viresh Kumar, Rob Herring
  Cc: linux-kernel, 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      |  141 ++++++++++++++++++++
 1 file changed, 141 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..118a163
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
@@ -0,0 +1,141 @@
+# 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.
+
+  reg-names:
+    items:
+      - const: "freq-domain0"
+      - const: "freq-domain1"
+    description: |
+      Frequency domain name. i.e.
+      "freq-domain0", "freq-domain1".
+
+  "#freq-domain-cells":
+    const: 1
+    description: |
+      Number of cells in a freqency domain specifier.
+
+  mtk-freq-domain:
+    maxItems: 1
+    description: |
+      Define this cpu belongs to which frequency domain. i.e.
+      cpu0-3 belong to frequency domain0,
+      cpu4-6 belong to frequency domain1.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - "#freq-domain-cells"
+
+examples:
+  - |
+    cpus {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            cpu0: cpu@0 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a55";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 0>;
+                reg = <0x000>;
+            };
+
+            cpu1: cpu@1 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a55";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 0>;
+                reg = <0x100>;
+            };
+
+            cpu2: cpu@2 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a55";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 0>;
+                reg = <0x200>;
+            };
+
+            cpu3: cpu@3 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a55";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 0>;
+                reg = <0x300>;
+            };
+
+            cpu4: cpu@4 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a55";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 1>;
+                reg = <0x400>;
+            };
+
+            cpu5: cpu@5 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a55";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 1>;
+                reg = <0x500>;
+            };
+
+            cpu6: cpu@6 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a75";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 1>;
+                reg = <0x600>;
+            };
+
+            cpu7: cpu@7 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a75";
+                enable-method = "psci";
+                mtk-freq-domain = <&cpufreq_hw 1>;
+                reg = <0x700>;
+            };
+    };
+
+    /* ... */
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        cpufreq_hw: cpufreq@11bc00 {
+            compatible = "mediatek,cpufreq-hw";
+            reg = <0 0x11bc10 0 0x8c>,
+               <0 0x11bca0 0 0x8c>;
+            reg-names = "freq-domain0", "freq-domain1";
+            #freq-domain-cells = <1>;
+        };
+    };
+
+
+
+
-- 
1.7.9.5

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

* Re: [PATCH v5 1/2] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver
  2020-09-09  9:51 ` [PATCH v5 1/2] " Hector Yuan
@ 2020-09-09 11:29   ` Viresh Kumar
  2020-09-09 12:26     ` Hector Yuan
  0 siblings, 1 reply; 5+ messages in thread
From: Viresh Kumar @ 2020-09-09 11:29 UTC (permalink / raw)
  To: Hector Yuan
  Cc: linux-mediatek, linux-arm-kernel, linux-pm, devicetree,
	Rafael J. Wysocki, Rob Herring, linux-kernel, wsd_upstream

On 09-09-20, 17:51, Hector Yuan wrote:
> From: "Hector.Yuan" <hector.yuan@mediatek.com>
> 
> Add MT6779 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 |  289 +++++++++++++++++++++++++++++++++
>  3 files changed, 302 insertions(+)
>  create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c
> 
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index c6cbfc8..8e58c12 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -121,6 +121,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 f6670c4..dc1f371 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -57,6 +57,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..ae4b38b
> --- /dev/null
> +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
> @@ -0,0 +1,289 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/cpufreq.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/slab.h>
> +
> +#define LUT_MAX_ENTRIES			32U
> +#define LUT_FREQ			GENMASK(11, 0)
> +#define LUT_ROW_SIZE			0x4
> +
> +enum {
> +	REG_LUT_TABLE,
> +	REG_ENABLE,
> +	REG_PERF_STATE,
> +
> +	REG_ARRAY_SIZE,
> +};
> +
> +struct cpufreq_mtk {
> +	struct cpufreq_frequency_table *table;
> +	void __iomem *reg_bases[REG_ARRAY_SIZE];
> +	cpumask_t related_cpus;
> +};
> +
> +static const u16 cpufreq_mtk_offsets[REG_ARRAY_SIZE] = {
> +	[REG_LUT_TABLE]		= 0x0,
> +	[REG_ENABLE]		= 0x84,
> +	[REG_PERF_STATE]	= 0x88,
> +};
> +
> +static struct cpufreq_mtk *mtk_freq_domain_map[NR_CPUS];
> +
> +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_PERF_STATE]);
> +	arch_set_freq_scale(policy->related_cpus,
> +			    policy->freq_table[index].frequency,
> +			    policy->cpuinfo.max_freq);

Please drop the arch_set_freq_scale() stuff. This is getting moved to
cpufreq core in another series and will be done by core code for
everyone. Sorry I forgot to mention that earlier.

> +
> +	return 0;
> +}
> +
> +static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
> +{
> +	struct cpufreq_mtk *c;
> +	struct cpufreq_policy *policy;
> +	unsigned int index;
> +
> +	policy = cpufreq_cpu_get_raw(cpu);
> +	if (!policy)
> +		return 0;
> +
> +	c = policy->driver_data;

What about doing this instead ?

        c = mtk_freq_domain_map[cpu];

You won't require to get policy here, right ?

> +
> +	index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
> +	index = min(index, LUT_MAX_ENTRIES - 1);
> +
> +	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;
> +
> +	cpu_dev = get_cpu_device(policy->cpu);
> +	if (!cpu_dev) {

Why do you need this here ? You don't use it.

> +		pr_err("%s: failed to get cpu%d device\n", __func__,
> +		       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;
> +
> +	/* HW should be in enabled state to proceed now */
> +	writel_relaxed(0x1, c->reg_bases[REG_ENABLE]);

Don't you need to do the opposite of this as well ?
cpufreq_driver->exit ?

> +
> +	return 0;
> +}
> +
> +static struct cpufreq_driver cpufreq_mtk_hw_driver = {
> +	.flags		= CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> +			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> +	.verify		= cpufreq_generic_frequency_table_verify,
> +	.target_index	= mtk_cpufreq_hw_target_index,
> +	.get		= mtk_cpufreq_hw_get,
> +	.init		= mtk_cpufreq_hw_cpu_init,
> +	.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_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;
> +
> +	return 0;
> +}
> +
> +static int mtk_get_related_cpus(int index, struct cpumask *m)
> +{
> +	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, "mtk-freq-domain",
> +						 "#freq-domain-cells", 0,
> +						 &args);
> +		of_node_put(cpu_np);
> +		if (ret < 0)
> +			continue;
> +
> +		if (index == args.args[0])
> +			cpumask_set_cpu(cpu, m);
> +	}
> +
> +	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 resource *res;
> +	struct device *dev = &pdev->dev;
> +	int ret, i, cpu_r;
> +	void __iomem *base;
> +
> +	if (mtk_freq_domain_map[cpu])
> +		return 0;
> +
> +	c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> +	if (!c)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);

Can you use devm_platform_ioremap_resource() here ?

> +
> +	for (i = REG_LUT_TABLE; i < REG_ARRAY_SIZE; i++)
> +		c->reg_bases[i] = base + offsets[i];
> +
> +	ret = mtk_get_related_cpus(index, &c->related_cpus);
> +	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 OPP\n", index);

Still the wrong error. You didn't fail to create OPP here :)

Just search for OPP in the driver and see if you need to fix that.

> +		return ret;
> +	}
> +
> +	for_each_cpu(cpu_r, &c->related_cpus)
> +		mtk_freq_domain_map[cpu_r] = c;

What about doing this from mtk_get_related_cpus() instead? We already
run the loop there.

> +
> +	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_dbg(&pdev->dev, "Failed to get cpu %d device\n",
> +				cpu);

This deserves dev_err and maybe you should error out completely ?
Currently if of_cpu_device_node_get() fails for all CPUs, you will
still try to register the cpufreq driver.

> +			continue;
> +		}
> +
> +		ret = of_parse_phandle_with_args(cpu_np, "mtk-freq-domain",
> +						 "#freq-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,
> +	},
> +};
> +
> +static int __init mtk_cpufreq_hw_init(void)
> +{
> +	return platform_driver_register(&mtk_cpufreq_hw_driver);
> +}
> +module_init(mtk_cpufreq_hw_init);
> +
> +static void __exit mtk_cpufreq_hw_exit(void)
> +{
> +	platform_driver_unregister(&mtk_cpufreq_hw_driver);
> +}
> +module_exit(mtk_cpufreq_hw_exit);

You can use this instead.

module_platform_driver(mtk_cpufreq_hw_driver);

> +
> +MODULE_DESCRIPTION("mtk CPUFREQ HW Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.7.9.5

-- 
viresh

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

* Re: [PATCH v5 1/2] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver
  2020-09-09 11:29   ` Viresh Kumar
@ 2020-09-09 12:26     ` Hector Yuan
  0 siblings, 0 replies; 5+ messages in thread
From: Hector Yuan @ 2020-09-09 12:26 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linux-mediatek, linux-arm-kernel, linux-pm, devicetree,
	Rafael J. Wysocki, Rob Herring, linux-kernel, wsd_upstream

On Wed, 2020-09-09 at 16:59 +0530, Viresh Kumar wrote:
> On 09-09-20, 17:51, Hector Yuan wrote:
> > From: "Hector.Yuan" <hector.yuan@mediatek.com>
> > 
> > Add MT6779 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 |  289 +++++++++++++++++++++++++++++++++
> >  3 files changed, 302 insertions(+)
> >  create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c
> > 
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index c6cbfc8..8e58c12 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -121,6 +121,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 f6670c4..dc1f371 100644
> > --- a/drivers/cpufreq/Makefile
> > +++ b/drivers/cpufreq/Makefile
> > @@ -57,6 +57,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..ae4b38b
> > --- /dev/null
> > +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
> > @@ -0,0 +1,289 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2020 MediaTek Inc.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/cpufreq.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/slab.h>
> > +
> > +#define LUT_MAX_ENTRIES			32U
> > +#define LUT_FREQ			GENMASK(11, 0)
> > +#define LUT_ROW_SIZE			0x4
> > +
> > +enum {
> > +	REG_LUT_TABLE,
> > +	REG_ENABLE,
> > +	REG_PERF_STATE,
> > +
> > +	REG_ARRAY_SIZE,
> > +};
> > +
> > +struct cpufreq_mtk {
> > +	struct cpufreq_frequency_table *table;
> > +	void __iomem *reg_bases[REG_ARRAY_SIZE];
> > +	cpumask_t related_cpus;
> > +};
> > +
> > +static const u16 cpufreq_mtk_offsets[REG_ARRAY_SIZE] = {
> > +	[REG_LUT_TABLE]		= 0x0,
> > +	[REG_ENABLE]		= 0x84,
> > +	[REG_PERF_STATE]	= 0x88,
> > +};
> > +
> > +static struct cpufreq_mtk *mtk_freq_domain_map[NR_CPUS];
> > +
> > +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_PERF_STATE]);
> > +	arch_set_freq_scale(policy->related_cpus,
> > +			    policy->freq_table[index].frequency,
> > +			    policy->cpuinfo.max_freq);
> 
> Please drop the arch_set_freq_scale() stuff. This is getting moved to
> cpufreq core in another series and will be done by core code for
> everyone. Sorry I forgot to mention that earlier.
> 
OK, will remove it, Thank you.
> > +
> > +	return 0;
> > +}
> > +
> > +static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
> > +{
> > +	struct cpufreq_mtk *c;
> > +	struct cpufreq_policy *policy;
> > +	unsigned int index;
> > +
> > +	policy = cpufreq_cpu_get_raw(cpu);
> > +	if (!policy)
> > +		return 0;
> > +
> > +	c = policy->driver_data;
> 
> What about doing this instead ?
> 
>         c = mtk_freq_domain_map[cpu];
> 
> You won't require to get policy here, right ?
> 
Yes, will get c from platform data.
> > +
> > +	index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
> > +	index = min(index, LUT_MAX_ENTRIES - 1);
> > +
> > +	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;
> > +
> > +	cpu_dev = get_cpu_device(policy->cpu);
> > +	if (!cpu_dev) {
> 
> Why do you need this here ? You don't use it.
> 
OK, will remove dummy code.
> > +		pr_err("%s: failed to get cpu%d device\n", __func__,
> > +		       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;
> > +
> > +	/* HW should be in enabled state to proceed now */
> > +	writel_relaxed(0x1, c->reg_bases[REG_ENABLE]);
> 
> Don't you need to do the opposite of this as well ?
> cpufreq_driver->exit ?
> 
OK, will define exit function.
> > +
> > +	return 0;
> > +}
> > +
> > +static struct cpufreq_driver cpufreq_mtk_hw_driver = {
> > +	.flags		= CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> > +			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> > +	.verify		= cpufreq_generic_frequency_table_verify,
> > +	.target_index	= mtk_cpufreq_hw_target_index,
> > +	.get		= mtk_cpufreq_hw_get,
> > +	.init		= mtk_cpufreq_hw_cpu_init,
> > +	.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_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;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_get_related_cpus(int index, struct cpumask *m)
> > +{
> > +	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, "mtk-freq-domain",
> > +						 "#freq-domain-cells", 0,
> > +						 &args);
> > +		of_node_put(cpu_np);
> > +		if (ret < 0)
> > +			continue;
> > +
> > +		if (index == args.args[0])
> > +			cpumask_set_cpu(cpu, m);
> > +	}
> > +
> > +	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 resource *res;
> > +	struct device *dev = &pdev->dev;
> > +	int ret, i, cpu_r;
> > +	void __iomem *base;
> > +
> > +	if (mtk_freq_domain_map[cpu])
> > +		return 0;
> > +
> > +	c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> > +	if (!c)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> > +	base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> 
> Can you use devm_platform_ioremap_resource() here ?
> 
OK, will use that.
> > +
> > +	for (i = REG_LUT_TABLE; i < REG_ARRAY_SIZE; i++)
> > +		c->reg_bases[i] = base + offsets[i];
> > +
> > +	ret = mtk_get_related_cpus(index, &c->related_cpus);
> > +	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 OPP\n", index);
> 
> Still the wrong error. You didn't fail to create OPP here :)
> 
> Just search for OPP in the driver and see if you need to fix that.
> 
OK, will modify this error message.
> > +		return ret;
> > +	}
> > +
> > +	for_each_cpu(cpu_r, &c->related_cpus)
> > +		mtk_freq_domain_map[cpu_r] = c;
> 
> What about doing this from mtk_get_related_cpus() instead? We already
> run the loop there.
> 
OK, will merge this to mtk_get_related_cpus().
> > +
> > +	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_dbg(&pdev->dev, "Failed to get cpu %d device\n",
> > +				cpu);
> 
> This deserves dev_err and maybe you should error out completely ?
> Currently if of_cpu_device_node_get() fails for all CPUs, you will
> still try to register the cpufreq driver.
> 
OK, will use dev_err and return fail.
> > +			continue;
> > +		}
> > +
> > +		ret = of_parse_phandle_with_args(cpu_np, "mtk-freq-domain",
> > +						 "#freq-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,
> > +	},
> > +};
> > +
> > +static int __init mtk_cpufreq_hw_init(void)
> > +{
> > +	return platform_driver_register(&mtk_cpufreq_hw_driver);
> > +}
> > +module_init(mtk_cpufreq_hw_init);
> > +
> > +static void __exit mtk_cpufreq_hw_exit(void)
> > +{
> > +	platform_driver_unregister(&mtk_cpufreq_hw_driver);
> > +}
> > +module_exit(mtk_cpufreq_hw_exit);
> 
> You can use this instead.
> 
> module_platform_driver(mtk_cpufreq_hw_driver);
> 
OK, will do that.
> > +
> > +MODULE_DESCRIPTION("mtk CPUFREQ HW Driver");
> > +MODULE_LICENSE("GPL v2");
> > -- 
> > 1.7.9.5
> 


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

end of thread, other threads:[~2020-09-09 15:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09  9:51 [PATCH v5] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver Hector Yuan
2020-09-09  9:51 ` [PATCH v5 1/2] " Hector Yuan
2020-09-09 11:29   ` Viresh Kumar
2020-09-09 12:26     ` Hector Yuan
2020-09-09  9:51 ` [PATCH v5 2/2] dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW 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).