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...
> + 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 ACPI method\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.
> +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.
thanks,
Takashi
