Re: [PATCH v4 1/2] ACPI/PPTT: Add support for ACPI 6.3 thread flag

[Jeremy Linton <jeremy.linton@arm.com>]

Hi,


Thanks for looking at this.

On 8/1/19 10:57 AM, Sudeep Holla wrote:
> 
> Hi Jeremy,
> 
> On Wed, Jul 31, 2019 at 10:46:33PM -0500, Jeremy Linton wrote:
>> ACPI 6.3 adds a flag to the CPU node to indicate whether
>> the given PE is a thread. Add a function to return that
>> information for a given linux logical CPU.
>>
> 
> Apart from few minor nits,
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   drivers/acpi/pptt.c  | 54 +++++++++++++++++++++++++++++++++++++++++++-
>>   include/linux/acpi.h |  5 ++++
>>   2 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>> index 1e7ac0bd0d3a..84718f6cb741 100644
>> --- a/drivers/acpi/pptt.c
>> +++ b/drivers/acpi/pptt.c
>> @@ -540,6 +540,44 @@ static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
>>   	return retval;
>>   }
>>
>> +/**
>> + * check_acpi_cpu_flag() - Determine if CPU node has a flag set
>> + * @cpu: Kernel logical CPU number
>> + * @rev: The PPTT revision defining the flag
> 
> [nit] I would rather put it as minimum PPTT revision that supports the
> flag. It aligns with the code too as we are not looking for exact match.

Ok, sure.


> 
>> + * @flag: The flag itself
>> + *
>> + * Check the node representing a CPU for a given flag.
>> + *
>> + * Return: -ENOENT if the PPTT doesn't exist, the CPU cannot be found or
>> + *	   the table revision isn't new enough.
>> + *	   1, any passed flag set
>> + *	   0, flag unset
>> + */
>> +static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
>> +{
>> +	struct acpi_table_header *table;
>> +	acpi_status status;
>> +	u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
>> +	struct acpi_pptt_processor *cpu_node = NULL;
>> +	int ret = -ENOENT;
>> +
>> +	status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
>> +	if (ACPI_FAILURE(status)) {
>> +		acpi_pptt_warn_missing();
>> +		return ret;
>> +	}
>> +
>> +	if (table->revision >= rev)
>> +		cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
>> +
>> +	if (cpu_node)
>> +		ret = (cpu_node->flags & flag) != 0;
>> +
>> +	acpi_put_table(table);
>> +
>> +	return ret;
>> +}
>> +
>>   /**
>>    * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
>>    * @cpu: Kernel logical CPU number
>> @@ -604,6 +642,21 @@ int cache_setup_acpi(unsigned int cpu)
>>   	return status;
>>   }
>>
>> +/**
>> + * acpi_pptt_cpu_is_thread() - Determine if CPU is a thread
>> + * @cpu: Kernel logical CPU number
>> + *
> 
> [nit] If you spin the patch again, you can drop extra line space here.

Sure..


> 
> --
> Regards,
> Sudeep
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>