All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
@ 2016-07-22 20:01 Mario Limonciello
  2016-07-27  5:19 ` Darren Hart
  0 siblings, 1 reply; 20+ messages in thread
From: Mario Limonciello @ 2016-07-22 20:01 UTC (permalink / raw)
  To: dvhart; +Cc: LKML, platform-driver-x86, Mario Limonciello

The Dell Rugged 7202 has 3 programmable buttons (labeled P1, P2, P3)
and a detachable magnetic keyboard/mouse.

Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
 drivers/platform/x86/dell-wmi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 15c6f11..6ba42d0 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -222,6 +222,16 @@ static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
 
 	/* Stealth mode toggle */
 	{ KE_IGNORE, 0x155, { KEY_RESERVED } },
+
+	/* Rugged magnetic keyboard/mouse events */
+	{ KE_IGNORE, 0x156, { KEY_RESERVED } },
+	{ KE_IGNORE, 0x157, { KEY_RESERVED } },
+
+	/* Rugged programmable (P1/P2/P3 keys) */
+	{ KE_KEY, 0x850, { KEY_PROG1 } },
+	{ KE_KEY, 0x851, { KEY_PROG2 } },
+	{ KE_KEY, 0x852, { KEY_PROG3 } },
+
 };
 
 static struct input_dev *dell_wmi_input_dev;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-22 20:01 [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s Mario Limonciello
@ 2016-07-27  5:19 ` Darren Hart
  2016-07-27  7:23   ` Pali Rohár
  0 siblings, 1 reply; 20+ messages in thread
From: Darren Hart @ 2016-07-27  5:19 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: LKML, platform-driver-x86, Pali Rohár

On Fri, Jul 22, 2016 at 03:01:24PM -0500, Mario Limonciello wrote:
> The Dell Rugged 7202 has 3 programmable buttons (labeled P1, P2, P3)
> and a detachable magnetic keyboard/mouse.
> 
> Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>

+Pali

Pali, any concerns?

> ---
>  drivers/platform/x86/dell-wmi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 15c6f11..6ba42d0 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -222,6 +222,16 @@ static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
>  
>  	/* Stealth mode toggle */
>  	{ KE_IGNORE, 0x155, { KEY_RESERVED } },
> +
> +	/* Rugged magnetic keyboard/mouse events */
> +	{ KE_IGNORE, 0x156, { KEY_RESERVED } },
> +	{ KE_IGNORE, 0x157, { KEY_RESERVED } },
> +
> +	/* Rugged programmable (P1/P2/P3 keys) */
> +	{ KE_KEY, 0x850, { KEY_PROG1 } },
> +	{ KE_KEY, 0x851, { KEY_PROG2 } },
> +	{ KE_KEY, 0x852, { KEY_PROG3 } },
> +
>  };
>  
>  static struct input_dev *dell_wmi_input_dev;
> -- 
> 2.7.4
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27  5:19 ` Darren Hart
@ 2016-07-27  7:23   ` Pali Rohár
  2016-07-27 15:55       ` Mario_Limonciello
  0 siblings, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2016-07-27  7:23 UTC (permalink / raw)
  To: Darren Hart; +Cc: Mario Limonciello, LKML, platform-driver-x86

On Tuesday 26 July 2016 22:19:09 Darren Hart wrote:
> On Fri, Jul 22, 2016 at 03:01:24PM -0500, Mario Limonciello wrote:
> > The Dell Rugged 7202 has 3 programmable buttons (labeled P1, P2, P3)
> > and a detachable magnetic keyboard/mouse.
> > 
> > Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> 
> +Pali
> 
> Pali, any concerns?

Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we already use
those for other actions (see bios_to_linux_keycode). Also there is:

/* Wifi Catcher */
{ KE_KEY,    0xe011, { KEY_PROG2 } },

But probably we do not have better names...

Another small cosmetic change: align WMI codes, so 0x157 and 0x850 are
at some column (similar like are formatted other actions).

> > ---
> >  drivers/platform/x86/dell-wmi.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> > index 15c6f11..6ba42d0 100644
> > --- a/drivers/platform/x86/dell-wmi.c
> > +++ b/drivers/platform/x86/dell-wmi.c
> > @@ -222,6 +222,16 @@ static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
> >  
> >  	/* Stealth mode toggle */
> >  	{ KE_IGNORE, 0x155, { KEY_RESERVED } },
> > +
> > +	/* Rugged magnetic keyboard/mouse events */
> > +	{ KE_IGNORE, 0x156, { KEY_RESERVED } },
> > +	{ KE_IGNORE, 0x157, { KEY_RESERVED } },
> > +
> > +	/* Rugged programmable (P1/P2/P3 keys) */
> > +	{ KE_KEY, 0x850, { KEY_PROG1 } },
> > +	{ KE_KEY, 0x851, { KEY_PROG2 } },
> > +	{ KE_KEY, 0x852, { KEY_PROG3 } },
> > +
> >  };
> >  
> >  static struct input_dev *dell_wmi_input_dev;
> > -- 
> > 2.7.4
> > 
> > 
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27  7:23   ` Pali Rohár
@ 2016-07-27 15:55       ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-27 15:55 UTC (permalink / raw)
  To: pali.rohar, dvhart; +Cc: linux-kernel, platform-driver-x86

> Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we already use
>those for other actions (see bios_to_linux_keycode). Also there is:

I had missed this, do you have some recommendations on what would be
better codes to map this to?

I'll double check what the things that "were" mapping to KEY_PROG1 etc
actually were.  This might be a case of an expected clash if the functions
aren't in current generations.

>
>/* Wifi Catcher */
> { KE_KEY,    0xe011, { KEY_PROG2 } },
>

It's worth mentioning that Wifi Catcher hasn't been used for any Dell laptops
for a handful generations.  The rugged 2in1's are current generation that have 
these programmable buttons and don't have wifi catcher.

So there should be no "real" clash here.
   
> But probably we do not have better names...

In this particular case, I was thinking PROG1/2/3 were really the best option
and would be most intuitive because the keys really are labels "P1" "P2" and
"P3".

Here's the front of the tablet:
http://shop-media.intel.com/api/v2/helperservice/getimage?url=http://images.icecat.biz/img/norm/high/30031725-706.jpg&height=550&width=550

>
> Another small cosmetic change: align WMI codes, so 0x157 and 0x850 are
> at some column (similar like are formatted other actions).

OK.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
@ 2016-07-27 15:55       ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-27 15:55 UTC (permalink / raw)
  To: pali.rohar, dvhart; +Cc: linux-kernel, platform-driver-x86

> Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we already use
>those for other actions (see bios_to_linux_keycode). Also there is:

I had missed this, do you have some recommendations on what would be
better codes to map this to?

I'll double check what the things that "were" mapping to KEY_PROG1 etc
actually were.  This might be a case of an expected clash if the functions
aren't in current generations.

>
>/* Wifi Catcher */
> { KE_KEY,    0xe011, { KEY_PROG2 } },
>

It's worth mentioning that Wifi Catcher hasn't been used for any Dell laptops
for a handful generations.  The rugged 2in1's are current generation that have 
these programmable buttons and don't have wifi catcher.

So there should be no "real" clash here.
   
> But probably we do not have better names...

In this particular case, I was thinking PROG1/2/3 were really the best option
and would be most intuitive because the keys really are labels "P1" "P2" and
"P3".

Here's the front of the tablet:
http://shop-media.intel.com/api/v2/helperservice/getimage?url=http://images.icecat.biz/img/norm/high/30031725-706.jpg&height=550&width=550

>
> Another small cosmetic change: align WMI codes, so 0x157 and 0x850 are
> at some column (similar like are formatted other actions).

OK.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 15:55       ` Mario_Limonciello
  (?)
@ 2016-07-27 16:05       ` Pali Rohár
  2016-07-27 16:38         ` Darren Hart
  2016-07-27 19:03           ` Mario_Limonciello
  -1 siblings, 2 replies; 20+ messages in thread
From: Pali Rohár @ 2016-07-27 16:05 UTC (permalink / raw)
  To: Mario_Limonciello; +Cc: dvhart, linux-kernel, platform-driver-x86

[-- Attachment #1: Type: Text/Plain, Size: 2115 bytes --]

On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com wrote:
> > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we already
> > use
> >
> >those for other actions (see bios_to_linux_keycode). Also there is:
> I had missed this, do you have some recommendations on what would be
> better codes to map this to?

Problem is that I do not know when those KEY_PROG keys from 
bios_to_linux_keycode table are emitted. There are missing comments or 
description.

Are you able to find out description for all those keys in that table? 
Maybe now (when linux key constants are extended), there could be better 
candidates...

> I'll double check what the things that "were" mapping to KEY_PROG1
> etc actually were.  This might be a case of an expected clash if the
> functions aren't in current generations.
> 
> >/* Wifi Catcher */
> >
> > { KE_KEY,    0xe011, { KEY_PROG2 } },
> 
> It's worth mentioning that Wifi Catcher hasn't been used for any Dell
> laptops for a handful generations.  The rugged 2in1's are current
> generation that have these programmable buttons and don't have wifi
> catcher.

Anyway, what is "Wifi Catcher"? HW switch buttton which enable/disable 
wifi? Or something else?

> So there should be no "real" clash here.

Problem can be in future. This driver is unified for all Dell products 
with wmi interface and so future product could do some nasty things...

> > But probably we do not have better names...
> 
> In this particular case, I was thinking PROG1/2/3 were really the
> best option and would be most intuitive because the keys really are
> labels "P1" "P2" and "P3".

Probably yes, as I wrote I do not have in my mind better names for now.

> Here's the front of the tablet:
> http://shop-media.intel.com/api/v2/helperservice/getimage?url=http://
> images.icecat.biz/img/norm/high/30031725-706.jpg&height=550&width=550
> 
> > Another small cosmetic change: align WMI codes, so 0x157 and 0x850
> > are at some column (similar like are formatted other actions).
> 
> OK.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 16:05       ` Pali Rohár
@ 2016-07-27 16:38         ` Darren Hart
  2016-07-27 19:03           ` Mario_Limonciello
  1 sibling, 0 replies; 20+ messages in thread
From: Darren Hart @ 2016-07-27 16:38 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Mario_Limonciello, linux-kernel, platform-driver-x86

On Wed, Jul 27, 2016 at 06:05:57PM +0200, Pali Rohár wrote:
> On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com wrote:
> > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we already
> > > use
> > >
> > >those for other actions (see bios_to_linux_keycode). Also there is:
> > I had missed this, do you have some recommendations on what would be
> > better codes to map this to?
> 
> Problem is that I do not know when those KEY_PROG keys from 
> bios_to_linux_keycode table are emitted. There are missing comments or 
> description.
> 
> Are you able to find out description for all those keys in that table? 
> Maybe now (when linux key constants are extended), there could be better 
> candidates...
> 
> > I'll double check what the things that "were" mapping to KEY_PROG1
> > etc actually were.  This might be a case of an expected clash if the
> > functions aren't in current generations.
> > 
> > >/* Wifi Catcher */
> > >
> > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> > 
> > It's worth mentioning that Wifi Catcher hasn't been used for any Dell
> > laptops for a handful generations.  The rugged 2in1's are current
> > generation that have these programmable buttons and don't have wifi
> > catcher.
> 
> Anyway, what is "Wifi Catcher"? HW switch buttton which enable/disable 
> wifi? Or something else?
> 
> > So there should be no "real" clash here.
> 
> Problem can be in future. This driver is unified for all Dell products 
> with wmi interface and so future product could do some nasty things...

I suggest not worrying too much about unknown future platforms when it comes to
things like key names, we cannot predict this, and can address it when/if it
becomes a problem.



-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 16:05       ` Pali Rohár
@ 2016-07-27 19:03           ` Mario_Limonciello
  2016-07-27 19:03           ` Mario_Limonciello
  1 sibling, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-27 19:03 UTC (permalink / raw)
  To: pali.rohar; +Cc: dvhart, linux-kernel, platform-driver-x86

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Wednesday, July 27, 2016 11:06 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org
> Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
> 
> On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com wrote:
> > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we already
> > > use
> > >
> > >those for other actions (see bios_to_linux_keycode). Also there is:
> > I had missed this, do you have some recommendations on what would be
> > better codes to map this to?
> 
> Problem is that I do not know when those KEY_PROG keys from
> bios_to_linux_keycode table are emitted. There are missing comments or
> description.
> 
> Are you able to find out description for all those keys in that table?
> Maybe now (when linux key constants are extended), there could be better
> candidates...

I'll ask around.  It's not immediately obvious to me though, maybe from old
specs?

> 
> > I'll double check what the things that "were" mapping to KEY_PROG1 etc
> > actually were.  This might be a case of an expected clash if the
> > functions aren't in current generations.
> >
> > >/* Wifi Catcher */
> > >
> > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> >
> > It's worth mentioning that Wifi Catcher hasn't been used for any Dell
> > laptops for a handful generations.  The rugged 2in1's are current
> > generation that have these programmable buttons and don't have wifi
> > catcher.
> 
> Anyway, what is "Wifi Catcher"? HW switch buttton which enable/disable wifi?
> Or something else?
>

Wifi catcher was a special hardware switch slider switch.  When the machine
was turned in S3 the sliding the switch beyond the on position would scan for 
available wifi networks and light up an LED if some from your predefined list
were found.

When the machine was on, it would open up the applet that let you configure
the behavior for the switch in S3.

> > So there should be no "real" clash here.
> 
> Problem can be in future. This driver is unified for all Dell products with wmi
> interface and so future product could do some nasty things...

I agree with Darren here.  

At least on Dell side we're creating new codes for "new" buttons and the
limitation is really on the kernel side for how many KEY_PROG# or similar
functions they can be re-assigned to.

Maybe it's time to think of another way to get this information to userspace
rather that always translating them into key codes?

There's a lot that are marked as KE_IGNORE because the kernel can't do
anything interesting with them as no 1-1 mapping exists to a keycode.

Those could still be passed on somehow to have things like gnome-settings-daemon
or other userspace tools to react however and show a notification.

> 
> > > But probably we do not have better names...
> >
> > In this particular case, I was thinking PROG1/2/3 were really the best
> > option and would be most intuitive because the keys really are labels
> > "P1" "P2" and "P3".
> 
> Probably yes, as I wrote I do not have in my mind better names for now.
> 

OK, I'll resend with the cosmetic tabbing change and a clearer description,
but keep the same mapping.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
@ 2016-07-27 19:03           ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-27 19:03 UTC (permalink / raw)
  To: pali.rohar; +Cc: dvhart, linux-kernel, platform-driver-x86

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Wednesday, July 27, 2016 11:06 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org
> Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
> 
> On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com wrote:
> > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we already
> > > use
> > >
> > >those for other actions (see bios_to_linux_keycode). Also there is:
> > I had missed this, do you have some recommendations on what would be
> > better codes to map this to?
> 
> Problem is that I do not know when those KEY_PROG keys from
> bios_to_linux_keycode table are emitted. There are missing comments or
> description.
> 
> Are you able to find out description for all those keys in that table?
> Maybe now (when linux key constants are extended), there could be better
> candidates...

I'll ask around.  It's not immediately obvious to me though, maybe from old
specs?

> 
> > I'll double check what the things that "were" mapping to KEY_PROG1 etc
> > actually were.  This might be a case of an expected clash if the
> > functions aren't in current generations.
> >
> > >/* Wifi Catcher */
> > >
> > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> >
> > It's worth mentioning that Wifi Catcher hasn't been used for any Dell
> > laptops for a handful generations.  The rugged 2in1's are current
> > generation that have these programmable buttons and don't have wifi
> > catcher.
> 
> Anyway, what is "Wifi Catcher"? HW switch buttton which enable/disable wifi?
> Or something else?
>

Wifi catcher was a special hardware switch slider switch.  When the machine
was turned in S3 the sliding the switch beyond the on position would scan for 
available wifi networks and light up an LED if some from your predefined list
were found.

When the machine was on, it would open up the applet that let you configure
the behavior for the switch in S3.

> > So there should be no "real" clash here.
> 
> Problem can be in future. This driver is unified for all Dell products with wmi
> interface and so future product could do some nasty things...

I agree with Darren here.  

At least on Dell side we're creating new codes for "new" buttons and the
limitation is really on the kernel side for how many KEY_PROG# or similar
functions they can be re-assigned to.

Maybe it's time to think of another way to get this information to userspace
rather that always translating them into key codes?

There's a lot that are marked as KE_IGNORE because the kernel can't do
anything interesting with them as no 1-1 mapping exists to a keycode.

Those could still be passed on somehow to have things like gnome-settings-daemon
or other userspace tools to react however and show a notification.

> 
> > > But probably we do not have better names...
> >
> > In this particular case, I was thinking PROG1/2/3 were really the best
> > option and would be most intuitive because the keys really are labels
> > "P1" "P2" and "P3".
> 
> Probably yes, as I wrote I do not have in my mind better names for now.
> 

OK, I'll resend with the cosmetic tabbing change and a clearer description,
but keep the same mapping.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 19:03           ` Mario_Limonciello
  (?)
@ 2016-07-27 20:15           ` Pali Rohár
  2016-07-27 22:43             ` Darren Hart
  2016-07-28 15:36               ` Mario_Limonciello
  -1 siblings, 2 replies; 20+ messages in thread
From: Pali Rohár @ 2016-07-27 20:15 UTC (permalink / raw)
  To: Mario_Limonciello; +Cc: dvhart, linux-kernel, platform-driver-x86

[-- Attachment #1: Type: Text/Plain, Size: 5199 bytes --]

On Wednesday 27 July 2016 21:03:57 Mario_Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > Sent: Wednesday, July 27, 2016 11:06 AM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org;
> > platform-driver- x86@vger.kernel.org
> > Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged
> > 2-in-1s
> > 
> > On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com wrote:
> > > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we
> > > > already use
> > > >
> > > >those for other actions (see bios_to_linux_keycode). Also there
> > > >is:
> > > I had missed this, do you have some recommendations on what would
> > > be better codes to map this to?
> > 
> > Problem is that I do not know when those KEY_PROG keys from
> > bios_to_linux_keycode table are emitted. There are missing comments
> > or description.
> > 
> > Are you able to find out description for all those keys in that
> > table? Maybe now (when linux key constants are extended), there
> > could be better candidates...
> 
> I'll ask around.  It's not immediately obvious to me though, maybe
> from old specs?

Do not know if it is old. At least some codes from it are used on my 
E6440 machine. Table itself was added by Rezwanul Kabir in this commit:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5ea2559726b786283236835dc2905c23b36ac91c

Maybe commit message could help you to indentify what it is...

> > > I'll double check what the things that "were" mapping to
> > > KEY_PROG1 etc actually were.  This might be a case of an
> > > expected clash if the functions aren't in current generations.
> > > 
> > > >/* Wifi Catcher */
> > > >
> > > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> > > 
> > > It's worth mentioning that Wifi Catcher hasn't been used for any
> > > Dell laptops for a handful generations.  The rugged 2in1's are
> > > current generation that have these programmable buttons and
> > > don't have wifi catcher.
> > 
> > Anyway, what is "Wifi Catcher"? HW switch buttton which
> > enable/disable wifi? Or something else?
> 
> Wifi catcher was a special hardware switch slider switch.  When the
> machine was turned in S3 the sliding the switch beyond the on
> position would scan for available wifi networks and light up an LED
> if some from your predefined list were found.
> 
> When the machine was on, it would open up the applet that let you
> configure the behavior for the switch in S3.

Hm... maybe it better fit KEY_WLAN then? Just speculation.

> > > So there should be no "real" clash here.
> > 
> > Problem can be in future. This driver is unified for all Dell
> > products with wmi interface and so future product could do some
> > nasty things...
> 
> I agree with Darren here.
> 
> At least on Dell side we're creating new codes for "new" buttons and
> the limitation is really on the kernel side for how many KEY_PROG#
> or similar functions they can be re-assigned to.

Ok.

> Maybe it's time to think of another way to get this information to
> userspace rather that always translating them into key codes?

If event is generated by pressing key, button or hw switch, then input 
key is correct way. If there is no KEY define which fit for new vendor 
hw button, then I think we should start discussion with input subsystem 
how to handle such situation.

> There's a lot that are marked as KE_IGNORE because the kernel can't
> do anything interesting with them as no 1-1 mapping exists to a
> keycode.

Most marked as KE_IGNORE are because duplicate event is sent via 
keyboard controller or via other subsystem (e.g. rfkill).

But events which are not keys or buttons should not be sent via input 
devices. At least I think it is against usage of input devices.

Events like "battery was removed" or "keyboard backlight was changed" or 
"battery lifetime decreased under X %" can be useful for userspace 
applications. I agree. But I think we do not have any subsystem or 
interface for sending them from kernel to userspace.

If we start talking about creating interface for it, I would rather see 
something more generic, not Dell-only specific or created specially for 
Dell machines which will not fit for others...

Darren, what do you think about it?

> Those could still be passed on somehow to have things like
> gnome-settings-daemon or other userspace tools to react however and
> show a notification.

Yes, that can be useful and with some genetic way, other hardware can 
benefit from it too...

> > > > But probably we do not have better names...
> > > 
> > > In this particular case, I was thinking PROG1/2/3 were really the
> > > best option and would be most intuitive because the keys really
> > > are labels "P1" "P2" and "P3".
> > 
> > Probably yes, as I wrote I do not have in my mind better names for
> > now.
> 
> OK, I'll resend with the cosmetic tabbing change and a clearer
> description, but keep the same mapping.

Ok.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 20:15           ` Pali Rohár
@ 2016-07-27 22:43             ` Darren Hart
  2016-07-27 23:07               ` Dmitry Torokhov
  2016-07-28 15:36               ` Mario_Limonciello
  1 sibling, 1 reply; 20+ messages in thread
From: Darren Hart @ 2016-07-27 22:43 UTC (permalink / raw)
  To: Pali Rohár, Dmitry Torokhov
  Cc: Mario_Limonciello, linux-kernel, platform-driver-x86

Dmitry, we'd appreciate your thoughts as the input maintainer on a question
below (left the thread in tact so you have all the context).

On Wed, Jul 27, 2016 at 10:15:12PM +0200, Pali Rohár wrote:
> On Wednesday 27 July 2016 21:03:57 Mario_Limonciello@dell.com wrote:
> > > -----Original Message-----
> > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > > Sent: Wednesday, July 27, 2016 11:06 AM
> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > > Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org;
> > > platform-driver- x86@vger.kernel.org
> > > Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged
> > > 2-in-1s
> > > 
> > > On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com wrote:
> > > > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we
> > > > > already use
> > > > >
> > > > >those for other actions (see bios_to_linux_keycode). Also there
> > > > >is:
> > > > I had missed this, do you have some recommendations on what would
> > > > be better codes to map this to?
> > > 
> > > Problem is that I do not know when those KEY_PROG keys from
> > > bios_to_linux_keycode table are emitted. There are missing comments
> > > or description.
> > > 
> > > Are you able to find out description for all those keys in that
> > > table? Maybe now (when linux key constants are extended), there
> > > could be better candidates...
> > 
> > I'll ask around.  It's not immediately obvious to me though, maybe
> > from old specs?
> 
> Do not know if it is old. At least some codes from it are used on my 
> E6440 machine. Table itself was added by Rezwanul Kabir in this commit:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5ea2559726b786283236835dc2905c23b36ac91c
> 
> Maybe commit message could help you to indentify what it is...
> 
> > > > I'll double check what the things that "were" mapping to
> > > > KEY_PROG1 etc actually were.  This might be a case of an
> > > > expected clash if the functions aren't in current generations.
> > > > 
> > > > >/* Wifi Catcher */
> > > > >
> > > > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> > > > 
> > > > It's worth mentioning that Wifi Catcher hasn't been used for any
> > > > Dell laptops for a handful generations.  The rugged 2in1's are
> > > > current generation that have these programmable buttons and
> > > > don't have wifi catcher.
> > > 
> > > Anyway, what is "Wifi Catcher"? HW switch buttton which
> > > enable/disable wifi? Or something else?
> > 
> > Wifi catcher was a special hardware switch slider switch.  When the
> > machine was turned in S3 the sliding the switch beyond the on
> > position would scan for available wifi networks and light up an LED
> > if some from your predefined list were found.
> > 
> > When the machine was on, it would open up the applet that let you
> > configure the behavior for the switch in S3.
> 
> Hm... maybe it better fit KEY_WLAN then? Just speculation.
> 
> > > > So there should be no "real" clash here.
> > > 
> > > Problem can be in future. This driver is unified for all Dell
> > > products with wmi interface and so future product could do some
> > > nasty things...
> > 
> > I agree with Darren here.
> > 
> > At least on Dell side we're creating new codes for "new" buttons and
> > the limitation is really on the kernel side for how many KEY_PROG#
> > or similar functions they can be re-assigned to.
> 
> Ok.
> 
> > Maybe it's time to think of another way to get this information to
> > userspace rather that always translating them into key codes?
> 
> If event is generated by pressing key, button or hw switch, then input 
> key is correct way. If there is no KEY define which fit for new vendor 
> hw button, then I think we should start discussion with input subsystem 
> how to handle such situation.
> 
> > There's a lot that are marked as KE_IGNORE because the kernel can't
> > do anything interesting with them as no 1-1 mapping exists to a
> > keycode.
> 
> Most marked as KE_IGNORE are because duplicate event is sent via 
> keyboard controller or via other subsystem (e.g. rfkill).
> 
> But events which are not keys or buttons should not be sent via input 
> devices. At least I think it is against usage of input devices.
> 
> Events like "battery was removed" or "keyboard backlight was changed" or 
> "battery lifetime decreased under X %" can be useful for userspace 
> applications. I agree. But I think we do not have any subsystem or 
> interface for sending them from kernel to userspace.
> 
> If we start talking about creating interface for it, I would rather see 
> something more generic, not Dell-only specific or created specially for 
> Dell machines which will not fit for others...
> 
> Darren, what do you think about it?

It sounds like a good Kernel Summit topic... and they just happen to have the
call for topics going on right now... first step would be to get the input
maintainers thoughts...

+Dmitry Torokhov

Dmitry, what are your thoughts with respect to handing events up to userspace
which are not strictly key presses via the input system? Pali's suggestion makes
sense to me - do you think this is something we should discuss at KS... or is
this already decided and can you set us straight?

> 
> > Those could still be passed on somehow to have things like
> > gnome-settings-daemon or other userspace tools to react however and
> > show a notification.
> 
> Yes, that can be useful and with some genetic way, other hardware can 
> benefit from it too...
> 
> > > > > But probably we do not have better names...
> > > > 
> > > > In this particular case, I was thinking PROG1/2/3 were really the
> > > > best option and would be most intuitive because the keys really
> > > > are labels "P1" "P2" and "P3".
> > > 
> > > Probably yes, as I wrote I do not have in my mind better names for
> > > now.
> > 
> > OK, I'll resend with the cosmetic tabbing change and a clearer
> > description, but keep the same mapping.
> 
> Ok.
> 
> -- 
> Pali Rohár
> pali.rohar@gmail.com



-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 22:43             ` Darren Hart
@ 2016-07-27 23:07               ` Dmitry Torokhov
  2016-07-28 15:52                   ` Mario_Limonciello
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2016-07-27 23:07 UTC (permalink / raw)
  To: Darren Hart; +Cc: Pali Rohár, Mario Limonciello, lkml, platform-driver-x86

Hi Darren,

On Wed, Jul 27, 2016 at 3:43 PM, Darren Hart <dvhart@infradead.org> wrote:
> Dmitry, we'd appreciate your thoughts as the input maintainer on a question
> below (left the thread in tact so you have all the context).
>
> On Wed, Jul 27, 2016 at 10:15:12PM +0200, Pali Rohár wrote:
>> On Wednesday 27 July 2016 21:03:57 Mario_Limonciello@dell.com wrote:
>> > > -----Original Message-----
>> > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
>> > > Sent: Wednesday, July 27, 2016 11:06 AM
>> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
>> > > Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org;
>> > > platform-driver- x86@vger.kernel.org
>> > > Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged
>> > > 2-in-1s
>> > >
>> > > On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com wrote:
>> > > > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we
>> > > > > already use
>> > > > >
>> > > > >those for other actions (see bios_to_linux_keycode). Also there
>> > > > >is:
>> > > > I had missed this, do you have some recommendations on what would
>> > > > be better codes to map this to?
>> > >
>> > > Problem is that I do not know when those KEY_PROG keys from
>> > > bios_to_linux_keycode table are emitted. There are missing comments
>> > > or description.
>> > >
>> > > Are you able to find out description for all those keys in that
>> > > table? Maybe now (when linux key constants are extended), there
>> > > could be better candidates...
>> >
>> > I'll ask around.  It's not immediately obvious to me though, maybe
>> > from old specs?
>>
>> Do not know if it is old. At least some codes from it are used on my
>> E6440 machine. Table itself was added by Rezwanul Kabir in this commit:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5ea2559726b786283236835dc2905c23b36ac91c
>>
>> Maybe commit message could help you to indentify what it is...
>>
>> > > > I'll double check what the things that "were" mapping to
>> > > > KEY_PROG1 etc actually were.  This might be a case of an
>> > > > expected clash if the functions aren't in current generations.
>> > > >
>> > > > >/* Wifi Catcher */
>> > > > >
>> > > > > { KE_KEY,    0xe011, { KEY_PROG2 } },
>> > > >
>> > > > It's worth mentioning that Wifi Catcher hasn't been used for any
>> > > > Dell laptops for a handful generations.  The rugged 2in1's are
>> > > > current generation that have these programmable buttons and
>> > > > don't have wifi catcher.
>> > >
>> > > Anyway, what is "Wifi Catcher"? HW switch buttton which
>> > > enable/disable wifi? Or something else?
>> >
>> > Wifi catcher was a special hardware switch slider switch.  When the
>> > machine was turned in S3 the sliding the switch beyond the on
>> > position would scan for available wifi networks and light up an LED
>> > if some from your predefined list were found.
>> >
>> > When the machine was on, it would open up the applet that let you
>> > configure the behavior for the switch in S3.
>>
>> Hm... maybe it better fit KEY_WLAN then? Just speculation.
>>
>> > > > So there should be no "real" clash here.
>> > >
>> > > Problem can be in future. This driver is unified for all Dell
>> > > products with wmi interface and so future product could do some
>> > > nasty things...
>> >
>> > I agree with Darren here.
>> >
>> > At least on Dell side we're creating new codes for "new" buttons and
>> > the limitation is really on the kernel side for how many KEY_PROG#
>> > or similar functions they can be re-assigned to.
>>
>> Ok.
>>
>> > Maybe it's time to think of another way to get this information to
>> > userspace rather that always translating them into key codes?
>>
>> If event is generated by pressing key, button or hw switch, then input
>> key is correct way. If there is no KEY define which fit for new vendor
>> hw button, then I think we should start discussion with input subsystem
>> how to handle such situation.
>>
>> > There's a lot that are marked as KE_IGNORE because the kernel can't
>> > do anything interesting with them as no 1-1 mapping exists to a
>> > keycode.
>>
>> Most marked as KE_IGNORE are because duplicate event is sent via
>> keyboard controller or via other subsystem (e.g. rfkill).
>>
>> But events which are not keys or buttons should not be sent via input
>> devices. At least I think it is against usage of input devices.
>>
>> Events like "battery was removed" or "keyboard backlight was changed" or
>> "battery lifetime decreased under X %" can be useful for userspace
>> applications. I agree. But I think we do not have any subsystem or
>> interface for sending them from kernel to userspace.
>>
>> If we start talking about creating interface for it, I would rather see
>> something more generic, not Dell-only specific or created specially for
>> Dell machines which will not fit for others...
>>
>> Darren, what do you think about it?
>
> It sounds like a good Kernel Summit topic... and they just happen to have the
> call for topics going on right now... first step would be to get the input
> maintainers thoughts...
>
> +Dmitry Torokhov
>
> Dmitry, what are your thoughts with respect to handing events up to userspace
> which are not strictly key presses via the input system? Pali's suggestion makes
> sense to me - do you think this is something we should discuss at KS... or is
> this already decided and can you set us straight?

So here is what I believe:

- Stuff that are not keys/buttons/switches/other bits of hardware
directly manipulated by user, but rather state change notifications
should not be delivered via input subsystem

- Even if something is a key/button/switch it does not necessarily
need to be sent through input subsystem or we may want to wait until
we add a new input event. This is because vendors love to come up with
"value add" features that require vendor specific driver that noone
ends up using. I mean, for the "WiFi catcher" do you have any kind of
numbers for the users of the feature?

- In the same vein, we can come up with something generic to shuffle
the crap over to userspace ("com.vendor.crap.morecrap..." over
connector? I think I just threw into my mouth a little), but do we
have consumer for this?

Actually it might be fun topic to discuss at KS, even if in hallway
only. We might end up deciding that we should work events that make
sense generically into existing subsystems and leave "value add"
suppressed.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 20:15           ` Pali Rohár
@ 2016-07-28 15:36               ` Mario_Limonciello
  2016-07-28 15:36               ` Mario_Limonciello
  1 sibling, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-28 15:36 UTC (permalink / raw)
  To: pali.rohar; +Cc: dvhart, linux-kernel, platform-driver-x86

> > >
> > > Anyway, what is "Wifi Catcher"? HW switch buttton which
> > > enable/disable wifi? Or something else?
> >
> > Wifi catcher was a special hardware switch slider switch.  When the
> > machine was turned in S3 the sliding the switch beyond the on
> > position would scan for available wifi networks and light up an LED
> > if some from your predefined list were found.
> >
> > When the machine was on, it would open up the applet that let you
> > configure the behavior for the switch in S3.
> 
> Hm... maybe it better fit KEY_WLAN then? Just speculation.
> 

Yeah, I think that's a good fit.  I'll adjust that when I resubmit the
rugged patch shortly.

> > > > So there should be no "real" clash here.
> > >
> > > Problem can be in future. This driver is unified for all Dell
> > > products with wmi interface and so future product could do some
> > > nasty things...
> >
> > I agree with Darren here.
> >
> > At least on Dell side we're creating new codes for "new" buttons and
> > the limitation is really on the kernel side for how many KEY_PROG#
> > or similar functions they can be re-assigned to.
> 
> Ok.
> 
> > Maybe it's time to think of another way to get this information to
> > userspace rather that always translating them into key codes?
> 
> If event is generated by pressing key, button or hw switch, then input
> key is correct way. If there is no KEY define which fit for new vendor
> hw button, then I think we should start discussion with input subsystem
> how to handle such situation.
> 
> > There's a lot that are marked as KE_IGNORE because the kernel can't
> > do anything interesting with them as no 1-1 mapping exists to a
> > keycode.
> 
> Most marked as KE_IGNORE are because duplicate event is sent via
> keyboard controller or via other subsystem (e.g. rfkill).
> 
> But events which are not keys or buttons should not be sent via input
> devices. At least I think it is against usage of input devices.
> 
> Events like "battery was removed" or "keyboard backlight was changed" or
> "battery lifetime decreased under X %" can be useful for userspace
> applications. I agree. But I think we do not have any subsystem or
> interface for sending them from kernel to userspace.
> 
> If we start talking about creating interface for it, I would rather see
> something more generic, not Dell-only specific or created specially for
> Dell machines which will not fit for others...
> 

I don't think it needs to be Dell specific, I'm sure plenty of other platform
drivers would use it too.

If this interface is created we can figure out which ones really are useful
notifications to userspace.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
@ 2016-07-28 15:36               ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-28 15:36 UTC (permalink / raw)
  To: pali.rohar; +Cc: dvhart, linux-kernel, platform-driver-x86

> > >
> > > Anyway, what is "Wifi Catcher"? HW switch buttton which
> > > enable/disable wifi? Or something else?
> >
> > Wifi catcher was a special hardware switch slider switch.  When the
> > machine was turned in S3 the sliding the switch beyond the on
> > position would scan for available wifi networks and light up an LED
> > if some from your predefined list were found.
> >
> > When the machine was on, it would open up the applet that let you
> > configure the behavior for the switch in S3.
> 
> Hm... maybe it better fit KEY_WLAN then? Just speculation.
> 

Yeah, I think that's a good fit.  I'll adjust that when I resubmit the
rugged patch shortly.

> > > > So there should be no "real" clash here.
> > >
> > > Problem can be in future. This driver is unified for all Dell
> > > products with wmi interface and so future product could do some
> > > nasty things...
> >
> > I agree with Darren here.
> >
> > At least on Dell side we're creating new codes for "new" buttons and
> > the limitation is really on the kernel side for how many KEY_PROG#
> > or similar functions they can be re-assigned to.
> 
> Ok.
> 
> > Maybe it's time to think of another way to get this information to
> > userspace rather that always translating them into key codes?
> 
> If event is generated by pressing key, button or hw switch, then input
> key is correct way. If there is no KEY define which fit for new vendor
> hw button, then I think we should start discussion with input subsystem
> how to handle such situation.
> 
> > There's a lot that are marked as KE_IGNORE because the kernel can't
> > do anything interesting with them as no 1-1 mapping exists to a
> > keycode.
> 
> Most marked as KE_IGNORE are because duplicate event is sent via
> keyboard controller or via other subsystem (e.g. rfkill).
> 
> But events which are not keys or buttons should not be sent via input
> devices. At least I think it is against usage of input devices.
> 
> Events like "battery was removed" or "keyboard backlight was changed" or
> "battery lifetime decreased under X %" can be useful for userspace
> applications. I agree. But I think we do not have any subsystem or
> interface for sending them from kernel to userspace.
> 
> If we start talking about creating interface for it, I would rather see
> something more generic, not Dell-only specific or created specially for
> Dell machines which will not fit for others...
> 

I don't think it needs to be Dell specific, I'm sure plenty of other platform
drivers would use it too.

If this interface is created we can figure out which ones really are useful
notifications to userspace.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-27 23:07               ` Dmitry Torokhov
@ 2016-07-28 15:52                   ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-28 15:52 UTC (permalink / raw)
  To: dmitry.torokhov, dvhart; +Cc: pali.rohar, linux-kernel, platform-driver-x86

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Wednesday, July 27, 2016 6:08 PM
> To: Darren Hart <dvhart@infradead.org>
> Cc: Pali Rohár <pali.rohar@gmail.com>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>; lkml <linux-kernel@vger.kernel.org>;
> platform-driver-x86@vger.kernel.org
> Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
> 
> Hi Darren,
> 
> On Wed, Jul 27, 2016 at 3:43 PM, Darren Hart <dvhart@infradead.org> wrote:
> > Dmitry, we'd appreciate your thoughts as the input maintainer on a
> question
> > below (left the thread in tact so you have all the context).
> >
> > On Wed, Jul 27, 2016 at 10:15:12PM +0200, Pali Rohár wrote:
> >> On Wednesday 27 July 2016 21:03:57 Mario_Limonciello@dell.com wrote:
> >> > > -----Original Message-----
> >> > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> >> > > Sent: Wednesday, July 27, 2016 11:06 AM
> >> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> >> > > Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org;
> >> > > platform-driver- x86@vger.kernel.org
> >> > > Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged
> >> > > 2-in-1s
> >> > >
> >> > > On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com
> wrote:
> >> > > > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we
> >> > > > > already use
> >> > > > >
> >> > > > >those for other actions (see bios_to_linux_keycode). Also there
> >> > > > >is:
> >> > > > I had missed this, do you have some recommendations on what
> would
> >> > > > be better codes to map this to?
> >> > >
> >> > > Problem is that I do not know when those KEY_PROG keys from
> >> > > bios_to_linux_keycode table are emitted. There are missing
> comments
> >> > > or description.
> >> > >
> >> > > Are you able to find out description for all those keys in that
> >> > > table? Maybe now (when linux key constants are extended), there
> >> > > could be better candidates...
> >> >
> >> > I'll ask around.  It's not immediately obvious to me though, maybe
> >> > from old specs?
> >>
> >> Do not know if it is old. At least some codes from it are used on my
> >> E6440 machine. Table itself was added by Rezwanul Kabir in this commit:
> >>
> >>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5e
> a2559726b786283236835dc2905c23b36ac91c
> >>
> >> Maybe commit message could help you to indentify what it is...
> >>
> >> > > > I'll double check what the things that "were" mapping to
> >> > > > KEY_PROG1 etc actually were.  This might be a case of an
> >> > > > expected clash if the functions aren't in current generations.
> >> > > >
> >> > > > >/* Wifi Catcher */
> >> > > > >
> >> > > > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> >> > > >
> >> > > > It's worth mentioning that Wifi Catcher hasn't been used for any
> >> > > > Dell laptops for a handful generations.  The rugged 2in1's are
> >> > > > current generation that have these programmable buttons and
> >> > > > don't have wifi catcher.
> >> > >
> >> > > Anyway, what is "Wifi Catcher"? HW switch buttton which
> >> > > enable/disable wifi? Or something else?
> >> >
> >> > Wifi catcher was a special hardware switch slider switch.  When the
> >> > machine was turned in S3 the sliding the switch beyond the on
> >> > position would scan for available wifi networks and light up an LED
> >> > if some from your predefined list were found.
> >> >
> >> > When the machine was on, it would open up the applet that let you
> >> > configure the behavior for the switch in S3.
> >>
> >> Hm... maybe it better fit KEY_WLAN then? Just speculation.
> >>
> >> > > > So there should be no "real" clash here.
> >> > >
> >> > > Problem can be in future. This driver is unified for all Dell
> >> > > products with wmi interface and so future product could do some
> >> > > nasty things...
> >> >
> >> > I agree with Darren here.
> >> >
> >> > At least on Dell side we're creating new codes for "new" buttons and
> >> > the limitation is really on the kernel side for how many KEY_PROG#
> >> > or similar functions they can be re-assigned to.
> >>
> >> Ok.
> >>
> >> > Maybe it's time to think of another way to get this information to
> >> > userspace rather that always translating them into key codes?
> >>
> >> If event is generated by pressing key, button or hw switch, then input
> >> key is correct way. If there is no KEY define which fit for new vendor
> >> hw button, then I think we should start discussion with input subsystem
> >> how to handle such situation.
> >>
> >> > There's a lot that are marked as KE_IGNORE because the kernel can't
> >> > do anything interesting with them as no 1-1 mapping exists to a
> >> > keycode.
> >>
> >> Most marked as KE_IGNORE are because duplicate event is sent via
> >> keyboard controller or via other subsystem (e.g. rfkill).
> >>
> >> But events which are not keys or buttons should not be sent via input
> >> devices. At least I think it is against usage of input devices.
> >>
> >> Events like "battery was removed" or "keyboard backlight was changed"
> or
> >> "battery lifetime decreased under X %" can be useful for userspace
> >> applications. I agree. But I think we do not have any subsystem or
> >> interface for sending them from kernel to userspace.
> >>
> >> If we start talking about creating interface for it, I would rather see
> >> something more generic, not Dell-only specific or created specially for
> >> Dell machines which will not fit for others...
> >>
> >> Darren, what do you think about it?
> >
> > It sounds like a good Kernel Summit topic... and they just happen to have
> the
> > call for topics going on right now... first step would be to get the input
> > maintainers thoughts...
> >
> > +Dmitry Torokhov
> >
> > Dmitry, what are your thoughts with respect to handing events up to
> userspace
> > which are not strictly key presses via the input system? Pali's suggestion
> makes
> > sense to me - do you think this is something we should discuss at KS... or is
> > this already decided and can you set us straight?
> 
> So here is what I believe:
> 
> - Stuff that are not keys/buttons/switches/other bits of hardware
> directly manipulated by user, but rather state change notifications
> should not be delivered via input subsystem

Yeah this matches existing kernel documentation too.

> 
> - Even if something is a key/button/switch it does not necessarily
> need to be sent through input subsystem or we may want to wait until
> we add a new input event. This is because vendors love to come up with
> "value add" features that require vendor specific driver that noone
> ends up using. I mean, for the "WiFi catcher" do you have any kind of
> numbers for the users of the feature?

Like I said, this particular feature hasn't been in use for quite a few generations.  
In its time it was deemed "beneficial" in that day because of the amount of time it 
took for waking up the machine only to determine there was no available wifi nearby.
It's been superseded by other technology improvements.

There are other types of "notification" only events that could be useful to userspace.
I don't think they need to all be generally demonized with the connotation as a negative 
vendor specific value add, there are plenty of generic things that userspace could notify on
that are essentially "killed" at the WMI driver today.

Here's a few I find:
"Keyboard illumination toggle"
"Mic mute toggle"
"ALS sensor toggle"
" Rotation lock toggle"
"Airplane mode toggle"

> 
> - In the same vein, we can come up with something generic to shuffle
> the crap over to userspace ("com.vendor.crap.morecrap..." over
> connector? I think I just threw into my mouth a little), but do we
> have consumer for this?
> 

I think it would be most equitable to shuffle all notification events over
to userspace with a generic connector and let userspace maintainers 
decide which ones make sense to show to the user.

The most logical one to me will be adding patches into gnome-settings-daemon
and similar UI interfaces that already display things like brightness and volume
changes.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
@ 2016-07-28 15:52                   ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-07-28 15:52 UTC (permalink / raw)
  To: dmitry.torokhov, dvhart; +Cc: pali.rohar, linux-kernel, platform-driver-x86

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Wednesday, July 27, 2016 6:08 PM
> To: Darren Hart <dvhart@infradead.org>
> Cc: Pali Rohár <pali.rohar@gmail.com>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>; lkml <linux-kernel@vger.kernel.org>;
> platform-driver-x86@vger.kernel.org
> Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
> 
> Hi Darren,
> 
> On Wed, Jul 27, 2016 at 3:43 PM, Darren Hart <dvhart@infradead.org> wrote:
> > Dmitry, we'd appreciate your thoughts as the input maintainer on a
> question
> > below (left the thread in tact so you have all the context).
> >
> > On Wed, Jul 27, 2016 at 10:15:12PM +0200, Pali Rohár wrote:
> >> On Wednesday 27 July 2016 21:03:57 Mario_Limonciello@dell.com wrote:
> >> > > -----Original Message-----
> >> > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> >> > > Sent: Wednesday, July 27, 2016 11:06 AM
> >> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> >> > > Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org;
> >> > > platform-driver- x86@vger.kernel.org
> >> > > Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged
> >> > > 2-in-1s
> >> > >
> >> > > On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com
> wrote:
> >> > > > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we
> >> > > > > already use
> >> > > > >
> >> > > > >those for other actions (see bios_to_linux_keycode). Also there
> >> > > > >is:
> >> > > > I had missed this, do you have some recommendations on what
> would
> >> > > > be better codes to map this to?
> >> > >
> >> > > Problem is that I do not know when those KEY_PROG keys from
> >> > > bios_to_linux_keycode table are emitted. There are missing
> comments
> >> > > or description.
> >> > >
> >> > > Are you able to find out description for all those keys in that
> >> > > table? Maybe now (when linux key constants are extended), there
> >> > > could be better candidates...
> >> >
> >> > I'll ask around.  It's not immediately obvious to me though, maybe
> >> > from old specs?
> >>
> >> Do not know if it is old. At least some codes from it are used on my
> >> E6440 machine. Table itself was added by Rezwanul Kabir in this commit:
> >>
> >>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5e
> a2559726b786283236835dc2905c23b36ac91c
> >>
> >> Maybe commit message could help you to indentify what it is...
> >>
> >> > > > I'll double check what the things that "were" mapping to
> >> > > > KEY_PROG1 etc actually were.  This might be a case of an
> >> > > > expected clash if the functions aren't in current generations.
> >> > > >
> >> > > > >/* Wifi Catcher */
> >> > > > >
> >> > > > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> >> > > >
> >> > > > It's worth mentioning that Wifi Catcher hasn't been used for any
> >> > > > Dell laptops for a handful generations.  The rugged 2in1's are
> >> > > > current generation that have these programmable buttons and
> >> > > > don't have wifi catcher.
> >> > >
> >> > > Anyway, what is "Wifi Catcher"? HW switch buttton which
> >> > > enable/disable wifi? Or something else?
> >> >
> >> > Wifi catcher was a special hardware switch slider switch.  When the
> >> > machine was turned in S3 the sliding the switch beyond the on
> >> > position would scan for available wifi networks and light up an LED
> >> > if some from your predefined list were found.
> >> >
> >> > When the machine was on, it would open up the applet that let you
> >> > configure the behavior for the switch in S3.
> >>
> >> Hm... maybe it better fit KEY_WLAN then? Just speculation.
> >>
> >> > > > So there should be no "real" clash here.
> >> > >
> >> > > Problem can be in future. This driver is unified for all Dell
> >> > > products with wmi interface and so future product could do some
> >> > > nasty things...
> >> >
> >> > I agree with Darren here.
> >> >
> >> > At least on Dell side we're creating new codes for "new" buttons and
> >> > the limitation is really on the kernel side for how many KEY_PROG#
> >> > or similar functions they can be re-assigned to.
> >>
> >> Ok.
> >>
> >> > Maybe it's time to think of another way to get this information to
> >> > userspace rather that always translating them into key codes?
> >>
> >> If event is generated by pressing key, button or hw switch, then input
> >> key is correct way. If there is no KEY define which fit for new vendor
> >> hw button, then I think we should start discussion with input subsystem
> >> how to handle such situation.
> >>
> >> > There's a lot that are marked as KE_IGNORE because the kernel can't
> >> > do anything interesting with them as no 1-1 mapping exists to a
> >> > keycode.
> >>
> >> Most marked as KE_IGNORE are because duplicate event is sent via
> >> keyboard controller or via other subsystem (e.g. rfkill).
> >>
> >> But events which are not keys or buttons should not be sent via input
> >> devices. At least I think it is against usage of input devices.
> >>
> >> Events like "battery was removed" or "keyboard backlight was changed"
> or
> >> "battery lifetime decreased under X %" can be useful for userspace
> >> applications. I agree. But I think we do not have any subsystem or
> >> interface for sending them from kernel to userspace.
> >>
> >> If we start talking about creating interface for it, I would rather see
> >> something more generic, not Dell-only specific or created specially for
> >> Dell machines which will not fit for others...
> >>
> >> Darren, what do you think about it?
> >
> > It sounds like a good Kernel Summit topic... and they just happen to have
> the
> > call for topics going on right now... first step would be to get the input
> > maintainers thoughts...
> >
> > +Dmitry Torokhov
> >
> > Dmitry, what are your thoughts with respect to handing events up to
> userspace
> > which are not strictly key presses via the input system? Pali's suggestion
> makes
> > sense to me - do you think this is something we should discuss at KS... or is
> > this already decided and can you set us straight?
> 
> So here is what I believe:
> 
> - Stuff that are not keys/buttons/switches/other bits of hardware
> directly manipulated by user, but rather state change notifications
> should not be delivered via input subsystem

Yeah this matches existing kernel documentation too.

> 
> - Even if something is a key/button/switch it does not necessarily
> need to be sent through input subsystem or we may want to wait until
> we add a new input event. This is because vendors love to come up with
> "value add" features that require vendor specific driver that noone
> ends up using. I mean, for the "WiFi catcher" do you have any kind of
> numbers for the users of the feature?

Like I said, this particular feature hasn't been in use for quite a few generations.  
In its time it was deemed "beneficial" in that day because of the amount of time it 
took for waking up the machine only to determine there was no available wifi nearby.
It's been superseded by other technology improvements.

There are other types of "notification" only events that could be useful to userspace.
I don't think they need to all be generally demonized with the connotation as a negative 
vendor specific value add, there are plenty of generic things that userspace could notify on
that are essentially "killed" at the WMI driver today.

Here's a few I find:
"Keyboard illumination toggle"
"Mic mute toggle"
"ALS sensor toggle"
" Rotation lock toggle"
"Airplane mode toggle"

> 
> - In the same vein, we can come up with something generic to shuffle
> the crap over to userspace ("com.vendor.crap.morecrap..." over
> connector? I think I just threw into my mouth a little), but do we
> have consumer for this?
> 

I think it would be most equitable to shuffle all notification events over
to userspace with a generic connector and let userspace maintainers 
decide which ones make sense to show to the user.

The most logical one to me will be adding patches into gnome-settings-daemon
and similar UI interfaces that already display things like brightness and volume
changes.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-28 15:52                   ` Mario_Limonciello
  (?)
@ 2016-07-28 18:49                   ` Dmitry Torokhov
  2016-07-28 19:22                     ` Pali Rohár
  -1 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2016-07-28 18:49 UTC (permalink / raw)
  To: Mario_Limonciello; +Cc: dvhart, pali.rohar, linux-kernel, platform-driver-x86

On Thu, Jul 28, 2016 at 03:52:11PM +0000, Mario_Limonciello@Dell.com wrote:
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > Sent: Wednesday, July 27, 2016 6:08 PM
> > To: Darren Hart <dvhart@infradead.org>
> > Cc: Pali Rohár <pali.rohar@gmail.com>; Limonciello, Mario
> > <Mario_Limonciello@Dell.com>; lkml <linux-kernel@vger.kernel.org>;
> > platform-driver-x86@vger.kernel.org
> > Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
> > 
> > Hi Darren,
> > 
> > On Wed, Jul 27, 2016 at 3:43 PM, Darren Hart <dvhart@infradead.org> wrote:
> > > Dmitry, we'd appreciate your thoughts as the input maintainer on a
> > question
> > > below (left the thread in tact so you have all the context).
> > >
> > > On Wed, Jul 27, 2016 at 10:15:12PM +0200, Pali Rohár wrote:
> > >> On Wednesday 27 July 2016 21:03:57 Mario_Limonciello@dell.com wrote:
> > >> > > -----Original Message-----
> > >> > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > >> > > Sent: Wednesday, July 27, 2016 11:06 AM
> > >> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > >> > > Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org;
> > >> > > platform-driver- x86@vger.kernel.org
> > >> > > Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged
> > >> > > 2-in-1s
> > >> > >
> > >> > > On Wednesday 27 July 2016 17:55:09 Mario_Limonciello@dell.com
> > wrote:
> > >> > > > > Hi! I'm not sure if KEY_PROG1/2/3 are good names here as we
> > >> > > > > already use
> > >> > > > >
> > >> > > > >those for other actions (see bios_to_linux_keycode). Also there
> > >> > > > >is:
> > >> > > > I had missed this, do you have some recommendations on what
> > would
> > >> > > > be better codes to map this to?
> > >> > >
> > >> > > Problem is that I do not know when those KEY_PROG keys from
> > >> > > bios_to_linux_keycode table are emitted. There are missing
> > comments
> > >> > > or description.
> > >> > >
> > >> > > Are you able to find out description for all those keys in that
> > >> > > table? Maybe now (when linux key constants are extended), there
> > >> > > could be better candidates...
> > >> >
> > >> > I'll ask around.  It's not immediately obvious to me though, maybe
> > >> > from old specs?
> > >>
> > >> Do not know if it is old. At least some codes from it are used on my
> > >> E6440 machine. Table itself was added by Rezwanul Kabir in this commit:
> > >>
> > >>
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5e
> > a2559726b786283236835dc2905c23b36ac91c
> > >>
> > >> Maybe commit message could help you to indentify what it is...
> > >>
> > >> > > > I'll double check what the things that "were" mapping to
> > >> > > > KEY_PROG1 etc actually were.  This might be a case of an
> > >> > > > expected clash if the functions aren't in current generations.
> > >> > > >
> > >> > > > >/* Wifi Catcher */
> > >> > > > >
> > >> > > > > { KE_KEY,    0xe011, { KEY_PROG2 } },
> > >> > > >
> > >> > > > It's worth mentioning that Wifi Catcher hasn't been used for any
> > >> > > > Dell laptops for a handful generations.  The rugged 2in1's are
> > >> > > > current generation that have these programmable buttons and
> > >> > > > don't have wifi catcher.
> > >> > >
> > >> > > Anyway, what is "Wifi Catcher"? HW switch buttton which
> > >> > > enable/disable wifi? Or something else?
> > >> >
> > >> > Wifi catcher was a special hardware switch slider switch.  When the
> > >> > machine was turned in S3 the sliding the switch beyond the on
> > >> > position would scan for available wifi networks and light up an LED
> > >> > if some from your predefined list were found.
> > >> >
> > >> > When the machine was on, it would open up the applet that let you
> > >> > configure the behavior for the switch in S3.
> > >>
> > >> Hm... maybe it better fit KEY_WLAN then? Just speculation.
> > >>
> > >> > > > So there should be no "real" clash here.
> > >> > >
> > >> > > Problem can be in future. This driver is unified for all Dell
> > >> > > products with wmi interface and so future product could do some
> > >> > > nasty things...
> > >> >
> > >> > I agree with Darren here.
> > >> >
> > >> > At least on Dell side we're creating new codes for "new" buttons and
> > >> > the limitation is really on the kernel side for how many KEY_PROG#
> > >> > or similar functions they can be re-assigned to.
> > >>
> > >> Ok.
> > >>
> > >> > Maybe it's time to think of another way to get this information to
> > >> > userspace rather that always translating them into key codes?
> > >>
> > >> If event is generated by pressing key, button or hw switch, then input
> > >> key is correct way. If there is no KEY define which fit for new vendor
> > >> hw button, then I think we should start discussion with input subsystem
> > >> how to handle such situation.
> > >>
> > >> > There's a lot that are marked as KE_IGNORE because the kernel can't
> > >> > do anything interesting with them as no 1-1 mapping exists to a
> > >> > keycode.
> > >>
> > >> Most marked as KE_IGNORE are because duplicate event is sent via
> > >> keyboard controller or via other subsystem (e.g. rfkill).
> > >>
> > >> But events which are not keys or buttons should not be sent via input
> > >> devices. At least I think it is against usage of input devices.
> > >>
> > >> Events like "battery was removed" or "keyboard backlight was changed"
> > or
> > >> "battery lifetime decreased under X %" can be useful for userspace
> > >> applications. I agree. But I think we do not have any subsystem or
> > >> interface for sending them from kernel to userspace.
> > >>
> > >> If we start talking about creating interface for it, I would rather see
> > >> something more generic, not Dell-only specific or created specially for
> > >> Dell machines which will not fit for others...
> > >>
> > >> Darren, what do you think about it?
> > >
> > > It sounds like a good Kernel Summit topic... and they just happen to have
> > the
> > > call for topics going on right now... first step would be to get the input
> > > maintainers thoughts...
> > >
> > > +Dmitry Torokhov
> > >
> > > Dmitry, what are your thoughts with respect to handing events up to
> > userspace
> > > which are not strictly key presses via the input system? Pali's suggestion
> > makes
> > > sense to me - do you think this is something we should discuss at KS... or is
> > > this already decided and can you set us straight?
> > 
> > So here is what I believe:
> > 
> > - Stuff that are not keys/buttons/switches/other bits of hardware
> > directly manipulated by user, but rather state change notifications
> > should not be delivered via input subsystem
> 
> Yeah this matches existing kernel documentation too.
> 
> > 
> > - Even if something is a key/button/switch it does not necessarily
> > need to be sent through input subsystem or we may want to wait until
> > we add a new input event. This is because vendors love to come up with
> > "value add" features that require vendor specific driver that noone
> > ends up using. I mean, for the "WiFi catcher" do you have any kind of
> > numbers for the users of the feature?
> 
> Like I said, this particular feature hasn't been in use for quite a few generations.  
> In its time it was deemed "beneficial" in that day because of the amount of time it 
> took for waking up the machine only to determine there was no available wifi nearby.
> It's been superseded by other technology improvements.
> 
> There are other types of "notification" only events that could be useful to userspace.
> I don't think they need to all be generally demonized with the connotation as a negative 
> vendor specific value add, there are plenty of generic things that userspace could notify on

Generic things should find a specific system they are part of (i.e. wifi
notifications -> rflill, wifi switching -> input, etc), it is one-off
that "sounded good" but are hard to discover by user and nobody ends up
using that I am demonizing.

> that are essentially "killed" at the WMI driver today.
> 
> Here's a few I find:
> "Keyboard illumination toggle"
> "Mic mute toggle"
> "ALS sensor toggle"
> " Rotation lock toggle"
> "Airplane mode toggle"

These all seem valid key events (unless they are not requests to execute
said actions but rather notifiers of events already happened).

Are they "killed" because they are also delivered via i8042?

> 
> > 
> > - In the same vein, we can come up with something generic to shuffle
> > the crap over to userspace ("com.vendor.crap.morecrap..." over
> > connector? I think I just threw into my mouth a little), but do we
> > have consumer for this?
> > 
> 
> I think it would be most equitable to shuffle all notification events over
> to userspace with a generic connector and let userspace maintainers 
> decide which ones make sense to show to the user.
> 
> The most logical one to me will be adding patches into gnome-settings-daemon
> and similar UI interfaces that already display things like brightness and volume
> changes.

Well, it is really easy to shove stuff up into userspace and say "let
them deal with it", but we if stuff is useful and applicable to many
devices, then I am sure there are more specific interfaces that would be
better suited for such events, and one off will require writing "tray
notification apps" that we all so love.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-28 18:49                   ` Dmitry Torokhov
@ 2016-07-28 19:22                     ` Pali Rohár
  2016-08-01 22:41                         ` Mario_Limonciello
  0 siblings, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2016-07-28 19:22 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Mario_Limonciello, dvhart, linux-kernel, platform-driver-x86, pavel

[-- Attachment #1: Type: Text/Plain, Size: 5530 bytes --]

On Thursday 28 July 2016 20:49:30 Dmitry Torokhov wrote:
> On Thu, Jul 28, 2016 at 03:52:11PM +0000, Mario_Limonciello@Dell.com
> wrote: 
> > > - Even if something is a key/button/switch it does not
> > > necessarily need to be sent through input subsystem or we may
> > > want to wait until we add a new input event. This is because
> > > vendors love to come up with "value add" features that require
> > > vendor specific driver that noone ends up using. I mean, for the
> > > "WiFi catcher" do you have any kind of numbers for the users of
> > > the feature?
> > 
> > Like I said, this particular feature hasn't been in use for quite a
> > few generations. In its time it was deemed "beneficial" in that
> > day because of the amount of time it took for waking up the
> > machine only to determine there was no available wifi nearby. It's
> > been superseded by other technology improvements.
> > 
> > There are other types of "notification" only events that could be
> > useful to userspace. I don't think they need to all be generally
> > demonized with the connotation as a negative vendor specific value
> > add, there are plenty of generic things that userspace could
> > notify on
> 
> Generic things should find a specific system they are part of (i.e.
> wifi notifications -> rflill, wifi switching -> input, etc), it is
> one-off that "sounded good" but are hard to discover by user and
> nobody ends up using that I am demonizing.

User (as consumer) of applications should not discovering such parts. 
User just open/configure his favourite gnome/kde/unity/xface/... 
application. But developers of wifi/power/led applications must know 
where to find those events.

And I need to say, if I change wifi state via rfkill interface, I would 
expect that also rfkill interface provides me information about changes. 
If I change keyboard backlight level via LED interface, then I somehow 
expect that LED interface should be able to tell me information that it 
was changed (but that's not truth... yet).

> > that are essentially "killed" at the WMI driver today.
> > 
> > Here's a few I find:
> > "Keyboard illumination toggle"
> > "Mic mute toggle"
> > "ALS sensor toggle"
> > " Rotation lock toggle"
> > "Airplane mode toggle"
> 
> These all seem valid key events (unless they are not requests to
> execute said actions but rather notifiers of events already
> happened).

Previously "Keyboard illumination toggle" event was translated to 
KEY_KBDILLUMTOGGLE, but it later after implementing dell::kbd_backlight 
LED driver (software control via /sys/ of keyboard backlight) it was 
changed to KE_IGNORE.

Reason was following: All userspace applications (KDE, Unity, upowerd) 
understand KEY_KBDILLUMTOGGLE input event as "key for toggling keyboard 
backlight level" was pressed. Their answer to that input key is to find 
*::kbd_backlight led device in /sys/class/leds and switch keyboard 
backlight level to next value.

Dell ACPI/firmware send "Keyboard illumination toggle" event every time 
when keyboard backlight level is changed (software via driver or by HW 
key press) and all above applications started infinite loop...

So there are difference between:

*) user pressed key with icon of "keyboard illumination", but hardware 
did nothing

*) hardware changed keyboard illumination level (either by its own --- 
e.g. because of long inactivity of user or user closed LID --- or as 
reaction of user request via special driver)

Months ago we agreed that if pressing key with icon "keyboard 
illumination" cause 1) emitting key press event (via ACPI/WMI) and also 
2) hardware unconditionally change keyboard backlight level, then we 
should not send that key press to userspace (as it was already processed 
by hardware). Same is for hardware key which block WIFI (as userspace 
react on input KEY_RFKILL to block all rfkills).

But I agree that could somehow inform userspace about changes which 
hardware did. For rfkills we already support interface when rfkill 
device send to userspace all change events.

For keyboard backlight we do not have notify interface yet, but I 
proposed that LED subsystem could be extended, so select/poll syscalls 
could be used on "brightness" sysfs node (which is used for changing LED 
level).

> Are they "killed" because they are also delivered via i8042?

No, (at least on my machine they are sent via WMI).

> > > - In the same vein, we can come up with something generic to
> > > shuffle the crap over to userspace
> > > ("com.vendor.crap.morecrap..." over connector? I think I just
> > > threw into my mouth a little), but do we have consumer for this?
> > 
> > I think it would be most equitable to shuffle all notification
> > events over to userspace with a generic connector and let
> > userspace maintainers decide which ones make sense to show to the
> > user.
> > 
> > The most logical one to me will be adding patches into
> > gnome-settings-daemon and similar UI interfaces that already
> > display things like brightness and volume changes.
> 
> Well, it is really easy to shove stuff up into userspace and say "let
> them deal with it", but we if stuff is useful and applicable to many
> devices, then I am sure there are more specific interfaces that would
> be better suited for such events, and one off will require writing
> "tray notification apps" that we all so love.
> 
> Thanks.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
  2016-07-28 19:22                     ` Pali Rohár
@ 2016-08-01 22:41                         ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-08-01 22:41 UTC (permalink / raw)
  To: pali.rohar, dmitry.torokhov
  Cc: dvhart, linux-kernel, platform-driver-x86, pavel

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Thursday, July 28, 2016 2:23 PM
> To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Limonciello, Mario <Mario_Limonciello@Dell.com>;
> dvhart@infradead.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; pavel@ucw.cz
> Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
> 
> On Thursday 28 July 2016 20:49:30 Dmitry Torokhov wrote:
> > On Thu, Jul 28, 2016 at 03:52:11PM +0000, Mario_Limonciello@Dell.com
> > wrote:
> > > > - Even if something is a key/button/switch it does not
> > > > necessarily need to be sent through input subsystem or we may
> > > > want to wait until we add a new input event. This is because
> > > > vendors love to come up with "value add" features that require
> > > > vendor specific driver that noone ends up using. I mean, for the
> > > > "WiFi catcher" do you have any kind of numbers for the users of
> > > > the feature?
> > >
> > > Like I said, this particular feature hasn't been in use for quite a
> > > few generations. In its time it was deemed "beneficial" in that
> > > day because of the amount of time it took for waking up the
> > > machine only to determine there was no available wifi nearby. It's
> > > been superseded by other technology improvements.
> > >
> > > There are other types of "notification" only events that could be
> > > useful to userspace. I don't think they need to all be generally
> > > demonized with the connotation as a negative vendor specific value
> > > add, there are plenty of generic things that userspace could
> > > notify on
> >
> > Generic things should find a specific system they are part of (i.e.
> > wifi notifications -> rflill, wifi switching -> input, etc), it is
> > one-off that "sounded good" but are hard to discover by user and
> > nobody ends up using that I am demonizing.
> 
> User (as consumer) of applications should not discovering such parts.
> User just open/configure his favourite gnome/kde/unity/xface/...
> application. But developers of wifi/power/led applications must know
> where to find those events.
> 
> And I need to say, if I change wifi state via rfkill interface, I would
> expect that also rfkill interface provides me information about changes.
> If I change keyboard backlight level via LED interface, then I somehow
> expect that LED interface should be able to tell me information that it
> was changed (but that's not truth... yet).
> 
> > > that are essentially "killed" at the WMI driver today.
> > >
> > > Here's a few I find:
> > > "Keyboard illumination toggle"
> > > "Mic mute toggle"
> > > "ALS sensor toggle"
> > > " Rotation lock toggle"
> > > "Airplane mode toggle"
> >
> > These all seem valid key events (unless they are not requests to
> > execute said actions but rather notifiers of events already
> > happened).
> 
> Previously "Keyboard illumination toggle" event was translated to
> KEY_KBDILLUMTOGGLE, but it later after implementing dell::kbd_backlight
> LED driver (software control via /sys/ of keyboard backlight) it was
> changed to KE_IGNORE.
> 
> Reason was following: All userspace applications (KDE, Unity, upowerd)
> understand KEY_KBDILLUMTOGGLE input event as "key for toggling
> keyboard
> backlight level" was pressed. Their answer to that input key is to find
> *::kbd_backlight led device in /sys/class/leds and switch keyboard
> backlight level to next value.
> 
> Dell ACPI/firmware send "Keyboard illumination toggle" event every time
> when keyboard backlight level is changed (software via driver or by HW
> key press) and all above applications started infinite loop...
> 
> So there are difference between:
> 
> *) user pressed key with icon of "keyboard illumination", but hardware
> did nothing
> 
> *) hardware changed keyboard illumination level (either by its own ---
> e.g. because of long inactivity of user or user closed LID --- or as
> reaction of user request via special driver)
> 
> Months ago we agreed that if pressing key with icon "keyboard
> illumination" cause 1) emitting key press event (via ACPI/WMI) and also
> 2) hardware unconditionally change keyboard backlight level, then we
> should not send that key press to userspace (as it was already processed
> by hardware). Same is for hardware key which block WIFI (as userspace
> react on input KEY_RFKILL to block all rfkills).
> 
> But I agree that could somehow inform userspace about changes which
> hardware did. For rfkills we already support interface when rfkill
> device send to userspace all change events.
> 
> For keyboard backlight we do not have notify interface yet, but I
> proposed that LED subsystem could be extended, so select/poll syscalls
> could be used on "brightness" sysfs node (which is used for changing LED
> level).
> 
> > Are they "killed" because they are also delivered via i8042?
> 
> No, (at least on my machine they are sent via WMI).

It really depends upon the vendor's implementation and that key's particular
implementation.  Some of the ones that are KE_IGNORE'ed by the wmi driver
will send something on i8042, others won't.

The ones I'm particularly interested about are the ones that are "NOTIFY" only
and only come across on WMI.

On Windows these are picked up by some application or a filter driver and a 
notification event is shown.

> 
> > > > - In the same vein, we can come up with something generic to
> > > > shuffle the crap over to userspace
> > > > ("com.vendor.crap.morecrap..." over connector? I think I just
> > > > threw into my mouth a little), but do we have consumer for this?
> > >
> > > I think it would be most equitable to shuffle all notification
> > > events over to userspace with a generic connector and let
> > > userspace maintainers decide which ones make sense to show to the
> > > user.
> > >
> > > The most logical one to me will be adding patches into
> > > gnome-settings-daemon and similar UI interfaces that already
> > > display things like brightness and volume changes.
> >
> > Well, it is really easy to shove stuff up into userspace and say "let
> > them deal with it", but we if stuff is useful and applicable to many
> > devices, then I am sure there are more specific interfaces that would
> > be better suited for such events, and one off will require writing
> > "tray notification apps" that we all so love.
> >
> > Thanks.
> 

I guess I don't see the use case on reacting to events like I said above
in userspace other than the basic libnotify toast popup.

That at least relays the message to the user that "something" happened
when that key was pressed.  Either FW or kernel would do the actual
activities but userspace would have to do the notification.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
@ 2016-08-01 22:41                         ` Mario_Limonciello
  0 siblings, 0 replies; 20+ messages in thread
From: Mario_Limonciello @ 2016-08-01 22:41 UTC (permalink / raw)
  To: pali.rohar, dmitry.torokhov
  Cc: dvhart, linux-kernel, platform-driver-x86, pavel

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Thursday, July 28, 2016 2:23 PM
> To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Limonciello, Mario <Mario_Limonciello@Dell.com>;
> dvhart@infradead.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; pavel@ucw.cz
> Subject: Re: [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s
> 
> On Thursday 28 July 2016 20:49:30 Dmitry Torokhov wrote:
> > On Thu, Jul 28, 2016 at 03:52:11PM +0000, Mario_Limonciello@Dell.com
> > wrote:
> > > > - Even if something is a key/button/switch it does not
> > > > necessarily need to be sent through input subsystem or we may
> > > > want to wait until we add a new input event. This is because
> > > > vendors love to come up with "value add" features that require
> > > > vendor specific driver that noone ends up using. I mean, for the
> > > > "WiFi catcher" do you have any kind of numbers for the users of
> > > > the feature?
> > >
> > > Like I said, this particular feature hasn't been in use for quite a
> > > few generations. In its time it was deemed "beneficial" in that
> > > day because of the amount of time it took for waking up the
> > > machine only to determine there was no available wifi nearby. It's
> > > been superseded by other technology improvements.
> > >
> > > There are other types of "notification" only events that could be
> > > useful to userspace. I don't think they need to all be generally
> > > demonized with the connotation as a negative vendor specific value
> > > add, there are plenty of generic things that userspace could
> > > notify on
> >
> > Generic things should find a specific system they are part of (i.e.
> > wifi notifications -> rflill, wifi switching -> input, etc), it is
> > one-off that "sounded good" but are hard to discover by user and
> > nobody ends up using that I am demonizing.
> 
> User (as consumer) of applications should not discovering such parts.
> User just open/configure his favourite gnome/kde/unity/xface/...
> application. But developers of wifi/power/led applications must know
> where to find those events.
> 
> And I need to say, if I change wifi state via rfkill interface, I would
> expect that also rfkill interface provides me information about changes.
> If I change keyboard backlight level via LED interface, then I somehow
> expect that LED interface should be able to tell me information that it
> was changed (but that's not truth... yet).
> 
> > > that are essentially "killed" at the WMI driver today.
> > >
> > > Here's a few I find:
> > > "Keyboard illumination toggle"
> > > "Mic mute toggle"
> > > "ALS sensor toggle"
> > > " Rotation lock toggle"
> > > "Airplane mode toggle"
> >
> > These all seem valid key events (unless they are not requests to
> > execute said actions but rather notifiers of events already
> > happened).
> 
> Previously "Keyboard illumination toggle" event was translated to
> KEY_KBDILLUMTOGGLE, but it later after implementing dell::kbd_backlight
> LED driver (software control via /sys/ of keyboard backlight) it was
> changed to KE_IGNORE.
> 
> Reason was following: All userspace applications (KDE, Unity, upowerd)
> understand KEY_KBDILLUMTOGGLE input event as "key for toggling
> keyboard
> backlight level" was pressed. Their answer to that input key is to find
> *::kbd_backlight led device in /sys/class/leds and switch keyboard
> backlight level to next value.
> 
> Dell ACPI/firmware send "Keyboard illumination toggle" event every time
> when keyboard backlight level is changed (software via driver or by HW
> key press) and all above applications started infinite loop...
> 
> So there are difference between:
> 
> *) user pressed key with icon of "keyboard illumination", but hardware
> did nothing
> 
> *) hardware changed keyboard illumination level (either by its own ---
> e.g. because of long inactivity of user or user closed LID --- or as
> reaction of user request via special driver)
> 
> Months ago we agreed that if pressing key with icon "keyboard
> illumination" cause 1) emitting key press event (via ACPI/WMI) and also
> 2) hardware unconditionally change keyboard backlight level, then we
> should not send that key press to userspace (as it was already processed
> by hardware). Same is for hardware key which block WIFI (as userspace
> react on input KEY_RFKILL to block all rfkills).
> 
> But I agree that could somehow inform userspace about changes which
> hardware did. For rfkills we already support interface when rfkill
> device send to userspace all change events.
> 
> For keyboard backlight we do not have notify interface yet, but I
> proposed that LED subsystem could be extended, so select/poll syscalls
> could be used on "brightness" sysfs node (which is used for changing LED
> level).
> 
> > Are they "killed" because they are also delivered via i8042?
> 
> No, (at least on my machine they are sent via WMI).

It really depends upon the vendor's implementation and that key's particular
implementation.  Some of the ones that are KE_IGNORE'ed by the wmi driver
will send something on i8042, others won't.

The ones I'm particularly interested about are the ones that are "NOTIFY" only
and only come across on WMI.

On Windows these are picked up by some application or a filter driver and a 
notification event is shown.

> 
> > > > - In the same vein, we can come up with something generic to
> > > > shuffle the crap over to userspace
> > > > ("com.vendor.crap.morecrap..." over connector? I think I just
> > > > threw into my mouth a little), but do we have consumer for this?
> > >
> > > I think it would be most equitable to shuffle all notification
> > > events over to userspace with a generic connector and let
> > > userspace maintainers decide which ones make sense to show to the
> > > user.
> > >
> > > The most logical one to me will be adding patches into
> > > gnome-settings-daemon and similar UI interfaces that already
> > > display things like brightness and volume changes.
> >
> > Well, it is really easy to shove stuff up into userspace and say "let
> > them deal with it", but we if stuff is useful and applicable to many
> > devices, then I am sure there are more specific interfaces that would
> > be better suited for such events, and one off will require writing
> > "tray notification apps" that we all so love.
> >
> > Thanks.
> 

I guess I don't see the use case on reacting to events like I said above
in userspace other than the basic libnotify toast popup.

That at least relays the message to the user that "something" happened
when that key was pressed.  Either FW or kernel would do the actual
activities but userspace would have to do the notification.


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-08-01 22:44 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-22 20:01 [PATCH] dell-wmi: Add events created by Dell Rugged 2-in-1s Mario Limonciello
2016-07-27  5:19 ` Darren Hart
2016-07-27  7:23   ` Pali Rohár
2016-07-27 15:55     ` Mario_Limonciello
2016-07-27 15:55       ` Mario_Limonciello
2016-07-27 16:05       ` Pali Rohár
2016-07-27 16:38         ` Darren Hart
2016-07-27 19:03         ` Mario_Limonciello
2016-07-27 19:03           ` Mario_Limonciello
2016-07-27 20:15           ` Pali Rohár
2016-07-27 22:43             ` Darren Hart
2016-07-27 23:07               ` Dmitry Torokhov
2016-07-28 15:52                 ` Mario_Limonciello
2016-07-28 15:52                   ` Mario_Limonciello
2016-07-28 18:49                   ` Dmitry Torokhov
2016-07-28 19:22                     ` Pali Rohár
2016-08-01 22:41                       ` Mario_Limonciello
2016-08-01 22:41                         ` Mario_Limonciello
2016-07-28 15:36             ` Mario_Limonciello
2016-07-28 15:36               ` Mario_Limonciello

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.