On Thu, May 31, 2018 at 12:43 PM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Tue, May 22, 2018 at 11:55 PM, Daniel Drake <drake@xxxxxxxxxxxx> wrote:
>> The Asus GL502VSK has the same 0B05:1837 keyboard as we've seen in
>> several Republic of Gamers laptops.
>>
>> However, in this model, the keybard backlight control exposed by hid-asus
>> has no effect on the keyboard backlight. Instead, the keyboard
>> backlight is correctly driven by asus-wmi.
>>
>> With two keyboard backlight devices available (and only the acer-wmi
>> one working), GNOME is picking the wrong one to drive in the UI.
>>
>> Avoid this problem by not creating the backlight interface when we
>> detect a WMI-driven keyboard backlight.
>>
>> We have also tested Asus GL702VMK which does have the hid-asus
>> backlight present, and it still works fine with this patch (WMI method
>> call returns UNSUPPORTED_METHOD).
>>
>> Signed-off-by: Daniel Drake <drake@xxxxxxxxxxxx>
>> ---
>> drivers/hid/Kconfig | 1 +
>> drivers/hid/hid-asus.c | 24 +++++++++++++++++++++++-
>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 0000434a1fbd..3c0d461bb830 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -149,6 +149,7 @@ config HID_APPLEIR
>> config HID_ASUS
>> tristate "Asus"
>> depends on LEDS_CLASS
>
>> + depends on ASUS_WMI
>
> I'm not sure it's a good idea.
>
> Rather you may use ifdef inside header which also provides a stub for
> the function. But be careful with corner cases, like ASUS_WMI=m &&
> HID_ASUS=y.
I wonder if the report descriptors are not enough to provide the
information that the keyboard backlights are driven by wmi instead of
plain HID (or if there is any register that tells us that the
backlights are not handled by HID).
If not, then I have nothing against the approach.
Acked-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
Cheers,
Benjamin
>
>> ---help---
>> Support for Asus notebook built-in keyboard and touchpad via i2c, and
>> the Asus Republic of Gamers laptop keyboard special keys.
>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>> index 88a5672f42cd..c42af8511b38 100644
>> --- a/drivers/hid/hid-asus.c
>> +++ b/drivers/hid/hid-asus.c
>> @@ -26,6 +26,7 @@
>> * any later version.
>> */
>>
>> +#include <linux/asus-wmi.h>
>> #include <linux/dmi.h>
>> #include <linux/hid.h>
>> #include <linux/module.h>
>> @@ -349,6 +350,25 @@ static void asus_kbd_backlight_work(struct work_struct *work)
>> hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
>> }
>>
>> +/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
>> + * precedence. We only activate HID-based backlight control when the
>> + * WMI control is not available.
>> + */
>> +static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
>> +{
>> + u32 value;
>> + int ret;
>> +
>> + ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS2,
>> + ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
>> + hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
>> +
>> + if (ret != 0)
>> + return false;
>> +
>> + return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
>> +}
>> +
>> static int asus_kbd_register_leds(struct hid_device *hdev)
>> {
>> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>> @@ -436,7 +456,9 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>>
>> drvdata->input = input;
>>
>> - if (drvdata->enable_backlight && asus_kbd_register_leds(hdev))
>> + if (drvdata->enable_backlight &&
>> + !asus_kbd_wmi_led_control_present(hdev) &&
>> + asus_kbd_register_leds(hdev))
>> hid_warn(hdev, "Failed to initialize backlight.\n");
>>
>> return 0;
>> --
>> 2.17.0
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
