[PATCH v2 0/4] MT8196 CPUFreq Support

[PATCH v2 0/4] MT8196 CPUFreq Support

[Nicolas Frattaroli]

This series adds the necessary bindings and driver changes to integrate
MT8196 CPUFreq into the existing mediatek-cpufreq-hw driver. This
necessitated two preparatory cleanup patches to the driver.

The CPU frequency was verified to actually be changing by comparing
sysbench cpu numbers between fdvfs being enabled and it not being
enabled.

Enablement in the DT will be done once the MT8196 DT lands, so don't be
surprised that no node uses these new compatibles so far.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v2:
- Split off mt8196-cpufreq-hw into a new binding.
- Made the fdvfs register regions part of the cpufreq "performance"
  node, instead of using syscons for this.
- Adjusted the driver to iomap those, and use the per-variant struct to
  add an offset for the domains index.
- Link to v1: https://lore.kernel.org/r/20250711-mt8196-cpufreq-v1-0-e1b0a3b4ac61@collabora.com

---
Nicolas Frattaroli (4):
      dt-bindings: cpufreq: Add mediatek,mt8196-cpufreq-hw binding
      cpufreq: mediatek-hw: Refactor match data into struct
      cpufreq: mediatek-hw: Separate per-domain and per-instance data
      cpufreq: mediatek-hw: Add support for MT8196

 .../cpufreq/mediatek,mt8196-cpufreq-hw.yaml        |  86 +++++++++++++
 drivers/cpufreq/mediatek-cpufreq-hw.c              | 139 +++++++++++++++++----
 2 files changed, 200 insertions(+), 25 deletions(-)
---
base-commit: 42f78243e0c6fe42f2710f98513a55c102347ff0
change-id: 20250711-mt8196-cpufreq-86d961e2300b

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v2 1/4] dt-bindings: cpufreq: Add mediatek,mt8196-cpufreq-hw binding

[Nicolas Frattaroli]

The MediaTek MT8196 SoC has new cpufreq hardware, with added memory
register ranges to control Dynamic-Voltage-Frequency-Scaling.

The DVFS hardware is controlled through a set of registers referred to
as "FDVFS"; one is a location from which a magic number is read to
ensure DVFS should be used, the other is a region to set the desired
target frequency that DVFS should aim towards for each performance
domain.

Instead of working around the old binding and its already established
meanings for the reg items, add a new binding. The FDVFS register memory
regions are at the beginning, which allows us to easily expand this
binding for future SoCs which may have more than 3 performance domains.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../cpufreq/mediatek,mt8196-cpufreq-hw.yaml        | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml b/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..26bf21e05888646b4d1bdac95bfba0f36e037ffd
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cpufreq/mediatek,mt8196-cpufreq-hw.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek CPUFreq for MT8196 and related SoCs
+
+maintainers:
+  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+
+description:
+  MT8196 uses CPUFreq management hardware that supports dynamic voltage
+  frequency scaling (dvfs), and can support several performance domains.
+
+properties:
+  compatible:
+    const: mediatek,mt8196-cpufreq-hw
+
+  reg:
+    items:
+      - description: FDVFS magic number register region
+      - description: FDVFS control register region
+      - description: OPP tables and control for performance domain 0
+      - description: OPP tables and control for performance domain 1
+      - description: OPP tables and control for performance domain 2
+
+  "#performance-domain-cells":
+    description:
+      Number of cells in a performance domain specifier. Must be 1.
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#performance-domain-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    cpus {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            cpu0: cpu@0 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a720";
+                enable-method = "psci";
+                performance-domains = <&performance 0>;
+                reg = <0x000>;
+            };
+
+            /* ... */
+
+            cpu6: cpu@600 {
+                device_type = "cpu";
+                compatible = "arm,cortex-x4";
+                enable-method = "psci";
+                performance-domains = <&performance 1>;
+                reg = <0x600>;
+            };
+
+            cpu7: cpu@700 {
+                device_type = "cpu";
+                compatible = "arm,cortex-x925";
+                enable-method = "psci";
+                performance-domains = <&performance 2>;
+                reg = <0x700>;
+            };
+    };
+
+    /* ... */
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        performance: performance-controller@c2c2034 {
+            compatible = "mediatek,mt8196-cpufreq-hw";
+            reg = <0 0xc2c2034 0 0x4>, <0 0xc220400 0 0x20>,
+                  <0 0xc2c0f20 0 0x120>, <0 0xc2c1040 0 0x120>,
+                  <0 0xc2c1160 0 0x120>;
+            #performance-domain-cells = <1>;
+        };
+    };

-- 
2.50.1

[PATCH v2 2/4] cpufreq: mediatek-hw: Refactor match data into struct

[Nicolas Frattaroli]

While the driver could get away with having the per-compatible match
data just be an array of the reg offsets, the only thing it used it for
right now, this doesn't really allow it to be extended in any meaningful
way if some other per-variant information needs to be communicated.

Refactor the code to make the DT match data a struct, which currently
only contains a single member: the reg offsets. This will allow this
struct to be extended with other members for other hardware variants.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq-hw.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
index 74f1b4c796e4cc9ebccf50dd4e165a1eba03136a..b2aba1842226c7d24a8b9599ea62408cac9f803c 100644
--- a/drivers/cpufreq/mediatek-cpufreq-hw.c
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -41,15 +41,22 @@ struct mtk_cpufreq_data {
 	struct resource *res;
 	void __iomem *base;
 	int nr_opp;
+	const struct mtk_cpufreq_variant *variant;
 };
 
-static const u16 cpufreq_mtk_offsets[REG_ARRAY_SIZE] = {
-	[REG_FREQ_LUT_TABLE]	= 0x0,
-	[REG_FREQ_ENABLE]	= 0x84,
-	[REG_FREQ_PERF_STATE]	= 0x88,
-	[REG_FREQ_HW_STATE]	= 0x8c,
-	[REG_EM_POWER_TBL]	= 0x90,
-	[REG_FREQ_LATENCY]	= 0x110,
+struct mtk_cpufreq_variant {
+	const u16 reg_offsets[REG_ARRAY_SIZE];
+};
+
+static const struct mtk_cpufreq_variant cpufreq_mtk_base_variant = {
+	.reg_offsets = {
+		[REG_FREQ_LUT_TABLE]	= 0x0,
+		[REG_FREQ_ENABLE]	= 0x84,
+		[REG_FREQ_PERF_STATE]	= 0x88,
+		[REG_FREQ_HW_STATE]	= 0x8c,
+		[REG_EM_POWER_TBL]	= 0x90,
+		[REG_FREQ_LATENCY]	= 0x110,
+	},
 };
 
 static int __maybe_unused
@@ -157,7 +164,7 @@ static int mtk_cpu_create_freq_table(struct platform_device *pdev,
 
 static int mtk_cpu_resources_init(struct platform_device *pdev,
 				  struct cpufreq_policy *policy,
-				  const u16 *offsets)
+				  const struct mtk_cpufreq_variant *variant)
 {
 	struct mtk_cpufreq_data *data;
 	struct device *dev = &pdev->dev;
@@ -200,9 +207,10 @@ static int mtk_cpu_resources_init(struct platform_device *pdev,
 
 	data->base = base;
 	data->res = res;
+	data->variant = variant;
 
 	for (i = REG_FREQ_LUT_TABLE; i < REG_ARRAY_SIZE; i++)
-		data->reg_bases[i] = base + offsets[i];
+		data->reg_bases[i] = base + variant->reg_offsets[i];
 
 	ret = mtk_cpu_create_freq_table(pdev, data);
 	if (ret) {
@@ -336,7 +344,7 @@ static void mtk_cpufreq_hw_driver_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id mtk_cpufreq_hw_match[] = {
-	{ .compatible = "mediatek,cpufreq-hw", .data = &cpufreq_mtk_offsets },
+	{ .compatible = "mediatek,cpufreq-hw", .data = &cpufreq_mtk_base_variant },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mtk_cpufreq_hw_match);

-- 
2.50.1

[PATCH v2 3/4] cpufreq: mediatek-hw: Separate per-domain and per-instance data

[Nicolas Frattaroli]

As it stood, the mediatek cpufreq driver could get away with never
really having a private driver instance struct. This is because all data
was stored in the per-domain structs.

However, this complicates matters when actual per-instance data like the
variant struct is introduced. Instead of having a pointer to it for
every domain, have a pointer to a global "priv" struct that can be
extended over time, and rename the "data" struct to "domain" to
distinguish its purpose better.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq-hw.c | 42 ++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
index b2aba1842226c7d24a8b9599ea62408cac9f803c..53611077d0d9a2d9865cf771568ab71abc0e6fbd 100644
--- a/drivers/cpufreq/mediatek-cpufreq-hw.c
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -35,13 +35,17 @@ enum {
 	REG_ARRAY_SIZE,
 };
 
-struct mtk_cpufreq_data {
+struct mtk_cpufreq_priv {
+	const struct mtk_cpufreq_variant *variant;
+};
+
+struct mtk_cpufreq_domain {
+	struct mtk_cpufreq_priv *parent;
 	struct cpufreq_frequency_table *table;
 	void __iomem *reg_bases[REG_ARRAY_SIZE];
 	struct resource *res;
 	void __iomem *base;
 	int nr_opp;
-	const struct mtk_cpufreq_variant *variant;
 };
 
 struct mtk_cpufreq_variant {
@@ -63,7 +67,7 @@ static int __maybe_unused
 mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
 			  unsigned long *KHz)
 {
-	struct mtk_cpufreq_data *data;
+	struct mtk_cpufreq_domain *data;
 	struct cpufreq_policy *policy;
 	int i;
 
@@ -90,7 +94,7 @@ mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
 static int mtk_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 				       unsigned int index)
 {
-	struct mtk_cpufreq_data *data = policy->driver_data;
+	struct mtk_cpufreq_domain *data = policy->driver_data;
 
 	writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
 
@@ -99,7 +103,7 @@ static int mtk_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 
 static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
 {
-	struct mtk_cpufreq_data *data;
+	struct mtk_cpufreq_domain *data;
 	struct cpufreq_policy *policy;
 	unsigned int index;
 
@@ -118,7 +122,7 @@ static unsigned int mtk_cpufreq_hw_get(unsigned int cpu)
 static unsigned int mtk_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
 					       unsigned int target_freq)
 {
-	struct mtk_cpufreq_data *data = policy->driver_data;
+	struct mtk_cpufreq_domain *data = policy->driver_data;
 	unsigned int index;
 
 	index = cpufreq_table_find_index_dl(policy, target_freq, false);
@@ -129,7 +133,7 @@ static unsigned int mtk_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
 }
 
 static int mtk_cpu_create_freq_table(struct platform_device *pdev,
-				     struct mtk_cpufreq_data *data)
+				     struct mtk_cpufreq_domain *data)
 {
 	struct device *dev = &pdev->dev;
 	u32 temp, i, freq, prev_freq = 0;
@@ -164,9 +168,9 @@ static int mtk_cpu_create_freq_table(struct platform_device *pdev,
 
 static int mtk_cpu_resources_init(struct platform_device *pdev,
 				  struct cpufreq_policy *policy,
-				  const struct mtk_cpufreq_variant *variant)
+				  struct mtk_cpufreq_priv *priv)
 {
-	struct mtk_cpufreq_data *data;
+	struct mtk_cpufreq_domain *data;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct of_phandle_args args;
@@ -187,6 +191,8 @@ static int mtk_cpu_resources_init(struct platform_device *pdev,
 	index = args.args[0];
 	of_node_put(args.np);
 
+	data->parent = priv;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
 	if (!res) {
 		dev_err(dev, "failed to get mem resource %d\n", index);
@@ -207,10 +213,9 @@ static int mtk_cpu_resources_init(struct platform_device *pdev,
 
 	data->base = base;
 	data->res = res;
-	data->variant = variant;
 
 	for (i = REG_FREQ_LUT_TABLE; i < REG_ARRAY_SIZE; i++)
-		data->reg_bases[i] = base + variant->reg_offsets[i];
+		data->reg_bases[i] = base + priv->variant->reg_offsets[i];
 
 	ret = mtk_cpu_create_freq_table(pdev, data);
 	if (ret) {
@@ -231,7 +236,7 @@ static int mtk_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 {
 	struct platform_device *pdev = cpufreq_get_driver_data();
 	int sig, pwr_hw = CPUFREQ_HW_STATUS | SVS_HW_STATUS;
-	struct mtk_cpufreq_data *data;
+	struct mtk_cpufreq_domain *data;
 	unsigned int latency;
 	int ret;
 
@@ -270,7 +275,7 @@ static int mtk_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 
 static void mtk_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
 {
-	struct mtk_cpufreq_data *data = policy->driver_data;
+	struct mtk_cpufreq_domain *data = policy->driver_data;
 	struct resource *res = data->res;
 	void __iomem *base = data->base;
 
@@ -283,7 +288,7 @@ static void mtk_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
 static void mtk_cpufreq_register_em(struct cpufreq_policy *policy)
 {
 	struct em_data_callback em_cb = EM_DATA_CB(mtk_cpufreq_get_cpu_power);
-	struct mtk_cpufreq_data *data = policy->driver_data;
+	struct mtk_cpufreq_domain *data = policy->driver_data;
 
 	em_dev_register_perf_domain(get_cpu_device(policy->cpu), data->nr_opp,
 				    &em_cb, policy->cpus, true);
@@ -305,6 +310,7 @@ static struct cpufreq_driver cpufreq_mtk_hw_driver = {
 
 static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
 {
+	struct mtk_cpufreq_priv *priv;
 	const void *data;
 	int ret, cpu;
 	struct device *cpu_dev;
@@ -328,7 +334,13 @@ static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
 	if (!data)
 		return -EINVAL;
 
-	platform_set_drvdata(pdev, (void *) data);
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->variant = data;
+
+	platform_set_drvdata(pdev, priv);
 	cpufreq_mtk_hw_driver.driver_data = pdev;
 
 	ret = cpufreq_register_driver(&cpufreq_mtk_hw_driver);

-- 
2.50.1

[PATCH v2 4/4] cpufreq: mediatek-hw: Add support for MT8196

[Nicolas Frattaroli]

The MT8196 SoC uses DVFS to set a desired target frequency for each CPU
core. It also uses slightly different register offsets.

Add support for it, which necessitates reworking how the mmio regs are
acquired, as mt8196 has the fdvfs config register and the fdvfs
registers as two reg items before the performance domains.

I've verified with both `sysbench cpu run` and `head -c 10G \
/dev/urandom | pigz -p 8 -c - | pv -ba > /dev/null` that we don't just
get a higher reported clock frequency, but that the observed performance
also increases, by a factor of 2.64 in an 8 thread sysbench test.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq-hw.c | 75 +++++++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
index 53611077d0d9a2d9865cf771568ab71abc0e6fbd..de6b2b8f6600f23148a7a24cafeae339903ba119 100644
--- a/drivers/cpufreq/mediatek-cpufreq-hw.c
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -24,6 +24,8 @@
 #define POLL_USEC			1000
 #define TIMEOUT_USEC			300000
 
+#define MT8196_FDVFS_MAGIC		0xABCD0001UL
+
 enum {
 	REG_FREQ_LUT_TABLE,
 	REG_FREQ_ENABLE,
@@ -36,7 +38,10 @@ enum {
 };
 
 struct mtk_cpufreq_priv {
+	struct device *dev;
 	const struct mtk_cpufreq_variant *variant;
+	void __iomem *fdvfs_config;
+	void __iomem *fdvfs;
 };
 
 struct mtk_cpufreq_domain {
@@ -49,7 +54,10 @@ struct mtk_cpufreq_domain {
 };
 
 struct mtk_cpufreq_variant {
+	int (*init)(struct mtk_cpufreq_priv *priv);
 	const u16 reg_offsets[REG_ARRAY_SIZE];
+	const unsigned int fdvfs_fdiv;
+	const unsigned int domain_offset;
 };
 
 static const struct mtk_cpufreq_variant cpufreq_mtk_base_variant = {
@@ -63,6 +71,37 @@ static const struct mtk_cpufreq_variant cpufreq_mtk_base_variant = {
 	},
 };
 
+static int mtk_cpufreq_hw_mt8196_init(struct mtk_cpufreq_priv *priv)
+{
+	priv->fdvfs_config = devm_of_iomap(priv->dev, priv->dev->of_node, 0, NULL);
+	if (IS_ERR_OR_NULL(priv->fdvfs_config))
+		return dev_err_probe(priv->dev, PTR_ERR(priv->fdvfs_config),
+				     "failed to get fdvfs-config iomem\n");
+
+	if (readl_relaxed(priv->fdvfs_config) == MT8196_FDVFS_MAGIC) {
+		priv->fdvfs = devm_of_iomap(priv->dev, priv->dev->of_node, 1, NULL);
+		if (IS_ERR_OR_NULL(priv->fdvfs))
+			return dev_err_probe(priv->dev, PTR_ERR(priv->fdvfs),
+					     "failed to get fdvfs iomem\n");
+	}
+
+	return 0;
+}
+
+static const struct mtk_cpufreq_variant cpufreq_mtk_mt8196_variant = {
+	.init = mtk_cpufreq_hw_mt8196_init,
+	.reg_offsets = {
+		[REG_FREQ_LUT_TABLE]	= 0x0,
+		[REG_FREQ_ENABLE]	= 0x84,
+		[REG_FREQ_PERF_STATE]	= 0x88,
+		[REG_FREQ_HW_STATE]	= 0x8c,
+		[REG_EM_POWER_TBL]	= 0x90,
+		[REG_FREQ_LATENCY]	= 0x114,
+	},
+	.fdvfs_fdiv = 26000,
+	.domain_offset = 2,
+};
+
 static int __maybe_unused
 mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
 			  unsigned long *KHz)
@@ -91,12 +130,31 @@ mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
 	return 0;
 }
 
+static void mtk_cpufreq_hw_fdvfs_switch(unsigned int target_freq,
+				       struct cpufreq_policy *policy)
+{
+	struct mtk_cpufreq_domain *data = policy->driver_data;
+	struct mtk_cpufreq_priv *priv = data->parent;
+	unsigned int cpu;
+
+	target_freq = DIV_ROUND_UP(target_freq, priv->variant->fdvfs_fdiv);
+	for_each_cpu(cpu, policy->real_cpus) {
+		writel_relaxed(target_freq, priv->fdvfs + cpu * 4);
+	}
+}
+
 static int mtk_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 				       unsigned int index)
 {
 	struct mtk_cpufreq_domain *data = policy->driver_data;
+	unsigned int target_freq;
 
-	writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
+	if (data->parent->fdvfs) {
+		target_freq = policy->freq_table[index].frequency;
+		mtk_cpufreq_hw_fdvfs_switch(target_freq, policy);
+	} else {
+		writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
+	}
 
 	return 0;
 }
@@ -127,7 +185,10 @@ static unsigned int mtk_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
 
 	index = cpufreq_table_find_index_dl(policy, target_freq, false);
 
-	writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
+	if (data->parent->fdvfs)
+		mtk_cpufreq_hw_fdvfs_switch(target_freq, policy);
+	else
+		writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
 
 	return policy->freq_table[index].frequency;
 }
@@ -188,7 +249,7 @@ static int mtk_cpu_resources_init(struct platform_device *pdev,
 	if (ret < 0)
 		return ret;
 
-	index = args.args[0];
+	index = args.args[0] + priv->variant->domain_offset;
 	of_node_put(args.np);
 
 	data->parent = priv;
@@ -339,6 +400,13 @@ static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv->variant = data;
+	priv->dev = &pdev->dev;
+
+	if (priv->variant->init) {
+		ret = priv->variant->init(priv);
+		if (ret)
+			return ret;
+	}
 
 	platform_set_drvdata(pdev, priv);
 	cpufreq_mtk_hw_driver.driver_data = pdev;
@@ -357,6 +425,7 @@ static void mtk_cpufreq_hw_driver_remove(struct platform_device *pdev)
 
 static const struct of_device_id mtk_cpufreq_hw_match[] = {
 	{ .compatible = "mediatek,cpufreq-hw", .data = &cpufreq_mtk_base_variant },
+	{ .compatible = "mediatek,mt8196-cpufreq-hw", .data = &cpufreq_mtk_mt8196_variant },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mtk_cpufreq_hw_match);

-- 
2.50.1

Re: [PATCH v2 4/4] cpufreq: mediatek-hw: Add support for MT8196

[AngeloGioacchino Del Regno]

Il 14/07/25 16:08, Nicolas Frattaroli ha scritto:
> The MT8196 SoC uses DVFS to set a desired target frequency for each CPU
> core. It also uses slightly different register offsets.
> 
> Add support for it, which necessitates reworking how the mmio regs are
> acquired, as mt8196 has the fdvfs config register and the fdvfs
> registers as two reg items before the performance domains.
> 
> I've verified with both `sysbench cpu run` and `head -c 10G \
> /dev/urandom | pigz -p 8 -c - | pv -ba > /dev/null` that we don't just
> get a higher reported clock frequency, but that the observed performance
> also increases, by a factor of 2.64 in an 8 thread sysbench test.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>   drivers/cpufreq/mediatek-cpufreq-hw.c | 75 +++++++++++++++++++++++++++++++++--
>   1 file changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
> index 53611077d0d9a2d9865cf771568ab71abc0e6fbd..de6b2b8f6600f23148a7a24cafeae339903ba119 100644
> --- a/drivers/cpufreq/mediatek-cpufreq-hw.c
> +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
> @@ -24,6 +24,8 @@
>   #define POLL_USEC			1000
>   #define TIMEOUT_USEC			300000
>   
> +#define MT8196_FDVFS_MAGIC		0xABCD0001UL
> +
>   enum {
>   	REG_FREQ_LUT_TABLE,
>   	REG_FREQ_ENABLE,
> @@ -36,7 +38,10 @@ enum {
>   };
>   
>   struct mtk_cpufreq_priv {
> +	struct device *dev;
>   	const struct mtk_cpufreq_variant *variant;
> +	void __iomem *fdvfs_config;
> +	void __iomem *fdvfs;
>   };
>   
>   struct mtk_cpufreq_domain {
> @@ -49,7 +54,10 @@ struct mtk_cpufreq_domain {
>   };
>   
>   struct mtk_cpufreq_variant {
> +	int (*init)(struct mtk_cpufreq_priv *priv);
>   	const u16 reg_offsets[REG_ARRAY_SIZE];
> +	const unsigned int fdvfs_fdiv;
> +	const unsigned int domain_offset;

I don't think you need a domain_offset here, but just a boolean saying that this
"has_fdvfs" or "is_dvfs_hybrid" or something else...

Besides, what about having just a

#define FDVFS_FDIV_HZ (26 * 1000) ?

This is the only platform that uses FDVFS and needs a FDIV, and I don't think that
other FDVFS MCUs will have a different divider.

If so, we can always add that param later to the variant structure, I think?

That'd reduce the change to

struct mtk_cpufreq_pdata {
	int (*hw_init)(struct mtk_cpufreq_priv *priv);
	const u16 reg_offsets[REG_ARRAY_SIZE];
	const bool is_hybrid_dvfs;
};

(don't mind about the naming, I'm just giving you alternatives if you didn't think
about those, I'm not complaining)

>   };
>   
>   static const struct mtk_cpufreq_variant cpufreq_mtk_base_variant = {
> @@ -63,6 +71,37 @@ static const struct mtk_cpufreq_variant cpufreq_mtk_base_variant = {
>   	},
>   };
>   
> +static int mtk_cpufreq_hw_mt8196_init(struct mtk_cpufreq_priv *priv)
> +{
> +	priv->fdvfs_config = devm_of_iomap(priv->dev, priv->dev->of_node, 0, NULL);
> +	if (IS_ERR_OR_NULL(priv->fdvfs_config))
> +		return dev_err_probe(priv->dev, PTR_ERR(priv->fdvfs_config),
> +				     "failed to get fdvfs-config iomem\n");
> +
> +	if (readl_relaxed(priv->fdvfs_config) == MT8196_FDVFS_MAGIC) {

...but then, wait a minute, is the FDVFS_CONFIG used only to check if FDVFS_MAGIC
is present?

If that's all for which it is used for, we might as well just assume that MT8196
always has FDVFS, because, well, it's true - MT8196 and MT6991 always have that
because it's initialized by the firmware before booting the kernel.

If there will be any need at all to initialize the FDVFS in Linux, we can use that
MMIO in a driver that does only that, perhaps...

> +		priv->fdvfs = devm_of_iomap(priv->dev, priv->dev->of_node, 1, NULL);
> +		if (IS_ERR_OR_NULL(priv->fdvfs))
> +			return dev_err_probe(priv->dev, PTR_ERR(priv->fdvfs),
> +					     "failed to get fdvfs iomem\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct mtk_cpufreq_variant cpufreq_mtk_mt8196_variant = {
> +	.init = mtk_cpufreq_hw_mt8196_init,
> +	.reg_offsets = {
> +		[REG_FREQ_LUT_TABLE]	= 0x0,
> +		[REG_FREQ_ENABLE]	= 0x84,
> +		[REG_FREQ_PERF_STATE]	= 0x88,
> +		[REG_FREQ_HW_STATE]	= 0x8c,
> +		[REG_EM_POWER_TBL]	= 0x90,
> +		[REG_FREQ_LATENCY]	= 0x114,
> +	},
> +	.fdvfs_fdiv = 26000,
> +	.domain_offset = 2,
> +};
> +
>   static int __maybe_unused
>   mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
>   			  unsigned long *KHz)
> @@ -91,12 +130,31 @@ mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
>   	return 0;
>   }
>   
> +static void mtk_cpufreq_hw_fdvfs_switch(unsigned int target_freq,
> +				       struct cpufreq_policy *policy)
> +{
> +	struct mtk_cpufreq_domain *data = policy->driver_data;
> +	struct mtk_cpufreq_priv *priv = data->parent;
> +	unsigned int cpu;
> +
> +	target_freq = DIV_ROUND_UP(target_freq, priv->variant->fdvfs_fdiv);
> +	for_each_cpu(cpu, policy->real_cpus) {
> +		writel_relaxed(target_freq, priv->fdvfs + cpu * 4);
> +	}
> +}
> +
>   static int mtk_cpufreq_hw_target_index(struct cpufreq_policy *policy,
>   				       unsigned int index)
>   {
>   	struct mtk_cpufreq_domain *data = policy->driver_data;
> +	unsigned int target_freq;
>   
> -	writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
> +	if (data->parent->fdvfs) {
> +		target_freq = policy->freq_table[index].frequency;
> +		mtk_cpufreq_hw_fdvfs_switch(target_freq, policy);
> +	} else {
> +		writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
> +	}
>   
>   	return 0;
>   }
> @@ -127,7 +185,10 @@ static unsigned int mtk_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
>   
>   	index = cpufreq_table_find_index_dl(policy, target_freq, false);
>   
> -	writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
> +	if (data->parent->fdvfs)
> +		mtk_cpufreq_hw_fdvfs_switch(target_freq, policy);
> +	else
> +		writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
>   
>   	return policy->freq_table[index].frequency;
>   }
> @@ -188,7 +249,7 @@ static int mtk_cpu_resources_init(struct platform_device *pdev,
>   	if (ret < 0)
>   		return ret;
>   
> -	index = args.args[0];

	index = args.args[0];

	/*
	 * If this is cpufreq with hybrid dvfs, the first declared register range
	 * is FDVFS and each of the frequency domain MMIOs follow that.
	 */
	if (priv->variant->cpufreq_is_hybrid_dvfs)
		index++

Do you like that? Any concern with such a solution? :-)

Cheers,
Angelo

> +	index = args.args[0] + priv->variant->domain_offset;
>   	of_node_put(args.np);
>   
>   	data->parent = priv;
> @@ -339,6 +400,13 @@ static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   
>   	priv->variant = data;
> +	priv->dev = &pdev->dev;
> +
> +	if (priv->variant->init) {
> +		ret = priv->variant->init(priv);
> +		if (ret)
> +			return ret;
> +	}
>   
>   	platform_set_drvdata(pdev, priv);
>   	cpufreq_mtk_hw_driver.driver_data = pdev;
> @@ -357,6 +425,7 @@ static void mtk_cpufreq_hw_driver_remove(struct platform_device *pdev)
>   
>   static const struct of_device_id mtk_cpufreq_hw_match[] = {
>   	{ .compatible = "mediatek,cpufreq-hw", .data = &cpufreq_mtk_base_variant },
> +	{ .compatible = "mediatek,mt8196-cpufreq-hw", .data = &cpufreq_mtk_mt8196_variant },
>   	{}
>   };
>   MODULE_DEVICE_TABLE(of, mtk_cpufreq_hw_match);
> 

Re: [PATCH v2 2/4] cpufreq: mediatek-hw: Refactor match data into struct

[AngeloGioacchino Del Regno]

Il 14/07/25 16:08, Nicolas Frattaroli ha scritto:
> While the driver could get away with having the per-compatible match
> data just be an array of the reg offsets, the only thing it used it for
> right now, this doesn't really allow it to be extended in any meaningful
> way if some other per-variant information needs to be communicated.
> 
> Refactor the code to make the DT match data a struct, which currently
> only contains a single member: the reg offsets. This will allow this
> struct to be extended with other members for other hardware variants.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

You could explicitly state, in the commit description, something like:
"In preparation to add support for new Hybrid "FDVFS" CPUFreq for MT8196,
refactor the code to make the DT match data...."

Regardless of that:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v2 1/4] dt-bindings: cpufreq: Add mediatek,mt8196-cpufreq-hw binding

[AngeloGioacchino Del Regno]

Il 14/07/25 16:08, Nicolas Frattaroli ha scritto:
> The MediaTek MT8196 SoC has new cpufreq hardware, with added memory
> register ranges to control Dynamic-Voltage-Frequency-Scaling.
> 
> The DVFS hardware is controlled through a set of registers referred to
> as "FDVFS"; one is a location from which a magic number is read to
> ensure DVFS should be used, the other is a region to set the desired
> target frequency that DVFS should aim towards for each performance
> domain.
> 
> Instead of working around the old binding and its already established
> meanings for the reg items, add a new binding. The FDVFS register memory
> regions are at the beginning, which allows us to easily expand this
> binding for future SoCs which may have more than 3 performance domains.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>   .../cpufreq/mediatek,mt8196-cpufreq-hw.yaml        | 86 ++++++++++++++++++++++
>   1 file changed, 86 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml b/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..26bf21e05888646b4d1bdac95bfba0f36e037ffd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cpufreq/mediatek,mt8196-cpufreq-hw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek CPUFreq for MT8196 and related SoCs

title: MediaTek Hybrid CPUFreq for MT8196/MT6991 series SoCs

> +
> +maintainers:
> +  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> +
> +description:
> +  MT8196 uses CPUFreq management hardware that supports dynamic voltage
> +  frequency scaling (dvfs), and can support several performance domains.
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8196-cpufreq-hw
> +
> +  reg:
> +    items:
> +      - description: FDVFS magic number register region

As already said in the other commit, we might just be able to avoid adding the
magic number register region :-)

> +      - description: FDVFS control register region
> +      - description: OPP tables and control for performance domain 0
> +      - description: OPP tables and control for performance domain 1
> +      - description: OPP tables and control for performance domain 2
> +
> +  "#performance-domain-cells":
> +    description:
> +      Number of cells in a performance domain specifier. Must be 1.

The description is redundant and doesn't add any real information, I think you
should drop it.

Bindings maintainers, please, opinions?

> +    const: 1
> +

Everything else looks good to me.

Cheers,
Angelo

> +required:
> +  - compatible
> +  - reg
> +  - "#performance-domain-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    cpus {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            cpu0: cpu@0 {
> +                device_type = "cpu";
> +                compatible = "arm,cortex-a720";
> +                enable-method = "psci";
> +                performance-domains = <&performance 0>;
> +                reg = <0x000>;
> +            };
> +
> +            /* ... */
> +
> +            cpu6: cpu@600 {
> +                device_type = "cpu";
> +                compatible = "arm,cortex-x4";
> +                enable-method = "psci";
> +                performance-domains = <&performance 1>;
> +                reg = <0x600>;
> +            };
> +
> +            cpu7: cpu@700 {
> +                device_type = "cpu";
> +                compatible = "arm,cortex-x925";
> +                enable-method = "psci";
> +                performance-domains = <&performance 2>;
> +                reg = <0x700>;
> +            };
> +    };
> +
> +    /* ... */
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        performance: performance-controller@c2c2034 {
> +            compatible = "mediatek,mt8196-cpufreq-hw";
> +            reg = <0 0xc2c2034 0 0x4>, <0 0xc220400 0 0x20>,
> +                  <0 0xc2c0f20 0 0x120>, <0 0xc2c1040 0 0x120>,
> +                  <0 0xc2c1160 0 0x120>;
> +            #performance-domain-cells = <1>;
> +        };
> +    };
> 

Re: [PATCH v2 3/4] cpufreq: mediatek-hw: Separate per-domain and per-instance data

[AngeloGioacchino Del Regno]

Il 14/07/25 16:08, Nicolas Frattaroli ha scritto:
> As it stood, the mediatek cpufreq driver could get away with never
> really having a private driver instance struct. This is because all data
> was stored in the per-domain structs.
> 
> However, this complicates matters when actual per-instance data like the
> variant struct is introduced. Instead of having a pointer to it for
> every domain, have a pointer to a global "priv" struct that can be
> extended over time, and rename the "data" struct to "domain" to
> distinguish its purpose better.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v2 1/4] dt-bindings: cpufreq: Add mediatek,mt8196-cpufreq-hw binding

[Rob Herring]

On Mon, Jul 14, 2025 at 04:41:30PM +0200, AngeloGioacchino Del Regno wrote:
> Il 14/07/25 16:08, Nicolas Frattaroli ha scritto:
> > The MediaTek MT8196 SoC has new cpufreq hardware, with added memory
> > register ranges to control Dynamic-Voltage-Frequency-Scaling.
> > 
> > The DVFS hardware is controlled through a set of registers referred to
> > as "FDVFS"; one is a location from which a magic number is read to
> > ensure DVFS should be used, the other is a region to set the desired
> > target frequency that DVFS should aim towards for each performance
> > domain.
> > 
> > Instead of working around the old binding and its already established
> > meanings for the reg items, add a new binding. The FDVFS register memory
> > regions are at the beginning, which allows us to easily expand this
> > binding for future SoCs which may have more than 3 performance domains.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >   .../cpufreq/mediatek,mt8196-cpufreq-hw.yaml        | 86 ++++++++++++++++++++++
> >   1 file changed, 86 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml b/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..26bf21e05888646b4d1bdac95bfba0f36e037ffd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cpufreq/mediatek,mt8196-cpufreq-hw.yaml
> > @@ -0,0 +1,86 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/cpufreq/mediatek,mt8196-cpufreq-hw.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek CPUFreq for MT8196 and related SoCs
> 
> title: MediaTek Hybrid CPUFreq for MT8196/MT6991 series SoCs
> 
> > +
> > +maintainers:
> > +  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > +
> > +description:
> > +  MT8196 uses CPUFreq management hardware that supports dynamic voltage
> > +  frequency scaling (dvfs), and can support several performance domains.
> > +
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt8196-cpufreq-hw
> > +
> > +  reg:
> > +    items:
> > +      - description: FDVFS magic number register region
> 
> As already said in the other commit, we might just be able to avoid adding the
> magic number register region :-)
> 
> > +      - description: FDVFS control register region
> > +      - description: OPP tables and control for performance domain 0
> > +      - description: OPP tables and control for performance domain 1
> > +      - description: OPP tables and control for performance domain 2
> > +
> > +  "#performance-domain-cells":
> > +    description:
> > +      Number of cells in a performance domain specifier. Must be 1.
> 
> The description is redundant and doesn't add any real information, I think you
> should drop it.
> 
> Bindings maintainers, please, opinions?

Drop.

Rob