On Thu, Dec 17, 2015 at 02:50:39PM -0800, Andy Lutomirski wrote:
> On Thu, Dec 17, 2015 at 2:15 PM, Darren Hart <dvhart@xxxxxxxxxxxxx> wrote:
> > On Thu, Dec 17, 2015 at 03:30:02PM +0800, Alex Hung wrote:
> >> This driver supports various hid events including hotkeys.
> >> Dell XPS 13 9350 requires it for wireless hotkey.
> >>
> >> Andy Lutomirski contributed greatly to this driver.
> >>
> >> Signed-off-by: Alex Hung <alex.hung@xxxxxxxxxxxxx>
> >
> > Queued for testing.
> >
> > Andy, please provide your Reviewed-by if you're happy with it.
>
> Reviewed-and-tested-by: Andy Lutomirski <luto@xxxxxxxxxx>
>
> Minor nits, though:
>
> + This driver provides supports for Intel HID event. Some laptops
> + require this driver for hotkey supports.
>
> Should that be "This driver provides support for the Intel HID Event
> hotkey interface. Some laptops require this driver for hotkey
> support."?
>
> Also, can we remove the word "Filter" a couple lines up in the Kconfig
> file? I'd guess the the Windows equivalent is a "filter' driver in
> the Windows model, but it's not logically filtering anything, and
> there's nothing filter-like about the Linux driver.
>
> (I think it's slightly sad that HID is in the name, too, but that's
> indeed what it seems to be called.)
These are minor, but I had similar thoughts. I'd be happy to see these included
and respun.
>
> >
> > Alex, just one question maybe you can answer quickly and save me some time
> > below:
> >
> >
> >> +static int intel_hid_pl_suspend_handler(struct device *device)
> >> +{
> >> + intel_hid_set_enable(device, 0);
> >> + return 0;
> >> +}
> >> +
> >> +static int intel_hid_pl_resume_handler(struct device *device)
> >> +{
> >> + intel_hid_set_enable(device, 1);
> >> + return 0;
> >> +}
> >
> > Why not propagate the intel_hid_set_enable() return code? Is it because it just
> > doesn't really impact suspend/resume, even if we do get an error?
>
> That was me, actually. I definitely don't want to cause suspect to
> fail if the ACPI call fails. At worst we'll report a spurious key
> event when we resume.
>
OK, thanks for the clarification (reminder :-)
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
