[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb0466ae-7a6c-20c8-4e90-e4b9b3432e72@marcan.st>
Date: Thu, 10 Nov 2022 00:39:37 +0900
From: Hector Martin <marcan@...can.st>
To: Marc Zyngier <maz@...nel.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Sven Peter <sven@...npeter.dev>,
Alyssa Rosenzweig <alyssa@...enzweig.io>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Stephen Boyd <sboyd@...nel.org>,
Ulf Hansson <ulf.hansson@...aro.org>,
Mark Kettenis <mark.kettenis@...all.nl>, asahi@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-pm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/5] cpufreq: apple-soc: Add new driver to control
Apple SoC CPU P-states
On 09/11/2022 23.20, Marc Zyngier wrote:
> On Wed, 09 Nov 2022 12:13:33 +0000,
> Hector Martin <marcan@...can.st> wrote:
>>
>> On 24/10/2022 17.27, Marc Zyngier wrote:
>>> On Mon, 24 Oct 2022 05:39:24 +0100,
>>> Hector Martin <marcan@...can.st> wrote:
>>>>
>>>> This driver implements CPU frequency scaling for Apple Silicon SoCs,
>>>> including M1 (t8103), M1 Max/Pro/Ultra (t600x), and M2 (t8112).
>>>>
>>>> Each CPU cluster has its own register set, and frequency management is
>>>> fully automated by the hardware; the driver only has to write one
>>>> register. There is boost frequency support, but the hardware will only
>>>> allow their use if only a subset of cores in a cluster are in
>>>> non-deep-idle. Since we don't support deep idle yet, these frequencies
>>>> are not achievable, but the driver supports them. They will remain
>>>> disabled in the device tree until deep idle is implemented, to avoid
>>>> confusing users.
>>>>
>>>> This driver does not yet implement the memory controller performance
>>>> state tuning that usually accompanies higher CPU p-states. This will be
>>>> done in a future patch.
>>>>
>>>> Signed-off-by: Hector Martin <marcan@...can.st>
>>>> ---
>>>> drivers/cpufreq/Kconfig.arm | 9 +
>>>> drivers/cpufreq/Makefile | 1 +
>>>> drivers/cpufreq/apple-soc-cpufreq.c | 352 +++++++++++++++++++++++++++
>>>> drivers/cpufreq/cpufreq-dt-platdev.c | 2 +
>>>> 4 files changed, 364 insertions(+)
>>>> create mode 100644 drivers/cpufreq/apple-soc-cpufreq.c
>>>>
>>>
>>> [...]
>>>
>>>> +static struct freq_attr *apple_soc_cpufreq_hw_attr[] = {
>>>> + &cpufreq_freq_attr_scaling_available_freqs,
>>>> + NULL,
>>>> + NULL,
>>>
>>> nit: extra NULL?
>>
>> That slot gets filled in later if boost is enabled, hence the need for
>> an extra terminating NULL in that case.
>
> Right. Consider placing a comment next to the first NULL so that
> someone else doesn't consider it useless and accidentally removes
> it...
>
Good point, done :)
- Hector
Powered by blists - more mailing lists