| Message ID | 20221215061550.18993-1-joewu@msi.corp-partner.google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cros_ec_keyb: Add 3 buttons for monitor function | expand |
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 >
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
Hi Prashant, Thank you for comments. I will correct the description to be imperative mood. BRs, Joe -----Original Message----- From: Prashant Malani <pmalani@chromium.org> Sent: Friday, December 16, 2022 6:59 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>; 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>; Dmitry Torokhov <dmitry.torokhov@gmail.com>; 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 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 *****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.
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 >
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 --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
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(+)