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
