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
From: "Hector.Yuan" <[email protected]>
Add devicetree bindings for MediaTek HW driver.
Signed-off-by: Hector.Yuan <[email protected]>
---
.../bindings/cpufreq/cpufreq-mediatek-hw.yaml | 61 ++++++++++++++++++++
1 file changed, 61 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..59bb24e
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
@@ -0,0 +1,61 @@
+# 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 <[email protected]>
+
+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:
+ - |
+ 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
From: "Hector.Yuan" <[email protected]>
Add MT6779 cpufreq HW support.
Signed-off-by: Hector.Yuan <[email protected]>
---
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..6752db9
--- /dev/null
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -0,0 +1,255 @@
+// 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/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
On 13-08-20, 15:07, Hector Yuan wrote:
> From: "Hector.Yuan" <[email protected]>
>
> Add MT6779 cpufreq HW support.
>
> Signed-off-by: Hector.Yuan <[email protected]>
> ---
> 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
What about a 'default m' in Kconfig itself ?
> 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..6752db9
> --- /dev/null
> +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
> @@ -0,0 +1,255 @@
> +// 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/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;
Use cpufreq_driver->driver_data for this, it is already used in other
drivers for similar use.
> +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,
This routine needs to be named better, it is creating the cpufreq
table after all.
> + struct cpufreq_policy *policy,
> + void __iomem *base)
Please make sure checkpatch --strict doesn't give any errors.
> +{
> + u32 data;
> + u32 freq, volt, prev_freq = 0;
Merge these two..
> + 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);
Why are you adding OPPs here and rather why using OPP specific stuff
at all in the driver ?
> + 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",
Where are bindings of this node and how does this look ?
> + "#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
--
viresh
On Thu, Aug 13, 2020 at 03:07:55PM +0800, Hector Yuan wrote:
> From: "Hector.Yuan" <[email protected]>
>
> Add devicetree bindings for MediaTek HW driver.
>
> Signed-off-by: Hector.Yuan <[email protected]>
> ---
> .../bindings/cpufreq/cpufreq-mediatek-hw.yaml | 61 ++++++++++++++++++++
> 1 file changed, 61 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..59bb24e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> @@ -0,0 +1,61 @@
> +# 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 <[email protected]>
> +
> +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"
Not all that useful of a name given it's based on the index.
> + description: |
> + Frequency domain name.
> +
> + "#freq-domain-cells":
> + const: 1
> + description: |
> + Number of cells in a freqency domain specifier.
What's this for?
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - "#freq-domain-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + 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
On Mon, 2020-08-24 at 20:04 -0600, Rob Herring wrote:
> On Thu, Aug 13, 2020 at 03:07:55PM +0800, Hector Yuan wrote:
> > From: "Hector.Yuan" <[email protected]>
> >
> > Add devicetree bindings for MediaTek HW driver.
> >
> > Signed-off-by: Hector.Yuan <[email protected]>
> > ---
> > .../bindings/cpufreq/cpufreq-mediatek-hw.yaml | 61 ++++++++++++++++++++
> > 1 file changed, 61 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..59bb24e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
> > @@ -0,0 +1,61 @@
> > +# 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 <[email protected]>
> > +
> > +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"
>
> Not all that useful of a name given it's based on the index.
Let me explain this index is about to map cpus to each frequency control
domain. Will update details usage in V3. Thank you.
> > + description: |
> > + Frequency domain name.
> > +
> > + "#freq-domain-cells":
> > + const: 1
> > + description: |
> > + Number of cells in a freqency domain specifier.
>
> What's this for?
> Like the previous mentioned, this is for index mapping. will update in V3. Thank you.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - "#freq-domain-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + 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
On Mon, 2020-08-24 at 15:36 +0530, Viresh Kumar wrote:
> On 13-08-20, 15:07, Hector Yuan wrote:
> > From: "Hector.Yuan" <[email protected]>
> >
> > Add MT6779 cpufreq HW support.
> >
> > Signed-off-by: Hector.Yuan <[email protected]>
> > ---
> > 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
>
> What about a 'default m' in Kconfig itself ?
> OK, will update in V3.
> > 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..6752db9
> > --- /dev/null
> > +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
> > @@ -0,0 +1,255 @@
> > +// 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/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;
>
> Use cpufreq_driver->driver_data for this, it is already used in other
> drivers for similar use.
> OK, I see.will update in V3.
> > +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,
>
> This routine needs to be named better, it is creating the cpufreq
> table after all.
> OK, will update in V3. rename to xxx_opp_create.
> > + struct cpufreq_policy *policy,
> > + void __iomem *base)
>
> Please make sure checkpatch --strict doesn't give any errors.
>
> > +{
> > + u32 data;
> > + u32 freq, volt, prev_freq = 0;
>
> Merge these two..
> OK
> > + 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);
>
> Why are you adding OPPs here and rather why using OPP specific stuff
> at all in the driver ?
> yes, the opp information is read from CPU HW engine.Then add it to the CPU dev OPP one by one.
> > + 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",
>
> Where are bindings of this node and how does this look ?
> Can refer to the same patch series, I split it to another patch.Each cpu will be group into one frequency domain for the CPU DVFS.
> > + "#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
>
On 26-08-20, 20:57, Hector Yuan wrote:
> On Mon, 2020-08-24 at 15:36 +0530, Viresh Kumar wrote:
> > On 13-08-20, 15:07, Hector Yuan wrote:
> > > CONFIG_ARM_ALLWINNER_SUN50I_CPUFREQ_NVMEM=m
> > > CONFIG_ARM_ARMADA_37XX_CPUFREQ=y
> > > +CONFIG_ARM_MEDIATEK_CPUFREQ_HW=m
> >
> > What about a 'default m' in Kconfig itself ?
> > OK, will update in V3.
Hector, you need to remove (or not add) the right bracket (>) before the
beginning of your lines. This makes it incredibly difficult to read.
> > > + 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);
> >
> > Why are you adding OPPs here and rather why using OPP specific stuff
> > at all in the driver ?
> > yes, the opp information is read from CPU HW engine.Then add it to the CPU dev OPP one by one.
I asked a different question, why are you adding OPPs ? You don't need the OPPs
at all in my opinion. You can just create the frequency table and that's it.
> > > + 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",
> >
> > Where are bindings of this node and how does this look ?
> > Can refer to the same patch series, I split it to another patch.Each cpu will be group into one frequency domain for the CPU DVFS.
That binding only defines "mediatek,cpufreq-hw" and not "mtk,freq-domain".
--
viresh
On Thu, 2020-08-27 at 09:56 +0530, Viresh Kumar wrote:
> On 26-08-20, 20:57, Hector Yuan wrote:
> > On Mon, 2020-08-24 at 15:36 +0530, Viresh Kumar wrote:
> > > On 13-08-20, 15:07, Hector Yuan wrote:
> > > > CONFIG_ARM_ALLWINNER_SUN50I_CPUFREQ_NVMEM=m
> > > > CONFIG_ARM_ARMADA_37XX_CPUFREQ=y
> > > > +CONFIG_ARM_MEDIATEK_CPUFREQ_HW=m
> > >
> > > What about a 'default m' in Kconfig itself ?
> > > OK, will update in V3.
>
> Hector, you need to remove (or not add) the right bracket (>) before the
> beginning of your lines. This makes it incredibly difficult to read.
OK, I get it. Sorry for the inconvenience.
> > > > + 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);
> > >
> > > Why are you adding OPPs here and rather why using OPP specific stuff
> > > at all in the driver ?
> > > yes, the opp information is read from CPU HW engine.Then add it to the CPU dev OPP one by one.
>
> I asked a different question, why are you adding OPPs ? You don't need the OPPs
> at all in my opinion. You can just create the frequency table and that's it.
I just add OPP info to OPP framework so that others modules can get it
from OPP framework.
But like you said, I don't need it in this driver. I will remove this
code segment in V4.
I already send V3 yesterday but not including this modification.
> > > > + 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",
> > >
> > > Where are bindings of this node and how does this look ?
> > > Can refer to the same patch series, I split it to another patch.Each cpu will be group into one frequency domain for the CPU DVFS.
>
> That binding only defines "mediatek,cpufreq-hw" and not "mtk,freq-domain".
Please refer to the dt binding in V3, thank you. (lkml not show up yet,
so I post the below link instead)
https://www.spinics.net/lists/arm-kernel/msg832592.html
>
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
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/
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
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.