Re: [PATCH 3/7] platform/x86: think-lmi: Use WMI bus API when accessing BIOS settings

[Armin Wolf]

Am 13.02.25 um 14:17 schrieb Ilpo Järvinen:

> On Mon, 3 Feb 2025, Armin Wolf wrote:
>
> > Since the driver already binds to LENOVO_BIOS_SETTING_GUID, using
> > wmidev_block_query() inside tlmi_setting() allows for faster
> > access to BIOS settings.
> >
> > Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
> > ---
> >   drivers/platform/x86/think-lmi.c | 23 +++++++++--------------
> >   drivers/platform/x86/think-lmi.h |  2 ++
> >   2 files changed, 11 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> > index 2c94a4af9a1d..0fc275e461be 100644
> > --- a/drivers/platform/x86/think-lmi.c
> > +++ b/drivers/platform/x86/think-lmi.c
> > @@ -344,20 +344,14 @@ static int tlmi_opcode_setting(char *setting, const char *value)
> >   	return ret;
> >   }
> >
> > -static int tlmi_setting(int item, char **value, const char *guid_string)
> > +static int tlmi_setting(struct wmi_device *wdev, int item, char **value)
> >   {
> > -	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))
> > -		return -EIO;
> > -
> > -	obj = output.pointer;
> > +	obj = wmidev_block_query(wdev, item);
> >   	if (!obj)
> > -		return -ENODATA;
> > +		return -EIO;
> Hi Armin,
>
> I'm trying to understand why there are these back and forth changes in the
> error code.
>
> It almost looks to me like wmidev_block_query() would want to return the
> error code itself because after you abstracted this code using
> wmidev_block_query(), you had to change the error code because you no
> longer have access to the key detail to decide which error code should be
> returned. That is, use ERR_PTR() inside wmidev_block_query() and the
> callers should just pass that error code on with IS_ERR & friends?
Hi,

the reason why wmidev_block_query() only returns NULL in case of an error is that
according to the WMI-ACPI specification, querying a WMI data block should return data.

So we have two error scenarios:

- ACPI error => firmware error => EIO
- no data returned => violation of firmware spec => EIO

Because of this always returning EIO is the correct approach in my opinion.

Thanks,
Armin Wolf