From: Matthew Garrett <matthew.garrett@nebula.com>
To: "alex.hung@canonical.com" <alex.hung@canonical.com>
Cc: "platform-driver-x86@vger.kernel.org"
<platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH] hp-wireless: new driver for hp wireless button for Windows 8
Date: Wed, 15 Jan 2014 13:19:22 +0000 [thread overview]
Message-ID: <1389791962.3720.4.camel@x230> (raw)
In-Reply-To: <1389767381-10211-1-git-send-email-alex.hung@canonical.com>
On Wed, 2014-01-15 at 14:29 +0800, Alex Hung wrote:
This looks pretty good, but:
> +static void hpwl_notify(struct acpi_device *acpi_dev, u32 event)
> +{
> + const struct key_entry *key;
> +
> + if (event != 0x80) {
> + pr_info("Received unknown event (0x%x)\n", event);
> + return;
> + }
You're checking that it's 0x80, then using
sparse_keymap_entry_from_scancode() to effectively check whether it's
0x80, so this is redundant. But I suspect that this is actually wrong -
0x80 is often used as a generic notification event, with a separate
method for looking up the actual keycode. Do you have some ASL
describing the device?
Also, could you add a description of what the device does to the commit
so it shows up nicely in the changelog?
--
Matthew Garrett <matthew.garrett@nebula.com>
next prev parent reply other threads:[~2014-01-15 13:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-15 6:29 [PATCH] hp-wireless: new driver for hp wireless button for Windows 8 Alex Hung
2014-01-15 13:19 ` Matthew Garrett [this message]
2014-01-15 13:21 ` Matthew Garrett
2014-01-16 9:34 ` Alex Hung
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=1389791962.3720.4.camel@x230 \
--to=matthew.garrett@nebula.com \
--cc=alex.hung@canonical.com \
--cc=platform-driver-x86@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.