Hi Armin
On Sun, Feb 9, 2025, at 7:31 PM, Armin Wolf wrote:
> Am 03.02.25 um 19:23 schrieb Armin Wolf:
>
>> Move the ACPI buffer handling out of tlmi_extract_output_string()
>> and instead pass the unpacked ACPI object to prepare for future
>> changes.
>
> Hi,
>
> i was hoping that maybe the driver maintainer could take a look at this patch
> and give some feedback.
>
My apologies - because of this patch (and a couple of others) I've just realised I never updated the MAINTAINERS file.
I have been mothballing the markpearson@xxxxxxxxxx address as it's a nightmare to use and switched to using my personal email domain instead. It seems my email filters aren't flagging these messages the way they are supposed to be - I have to figure that out :(
> Thanks,
> Armin Wolf
>
>> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
>> ---
>> drivers/platform/x86/think-lmi.c | 38 +++++++++++++++++---------------
>> 1 file changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
>> index 323316ac6783..2c94a4af9a1d 100644
>> --- a/drivers/platform/x86/think-lmi.c
>> +++ b/drivers/platform/x86/think-lmi.c
>> @@ -262,16 +262,11 @@ static int tlmi_simple_call(const char *guid, const char *arg)
>> return 0;
>> }
>>
>> -/* Extract output string from WMI return buffer */
>> -static int tlmi_extract_output_string(const struct acpi_buffer *output,
>> - char **string)
>> +/* Extract output string from WMI return value */
>> +static int tlmi_extract_output_string(union acpi_object *obj, char **string)
>> {
>> - const union acpi_object *obj;
>> char *s;
>>
>> - obj = output->pointer;
>> - if (!obj)
>> - return -ENOMEM;
>> if (obj->type != ACPI_TYPE_STRING || !obj->string.pointer)
>> return -EIO;
>>
>> @@ -352,17 +347,21 @@ static int tlmi_opcode_setting(char *setting, const char *value)
>> static int tlmi_setting(int item, char **value, const char *guid_string)
>> {
>> struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> + union acpi_object *obj;
>> acpi_status status;
>> int ret;
>>
>> status = wmi_query_block(guid_string, item, &output);
>> - if (ACPI_FAILURE(status)) {
>> - kfree(output.pointer);
>> + if (ACPI_FAILURE(status))
>> return -EIO;
>> - }
>>
>> - ret = tlmi_extract_output_string(&output, value);
>> - kfree(output.pointer);
>> + obj = output.pointer;
>> + if (!obj)
>> + return -ENODATA;
>> +
>> + ret = tlmi_extract_output_string(obj, value);
>> + kfree(obj);
>> +
>> return ret;
>> }
>>
>> @@ -370,19 +369,22 @@ static int tlmi_get_bios_selections(const char *item, char **value)
>> {
>> const struct acpi_buffer input = { strlen(item), (char *)item };
>> struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> + union acpi_object *obj;
>> acpi_status status;
>> int ret;
>>
>> status = wmi_evaluate_method(LENOVO_GET_BIOS_SELECTIONS_GUID,
>> 0, 0, &input, &output);
>> -
>> - if (ACPI_FAILURE(status)) {
>> - kfree(output.pointer);
>> + if (ACPI_FAILURE(status))
>> return -EIO;
>> - }
>>
>> - ret = tlmi_extract_output_string(&output, value);
>> - kfree(output.pointer);
>> + obj = output.pointer;
>> + if (!obj)
>> + return -ENODATA;
>> +
>> + ret = tlmi_extract_output_string(obj, value);
>> + kfree(obj);
>> +
>> return ret;
>> }
>>
>> --
>> 2.39.5
>>
>>
Changes look good to me. If you can hold on a bit I'll see if I can test them on a few platforms to make sure no surprises.
Reviewed-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Mark
