[PATCH] cros_ec_keyb: Add 3 buttons for monitor function

[PATCH] cros_ec_keyb: Add 3 buttons for monitor function

[Joe Wu]

Add 3 extra buttons: 'brightness up', 'brightness down'
and 'screen lock' to support monitor manipulating function.

Signed-off-by: Joe Wu <joewu@msi.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 6f435125ec03..e7ecfca838df 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_SCREENLOCK,
+		.bit            = EC_MKBP_SCREEN_LOCK,
+	},
 
 	/* Switches */
 	{
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 5744a2d746aa..a2073ed43972 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_SCREEN_LOCK	6
 
 /* Switches */
 #define EC_MKBP_LID_OPEN	0
-- 
2.17.1

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

[Greg Kroah-Hartman]

On Thu, Dec 22, 2022 at 02:39:50PM +0800, Joe Wu wrote:
> Add 3 extra buttons: 'brightness up', 'brightness down'
> and 'screen lock' to support monitor manipulating function.
> 
> Signed-off-by: Joe Wu <joewu@msi.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 6f435125ec03..e7ecfca838df 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_SCREENLOCK,
> +		.bit            = EC_MKBP_SCREEN_LOCK,
> +	},
>  
>  	/* Switches */
>  	{
> diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> index 5744a2d746aa..a2073ed43972 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_SCREEN_LOCK	6
>  
>  /* Switches */
>  #define EC_MKBP_LID_OPEN	0
> -- 
> 2.17.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- It looks like you did not use your "real" name for the patch on either
  the Signed-off-by: line, or the From: line (both of which have to
  match).  Please read the kernel file,
  Documentation/process/submitting-patches.rst for how to do this
  correctly.

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

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

[Greg Kroah-Hartman]

On Thu, Dec 22, 2022 at 02:39:50PM +0800, Joe Wu wrote:
> Add 3 extra buttons: 'brightness up', 'brightness down'
> and 'screen lock' to support monitor manipulating function.
> 
> Signed-off-by: Joe Wu <joewu@msi.com>

From: line does not match the signed-off-by (and is an invalid email
address...)

Please fix.

thanks,

greg k-h

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

[Dmitry Torokhov]

On Fri, Jan 20, 2023 at 12:26:04PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Dec 22, 2022 at 02:39:50PM +0800, Joe Wu wrote:
> > Add 3 extra buttons: 'brightness up', 'brightness down'
> > and 'screen lock' to support monitor manipulating function.
> > 
> > Signed-off-by: Joe Wu <joewu@msi.com>
> 
> From: line does not match the signed-off-by (and is an invalid email
> address...)

What do you mean "it's an invalid email address"? You can definitely
send emails there... I prefer people not to use Google partner domain
accounts in the hope that their employment might outlast their
involvement in Google projects, but that is it.

I think if we ask people to stick "From: <whatever the company address
is" in the body of the patch we can ignore the difference between sender
address and from/signed-off-by when they use partner domain accounts. If
anything, such accounts have better vetting than a random gmail or other
free email service account some vendors have to create to be able to
send a plain-text emails that we require. I mean, we have
"Signed-off-by: George Spelvin <lkml@sdf.org>" present in our git
history and nobody bats an eye...

Thanks.

-- 
Dmitry

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

[Greg Kroah-Hartman]

On Fri, Jan 20, 2023 at 09:24:18AM -0800, Dmitry Torokhov wrote:
> On Fri, Jan 20, 2023 at 12:26:04PM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Dec 22, 2022 at 02:39:50PM +0800, Joe Wu wrote:
> > > Add 3 extra buttons: 'brightness up', 'brightness down'
> > > and 'screen lock' to support monitor manipulating function.
> > > 
> > > Signed-off-by: Joe Wu <joewu@msi.com>
> > 
> > From: line does not match the signed-off-by (and is an invalid email
> > address...)
> 
> What do you mean "it's an invalid email address"? You can definitely
> send emails there... I prefer people not to use Google partner domain
> accounts in the hope that their employment might outlast their
> involvement in Google projects, but that is it.

I was told that this was not a valid email address to send and receive
emails from, and was only an email alias given to companies to interact
with Google through their gerrit systems.  If that is incorrect, I'll
not complain about this anymore, but someone needs to please verify this
for me before I do so :)

But even if it is valid, should we accept it as a way to get in contact
with the original submitter over time?

> I think if we ask people to stick "From: <whatever the company address
> is" in the body of the patch we can ignore the difference between sender
> address and from/signed-off-by when they use partner domain accounts. If
> anything, such accounts have better vetting than a random gmail or other
> free email service account some vendors have to create to be able to
> send a plain-text emails that we require. I mean, we have
> "Signed-off-by: George Spelvin <lkml@sdf.org>" present in our git
> history and nobody bats an eye...

Oh lots of people "batted an eye" about that one, I've had too many
meetings with lawyers about that, which is one reason I now verify email
addresses like I did here.

thanks,

greg k-h

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

[Dmitry Torokhov]

On Sat, Jan 21, 2023 at 08:26:47AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Jan 20, 2023 at 09:24:18AM -0800, Dmitry Torokhov wrote:
> > On Fri, Jan 20, 2023 at 12:26:04PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Dec 22, 2022 at 02:39:50PM +0800, Joe Wu wrote:
> > > > Add 3 extra buttons: 'brightness up', 'brightness down'
> > > > and 'screen lock' to support monitor manipulating function.
> > > > 
> > > > Signed-off-by: Joe Wu <joewu@msi.com>
> > > 
> > > From: line does not match the signed-off-by (and is an invalid email
> > > address...)
> > 
> > What do you mean "it's an invalid email address"? You can definitely
> > send emails there... I prefer people not to use Google partner domain
> > accounts in the hope that their employment might outlast their
> > involvement in Google projects, but that is it.
> 
> I was told that this was not a valid email address to send and receive
> emails from, and was only an email alias given to companies to interact
> with Google through their gerrit systems.  If that is incorrect, I'll
> not complain about this anymore, but someone needs to please verify this
> for me before I do so :)

Not solely for gerrit, there are other systems at Google (for example
the issue tracker) that require Google account and that is why we create
them for partners. It is however a real account that can send and
receive e-mails. Whether anyone is looking at it, especially after they
moved to other projects, is a separate topic, but it is the same with a
random gmail or whatever account that people are creating to submit a
patch or two.

> 
> But even if it is valid, should we accept it as a way to get in contact
> with the original submitter over time?

I believe that a real corp account is preferable for getting in contact
with the engineer who submitted the patch. Unfortunately corp accounts
often unsuitable for submitting patches to the kernel. Outgoing mail
servers either force HTML, mangle the text, or add legal-sounding
footers that result in snark replies. So submitters often try to use
another account to submit the code, such us a throwaway gmail account,
or as in this case, our "partner domain" account. Neither is ideal but
we require from to match sign-off (including email part) and complain
about sane option of overriding "from" in the mail body to be something
more sensitive, like the email that is actually used by the person in
question.

Again, I have more trust for patches sent as "Joe Wu
<joewu@msi.corp-partner.google.com>" with body

From: Joe Wu <joewu@msi.com>
...
Signed-off-by: Joe Wu <joewu@msi.com>
...

then patches sent and signed off as "Joe Wu <joemsi-oss@gmail.com>"
because I actually know that there is a process for establishing and
managing that @msi.corp-partner.google.com.

> 
> > I think if we ask people to stick "From: <whatever the company address
> > is" in the body of the patch we can ignore the difference between sender
> > address and from/signed-off-by when they use partner domain accounts. If
> > anything, such accounts have better vetting than a random gmail or other
> > free email service account some vendors have to create to be able to
> > send a plain-text emails that we require. I mean, we have
> > "Signed-off-by: George Spelvin <lkml@sdf.org>" present in our git
> > history and nobody bats an eye...
> 
> Oh lots of people "batted an eye" about that one, I've had too many
> meetings with lawyers about that, which is one reason I now verify email
> addresses like I did here.

Would you attempt to verify it is you saw a sign-off from "Daniil Kharms
<dank@gmail.com>"? What is the trigger for verification? How rigorous is
it?

Thanks.

-- 
Dmitry

[PATCH] cros_ec_keyb: Add 3 buttons for monitor function

[Joe Wu]

From: Joe Wu <joewu@msi.com>

Add 3 extra buttons: 'brightness up', 'brightness down'
and 'screen lock' to support monitor manipulating function.

Signed-off-by: Joe Wu <joewu@msi.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 6f435125ec03..e7ecfca838df 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_SCREENLOCK,
+		.bit            = EC_MKBP_SCREEN_LOCK,
+	},
 
 	/* Switches */
 	{
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 5744a2d746aa..502f0397a402 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_SCREEN_LOCK     6
 
 /* Switches */
 #define EC_MKBP_LID_OPEN	0
-- 
2.17.1

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

[Greg Kroah-Hartman]

On Thu, Dec 22, 2022 at 10:15:42AM +0800, Joe Wu wrote:
> --- 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_SCREEN_LOCK     6

You forgot to use tabs :(

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

[joewu(吳仲振)]

Hi Greg,

I'll fix it. Thanks.

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

On Thu, Dec 22, 2022 at 10:15:42AM +0800, Joe Wu wrote:
> --- 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_SCREEN_LOCK     6

You forgot to use tabs :(


*****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.

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

[Greg Kroah-Hartman]

On Thu, Dec 22, 2022 at 10:15:42AM +0800, Joe Wu wrote:
> From: Joe Wu <joewu@msi.com>

Does not match the "From:" line on your email, why?

thanks,

greg k-h

[PATCH] cros_ec_keyb: Add 3 buttons for monitor function

[Joe Wu]

Add 3 extra buttons: 'brightness up', 'brightness down'
and 'screen lock' to support monitor manipulating 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(+)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 6f435125ec03..e7ecfca838df 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_SCREENLOCK,
+		.bit            = EC_MKBP_SCREEN_LOCK,
+	},
 
 	/* Switches */
 	{
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 5744a2d746aa..502f0397a402 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_SCREEN_LOCK     6
 
 /* Switches */
 #define EC_MKBP_LID_OPEN	0
-- 
2.17.1

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

[Greg Kroah-Hartman]

On Tue, Dec 20, 2022 at 04:02:26PM +0800, Joe Wu wrote:
> Add 3 extra buttons: 'brightness up', 'brightness down'
> and 'screen lock' to support monitor manipulating function.
> 
> Signed-off-by: Joe Wu <joewu@msi.corp-partner.google.com>

Note, this is a "fake" email address, please always use your real one
and not a google-gerrit-assigned address.

thanks,

greg k-h

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

[joewu(吳仲振)]

Hi Greg,

I will correct it. Thank you for comments.

Joe

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

On Tue, Dec 20, 2022 at 04:02:26PM +0800, Joe Wu wrote:
> Add 3 extra buttons: 'brightness up', 'brightness down'
> and 'screen lock' to support monitor manipulating function.
> 
> Signed-off-by: Joe Wu <joewu@msi.corp-partner.google.com>

Note, this is a "fake" email address, please always use your real one and not a google-gerrit-assigned address.

thanks,

greg k-h

*****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.

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

[Greg Kroah-Hartman]

On Thu, Dec 22, 2022 at 01:50:15AM +0000, joewu(吳仲振) wrote:
> *****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.

Now deleted.

[PATCH] cros_ec_keyb: Add 3 buttons for monitor function

[Joe Wu]

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(+)

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

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

[Dmitry Torokhov]

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

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

[joewu(吳仲振)]

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

*****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.

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

[Dmitry Torokhov]

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.

-- 
Dmitry

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

[Prashant Malani]

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

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

[joewu(吳仲振)]

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.