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?
--
i.
> ret = tlmi_extract_output_string(obj, value);
> kfree(obj);
> @@ -995,7 +989,7 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
> char *item, *value;
> int ret;
>
> - ret = tlmi_setting(setting->index, &item, LENOVO_BIOS_SETTING_GUID);
> + ret = tlmi_setting(setting->wdev, setting->index, &item);
> if (ret)
> return ret;
>
> @@ -1588,7 +1582,7 @@ static struct tlmi_pwd_setting *tlmi_create_auth(const char *pwd_type,
> return new_pwd;
> }
>
> -static int tlmi_analyze(void)
> +static int tlmi_analyze(struct wmi_device *wdev)
> {
> int i, ret;
>
> @@ -1625,7 +1619,7 @@ static int tlmi_analyze(void)
> char *item = NULL;
>
> tlmi_priv.setting[i] = NULL;
> - ret = tlmi_setting(i, &item, LENOVO_BIOS_SETTING_GUID);
> + ret = tlmi_setting(wdev, i, &item);
> if (ret)
> break;
> if (!item)
> @@ -1648,6 +1642,7 @@ static int tlmi_analyze(void)
> kfree(item);
> goto fail_clear_attr;
> }
> + setting->wdev = wdev;
> setting->index = i;
> strscpy(setting->display_name, item);
> /* If BIOS selections supported, load those */
> @@ -1666,7 +1661,7 @@ static int tlmi_analyze(void)
> */
> char *optitem, *optstart, *optend;
>
> - if (!tlmi_setting(setting->index, &optitem, LENOVO_BIOS_SETTING_GUID)) {
> + if (!tlmi_setting(setting->wdev, setting->index, &optitem)) {
> optstart = strstr(optitem, "[Optional:");
> if (optstart) {
> optstart += strlen("[Optional:");
> @@ -1791,7 +1786,7 @@ static int tlmi_probe(struct wmi_device *wdev, const void *context)
> {
> int ret;
>
> - ret = tlmi_analyze();
> + ret = tlmi_analyze(wdev);
> if (ret)
> return ret;
>
> diff --git a/drivers/platform/x86/think-lmi.h b/drivers/platform/x86/think-lmi.h
> index f267d8b46957..a80452482227 100644
> --- a/drivers/platform/x86/think-lmi.h
> +++ b/drivers/platform/x86/think-lmi.h
> @@ -4,6 +4,7 @@
> #define _THINK_LMI_H_
>
> #include <linux/types.h>
> +#include <linux/wmi.h>
>
> #define TLMI_SETTINGS_COUNT 256
> #define TLMI_SETTINGS_MAXLEN 512
> @@ -87,6 +88,7 @@ struct tlmi_pwd_setting {
> /* Attribute setting details */
> struct tlmi_attr_setting {
> struct kobject kobj;
> + struct wmi_device *wdev;
> int index;
> char display_name[TLMI_SETTINGS_MAXLEN];
> char *possible_values;
> --
> 2.39.5
>
