From: Dave Martin <Dave.Martin@arm•com>
To: James Morse <james.morse@arm•com>
Cc: linux-kernel@vger•kernel.org,
linux-arm-kernel@lists•infradead.org, linux-acpi@vger•kernel.org,
devicetree@vger•kernel.org,
D Scott Phillips OS <scott@os•amperecomputing.com>,
carl@os•amperecomputing.com, lcherian@marvell•com,
bobo.shaobowang@huawei•com, tan.shaopeng@fujitsu•com,
baolin.wang@linux•alibaba.com,
Jamie Iles <quic_jiles@quicinc•com>,
Xin Hao <xhao@linux•alibaba.com>,
peternewman@google•com, dfustini@baylibre•com,
amitsinght@marvell•com, David Hildenbrand <david@redhat•com>,
Rex Nie <rex.nie@jaguarmicro•com>, Koba Ko <kobak@nvidia•com>,
Shanker Donthineni <sdonthineni@nvidia•com>,
fenghuay@nvidia•com, baisheng.gao@unisoc•com,
Jonathan Cameron <jonathan.cameron@huawei•com>,
Rob Herring <robh@kernel•org>,
Rohit Mathew <rohit.mathew@arm•com>,
Rafael Wysocki <rafael@kernel•org>, Len Brown <lenb@kernel•org>,
Lorenzo Pieralisi <lpieralisi@kernel•org>,
Hanjun Guo <guohanjun@huawei•com>,
Sudeep Holla <sudeep.holla@arm•com>,
Krzysztof Kozlowski <krzk+dt@kernel•org>,
Conor Dooley <conor+dt@kernel•org>,
Catalin Marinas <catalin.marinas@arm•com>,
Will Deacon <will@kernel•org>,
Greg Kroah-Hartman <gregkh@linuxfoundation•org>,
Danilo Krummrich <dakr@kernel•org>
Subject: Re: [PATCH 04/33] ACPI / PPTT: Stop acpi_count_levels() expecting callers to clear levels
Date: Tue, 9 Sep 2025 11:06:52 +0100 [thread overview]
Message-ID: <aL/8PIcebYGoB/g6@e133380.arm.com> (raw)
In-Reply-To: <1914b7f0-10e6-4cf4-ad53-5ae03c69964d@arm.com>
Hi James,
On Thu, Aug 28, 2025 at 04:57:15PM +0100, James Morse wrote:
> Hi Dave,
>
> On 27/08/2025 11:49, Dave Martin wrote:
> > On Fri, Aug 22, 2025 at 03:29:45PM +0000, James Morse wrote:
> >> acpi_count_levels() passes the number of levels back via a pointer argument.
> >> It also passes this to acpi_find_cache_level() as the starting_level, and
> >> preserves this value as it walks up the cpu_node tree counting the levels.
> >>
> >> This means the caller must initialise 'levels' due to acpi_count_levels()
> >> internals. The only caller acpi_get_cache_info() happens to have already
> >> initialised levels to zero, which acpi_count_levels() depends on to get the
> >> correct result.
> >>
> >> Two results are passed back from acpi_count_levels(), unlike split_levels,
> >> levels is not optional.
> >>
> >> Split these two results up. The mandatory 'levels' is always returned,
> >> which hides the internal details from the caller, and avoids having
> >> duplicated initialisation in all callers. split_levels remains an
> >> optional argument passed back.
> >
> > Nit: I found all this a bit hard to follow.
> >
> > This seems to boil down to:
> >
> > --8<--
> >
> > In acpi_count_levels(), the initial value of *levels passed by the
> > caller is really an implementation detail of acpi_count_levels(), so it
> > is unreasonable to expect the callers of this function to know what to
> > pass in for this parameter. The only sensible initial value is 0,
> > which is what the only upstream caller (acpi_get_cache_info()) passes.
> >
> > Use a local variable for the starting cache level in acpi_count_levels(),
> > and pass the result back to the caller via the function return value.
> >
> > Gid rid of the levels parameter, which has no remaining purpose.
> >
> > Fix acpi_get_cache_info() to match.
> >
> > -->8--
>
> I've taken this instead,
OK
[...]
> >> @@ -731,7 +735,7 @@ int acpi_get_cache_info(unsigned int cpu, unsigned int *levels,
> >> if (!cpu_node)
> >> return -ENOENT;
> >>
> >> - acpi_count_levels(table, cpu_node, levels, split_levels);
> >> + *levels = acpi_count_levels(table, cpu_node, split_levels);
> >>
> >> pr_debug("Cache Setup: last_level=%d split_levels=%d\n",
> >> *levels, split_levels ? *split_levels : -1);
> >
> > Otherwise, looks reasonable to me.
> >
> > (But see my comments on the next patches re whether we really need this.)
>
> It was enough fun to debug that I'd like to save anyone else the trouble!
Fair enough.
Cheers
---Dave
next prev parent reply other threads:[~2025-09-09 15:42 UTC|newest]
Thread overview: 200+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-22 15:29 [PATCH 00/33] arm_mpam: Add basic mpam driver James Morse
2025-08-22 15:29 ` [PATCH 01/33] cacheinfo: Expose the code to generate a cache-id from a device_node James Morse
2025-08-27 10:46 ` Dave Martin
2025-08-27 17:11 ` James Morse
2025-08-28 14:08 ` Dave Martin
2025-08-22 15:29 ` [PATCH 02/33] drivers: base: cacheinfo: Add helper to find the cache size from cpu+level James Morse
2025-08-24 17:25 ` Krzysztof Kozlowski
2025-08-27 17:11 ` James Morse
2025-08-27 10:46 ` Dave Martin
2025-08-27 17:11 ` James Morse
2025-08-28 14:10 ` Dave Martin
2025-09-05 16:19 ` Dave Martin
2025-08-22 15:29 ` [PATCH 03/33] ACPI / PPTT: Add a helper to fill a cpumask from a processor container James Morse
2025-08-26 14:45 ` Ben Horgan
2025-08-28 15:56 ` James Morse
2025-08-27 10:48 ` Dave Martin
2025-08-28 15:57 ` James Morse
2025-09-05 16:24 ` Dave Martin
2025-09-10 19:29 ` James Morse
2025-08-22 15:29 ` [PATCH 04/33] ACPI / PPTT: Stop acpi_count_levels() expecting callers to clear levels James Morse
2025-08-27 10:49 ` Dave Martin
2025-08-28 15:57 ` James Morse
2025-09-09 10:06 ` Dave Martin [this message]
2025-08-22 15:29 ` [PATCH 05/33] ACPI / PPTT: Find cache level by cache-id James Morse
2025-08-23 12:14 ` Markus Elfring
2025-08-28 15:57 ` James Morse
2025-08-27 9:25 ` Ben Horgan
2025-08-28 15:57 ` James Morse
2025-08-27 10:50 ` Dave Martin
2025-08-28 15:58 ` James Morse
2025-09-05 16:27 ` Dave Martin
2025-09-10 19:29 ` James Morse
2025-08-22 15:29 ` [PATCH 06/33] ACPI / PPTT: Add a helper to fill a cpumask from a cache_id James Morse
2025-08-27 10:53 ` Dave Martin
2025-08-28 15:58 ` James Morse
2025-09-09 10:14 ` Dave Martin
2025-09-10 19:29 ` James Morse
2025-08-22 15:29 ` [PATCH 07/33] arm64: kconfig: Add Kconfig entry for MPAM James Morse
2025-08-27 8:53 ` Ben Horgan
2025-08-28 15:58 ` James Morse
2025-08-29 8:20 ` Ben Horgan
2025-08-27 11:01 ` Dave Martin
2025-09-04 17:28 ` James Morse
2025-09-09 10:26 ` Dave Martin
2025-08-22 15:29 ` [PATCH 08/33] ACPI / MPAM: Parse the MPAM table James Morse
2025-08-23 10:55 ` Markus Elfring
2025-09-04 17:28 ` James Morse
2025-08-27 16:05 ` Dave Martin
2025-09-04 17:28 ` James Morse
2025-09-05 16:38 ` Dave Martin
2025-09-10 19:19 ` James Morse
2025-08-22 15:29 ` [PATCH 09/33] dt-bindings: arm: Add MPAM MSC binding James Morse
2025-08-27 16:22 ` Dave Martin
2025-09-05 9:11 ` James Morse
2025-09-09 11:02 ` Dave Martin
2025-08-22 15:29 ` [PATCH 10/33] arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate James Morse
2025-08-22 19:15 ` Markus Elfring
2025-08-22 19:55 ` Markus Elfring
2025-08-23 6:41 ` Greg Kroah-Hartman
2025-08-27 13:03 ` Ben Horgan
2025-09-05 18:48 ` James Morse
2025-09-08 10:54 ` Ben Horgan
2025-08-27 15:39 ` Rob Herring
2025-08-27 16:16 ` Rob Herring
2025-09-05 18:52 ` James Morse
2025-09-05 18:52 ` James Morse
2025-09-01 9:11 ` Ben Horgan
2025-09-05 18:49 ` James Morse
2025-09-01 11:21 ` Dave Martin
2025-09-05 18:49 ` James Morse
2025-09-08 15:25 ` Dave Martin
2025-09-10 19:19 ` James Morse
2025-08-22 15:29 ` [PATCH 11/33] arm_mpam: Add support for memory controller MSC on DT platforms James Morse
2025-08-22 15:29 ` [PATCH 12/33] arm_mpam: Add the class and component structures for ris firmware described James Morse
2025-08-28 1:29 ` Fenghua Yu
2025-09-08 17:57 ` James Morse
2025-09-01 11:09 ` Dave Martin
2025-09-08 17:57 ` James Morse
2025-09-09 11:28 ` Dave Martin
2025-09-10 19:19 ` James Morse
2025-08-22 15:29 ` [PATCH 13/33] arm_mpam: Add MPAM MSC register layout definitions James Morse
2025-08-29 8:42 ` Ben Horgan
2025-09-08 17:57 ` James Morse
2025-09-09 11:36 ` Shaopeng Tan (Fujitsu)
2025-09-10 19:31 ` James Morse
2025-08-22 15:29 ` [PATCH 14/33] arm_mpam: Add cpuhp callbacks to probe MSC hardware James Morse
2025-08-27 16:08 ` Rob Herring
2025-09-08 17:58 ` James Morse
2025-09-05 16:40 ` Dave Martin
2025-09-09 16:56 ` James Morse
2025-09-09 14:23 ` Dave Martin
2025-08-22 15:29 ` [PATCH 15/33] arm_mpam: Probe MSCs to find the supported partid/pmg values James Morse
2025-08-28 13:12 ` Ben Horgan
2025-09-09 16:56 ` James Morse
2025-09-10 9:01 ` Ben Horgan
2025-09-08 16:29 ` Dave Martin
2025-09-09 16:57 ` James Morse
2025-08-22 15:29 ` [PATCH 16/33] arm_mpam: Add helpers for managing the locking around the mon_sel registers James Morse
2025-08-28 17:07 ` Fenghua Yu
2025-09-09 16:57 ` James Morse
2025-09-09 15:39 ` Dave Martin
2025-09-10 19:19 ` James Morse
2025-08-22 15:29 ` [PATCH 17/33] arm_mpam: Probe the hardware features resctrl supports James Morse
2025-08-28 13:44 ` Ben Horgan
2025-09-09 16:57 ` James Morse
2025-09-10 9:11 ` Ben Horgan
2025-08-22 15:29 ` [PATCH 18/33] arm_mpam: Merge supported features during mpam_enable() into mpam_class James Morse
2025-08-29 13:54 ` Ben Horgan
2025-09-09 16:57 ` James Morse
2025-08-22 15:30 ` [PATCH 19/33] arm_mpam: Reset MSC controls from cpu hp callbacks James Morse
2025-08-27 16:19 ` Ben Horgan
2025-09-09 16:57 ` James Morse
2025-08-22 15:30 ` [PATCH 20/33] arm_mpam: Add a helper to touch an MSC from any CPU James Morse
2025-08-28 16:13 ` Ben Horgan
2025-09-09 16:57 ` James Morse
2025-08-22 15:30 ` [PATCH 21/33] arm_mpam: Extend reset logic to allow devices to be reset any time James Morse
2025-08-29 14:30 ` Ben Horgan
2025-09-09 16:58 ` James Morse
2025-08-22 15:30 ` [PATCH 22/33] arm_mpam: Register and enable IRQs James Morse
2025-09-09 16:58 ` James Morse
2025-08-22 15:30 ` [PATCH 23/33] arm_mpam: Use a static key to indicate when mpam is enabled James Morse
2025-08-22 15:30 ` [PATCH 24/33] arm_mpam: Allow configuration to be applied and restored during cpu online James Morse
2025-08-28 16:13 ` Ben Horgan
2025-09-10 19:29 ` James Morse
2025-08-22 15:30 ` [PATCH 25/33] arm_mpam: Probe and reset the rest of the features James Morse
2025-08-28 10:11 ` Ben Horgan
2025-09-10 19:30 ` James Morse
2025-08-22 15:30 ` [PATCH 26/33] arm_mpam: Add helpers to allocate monitors James Morse
2025-08-29 15:47 ` Ben Horgan
2025-08-22 15:30 ` [PATCH 27/33] arm_mpam: Add mpam_msmon_read() to read monitor value James Morse
2025-08-29 15:55 ` Ben Horgan
2025-09-10 19:30 ` James Morse
2025-08-22 15:30 ` [PATCH 28/33] arm_mpam: Track bandwidth counter state for overflow and power management James Morse
2025-08-29 16:09 ` Ben Horgan
2025-08-22 15:30 ` [PATCH 29/33] arm_mpam: Probe for long/lwd mbwu counters James Morse
2025-08-28 16:14 ` Ben Horgan
2025-09-10 19:30 ` James Morse
2025-08-22 15:30 ` [PATCH 30/33] arm_mpam: Use long MBWU counters if supported James Morse
2025-08-29 16:39 ` Ben Horgan
2025-09-10 19:30 ` James Morse
2025-08-22 15:30 ` [PATCH 31/33] arm_mpam: Add helper to reset saved mbwu state James Morse
2025-08-22 15:30 ` [PATCH 32/33] arm_mpam: Add kunit test for bitmap reset James Morse
2025-08-29 16:56 ` Ben Horgan
2025-09-10 19:30 ` James Morse
2025-08-22 15:30 ` [PATCH 33/33] arm_mpam: Add kunit tests for props_mismatch() James Morse
2025-08-29 17:11 ` Ben Horgan
2025-09-10 19:31 ` James Morse
2025-08-22 15:30 ` [PATCH 00/33] arm_mpam: Add basic mpam driver James Morse
2025-08-22 15:30 ` [PATCH 01/33] cacheinfo: Expose the code to generate a cache-id from a device_node James Morse
2025-08-22 15:30 ` [PATCH 02/33] drivers: base: cacheinfo: Add helper to find the cache size from cpu+level James Morse
2025-08-22 15:30 ` [PATCH 03/33] ACPI / PPTT: Add a helper to fill a cpumask from a processor container James Morse
2025-08-22 15:30 ` [PATCH 04/33] ACPI / PPTT: Stop acpi_count_levels() expecting callers to clear levels James Morse
2025-09-10 13:44 ` Lorenzo Pieralisi
2025-09-10 19:19 ` James Morse
2025-08-22 15:30 ` [PATCH 05/33] ACPI / PPTT: Find cache level by cache-id James Morse
2025-08-22 15:30 ` [PATCH 06/33] ACPI / PPTT: Add a helper to fill a cpumask from a cache_id James Morse
2025-09-10 16:06 ` Lorenzo Pieralisi
2025-09-10 19:18 ` James Morse
2025-08-22 15:30 ` [PATCH 07/33] arm64: kconfig: Add Kconfig entry for MPAM James Morse
2025-08-22 15:30 ` [PATCH 08/33] ACPI / MPAM: Parse the MPAM table James Morse
2025-09-09 6:54 ` Shaopeng Tan (Fujitsu)
2025-09-10 19:31 ` James Morse
2025-08-22 15:30 ` [PATCH 09/33] dt-bindings: arm: Add MPAM MSC binding James Morse
2025-08-22 15:30 ` [PATCH 10/33] arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate James Morse
2025-09-09 7:03 ` Shaopeng Tan (Fujitsu)
2025-09-10 19:31 ` James Morse
2025-08-22 15:30 ` [PATCH 11/33] arm_mpam: Add support for memory controller MSC on DT platforms James Morse
2025-09-09 7:11 ` Shaopeng Tan (Fujitsu)
2025-09-10 19:31 ` James Morse
2025-08-22 15:30 ` [PATCH 12/33] arm_mpam: Add the class and component structures for ris firmware described James Morse
2025-08-29 12:41 ` Ben Horgan
2025-09-10 19:32 ` James Morse
2025-09-09 7:30 ` Shaopeng Tan (Fujitsu)
2025-09-10 19:32 ` James Morse
2025-08-22 15:30 ` [PATCH 13/33] arm_mpam: Add MPAM MSC register layout definitions James Morse
2025-08-22 15:30 ` [PATCH 14/33] arm_mpam: Add cpuhp callbacks to probe MSC hardware James Morse
2025-08-22 15:30 ` [PATCH 15/33] arm_mpam: Probe MSCs to find the supported partid/pmg values James Morse
2025-08-22 15:30 ` [PATCH 16/33] arm_mpam: Add helpers for managing the locking around the mon_sel registers James Morse
2025-08-22 15:30 ` [PATCH 17/33] arm_mpam: Probe the hardware features resctrl supports James Morse
2025-08-22 15:30 ` [PATCH 18/33] arm_mpam: Merge supported features during mpam_enable() into mpam_class James Morse
2025-08-22 15:30 ` [PATCH 19/33] arm_mpam: Reset MSC controls from cpu hp callbacks James Morse
2025-08-22 15:30 ` [PATCH 20/33] arm_mpam: Add a helper to touch an MSC from any CPU James Morse
2025-08-22 15:30 ` [PATCH 21/33] arm_mpam: Extend reset logic to allow devices to be reset any time James Morse
2025-08-22 15:30 ` [PATCH 22/33] arm_mpam: Register and enable IRQs James Morse
2025-09-01 10:05 ` Ben Horgan
2025-08-22 15:30 ` [PATCH 23/33] arm_mpam: Use a static key to indicate when mpam is enabled James Morse
2025-08-22 15:30 ` [PATCH 24/33] arm_mpam: Allow configuration to be applied and restored during cpu online James Morse
2025-08-22 15:30 ` [PATCH 25/33] arm_mpam: Probe and reset the rest of the features James Morse
2025-08-22 15:30 ` [PATCH 26/33] arm_mpam: Add helpers to allocate monitors James Morse
2025-08-22 15:30 ` [PATCH 27/33] arm_mpam: Add mpam_msmon_read() to read monitor value James Morse
2025-08-22 15:30 ` [PATCH 28/33] arm_mpam: Track bandwidth counter state for overflow and power management James Morse
2025-08-28 0:58 ` Fenghua Yu
2025-09-10 19:29 ` James Morse
2025-08-22 15:30 ` [PATCH 29/33] arm_mpam: Probe for long/lwd mbwu counters James Morse
2025-08-22 15:30 ` [PATCH 30/33] arm_mpam: Use long MBWU counters if supported James Morse
2025-08-22 15:30 ` [PATCH 31/33] arm_mpam: Add helper to reset saved mbwu state James Morse
2025-08-22 15:30 ` [PATCH 32/33] arm_mpam: Add kunit test for bitmap reset James Morse
2025-08-22 15:30 ` [PATCH 33/33] arm_mpam: Add kunit tests for props_mismatch() James Morse
2025-09-02 16:59 ` Fenghua Yu
2025-08-24 17:24 ` [PATCH 00/33] arm_mpam: Add basic mpam driver Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aL/8PIcebYGoB/g6@e133380.arm.com \
--to=dave.martin@arm$(echo .)com \
--cc=amitsinght@marvell$(echo .)com \
--cc=baisheng.gao@unisoc$(echo .)com \
--cc=baolin.wang@linux$(echo .)alibaba.com \
--cc=bobo.shaobowang@huawei$(echo .)com \
--cc=carl@os$(echo .)amperecomputing.com \
--cc=catalin.marinas@arm$(echo .)com \
--cc=conor+dt@kernel$(echo .)org \
--cc=dakr@kernel$(echo .)org \
--cc=david@redhat$(echo .)com \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=dfustini@baylibre$(echo .)com \
--cc=fenghuay@nvidia$(echo .)com \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=guohanjun@huawei$(echo .)com \
--cc=james.morse@arm$(echo .)com \
--cc=jonathan.cameron@huawei$(echo .)com \
--cc=kobak@nvidia$(echo .)com \
--cc=krzk+dt@kernel$(echo .)org \
--cc=lcherian@marvell$(echo .)com \
--cc=lenb@kernel$(echo .)org \
--cc=linux-acpi@vger$(echo .)kernel.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=lpieralisi@kernel$(echo .)org \
--cc=peternewman@google$(echo .)com \
--cc=quic_jiles@quicinc$(echo .)com \
--cc=rafael@kernel$(echo .)org \
--cc=rex.nie@jaguarmicro$(echo .)com \
--cc=robh@kernel$(echo .)org \
--cc=rohit.mathew@arm$(echo .)com \
--cc=scott@os$(echo .)amperecomputing.com \
--cc=sdonthineni@nvidia$(echo .)com \
--cc=sudeep.holla@arm$(echo .)com \
--cc=tan.shaopeng@fujitsu$(echo .)com \
--cc=will@kernel$(echo .)org \
--cc=xhao@linux$(echo .)alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox