Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 09 Nov 2022 14:20:47 +0000
Message-ID: <86v8nop5ts.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Hector Martin <marcan@marcan.st>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Sven Peter <sven@svenpeter.dev>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Mark Kettenis <mark.kettenis@xs4all.nl>, asahi@lists.linux.dev,
        linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/5] cpufreq: apple-soc: Add new driver to control Apple SoC CPU P-states
In-Reply-To: <3d4536c9-4b3b-8312-4868-5e5b42a87424@marcan.st>
References: <20221024043925.25379-1-marcan@marcan.st>
        <20221024043925.25379-5-marcan@marcan.st>
        <8635bdocco.wl-maz@kernel.org>
        <3d4536c9-4b3b-8312-4868-5e5b42a87424@marcan.st>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 09 Nov 2022 12:13:33 +0000,
Hector Martin <marcan@marcan.st> wrote:
> 
> On 24/10/2022 17.27, Marc Zyngier wrote:
> > On Mon, 24 Oct 2022 05:39:24 +0100,
> > Hector Martin <marcan@marcan.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@marcan.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...

	M.

-- 
Without deviation from the norm, progress is not possible.