Re: [PATCH 2/4] ACPI / PPTT: Stop acpi_count_levels() expecting callers to clear levels

[James Morse <james.morse@arm.com>]

Hi Sudeep,

On 23/06/2025 14:10, Sudeep Holla wrote:
> On Thu, Jun 12, 2025 at 05:13:34PM +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.
>>
>> 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.
>>
>> Explicitly zero the levels variable, so the count always starts at zero.
>> This saves any additional callers having to work out they need to do this.

>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>> index aaf9b5a26d07..72e6bfc1e358 100644
>> --- a/drivers/acpi/pptt.c
>> +++ b/drivers/acpi/pptt.c
>> @@ -183,7 +183,7 @@ acpi_find_cache_level(struct acpi_table_header *table_hdr,
>>   * @cpu_node: processor node we wish to count caches for
>>   * @levels: Number of levels if success.
>>   * @split_levels:	Number of split cache levels (data/instruction) if
>> - *			success. Can by NULL.
>> + *			success. Can be NULL.
>>   *
>>   * Given a processor node containing a processing unit, walk into it and count
>>   * how many levels exist solely for it, and then walk up each level until we hit
>> @@ -196,6 +196,8 @@ static void acpi_count_levels(struct acpi_table_header *table_hdr,
>>  			      struct acpi_pptt_processor *cpu_node,
>>  			      unsigned int *levels, unsigned int *split_levels)
>>  {
>> +	*levels = 0;
>> +
> 
> Does it make sense to drop similar reset to 0 in acpi_get_cache_info(), just
> to be consistent across all callers of acpi_count_levels().

acpi_get_cache_info() does this because it can return early, it makes sense that it
already cleared the values it passes back.


> Otherwise, LGTM:
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>


Thanks!

James