Hi Daniel,
one more actual thing and two tiny nitpicks.
On 2021-09-02T16:47-0500, Daniel Dadap wrote:
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index d12db6c316ea..0df908ef8d7c 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -113,6 +113,22 @@ config PEAQ_WMI
> help
> Say Y here if you want to support WMI-based hotkeys on PEAQ 2-in-1s.
>
> +config WMAA_BACKLIGHT_WMI
> + tristate "ACPI WMAA Backlight Driver"
> + depends on ACPI_WMI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + This driver provides a sysfs backlight interface for notebook
> + systems which expose the WMAA ACPI method and an associated WMI
> + wrapper to drive LCD backlight levels through the system's
> + Embedded Controller (EC).
system's -> systems
> +
> + Say Y or M here if you want to control the backlight on a notebook
> + system with an EC-driven backlight using the ACPI WMAA method.
> +
> + If you choose to compile this driver as a module the module will be
> + called wmaa-backlight-wmi.
> +
> config XIAOMI_WMI
> tristate "Xiaomi WMI key driver"
> depends on ACPI_WMI
> diff --git a/drivers/platform/x86/wmaa-backlight-wmi.c b/drivers/platform/x86/wmaa-backlight-wmi.c
> new file mode 100644
> index 000000000000..fa3f41302299
> --- /dev/null
> +++ b/drivers/platform/x86/wmaa-backlight-wmi.c
> @@ -0,0 +1,194 @@
> [..]
> +static int wmaa_backlight_update_status(struct backlight_device *bd)
> +{
> + struct wmi_device *wdev = bl_get_data(bd);
> +
> + return wmi_call_wmaa(wdev, WMAA_METHOD_LEVEL, WMAA_MODE_SET,
> + &bd->props.brightness);
> +}
> +
> +static int wmaa_backlight_get_brightness(struct backlight_device *bd)
> +{
> + struct wmi_device *wdev = bl_get_data(bd);
> + u32 level;
> + int ret;
> +
> + ret = wmi_call_wmaa(wdev, WMAA_METHOD_LEVEL, WMAA_MODE_GET, &level);
> + if (ret > 0)
> + return -EINVAL;
> + else if (ret < 0)
> + return ret;
Nowhere else is the return value of wmi_call_wmaa() checked explicitly for
greater than zero and less than zero. Why here?
At least wmaa_backlight_update_status() should have identical semantics.
The case ret > 0 should never happen.
> +
> + return level;
> +}
> [..]
> +
> +
> +#define WMAA_WMI_GUID "603E9613-EF25-4338-A3D0-C46177516DB7"
> +
> +static const struct wmi_device_id wmaa_backlight_wmi_id_table[] = {
> + { .guid_string = WMAA_WMI_GUID },
This could also be inlined as:
{ "603E9613-EF25-4338-A3D0-C46177516DB7" },
More a stylistic thing though.
Thomas
