On Tue, 2018-11-27 at 10:45 +0100, Takashi Iwai wrote:
On Tue, 27 Nov 2018 03:57:48 +0100, Ayman Bagabas wrote:
+static const struct key_entry huawei_wmi_keymap[] __initconst = {
{ KE_KEY, 0x281, { KEY_BRIGHTNESSDOWN } },{ KE_KEY, 0x282, { KEY_BRIGHTNESSUP } },{ KE_KEY, 0x284, { KEY_MUTE } },{ KE_KEY, 0x285, { KEY_VOLUMEDOWN } },{ KE_KEY, 0x286, { KEY_VOLUMEUP } },{ KE_KEY, 0x287, { KEY_MICMUTE } },{ KE_KEY, 0x289, { KEY_WLAN } },// Huawei |M| button{ KE_KEY, 0x28a, { KEY_CONFIG } },// Keyboard light{ KE_IGNORE, 0x293, { KEY_KBDILLUMTOGGLE } },{ KE_IGNORE, 0x294, { KEY_KBDILLUMUP } },{ KE_IGNORE, 0x295, { KEY_KBDILLUMUP } },{ KE_END, 0 }The indentation looks too deep here.
+static int huawei_wmi_micmute_led_set(bool on) +{
- acpi_handle handle;
- char *method;
- union acpi_object args[3];
- struct acpi_object_list arg_list = {
.pointer = args,.count = ARRAY_SIZE(args),- };
- handle = ACPI_HANDLE(&inputdev->dev);
Just wondering whether the ACPI handle is assigned properly for this device...
It is, but it returns a handle at "".
- args[0].type = args[1].type = args[2].type = ACPI_TYPE_INTEGER;
- args[1].integer.value = 0x04;
- if (acpi_has_method(handle, method =
"\_SB.PCI0.LPCB.EC0.SPIN")) {
args[0].integer.value = 0;args[2].integer.value = on ? 1 : 0;- } else if (acpi_has_method(handle, method =
"\_SB.PCI0.LPCB.EC0.WPIN")) {
args[0].integer.value = 1;args[2].integer.value = on ? 0 : 1;- } else {
dev_err(&inputdev->dev, "Unable to find ACPImethod\n");
return -ENOSYS;- }
- acpi_evaluate_object(handle, method, &arg_list, NULL);
- return 0;
+}
+static int micmute_led_set(struct led_classdev *led_cdev,
enum led_brightness brightness)+{
- int state = brightness != LED_OFF;
- int err;
- err = huawei_wmi_micmute_led_set(state);
- return err < 0 ? err : 0;
No need for checking err here, you can return huawei_wmi_mute_led_set() directly.
Or even you can fold two functions into one.
I think folding them make more sense.
+static int __init huawei_wmi_init(void) +{
- int err;
- if (wmi_has_guid(MBX_EVENT_GUID)) {
event_guid = MBX_EVENT_GUID;- } else if (wmi_has_guid(MBXP_EVENT_GUID)) {
event_guid = MBXP_EVENT_GUID;- } else {
pr_warn("Compatible WMI GUID not found\n");return -ENODEV;- }
- err = huawei_wmi_input_init();
- if (err)
goto err_input;- micmute_led_cdev.brightness =
ledtrig_audio_get(LED_AUDIO_MICMUTE);
- err = led_classdev_register(&inputdev->dev, &micmute_led_cdev);
- if (err)
goto err_leds;- return 0;
Might it be cleaner to implement on top of wmi_driver? Then you can create both input and led devices on wmi device.
Done! Now led devices get registered when wmi probes a new device. Thanks.
thanks,
Takashi