linux-kernel - Re: [PATCH v3 5/5] dell-wmi: support Dell Inspiron M5110

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160217120848.GT1476@pali>
Date:	Wed, 17 Feb 2016 13:08:48 +0100
From:	Pali Rohár <pali.rohar@...il.com>
To:	Michał Kępień <kernel@...pniu.pl>
Cc:	Matthew Garrett <mjg59@...f.ucam.org>,
	Darren Hart <dvhart@...radead.org>,
	Darek Stojaczyk <darek.stojaczyk@...il.com>,
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 5/5] dell-wmi: support Dell Inspiron M5110

On Wednesday 17 February 2016 13:01:58 Michał Kępień wrote:
> > > > >  	/* Shortcut and audio panel keys */
> > > > >  	{ KE_KEY, 0xe025, { KEY_PROG4 } },
> > > > >  	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
> > > > > +	{ KE_KEY, 0xe029, { KEY_PROG4 } },
> > > > 
> > > > Hi! Above comment (Shortcut and audio panel keys) is not relevant for
> > > > this 0xe029, right?
> > > 
> > > I believe it is relevant.  The commit which added that comment
> > > (f1566f0d) was made by Seth Forshee to support Dell XPS L502X.  The term
> > > "shortcut key" is used in the bug report linked from that commit [1],
> > > but the manual for that model [2] describes the hotkey in question as
> > > "Instant launch control" (page 43), so I believe both Seth's commit and
> > > my patch refer to the same thing.
> > > 
> > > [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/815914/comments/6
> > > [2] http://downloads.dell.com/Manuals/all-products/esuprt_laptop/esuprt_xps_laptop/xps-l502x_setup%20guide_en-us.pdf
> > 
> > Then please split above group of keys and add comment which key is
> > shortcut and which audio panel. Because now there will be three keys and
> > comment just for two -- in future will not know which code 0x... is
> > mapped to which key.
> 
> Please note that except for the first 5 entries, the keymap is sorted by
> event code in ascending order.

I have prepared some patches which sort all event codes plus adds
missing comments... So after all dell patches are in Darren tree, I will
rewrite/rebase my and can send them.

> If I understand correctly, doing what you ask would break that ordering:
> 
>     /* Dell Instant Launch key */
>     { KE_KEY, 0xe025, { KEY_PROG4 } },
>     { KE_KEY, 0xe029, { KEY_PROG4 } },
> 
>     /* Audio panel key */
>     { KE_IGNORE, 0xe026, { KEY_RESERVED } },
> 
> Is this acceptable?  Or perhaps you had something else on your mind?

Thats better, now we know which event is audio and which launch key.

-- 
Pali Rohár
pali.rohar@...il.com