2022-12-15 22:39:29

by Dmitry Torokhov

[permalink] [raw]
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 <[email protected]>
>
> ---
>
> 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


2022-12-16 02:06:32

by joewu(吳仲振)

[permalink] [raw]
Subject: RE: [PATCH] cros_ec_keyb: Add 3 buttons for monitor function

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 <[email protected]>
Sent: Friday, December 16, 2022 6:01 AM
To: Joe Wu <[email protected]>
Cc: LKML <[email protected]>; Stephen Boyd <[email protected]>; Xiang wangx <[email protected]>; [email protected]; Derek Huang <[email protected]>; Greg Kroah-Hartman <[email protected]>; Benson Leung <[email protected]>; Daisuke Nojiri <[email protected]>; Prashant Malani <[email protected]>; Furquan Shaikh <[email protected]>; Tzung-Bi Shih <[email protected]>; [email protected]; Douglas Anderson <[email protected]>; Dustin L . Howett <[email protected]>; Guenter Roeck <[email protected]>; Gustavo A . R . Silva <[email protected]>; Sebastian Reichel <[email protected]>
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 <[email protected]>
>
> ---
>
> 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

*****CONFIDENTIAL INFORMATION*****

This email is intended only for the use of the person or entity to whom it is
addressed and contains information that may be subject to and/or may be
restricted from disclosure by contract or applicable law. If you are not the
intended recipient of this email, be advised that any disclosure, copy,
distribution or use of the contents of this message is strictly prohibited.
If you are not the intended recipient of this email, please notify the sender
that you have received this in error by replying to this message. Then,
please delete it from your system. Our Privacy Policy is available here
https://www.msi.com/page/privacy-policy. Thank you.

2022-12-16 19:53:58

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] cros_ec_keyb: Add 3 buttons for monitor function

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 [email protected] there?

Thanks.

--
Dmitry