Re: [PATCH 4/4] cpufreq: add an Amlogic speed-bin driver

[Chuan Liu <chuan.liu@amlogic.com>]

Hi Christian,

On 10/27/2026 6:22 PM, Christian Hewitt wrote:
> [ EXTERNAL EMAIL ]
> 
>> On 27 Oct 2026, at 1:26 pm, Chuan Liu <chuan.liu686@gmail.com> wrote:

Sorry, this is my personal email. I didn't notice it when I sent it 
earlier, please don't mind.

>>
>> Hi Viresh, Christian,
>>     Could you please hold off for now on the patches adding cpufreq driver support on S4? There are still two issues that need to be resolved first:
>>   - The clock driver currently does not support setting the CPU clock.
>>   - The CPU regulator needs some time to stabilize after the voltage is changed. Without a proper delay, the CPU clock frequency may be raised before the regulator voltage has actually reached the target value, which can hang the system. The S4 CPU regulator handling currently lacks the logic to wait for the regulator voltage to settle, so this part needs to be completed first.
>>
>> We are currently working on submitting our own cpufreq driver. We are considering how to address the issues above, taking into account the hardware characteristics of previous SoCs and evaluating factors such as the transition latency. S4 will be covered by that work (as well as SoCs from S4 onward, such as C3, T7, ..., A9). So please give us some time.
> 
> Please don’t top-post, keep inline threading!
> 
> FWIW, the recent bindings/clock/driver work I’ve done for S4 is here:
> 
> https://github.com/chewitt/linux/commits/amlogic-7.3.y

Sorry, that's my fault, my mailbox seems to have some issues. I only
received the follow-up replies commenting on this patchset, so I missed
the other patches in the series (e.g. 2/4 here).

I've now re-reviewed patch 2/4. From a functional point of view it looks
complete. As for whether implementing it in s4-pll.c is the right place,
Jerome may be able to help evaluate that. Thanks for your contribution!

> 
> My speed-bin driver is based on earlier work from Martin Blumenstingl:
> 
> https://github.com/chewitt/linux/commit/77e12d4d584d6a8bbd93304bbd4e9bbbb6ee159a

I had also noticed Martin's earlier patch, it is a useful reference for
us as well. Thanks!

> 
> Setting a ramp-delay was the key to opp transition stability:
> 
> https://github.com/chewitt/linux/commit/ca7a99e86e23677c7b5af887e6f3b3c8ce3470d2

This approach can also satisfy the CPU regulator requirement of waiting
200us per 30mV step for the voltage to stabilize. However, we are not 
entirely sure whether this approach might pose any risks (such as 
causing voltage overshoot). We will test this method later and evaluate 
its impact on the output voltage.


Regarding the concern I mentioned above about supporting a unified
cpufreq driver across S4, C3, T7, ..., A9, let me explain it in more detail:
    - These SoCs fall into several hardware architecture categories, and
I'm wondering whether we could take all of them into consideration and
implement them in a single amlogic-cpufreq.c, for example:
      * S4/C3: 1 * cluster
      * T7: 2 * clusters
      * S7/S7D/S6: 1 * cluster + DSU
      * A9: 2 * clusters + DSU (the 2 clusters share one DSU)
    - For different hardware architectures, the resources that the
cpufreq driver has to manage also differ and are coupled with each
other, for example:
      * S7/S7D/S6: the CPUs and the DSU share a single power rail, and
the DSU frequency scales proportionally with the CPU frequency
(typically dsu_clk = cpu_clk * 80%)
      * A9: the resources that A9 needs to manage are relatively more
complicated ...
    - Optimizing the cpufreq transition latency:
      * Clock latency: for most S4/C3/T7 customers, the released bl31.bin
only supports configuring the CPU clock through SMCs with discrete clock
rates. Frequent set_rate()/set_parent() calls in the kernel would result
in a relatively large clock latency, and would cause the sugov thread to
consume too much CPU time, hurting scheduling performance.
      * Voltage latency: voltage scaling consumes a significant part of
the transition latency; most of the time spent in target_index() goes
into this, which leads to slow load-response.

The amlogic-cpufreq.c as implemented here can indeed cover the case of a
single cluster with no shared resources, such as S4/C3/T7. However, for
the upcoming SoCs where multiple clusters share resources, it may not be
sufficient to cover: for example, on S7/S7D/S6 the CPUs and the DSU
share one regulator; on A9 the two clusters share the dsu_clk, and the
sharing relationships among cpu_reg, cpu_sram_reg, dsu_reg and
dsu_sram_reg are even more complicated. For such cases, extending the
current amlogic-cpufreq.c would be rather difficult, so we would like to
make an overall evaluation.

We have done a lot of optimization on these issues from both the
hardware and software sides, and we would like the cpufreq driver we
submit upstream to cover these optimizations as much as possible. For
example, the bl31.bin for S4/C3/T7 has already been released, and
updating bl31.bin may cause extra trouble for customers, so we would
also like to balance this and see how to minimize the impact.

I went through the commit history of your repositories, and some of the
approaches there are well worth learning from. As for the Amlogic
cpufreq driver, we would very much appreciate your comments and
suggestions.

So please give us some time, although, admittedly, we should have
considered these issues earlier.

> 
> CH.
> also a Christian, to confuse people :)
> 
>> On 10/26/2026 4:59 PM, Viresh Kumar wrote:
>>> On 26-10-26, 09:57, Jerome Brunet wrote:
>>>> On lun. 26 oct. 2026 at 11:00, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>>
>>>>> On 25-10-26, 20:22, christian.koever-draxl@student.uibk.ac.at wrote:
>>>>>> From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
>>>>>>
>>>>>> Every S4 speed bin runs the same CPU frequencies but needs its own
>>>>>> voltage curve, and the bin can only be read through the secure monitor.
>>>>>>
>>>>>> Read the bin from the firmware, select the matching
>>>>>> "opp-microvolt-speed<bin>" property for all CPUs, then register
>>>>>> cpufreq-dt. Bins outside the known range fall back to bin 0, which
>>>>>> has the highest voltages. Like sun50i-cpufreq-nvmem, the driver binds
>>>>>> on the SoC compatible and needs no node of its own, and S4 is added to
>>>>>> the cpufreq-dt-platdev blocklist so cpufreq-dt is only created after
>>>>>> the bin has been applied.
>>>>>>
>>>>>> Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
>>>>>> ---
>>>>>>   MAINTAINERS                          |   1 +
>>>>>>   drivers/cpufreq/Kconfig.arm          |  14 ++
>>>>>>   drivers/cpufreq/Makefile             |   1 +
>>>>>>   drivers/cpufreq/amlogic-cpufreq.c    | 184 +++++++++++++++++++++++++++
>>>>>>   drivers/cpufreq/cpufreq-dt-platdev.c |   2 +
>>>>>>   5 files changed, 202 insertions(+)
>>>>>>   create mode 100644 drivers/cpufreq/amlogic-cpufreq.c
>>>>>
>>>>> Is it safe to apply just this one ? I can take this via the PM tree then.
>>>>
>>>> I would advise to wait a little for Amlogic to chime in.
>>> Sure, I wasn't going to wait for the maintainers to respond anyway.
>>
>> --
>> Best regards,
>> Chuan
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
Best regards,
Chuan