Re: [PATCH 2/2] platform/x86: export thinkpad_acpi handles

["Mark Pearson" <mpearson-lenovo@squebb.ca>]

Hi Ilpo,

On Thu, May 8, 2025, at 10:03 AM, Ilpo Järvinen wrote:
> On Wed, 7 May 2025, Mark Pearson wrote:
>
>> Add API to be able to get the thinkpad_acpi various handles.
>> 
>> Will use this to start pulling some of the thinkpad_acpi functionality
>> into separate modules in the future.
>> 
>> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> ---
>>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 18 ++++++++++++++++++
>>  include/linux/thinkpad_acpi.h               | 17 +++++++++++++++++
>>  2 files changed, 35 insertions(+)
>>  create mode 100644 include/linux/thinkpad_acpi.h
>> 
>> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> index 7dd4abf47f61..0eb33b4c99cf 100644
>> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> @@ -67,6 +67,7 @@
>>  #include <linux/string.h>
>>  #include <linux/string_helpers.h>
>>  #include <linux/sysfs.h>
>> +#include <linux/thinkpad_acpi.h>
>>  #include <linux/types.h>
>>  #include <linux/uaccess.h>
>>  #include <linux/units.h>
>> @@ -606,6 +607,23 @@ TPACPI_HANDLE(hkey, ec, "\\_SB.HKEY",	/* 600e/x, 770e, 770x */
>>   * ACPI helpers
>>   */
>>  
>> +int tp_acpi_get_handle(enum tp_acpi_handle_id handle_id, acpi_handle *handle)
>> +{
>> +	switch (handle_id) {
>> +	case TP_ROOT_HANDLE:
>> +		*handle = root_handle;
>> +		return 0;
>> +	case TP_EC_HANDLE:
>> +		*handle = ec_handle;
>> +		return 0;
>> +	case TP_HKEY_HANDLE:
>> +		*handle = hkey_handle;
>> +		return 0;
>> +	}
>> +	return -ENODEV;
>> +}
>> +EXPORT_SYMBOL_GPL(tp_acpi_get_handle);
>
> Please put these symbols into a namespace.

Sorry, not quite sure what you mean here. Could you point me at an example?

>
>> +
>>  static int acpi_evalf(acpi_handle handle,
>>  		      int *res, char *method, char *fmt, ...)
>>  {
>> diff --git a/include/linux/thinkpad_acpi.h b/include/linux/thinkpad_acpi.h
>> new file mode 100644
>> index 000000000000..eb5273464658
>> --- /dev/null
>> +++ b/include/linux/thinkpad_acpi.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Thinkpad ACPI driver API
>> + */
>> +
>> +#ifndef _TP_ACPI_H_
>> +#define _TP_ACPI_H_
>> +
>> +enum tp_acpi_handle_id {
>> +	TP_ROOT_HANDLE,
>> +	TP_EC_HANDLE,
>> +	TP_HKEY_HANDLE,
>> +};
>> +
>> +int tp_acpi_get_handle(enum tp_acpi_handle_id handle_id, acpi_handle *handle);
>> +
>> +#endif /* _TP_ACPI_H */
>> 
>
> I suggest you send this in the series that introduces an user for this 
> interface.

OK - will do. 
Side note - I do have one planned (new battery ACPI interface that gives some extra battery related information).

Thanks for the review
Mark