Re: [PATCH v2 2/2] HID: asus: only support backlight when it's not driven by WMI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jun 2, 2018 at 8:08 AM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Sat, Jun 2, 2018 at 12:28 AM, Daniel Drake <drake@xxxxxxxxxxxx> wrote:
>> On Thu, May 31, 2018 at 4:43 AM, Andy Shevchenko
>> <andy.shevchenko@xxxxxxxxx> wrote:
>>>> +       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.
>>
>> What do you see as wrong with it? The driver already depends on e.g.
>> LEDS_CLASS for the functionality that it sometimes uses there.
>>
>> In my opinion the ideal solution to the corner cases you mention is
>> the Kconfig "depends on" which is exactly what the patch is
>> proposing...
>
> depends on in this case adds less generic dependency to the more generic module.
> Thus, we imply that we have more users when dependency is satisfied
> than otherwise.
> Which is simply not true.
>
> Thus, you add a bulk dependency to unsuspecting users w/o any need.

I'm not familiar with this argument, but I'm happy to work on a
solution along these lines.

Upon initial attempt I can't find a clean approach that avoids the
corner cases that you mentioned though.

Specifically in the case of HID_ASUS=y and ASUS_WMI=m, we cannot try
to branch out to the newly exposed asus-wmi functionality since we
cannot have the kernel binary depend on a module.

Are you envisioning something like this code in hid-asus?

static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
{
#if defined(CONFIG_HID_ASUS) && defined(CONFIG_ASUS_WMI_MODULE)
    return 0;
#elif defined(CONFIG_ASUS_WMI) || defined(CONFIG_ASUS_WMI_MODULE)
    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);
#endif
}

Thanks
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux