From: ayman.bagabas@gmail.com
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org, Hui Wang <hui.wang@canonical.com>,
Andy Shevchenko <andy@infradead.org>,
Darren Hart <dvhart@infradead.org>,
Jaroslav Kysela <perex@perex.cz>,
Kailang Yang <kailang@realtek.com>,
linux-kernel@vger.kernel.org,
platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.
Date: Tue, 27 Nov 2018 19:58:24 -0500 [thread overview]
Message-ID: <30a2b3bea95199ffc1a81f374deabbfadaeb2c22.camel@gmail.com> (raw)
In-Reply-To: <s5h5zwi7t1s.wl-tiwai@suse.de>
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 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.
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
next prev parent reply other threads:[~2018-11-28 0:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-27 2:57 [PATCH v7 0/3] Huawei laptops Ayman Bagabas
2018-11-27 2:57 ` [PATCH v7 1/3] ALSA: hda: fix front speakers on Huawei MBXP Ayman Bagabas
2018-11-27 2:57 ` [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys Ayman Bagabas
2018-11-27 9:45 ` Takashi Iwai
2018-11-28 0:58 ` ayman.bagabas [this message]
2018-11-27 11:02 ` Takashi Iwai
2018-11-27 15:52 ` Andy Shevchenko
2018-11-28 0:49 ` ayman.bagabas
2018-11-28 8:44 ` Takashi Iwai
2018-11-27 2:57 ` [PATCH v7 3/3] ALSA: hda: add support for Huawei WMI micmute LED Ayman Bagabas
[not found] <20181126234624.31264-1-ayman.bagabas@gmail.com>
2018-11-26 23:46 ` [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys Ayman Bagabas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=30a2b3bea95199ffc1a81f374deabbfadaeb2c22.camel@gmail.com \
--to=ayman.bagabas@gmail.com \
--cc=alsa-devel@alsa-project.org \
--cc=andy@infradead.org \
--cc=dvhart@infradead.org \
--cc=hui.wang@canonical.com \
--cc=kailang@realtek.com \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=platform-driver-x86@vger.kernel.org \
--cc=tiwai@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).