[PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[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
  dt-bindings: cpufreq: add  MediaTek cpufreq bindings

 .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |   56 +++++
 arch/arm64/configs/defconfig                       |    1 +
 drivers/cpufreq/Kconfig.arm                        |   11 +
 drivers/cpufreq/Makefile                           |    1 +
 drivers/cpufreq/mediatek-cpufreq-hw.c              |  255 ++++++++++++++++++++
 5 files changed, 324 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
 create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c

[PATCH v1 1/2] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[Hector Yuan]

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

Add MT6779 cpufreq HW support.

Signed-off-by: Hector.Yuan <hector.yuan@mediatek.com>
---
 arch/arm64/configs/defconfig          |    1 +
 drivers/cpufreq/Kconfig.arm           |   11 ++
 drivers/cpufreq/Makefile              |    1 +
 drivers/cpufreq/mediatek-cpufreq-hw.c |  255 +++++++++++++++++++++++++++++++++
 4 files changed, 268 insertions(+)
 create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 883e8ba..866a1bf 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -86,6 +86,7 @@ CONFIG_CPUFREQ_DT=y
 CONFIG_ACPI_CPPC_CPUFREQ=m
 CONFIG_ARM_ALLWINNER_SUN50I_CPUFREQ_NVMEM=m
 CONFIG_ARM_ARMADA_37XX_CPUFREQ=y
+CONFIG_ARM_MEDIATEK_CPUFREQ_HW=m
 CONFIG_ARM_SCPI_CPUFREQ=y
 CONFIG_ARM_IMX_CPUFREQ_DT=m
 CONFIG_ARM_QCOM_CPUFREQ_NVMEM=y
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index c6cbfc8..81f1cc1 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -121,6 +121,17 @@ 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
+	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..920cc87
--- /dev/null
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -0,0 +1,255 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#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/pm_opp.h>
+#include <linux/slab.h>
+
+#define LUT_MAX_ENTRIES			32U
+#define LUT_FREQ			GENMASK(11, 0)
+#define LUT_VOLT			GENMASK(28, 12)
+#define LUT_ROW_SIZE			0x4
+
+/* Register offsets */
+#define REG_ENABLE			0x84
+#define REG_PERF_STATE		0x88
+
+static struct platform_device *global_pdev;
+static int mtk_cpufreq_hw_target_index(struct cpufreq_policy *policy,
+				       unsigned int index)
+{
+	void __iomem *perf_state_reg = policy->driver_data;
+	unsigned long freq = policy->freq_table[index].frequency;
+
+	writel_relaxed(index, perf_state_reg);
+	arch_set_freq_scale(policy->related_cpus, freq,
+			    policy->cpuinfo.max_freq);
+	return 0;
+}
+
+static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
+{
+	void __iomem *perf_state_reg;
+	struct cpufreq_policy *policy;
+	unsigned int index;
+
+	policy = cpufreq_cpu_get_raw(cpu);
+	if (!policy)
+		return 0;
+
+	perf_state_reg = policy->driver_data;
+
+	index = readl_relaxed(perf_state_reg);
+	index = min(index, LUT_MAX_ENTRIES - 1);
+
+	return policy->freq_table[index].frequency;
+}
+
+static int mtk_cpufreq_hw_read_lut(struct device *cpu_dev,
+				   struct cpufreq_policy *policy,
+				   void __iomem *base)
+{
+	u32 data;
+	u32 freq, volt, prev_freq = 0;
+	int i = 0;
+	struct cpufreq_frequency_table	*table;
+
+	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		data = readl_relaxed(base + (i * LUT_ROW_SIZE));
+		freq = FIELD_GET(LUT_FREQ, data) * 1000;
+		volt = FIELD_GET(LUT_VOLT, data);
+		if (freq != prev_freq) {
+			table[i].frequency = freq;
+			dev_pm_opp_add(cpu_dev, freq * 1000, volt);
+			dev_dbg(cpu_dev, "index=%d freq=%d, volt=%d\n", i,
+				freq, volt);
+		} else {
+			break;
+		}
+		prev_freq = freq;
+	}
+	table[i].frequency = CPUFREQ_TABLE_END;
+	policy->freq_table = table;
+	dev_pm_opp_set_sharing_cpus(cpu_dev, policy->cpus);
+
+	return 0;
+}
+
+static void 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);
+	}
+}
+
+static int mtk_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
+{
+	struct device *dev = &global_pdev->dev;
+	struct of_phandle_args args;
+	struct device_node *cpu_np;
+	struct device *cpu_dev;
+	struct resource *res;
+	void __iomem *base;
+	int ret, index;
+
+	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;
+	}
+
+	cpu_np = of_cpu_device_node_get(policy->cpu);
+	if (!cpu_np)
+		return -EINVAL;
+
+	ret = of_parse_phandle_with_args(cpu_np, "mtk,freq-domain",
+					 "#freq-domain-cells", 0, &args);
+	of_node_put(cpu_np);
+	if (ret)
+		return ret;
+
+	index = args.args[0];
+	res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);
+	if (!res)
+		return -ENODEV;
+	base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!base)
+		return -ENOMEM;
+
+	mtk_get_related_cpus(index, policy->cpus);
+	if (!cpumask_weight(policy->cpus)) {
+		dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
+		ret = -ENOENT;
+		goto error;
+	}
+
+	policy->driver_data = base + REG_PERF_STATE;
+	ret = mtk_cpufreq_hw_read_lut(cpu_dev, policy, base);
+	if (ret) {
+		dev_err(dev, "Domain-%d failed to read LUT\n", index);
+		goto error;
+	}
+
+	ret = dev_pm_opp_get_opp_count(cpu_dev);
+	if (ret <= 0) {
+		dev_err(cpu_dev, "Failed to add OPPs\n");
+		ret = -ENODEV;
+		goto error;
+	}
+
+	dev_pm_opp_of_register_em(policy->cpus);
+
+	/* HW should be in enabled state to proceed now */
+	writel_relaxed(0x1, (base + REG_ENABLE));
+	return 0;
+error:
+	devm_iounmap(dev, base);
+	return ret;
+}
+
+static int mtk_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
+{
+	struct device *cpu_dev = get_cpu_device(policy->cpu);
+	void __iomem *base = policy->driver_data - REG_PERF_STATE;
+
+	dev_pm_opp_remove_all_dynamic(cpu_dev);
+	kfree(policy->freq_table);
+	devm_iounmap(&global_pdev->dev, base);
+
+	return 0;
+}
+
+static struct freq_attr *mtk_cpufreq_hw_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	NULL
+};
+
+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,
+	.exit		= mtk_cpufreq_hw_cpu_exit,
+	.name		= "mtk-cpufreq-hw",
+	.attr		= mtk_cpufreq_hw_attr,
+};
+
+static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	global_pdev = pdev;
+	ret = cpufreq_register_driver(&cpufreq_mtk_hw_driver);
+	if (ret)
+		dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
+	else
+		dev_dbg(&pdev->dev, "mtk CPUFreq HW driver initialized\n");
+
+	return ret;
+}
+
+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" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mtk_cpufreq_hw_match);
+
+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);
+}
+postcore_initcall(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

[PATCH v1 2/2] dt-bindings: cpufreq: add MediaTek cpufreq bindings

[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      |   56 ++++++++++++++++++++
 1 file changed, 56 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..2a35098
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
@@ -0,0 +1,56 @@
+# 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.
+
+  "#freq-domain-cells":
+    const: 1
+    description: |
+      Number of cells in a freqency domain specifier.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - "#freq-domain-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    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

Re: [PATCH v1 2/2] dt-bindings: cpufreq: add MediaTek cpufreq bindings

[Rob Herring]

On Thu, 06 Aug 2020 20:18:40 +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      |   56 ++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.example.dt.yaml: example-0: cpufreq@11bc00:reg:0: [0, 1162256, 0, 140] is too long
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.example.dt.yaml: example-0: cpufreq@11bc00:reg:1: [0, 1162400, 0, 140] is too long


See https://patchwork.ozlabs.org/patch/1341854

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

[PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[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 MT6779 DTS patch submitted by Hanks Chen
 https://lkml.org/lkml/2020/8/4/1094


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

 .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |   61 +++++
 arch/arm64/configs/defconfig                       |    1 +
 drivers/cpufreq/Kconfig.arm                        |   11 +
 drivers/cpufreq/Makefile                           |    1 +
 drivers/cpufreq/mediatek-cpufreq-hw.c              |  255 ++++++++++++++++++++
 5 files changed, 329 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
 create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c

[PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[Hector Yuan]

This patchset includes 6 patches and depends on the MT6779 DTS patch[1] submitted by Hanks Chen.
The first 3 patches are for CPUFREQ HW driver and device tree binding, which are already sent before separately [2][3]. For binding part, I add a new patch to add property in cpu schema.
Besides, we add three more patches including EM power table, SVS CPU initialize, and cooling device.

[1] https://lkml.org/lkml/2020/8/4/1094
[2] https://lkml.org/lkml/2020/9/10/13 (already reviewed by Viresh) [3] https://lkml.org/lkml/2020/9/23/384


Hector.Yuan (6):
  cpufreq: mediatek-hw: Add support for CPUFREQ HW
  dt-bindings: arm: cpus: Document 'mtk,freq-domain' property
  dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW
  cpufreq: mediatek-hw: register EM power table
  cpufreq: mediatek-hw: Add SVS CPU initialization
  cpufreq: mediatek-hw: Add cooling dev flag

 Documentation/devicetree/bindings/arm/cpus.yaml    |    6 +
 .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |   46 +++
 drivers/cpufreq/Kconfig.arm                        |   12 +
 drivers/cpufreq/Makefile                           |    1 +
 drivers/cpufreq/mediatek-cpufreq-hw.c              |  343 ++++++++++++++++++++
 5 files changed, 408 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
 create mode 100644 drivers/cpufreq/mediatek-cpufreq-hw.c

Re: [PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[Viresh Kumar]

On 23-10-20, 16:24, Hector Yuan wrote:
> This patchset includes 6 patches and depends on the MT6779 DTS patch[1] submitted by Hanks Chen.
> The first 3 patches are for CPUFREQ HW driver and device tree binding, which are already sent before separately [2][3]. For binding part, I add a new patch to add property in cpu schema.
> Besides, we add three more patches including EM power table, SVS CPU initialize, and cooling device.

And even after so many versions of these you chose to name this V1. It
is very difficult for reviewers to find time to review your stuff, and
they expect some sort of summary from you on what exactly changed from
last version and you also need to name the current version currently.

This should have been V8 and you should have added a "V7->V8 diff:"
section here, naming all the changes you did. Please send that as
reply to this email, so I can see what really changed.

-- 
viresh

Re: [PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[Hector Yuan]

On Fri, 2020-10-23 at 13:58 +0530, Viresh Kumar wrote:
> On 23-10-20, 16:24, Hector Yuan wrote:
> > This patchset includes 6 patches and depends on the MT6779 DTS patch[1] submitted by Hanks Chen.
> > The first 3 patches are for CPUFREQ HW driver and device tree binding, which are already sent before separately [2][3]. For binding part, I add a new patch to add property in cpu schema.
> > Besides, we add three more patches including EM power table, SVS CPU initialize, and cooling device.
> 
> And even after so many versions of these you chose to name this V1. It
> is very difficult for reviewers to find time to review your stuff, and
> they expect some sort of summary from you on what exactly changed from
> last version and you also need to name the current version currently.
> 
> This should have been V8 and you should have added a "V7->V8 diff:"
> section here, naming all the changes you did. Please send that as
> reply to this email, so I can see what really changed.
> 
Hi, Viresh

Sorry for your inconvenience.
#1~#3 is for cpufreq driver we have reviewed and the bindings which
separate freq domain to CPU schema.There is no change for the driver
itself.
  1. cpufreq: mediatek-hw: Add support for CPUFREQ HW
  2. dt-bindings: arm: cpus: Document 'mtk,freq-domain' property
  3. dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW

#4~#6 is for other CPU features, i.e. SVS [1]
  4. cpufreq: mediatek-hw: register EM power table
  5. cpufreq: mediatek-hw: Add SVS CPU initialization
  6. cpufreq: mediatek-hw: Add cooling dev flag

I supposed that it could be more clean to separate #4~#6 in another
patchset.May I know is it okay to you? Or I should merge all of changes
into v8 like you mentioned? Thank you.

[1]
https://patchwork.kernel.org/project/linux-mediatek/patch/20190906100514.30803-4-roger.lu%40mediatek.com/

Re: [PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[Viresh Kumar]

On 23-10-20, 17:08, Hector Yuan wrote:
> On Fri, 2020-10-23 at 13:58 +0530, Viresh Kumar wrote:
> > On 23-10-20, 16:24, Hector Yuan wrote:
> > > This patchset includes 6 patches and depends on the MT6779 DTS patch[1] submitted by Hanks Chen.
> > > The first 3 patches are for CPUFREQ HW driver and device tree binding, which are already sent before separately [2][3]. For binding part, I add a new patch to add property in cpu schema.
> > > Besides, we add three more patches including EM power table, SVS CPU initialize, and cooling device.
> > 
> > And even after so many versions of these you chose to name this V1. It
> > is very difficult for reviewers to find time to review your stuff, and
> > they expect some sort of summary from you on what exactly changed from
> > last version and you also need to name the current version currently.
> > 
> > This should have been V8 and you should have added a "V7->V8 diff:"
> > section here, naming all the changes you did. Please send that as
> > reply to this email, so I can see what really changed.
> > 
> Hi, Viresh
> 
> Sorry for your inconvenience.
> #1~#3 is for cpufreq driver we have reviewed and the bindings which
> separate freq domain to CPU schema.There is no change for the driver
> itself.
>   1. cpufreq: mediatek-hw: Add support for CPUFREQ HW
>   2. dt-bindings: arm: cpus: Document 'mtk,freq-domain' property
>   3. dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW
> 
> #4~#6 is for other CPU features, i.e. SVS [1]
>   4. cpufreq: mediatek-hw: register EM power table
>   5. cpufreq: mediatek-hw: Add SVS CPU initialization
>   6. cpufreq: mediatek-hw: Add cooling dev flag
> 
> I supposed that it could be more clean to separate #4~#6 in another
> patchset.May I know is it okay to you? Or I should merge all of changes
> into v8 like you mentioned? Thank you.

Merge them all together and explain any special features (like SVS) in
the commit log. It will also help in future when people want to
understand your driver. Explain whatever is worth explaining there and
is not straight forward.

Thanks for the details.

-- 
viresh

Re: [PATCH v1] cpufreq: mediatek-hw: Add support for Mediatek cpufreq HW driver

[Hector Yuan]

On Fri, 2020-10-23 at 14:57 +0530, Viresh Kumar wrote:
> On 23-10-20, 17:08, Hector Yuan wrote:
> > On Fri, 2020-10-23 at 13:58 +0530, Viresh Kumar wrote:
> > > On 23-10-20, 16:24, Hector Yuan wrote:
> > > > This patchset includes 6 patches and depends on the MT6779 DTS patch[1] submitted by Hanks Chen.
> > > > The first 3 patches are for CPUFREQ HW driver and device tree binding, which are already sent before separately [2][3]. For binding part, I add a new patch to add property in cpu schema.
> > > > Besides, we add three more patches including EM power table, SVS CPU initialize, and cooling device.
> > > 
> > > And even after so many versions of these you chose to name this V1. It
> > > is very difficult for reviewers to find time to review your stuff, and
> > > they expect some sort of summary from you on what exactly changed from
> > > last version and you also need to name the current version currently.
> > > 
> > > This should have been V8 and you should have added a "V7->V8 diff:"
> > > section here, naming all the changes you did. Please send that as
> > > reply to this email, so I can see what really changed.
> > > 
> > Hi, Viresh
> > 
> > Sorry for your inconvenience.
> > #1~#3 is for cpufreq driver we have reviewed and the bindings which
> > separate freq domain to CPU schema.There is no change for the driver
> > itself.
> >   1. cpufreq: mediatek-hw: Add support for CPUFREQ HW
> >   2. dt-bindings: arm: cpus: Document 'mtk,freq-domain' property
> >   3. dt-bindings: cpufreq: add bindings for MediaTek cpufreq HW
> > 
> > #4~#6 is for other CPU features, i.e. SVS [1]
> >   4. cpufreq: mediatek-hw: register EM power table
> >   5. cpufreq: mediatek-hw: Add SVS CPU initialization
> >   6. cpufreq: mediatek-hw: Add cooling dev flag
> > 
> > I supposed that it could be more clean to separate #4~#6 in another
> > patchset.May I know is it okay to you? Or I should merge all of changes
> > into v8 like you mentioned? Thank you.
> 
> Merge them all together and explain any special features (like SVS) in
> the commit log. It will also help in future when people want to
> understand your driver. Explain whatever is worth explaining there and
> is not straight forward.
> 
> Thanks for the details.
> 
OK, I will merge all changes in v8. And explain details in cover-letter.
Thank you.