diff mbox series

cros_ec_keyb: Add 3 buttons for monitor function

Message ID 20221215061550.18993-1-joewu@msi.corp-partner.google.com
State Superseded
Headers show
Series cros_ec_keyb: Add 3 buttons for monitor function | expand

Commit Message

Joe Wu Dec. 15, 2022, 6:15 a.m. UTC
This patch is to add extra 3 buttons: 'brightness up',
'brightness down' and 'leave PC(cros) mode' to support
monitor navigation function.

Signed-off-by: Joe Wu <joewu@msi.corp-partner.google.com>

---

 drivers/input/keyboard/cros_ec_keyb.c          | 15 +++++++++++++++
 include/linux/platform_data/cros_ec_commands.h |  3 +++
 2 files changed, 18 insertions(+)

Comments

Prashant Malani Dec. 15, 2022, 10:58 p.m. UTC | #1
Hi Joe,

On Wed, Dec 14, 2022 at 10:15 PM Joe Wu
<joewu@msi.corp-partner.google.com> wrote:
>
> This patch is to add extra 3 buttons: 'brightness up',
> 'brightness down' and 'leave PC(cros) mode' to support
> monitor navigation function.

nit: Please use the imperative form [1] while describing patches.
In this case, that would translate to:
"Add extra 3 buttons..."

Thanks,

-Prashant

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
joewu(吳仲振) Dec. 16, 2022, 1:36 a.m. UTC | #2
Hi Dmitry,

Thank you for comments. I will try to give a clear description and sent the patch again.
Actually, the function of "leave PC(cros) mode" button is to implement 'lock the screen' in chromeOS.
Google suggests us to sent 'F13' key to lock the screen, could we do it or do you have any suggestion? 

BRs,
Joe

-----Original Message-----
From: Dmitry Torokhov <dmitry.torokhov@gmail.com> 
Sent: Friday, December 16, 2022 6:01 AM
To: Joe Wu <joewu@msi.corp-partner.google.com>
Cc: LKML <linux-kernel@vger.kernel.org>; Stephen Boyd <swboyd@chromium.org>; Xiang wangx <wangxiang@cdjrlc.com>; linux-input@vger.kernel.org; Derek Huang <derekhuang@google.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Benson Leung <bleung@chromium.org>; Daisuke Nojiri <dnojiri@chromium.org>; Prashant Malani <pmalani@chromium.org>; Furquan Shaikh <furquan@chromium.org>; Tzung-Bi Shih <tzungbi@kernel.org>; chrome-platform@lists.linux.dev; Douglas Anderson <dianders@chromium.org>; Dustin L . Howett <dustin@howett.net>; Guenter Roeck <groeck@chromium.org>; Gustavo A . R . Silva <gustavoars@kernel.org>; Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: Re: [PATCH] cros_ec_keyb: Add 3 buttons for monitor function

Hi Joe,

On Thu, Dec 15, 2022 at 02:15:50PM +0800, Joe Wu wrote:
> This patch is to add extra 3 buttons: 'brightness up', 'brightness 
> down' and 'leave PC(cros) mode' to support monitor navigation 
> function.

I understand "brightness up" and "brightness down" but I have no idea what "leave PC(cros) mode" means. I also do not think we should be abusing F13 for this.

Thanks.

> 
> Signed-off-by: Joe Wu <joewu@msi.corp-partner.google.com>
> 
> ---
> 
>  drivers/input/keyboard/cros_ec_keyb.c          | 15 +++++++++++++++
>  include/linux/platform_data/cros_ec_commands.h |  3 +++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
> b/drivers/input/keyboard/cros_ec_keyb.c
> index c14136b733a9..bf1cf0b782d2 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -100,6 +100,21 @@ static const struct cros_ec_bs_map cros_ec_keyb_bs[] = {
>  		.code		= KEY_VOLUMEDOWN,
>  		.bit		= EC_MKBP_VOL_DOWN,
>  	},
> +	{
> +		.ev_type        = EV_KEY,
> +		.code           = KEY_BRIGHTNESSUP,
> +		.bit            = EC_MKBP_BRI_UP,
> +	},
> +	{
> +		.ev_type        = EV_KEY,
> +		.code           = KEY_BRIGHTNESSDOWN,
> +		.bit            = EC_MKBP_BRI_DOWN,
> +	},
> +	{
> +		.ev_type        = EV_KEY,
> +		.code           = KEY_F13,
> +		.bit            = EC_MKBP_CROS_LEAVE,
> +	},
>  
>  	/* Switches */
>  	{
> diff --git a/include/linux/platform_data/cros_ec_commands.h 
> b/include/linux/platform_data/cros_ec_commands.h
> index 5744a2d746aa..79218da8a8cb 100644
> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -3471,6 +3471,9 @@ struct ec_response_get_next_event_v1 {
>  #define EC_MKBP_VOL_UP		1
>  #define EC_MKBP_VOL_DOWN	2
>  #define EC_MKBP_RECOVERY	3
> +#define EC_MKBP_BRI_UP          4
> +#define EC_MKBP_BRI_DOWN        5
> +#define EC_MKBP_CROS_LEAVE      6
>  
>  /* Switches */
>  #define EC_MKBP_LID_OPEN	0
> --
> 2.17.1
>
Dmitry Torokhov Dec. 16, 2022, 7:19 p.m. UTC | #3
On Fri, Dec 16, 2022 at 01:36:24AM +0000, joewu(吳仲振) wrote:
> Hi Dmitry,
> 
> Thank you for comments. I will try to give a clear description and sent the patch again.
> Actually, the function of "leave PC(cros) mode" button is to implement 'lock the screen' in chromeOS.
> Google suggests us to sent 'F13' key to lock the screen, could we do it or do you have any suggestion? 

Is this different from the normal power button? If it is the key with
"padlock" glyph you want to map it to KEY_SLEEP or KEY_SCREENLOCK.

We should no longer use or recommend overriding F13 for this.  Is there
an internal Google bug for this? Could you add dtor@google.com there?

Thanks.
diff mbox series

Patch

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index c14136b733a9..bf1cf0b782d2 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -100,6 +100,21 @@  static const struct cros_ec_bs_map cros_ec_keyb_bs[] = {
 		.code		= KEY_VOLUMEDOWN,
 		.bit		= EC_MKBP_VOL_DOWN,
 	},
+	{
+		.ev_type        = EV_KEY,
+		.code           = KEY_BRIGHTNESSUP,
+		.bit            = EC_MKBP_BRI_UP,
+	},
+	{
+		.ev_type        = EV_KEY,
+		.code           = KEY_BRIGHTNESSDOWN,
+		.bit            = EC_MKBP_BRI_DOWN,
+	},
+	{
+		.ev_type        = EV_KEY,
+		.code           = KEY_F13,
+		.bit            = EC_MKBP_CROS_LEAVE,
+	},
 
 	/* Switches */
 	{
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 5744a2d746aa..79218da8a8cb 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -3471,6 +3471,9 @@  struct ec_response_get_next_event_v1 {
 #define EC_MKBP_VOL_UP		1
 #define EC_MKBP_VOL_DOWN	2
 #define EC_MKBP_RECOVERY	3
+#define EC_MKBP_BRI_UP          4
+#define EC_MKBP_BRI_DOWN        5
+#define EC_MKBP_CROS_LEAVE      6
 
 /* Switches */
 #define EC_MKBP_LID_OPEN	0