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
