[PATCH 0/5] MT8196 CPUFreq Support

[PATCH 0/5] 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>
---
Nicolas Frattaroli (5):
      dt-bindings: mfd: syscon: Add mt8196 fdvfs syscons
      dt-bindings: cpufreq: mediatek-hw: add mt8196 cpufreq 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

 .../bindings/cpufreq/cpufreq-mediatek-hw.yaml      |  64 +++++++++-
 Documentation/devicetree/bindings/mfd/syscon.yaml  |   4 +
 drivers/cpufreq/mediatek-cpufreq-hw.c              | 140 +++++++++++++++++----
 3 files changed, 184 insertions(+), 24 deletions(-)
---
base-commit: 42f78243e0c6fe42f2710f98513a55c102347ff0
change-id: 20250711-mt8196-cpufreq-86d961e2300b

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

[PATCH 1/5] dt-bindings: mfd: syscon: Add mt8196 fdvfs syscons

[Nicolas Frattaroli]

The MT8196 SoC uses two syscon ranges for CPU DVFS that are separate
from each other. One, mt8196-fdvfs-config, is used to check for a magic
number at that memory address to verify that fdvfs should be used. The
other, mt8196-fdvfs, is used to configure the desired frequency for the
DVFS controller for each CPU core.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 27672adeb1fedb7c81b8ae86c35f4f3b26d5516f..5ee49d2ba0cdb72dd697a0fd71c8416ad4fd2c1e 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -88,6 +88,8 @@ select:
           - mediatek,mt8135-pctl-a-syscfg
           - mediatek,mt8135-pctl-b-syscfg
           - mediatek,mt8173-pctl-a-syscfg
+          - mediatek,mt8196-fdvfs
+          - mediatek,mt8196-fdvfs-config
           - mediatek,mt8365-syscfg
           - microchip,lan966x-cpu-syscon
           - microchip,mpfs-sysreg-scb
@@ -194,6 +196,8 @@ properties:
           - mediatek,mt8135-pctl-a-syscfg
           - mediatek,mt8135-pctl-b-syscfg
           - mediatek,mt8173-pctl-a-syscfg
+          - mediatek,mt8196-fdvfs
+          - mediatek,mt8196-fdvfs-config
           - mediatek,mt8365-infracfg-nao
           - mediatek,mt8365-syscfg
           - microchip,lan966x-cpu-syscon

-- 
2.50.0

Re: [PATCH 1/5] dt-bindings: mfd: syscon: Add mt8196 fdvfs syscons

[AngeloGioacchino Del Regno]

Il 11/07/25 16:57, Nicolas Frattaroli ha scritto:
> The MT8196 SoC uses two syscon ranges for CPU DVFS that are separate
> from each other. One, mt8196-fdvfs-config, is used to check for a magic
> number at that memory address to verify that fdvfs should be used. The
> other, mt8196-fdvfs, is used to configure the desired frequency for the
> DVFS controller for each CPU core.
> 

What is the reason why you're using syscons here?

Can't we simply assign the FDVFS MMIO to the cpufreq-hw node?

Or is there any reason why we can't declare it as mmio-sram? ...because I'm not
entirely sure but the FDVFS space should effectively be a [c]SRAM memory range...

Cheers,
Angelo

> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>   Documentation/devicetree/bindings/mfd/syscon.yaml | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> index 27672adeb1fedb7c81b8ae86c35f4f3b26d5516f..5ee49d2ba0cdb72dd697a0fd71c8416ad4fd2c1e 100644
> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> @@ -88,6 +88,8 @@ select:
>             - mediatek,mt8135-pctl-a-syscfg
>             - mediatek,mt8135-pctl-b-syscfg
>             - mediatek,mt8173-pctl-a-syscfg
> +          - mediatek,mt8196-fdvfs
> +          - mediatek,mt8196-fdvfs-config
>             - mediatek,mt8365-syscfg
>             - microchip,lan966x-cpu-syscon
>             - microchip,mpfs-sysreg-scb
> @@ -194,6 +196,8 @@ properties:
>             - mediatek,mt8135-pctl-a-syscfg
>             - mediatek,mt8135-pctl-b-syscfg
>             - mediatek,mt8173-pctl-a-syscfg
> +          - mediatek,mt8196-fdvfs
> +          - mediatek,mt8196-fdvfs-config
>             - mediatek,mt8365-infracfg-nao
>             - mediatek,mt8365-syscfg
>             - microchip,lan966x-cpu-syscon
> 

Re: [PATCH 1/5] dt-bindings: mfd: syscon: Add mt8196 fdvfs syscons

[Nicolas Frattaroli]

On Monday, 14 July 2025 10:47:32 Central European Summer Time AngeloGioacchino Del Regno wrote:
> Il 11/07/25 16:57, Nicolas Frattaroli ha scritto:
> > The MT8196 SoC uses two syscon ranges for CPU DVFS that are separate
> > from each other. One, mt8196-fdvfs-config, is used to check for a magic
> > number at that memory address to verify that fdvfs should be used. The
> > other, mt8196-fdvfs, is used to configure the desired frequency for the
> > DVFS controller for each CPU core.
> > 
> 
> What is the reason why you're using syscons here?
> 
> Can't we simply assign the FDVFS MMIO to the cpufreq-hw node?

That would require refactoring the driver way more since it currently
gets the number of performance domains from the number of regs. If
you want me to do that, I'll need to know how we should disambiguate
performance domains from misc things like fdvfs. Stuff like string
comparisons on reg-names seems very ugly for the driver to do, but
adding a property to explicitly specify the number of performance
domains would then put into question what the existing binding did
by just assuming this information is something that implementations
can get without any ambiguity.

Even if we forget that Linux is the only kernel that cares about this
device tree, I'm not totally on board with having the smattering of
dozens of different tiny register ranges in every DT node on mediatek
like the vendor kernel does it. And not to forget, it'd change the
binding even more, to the point where I'd probably have to create a
new binding for mt8196.

> Or is there any reason why we can't declare it as mmio-sram? ...because I'm not
> entirely sure but the FDVFS space should effectively be a [c]SRAM memory range...

mmio-sram is fairly useless for the purposes of having something as
a fixed set of registers, hence why nobody else does it. From my
research, it appears to mainly be used if you want to actually treat
it like a pool of memory from which to then dynamically allocate
things.

To use it like a syscon, which is what we're doing, you'll have to
specify your mmio-sram node, then add a child node as a reserved range
for the "syscon-like" area, and then specify in ranges that you want
that child node's address translated into the global address space as
expected. Then in the driver, you can't just do a single function call
to get some regmap to write into, you have to follow a phandle in your
vendor property pointing to said sram range, then get its address,
translate said address to the global SoC address space, and then iomap
it. And the cleanup for error paths/driver remove isn't fun either.

Besides, we don't actually know whether this is an sram range, or how
large it is. The only confirmed sram range was the csram_sys thing at
like 0x11bc00 which is not used because it turned out to be useless,
and dealing with the kernel's sram interface to use it as a reserved
range just to read 4 bytes from it was a wasted afternoon. And that's
not even the real starting address of that sram area, that's just a
part we know is used because downstream uses 5 KiB there for a codepath
that's dead when fdvfs is present (except the one thing where it installs
a panic handler to shove something into a register to make it stop dvfs
logging if the kernel is in the process of crashing).

I can be convinced to go through the pain of making this mmio-sram if
I have documentation of the whole memory map of the SoC that shows me
where and how large each area of sram is, so that I don't need to come
up with my own starting addresses and offsets based on whatever random
stuff I can infer from downstream DT.

Really, every writable syscon is probably implemented as "sram" in
hardware. But an sram cell that is not general use memory but is
treated by the hardware as having some meaning is not mmio-sram. We
can't ever use it in any way other than as a syscon, and telling
implementations that they can except then slapping a huge reserved
range into it just makes them have to implement syscons twice, once
as syscons and once as syscons-except-it's-a-reserved-sram-mmio-range.

> 
> Cheers,
> Angelo
> 

Kind regards,
Nicolas Frattaroli

> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >   Documentation/devicetree/bindings/mfd/syscon.yaml | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > index 27672adeb1fedb7c81b8ae86c35f4f3b26d5516f..5ee49d2ba0cdb72dd697a0fd71c8416ad4fd2c1e 100644
> > --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > @@ -88,6 +88,8 @@ select:
> >             - mediatek,mt8135-pctl-a-syscfg
> >             - mediatek,mt8135-pctl-b-syscfg
> >             - mediatek,mt8173-pctl-a-syscfg
> > +          - mediatek,mt8196-fdvfs
> > +          - mediatek,mt8196-fdvfs-config
> >             - mediatek,mt8365-syscfg
> >             - microchip,lan966x-cpu-syscon
> >             - microchip,mpfs-sysreg-scb
> > @@ -194,6 +196,8 @@ properties:
> >             - mediatek,mt8135-pctl-a-syscfg
> >             - mediatek,mt8135-pctl-b-syscfg
> >             - mediatek,mt8173-pctl-a-syscfg
> > +          - mediatek,mt8196-fdvfs
> > +          - mediatek,mt8196-fdvfs-config
> >             - mediatek,mt8365-infracfg-nao
> >             - mediatek,mt8365-syscfg
> >             - microchip,lan966x-cpu-syscon
> > 
> 
> 
> 

Re: [PATCH 1/5] dt-bindings: mfd: syscon: Add mt8196 fdvfs syscons

[AngeloGioacchino Del Regno]

Il 14/07/25 13:58, Nicolas Frattaroli ha scritto:
> On Monday, 14 July 2025 10:47:32 Central European Summer Time AngeloGioacchino Del Regno wrote:
>> Il 11/07/25 16:57, Nicolas Frattaroli ha scritto:
>>> The MT8196 SoC uses two syscon ranges for CPU DVFS that are separate
>>> from each other. One, mt8196-fdvfs-config, is used to check for a magic
>>> number at that memory address to verify that fdvfs should be used. The
>>> other, mt8196-fdvfs, is used to configure the desired frequency for the
>>> DVFS controller for each CPU core.
>>>
>>
>> What is the reason why you're using syscons here?
>>
>> Can't we simply assign the FDVFS MMIO to the cpufreq-hw node?
> 
> That would require refactoring the driver way more since it currently
> gets the number of performance domains from the number of regs. If
> you want me to do that, I'll need to know how we should disambiguate
> performance domains from misc things like fdvfs. Stuff like string
> comparisons on reg-names seems very ugly for the driver to do, but
> adding a property to explicitly specify the number of performance
> domains would then put into question what the existing binding did
> by just assuming this information is something that implementations
> can get without any ambiguity.
> 
> Even if we forget that Linux is the only kernel that cares about this
> device tree, I'm not totally on board with having the smattering of
> dozens of different tiny register ranges in every DT node on mediatek
> like the vendor kernel does it. And not to forget, it'd change the
> binding even more, to the point where I'd probably have to create a
> new binding for mt8196.
> 

The problem that I see with having a syscon for this is that I can foresee
future SoCs using the same cpufreq-hybrid IP, which means that we'd surely
be cluttering the dt-bindings/mfd/syscon.yaml with a huge list of MediaTek
SoCs (x2, because it's two syscons, even) without having a really-really
valid reason to do so.


>> Or is there any reason why we can't declare it as mmio-sram? ...because I'm not
>> entirely sure but the FDVFS space should effectively be a [c]SRAM memory range...
> 
> mmio-sram is fairly useless for the purposes of having something as
> a fixed set of registers, hence why nobody else does it. From my
> research, it appears to mainly be used if you want to actually treat
> it like a pool of memory from which to then dynamically allocate
> things.
> 
> To use it like a syscon, which is what we're doing, you'll have to
> specify your mmio-sram node, then add a child node as a reserved range
> for the "syscon-like" area, and then specify in ranges that you want
> that child node's address translated into the global address space as
> expected. Then in the driver, you can't just do a single function call
> to get some regmap to write into, you have to follow a phandle in your
> vendor property pointing to said sram range, then get its address,
> translate said address to the global SoC address space, and then iomap
> it. And the cleanup for error paths/driver remove isn't fun either.
> 
> Besides, we don't actually know whether this is an sram range, or how
> large it is. The only confirmed sram range was the csram_sys thing at
> like 0x11bc00 which is not used because it turned out to be useless,
> and dealing with the kernel's sram interface to use it as a reserved
> range just to read 4 bytes from it was a wasted afternoon. And that's
> not even the real starting address of that sram area, that's just a
> part we know is used because downstream uses 5 KiB there for a codepath
> that's dead when fdvfs is present (except the one thing where it installs
> a panic handler to shove something into a register to make it stop dvfs
> logging if the kernel is in the process of crashing).
> 
> I can be convinced to go through the pain of making this mmio-sram if
> I have documentation of the whole memory map of the SoC that shows me
> where and how large each area of sram is, so that I don't need to come
> up with my own starting addresses and offsets based on whatever random
> stuff I can infer from downstream DT.
> 
> Really, every writable syscon is probably implemented as "sram" in
> hardware. But an sram cell that is not general use memory but is
> treated by the hardware as having some meaning is not mmio-sram. We
> can't ever use it in any way other than as a syscon, and telling
> implementations that they can except then slapping a huge reserved
> range into it just makes them have to implement syscons twice, once
> as syscons and once as syscons-except-it's-a-reserved-sram-mmio-range.
> 

I understand. That's a valid reason, it's ok.

I'm not sure though why you're talking about huge restructuring of the MediaTek
cpufreq driver to add the FDVFS registers in the reg list?

There are only two MMIOs that we care about here, right? FDVFS and FDVFS-CONFIG.

Imagine the devicetree node being like this:

performance-controller@1234 {
   compatible = "mediatek,mt8196-cpufreq-hw";
   reg = <0 0xdomain1 0 0xsz>, <0 0xdomain2 0 0xsz>, <0xdomainN 0 0xsz> ...
         <0 0xfdvfs 0 0xsz>, <0 0xfdvfsconfig 0 0xsz>;
   #performance-domain-cells = <1>;
};

If you want, you can even do without having reg-names (though that might be better
for readability), because:

   1. This is a MT8196-specific binding (and node); this means that you can safely
      assume that:
      - There are three clusters (ignorable)
      - There is FDVFS support (not ignorable)
   2. No other SoC will ever use the MT8196 binding without being compatible with it
      and anyway newer socs will need something like
      compatible = "mediatek,mt1234-cpufreq-hw", "mediatek,mt8196-cpufreq-hw";
      - This means that N.1 still stands for new SoCs

So I do see two solutions here, of which I kinda prefer the one without reg-names.

With reg-names:
  1. Check all MMIOs named "domain%u" (domain0, domain1, domain2): this kind-of
     reflects the naming of the cpufreq policies (policy0...N), which even keeps
     things consistent between both hardware names and cpufreq software names (but
     of course - as long as HW is described in bindings, we don't care about any
     software defined names);
     - This gives you the number of performance domains, 3 in the specific case
       of the MT8196 SoC (and some future others for sure!)
  2. Get reg by name: "fdvfs-base", "fdvfs-config"

Without reg-names:
  1. Define the following in the dt-bindings for the cpufreq with fdvfs:
     reg:
       - description: FDVFS Performance State (or voltage?) setting region
       - description: FDVFS Configuration region
       - description: Cluster 0 OPP tables and Performance State setting region
       - description: Cluster 1 OPP tables and ...
       - description: Cluster 2 OPP tables and ...
       (new socs) - description: Cluster 3 ... etc etc etc
  2. Add the MT8196 conditionals and exclusions:
     if: (blahblah) mediatek,mt8196-cpufreq-hw
     then: reg:   minItems: 5    maxItems: 5
  3. Modify the driver like so:
     - Add platform data for MT8196, and set a "uses_fdvfs = true" (or something
       like that, anyway, your call about the naming)
     - Get the FDVFS regions:
         fdvfs_base = devm_platform_ioremap_resource(pdev, 0);
         error_checking_here();
         fdvfs_config = devm_platform_ioremap_resource(pdev, 1);
         error_checking_here();

         static int mtk_cpu_resources_init(.....)
         {
           .....code......
           index = args.args[0]; /* performance-domains number after parsing */

           if (pdata->uses_fdvfs)
             index += 2;

           ...code... (no further modification required)
         }
   4. Profit!

Now, if you're thinking about *guaranteeing* that FDVFS is always the first two
MMIOs in the node... just do not care.

You are already enforcing that with devicetree bindings, so that's how you are
guaranteeing that.

Anyway - If you want to make an entirely new binding named
            mediatek,mt8196-cpufreq-hw.yaml
noone prevents you from doing that.

If the binding describes the same hardware, it is WRONG to create a new one, but
in this case it would be describing *new hardware* that is incompatible with the
previous version, so that's RIGHT to do.

You don't need to have two drivers for two bindings, either.

You can have one driver with two different bindings, as long as it makes sense to
have that (as in, if the two are incompatible in the binding, but in the actual
code it still makes sense to have both in the same driver, that's ok).

Cheers!
Angelo

>>
>> Cheers,
>> Angelo
>>
> 
> Kind regards,
> Nicolas Frattaroli
> 
>>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>>> ---
>>>    Documentation/devicetree/bindings/mfd/syscon.yaml | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
>>> index 27672adeb1fedb7c81b8ae86c35f4f3b26d5516f..5ee49d2ba0cdb72dd697a0fd71c8416ad4fd2c1e 100644
>>> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
>>> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
>>> @@ -88,6 +88,8 @@ select:
>>>              - mediatek,mt8135-pctl-a-syscfg
>>>              - mediatek,mt8135-pctl-b-syscfg
>>>              - mediatek,mt8173-pctl-a-syscfg
>>> +          - mediatek,mt8196-fdvfs
>>> +          - mediatek,mt8196-fdvfs-config
>>>              - mediatek,mt8365-syscfg
>>>              - microchip,lan966x-cpu-syscon
>>>              - microchip,mpfs-sysreg-scb
>>> @@ -194,6 +196,8 @@ properties:
>>>              - mediatek,mt8135-pctl-a-syscfg
>>>              - mediatek,mt8135-pctl-b-syscfg
>>>              - mediatek,mt8173-pctl-a-syscfg
>>> +          - mediatek,mt8196-fdvfs
>>> +          - mediatek,mt8196-fdvfs-config
>>>              - mediatek,mt8365-infracfg-nao
>>>              - mediatek,mt8365-syscfg
>>>              - microchip,lan966x-cpu-syscon
>>>
>>
>>
>>
> 
> 
> 
> 

[PATCH 2/5] dt-bindings: cpufreq: mediatek-hw: add mt8196 cpufreq binding

[Nicolas Frattaroli]

The MediaTek MT8196 SoC's cpufreq has three performance domains, each
with their own reg item, compared to the existing mediatek,cpufreq-hw
compatible that only uses two.

In addition, the hardware requires special handling by driver
implementations, so the new compatible is needed for more than just the
difference in regs.

Add the mediatek,mt8196-cpufreq-hw compatible, increase the maxItems of
reg, and reduce it back down to 2 for mediatek,cpufreq-hw in an if
condition.

A second example is added to help verify the binding's correctness and
document its use appropriately, though some awkward label names had to
be chosen as dt-extract-example concatenates the entire list of examples
for each binding into a single file, so naming conflicts between them
can occur.

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

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
index d0aecde2b89b1896c01ea8ae24f26032d8075a11..cee2678b926f845ab131cecef403e127a63fabb2 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek-hw.yaml
@@ -16,11 +16,14 @@ description:
 
 properties:
   compatible:
-    const: mediatek,cpufreq-hw
+    contains:
+      enum:
+        - mediatek,cpufreq-hw
+        - mediatek,mt8196-cpufreq-hw
 
   reg:
     minItems: 1
-    maxItems: 2
+    maxItems: 3
     description:
       Addresses and sizes for the memory of the HW bases in
       each frequency domain. Each entry corresponds to
@@ -40,6 +43,18 @@ required:
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,cpufreq-hw
+    then:
+      properties:
+        regs:
+          maxItems: 2
+
 examples:
   - |
     cpus {
@@ -68,3 +83,48 @@ examples:
             #performance-domain-cells = <1>;
         };
     };
+  - |
+    cpus {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            mt8196_cpu0: cpu@0 {
+                device_type = "cpu";
+                compatible = "arm,cortex-a720";
+                enable-method = "psci";
+                performance-domains = <&mt8196_performance 0>;
+                reg = <0x000>;
+            };
+
+            /* ... */
+
+            mt8196_cpu6: cpu@600 {
+                device_type = "cpu";
+                compatible = "arm,cortex-x4";
+                enable-method = "psci";
+                performance-domains = <&mt8196_performance 1>;
+                reg = <0x600>;
+            };
+
+            mt8196_cpu7: cpu@700 {
+                device_type = "cpu";
+                compatible = "arm,cortex-x925";
+                enable-method = "psci";
+                performance-domains = <&mt8196_performance 2>;
+                reg = <0x700>;
+            };
+    };
+
+    /* ... */
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        mt8196_performance: performance-controller@c2c0f20 {
+            compatible = "mediatek,mt8196-cpufreq-hw";
+            reg = <0 0xc2c0f20 0 0x120>, <0 0xc2c1040 0 0x120>,
+                  <0 0xc2c1160 0 0x120>;
+            #performance-domain-cells = <1>;
+        };
+    };

-- 
2.50.0

[PATCH 3/5] 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.0

[PATCH 4/5] 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.0

[PATCH 5/5] 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 acquiring some syscons as regmaps
to both check for the FDVFS magic number and to set target frequencies
for FDVFS.

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 | 78 ++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
index 53611077d0d9a2d9865cf771568ab71abc0e6fbd..30cf038babb6d02a0666ed39b926f3977a164236 100644
--- a/drivers/cpufreq/mediatek-cpufreq-hw.c
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -9,10 +9,12 @@
 #include <linux/init.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -24,6 +26,8 @@
 #define POLL_USEC			1000
 #define TIMEOUT_USEC			300000
 
+#define MT8196_FDVFS_MAGIC		0xABCD0001UL
+
 enum {
 	REG_FREQ_LUT_TABLE,
 	REG_FREQ_ENABLE,
@@ -36,7 +40,10 @@ enum {
 };
 
 struct mtk_cpufreq_priv {
+	struct device *dev;
 	const struct mtk_cpufreq_variant *variant;
+	struct regmap *fdvfs_config;
+	struct regmap *fdvfs;
 };
 
 struct mtk_cpufreq_domain {
@@ -49,7 +56,9 @@ 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;
 };
 
 static const struct mtk_cpufreq_variant cpufreq_mtk_base_variant = {
@@ -63,6 +72,38 @@ static const struct mtk_cpufreq_variant cpufreq_mtk_base_variant = {
 	},
 };
 
+static int mtk_cpufreq_hw_mt8196_init(struct mtk_cpufreq_priv *priv)
+{
+	u32 val;
+
+	priv->fdvfs_config = syscon_regmap_lookup_by_compatible("mediatek,mt8196-fdvfs-config");
+	if (!priv->fdvfs_config)
+		return dev_err_probe(priv->dev, PTR_ERR(priv->fdvfs_config),
+				     "failed to get fdvfs-config syscon\n");
+
+	if (!regmap_read(priv->fdvfs_config, 0x0, &val) && val == MT8196_FDVFS_MAGIC) {
+		priv->fdvfs = syscon_regmap_lookup_by_compatible("mediatek,mt8196-fdvfs");
+		if (!priv->fdvfs)
+			return dev_err_probe(priv->dev, PTR_ERR(priv->fdvfs),
+					     "failed to get fdvfs syscon\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,
+};
+
 static int __maybe_unused
 mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
 			  unsigned long *KHz)
@@ -91,10 +132,34 @@ mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *uW,
 	return 0;
 }
 
+static int 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;
+	int ret;
+
+	target_freq = DIV_ROUND_UP(target_freq, priv->variant->fdvfs_fdiv);
+	for_each_cpu(cpu, policy->real_cpus) {
+		ret = regmap_write(priv->fdvfs, cpu * 4, target_freq);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 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;
+
+	if (data->parent->fdvfs) {
+		target_freq = policy->freq_table[index].frequency;
+		return mtk_cpufreq_hw_fdvfs_switch(target_freq, policy);
+	}
 
 	writel_relaxed(index, data->reg_bases[REG_FREQ_PERF_STATE]);
 
@@ -127,7 +192,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;
 }
@@ -339,6 +407,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 +432,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.0