From: Bastien Nocera <hadess@hadess.net> To: "Stefan Brüns" <stefan.bruens@rwth-aachen.de> Cc: platform-driver-x86@vger.kernel.org, linux-input@vger.kernel.org, AceLan Kao <acelan.kao@canonical.com>, Andy Shevchenko <andy@infradead.org>, Darren Hart <dvhart@infradead.org>, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 4/5] platform/x86: intel-vbtn: support KEY_ROTATE_LOCK_TOGGLE Date: Fri, 10 Nov 2017 01:39:47 +0100 [thread overview] Message-ID: <1510274387.2624.78.camel@hadess.net> (raw) In-Reply-To: <6070680.Ajd9sKXNqZ@pebbles> On Fri, 2017-11-10 at 01:15 +0100, Stefan Brüns wrote: > On Friday, November 10, 2017 12:30:46 AM CET Bastien Nocera wrote: > > On Thu, 2017-11-09 at 23:44 +0100, Stefan Brüns wrote: > > > The Rotate Lock button event is emitted on the XPS 12 (BIOS A8, > > > but > > > not > > > on BIOS A2). > > > > > > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> > > > --- > > > > > > Changes in v2: > > > - Emit KEY_ROTATE_LOCK_TOGGLE instead of KEY_ROTATE_DISPLAY > > > - Use separate up/down events > > > > > > drivers/platform/x86/intel-vbtn.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/platform/x86/intel-vbtn.c > > > b/drivers/platform/x86/intel-vbtn.c index > > > e3f6375af85c..a484bcc6393b > > > 100644 > > > --- a/drivers/platform/x86/intel-vbtn.c > > > +++ b/drivers/platform/x86/intel-vbtn.c > > > @@ -42,6 +42,8 @@ static const struct key_entry > > > intel_vbtn_keymap[] = { > > > > > > { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* > > > volume-up key release */ > > > { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* > > > volume-down key press */ > > > { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* > > > volume-down key release > > */ > > > > > > + { KE_KEY, 0xC8, { KEY_ROTATE_LOCK_TOGGLE } }, > > > /* rotate-lock key > > > press */ + { KE_KEY, 0xC9, { KEY_ROTATE_LOCK_TOGGLE } > > > }, /* > > > rotate-lock key release */ > > > > How are those events sent? When pressing and releasing the key, do > > you > > receive 0xC8 followed by 0xC9? Or do you receive 0xC8 when pressing > > and > > releasing the first time, and 0xC9 when pressing and releasing a > > second > > time? > > > > If the former, then it's not going to work. The release is supposed > > to > > be ignored, as you send the event with > > sparse_keymap_report_event(). > > > > If the latter, and there's an actual state, does it disable a > > device > > on-board, or activate an LED? If so, then it would need to be a > > switch, > > not a key. > > Do you think I don't test the patches before sending? Let me tell > you, it > *does* work. > > You could also read the cover letter, where you find more details, > putting the > patches in relation to each other. I guess this was so clear that Darren made the same assumption as me. I also never said that it wouldn't work, which is why I used "pretty sure", and asked questions. > Just in case its not yet clear: > The codes are emitted when pressing a button. It is a button, not a > switch. > There is no state handled in hardware. On press (as noted by the > code > comment), event code 0xc8 is emitted. On release, event code 0xc9 is > emitted. >From the looks of things, some devices only send the keypresses, and never the key release, otherwise what would be the point of ignoring the key release in that table, say for the volume buttons? It's clear as mud, and there's no comments in the driver to explain that. If you don't want to answer questions, or are going to be as pissy as your replies have been, I'm happy to not continue reviewing your patches. I find patch 2 unreadable, but it might be my tiny tiny little brain.
WARNING: multiple messages have this Message-ID (diff)
From: Bastien Nocera <hadess@hadess.net> To: "Stefan Brüns" <stefan.bruens@rwth-aachen.de> Cc: platform-driver-x86@vger.kernel.org, linux-input@vger.kernel.org, AceLan Kao <acelan.kao@canonical.com>, Andy Shevchenko <andy@infradead.org>, Darren Hart <dvhart@infradead.org>, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 4/5] platform/x86: intel-vbtn: support KEY_ROTATE_LOCK_TOGGLE Date: Fri, 10 Nov 2017 01:39:47 +0100 [thread overview] Message-ID: <1510274387.2624.78.camel@hadess.net> (raw) In-Reply-To: <6070680.Ajd9sKXNqZ@pebbles> On Fri, 2017-11-10 at 01:15 +0100, Stefan Brüns wrote: > On Friday, November 10, 2017 12:30:46 AM CET Bastien Nocera wrote: > > On Thu, 2017-11-09 at 23:44 +0100, Stefan Brüns wrote: > > > The Rotate Lock button event is emitted on the XPS 12 (BIOS A8, > > > but > > > not > > > on BIOS A2). > > > > > > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> > > > --- > > > > > > Changes in v2: > > > - Emit KEY_ROTATE_LOCK_TOGGLE instead of KEY_ROTATE_DISPLAY > > > - Use separate up/down events > > > > > > drivers/platform/x86/intel-vbtn.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/platform/x86/intel-vbtn.c > > > b/drivers/platform/x86/intel-vbtn.c index > > > e3f6375af85c..a484bcc6393b > > > 100644 > > > --- a/drivers/platform/x86/intel-vbtn.c > > > +++ b/drivers/platform/x86/intel-vbtn.c > > > @@ -42,6 +42,8 @@ static const struct key_entry > > > intel_vbtn_keymap[] = { > > > > > > { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* > > > volume-up key release */ > > > { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* > > > volume-down key press */ > > > { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* > > > volume-down key release > > */ > > > > > > + { KE_KEY, 0xC8, { KEY_ROTATE_LOCK_TOGGLE } }, > > > /* rotate-lock key > > > press */ + { KE_KEY, 0xC9, { KEY_ROTATE_LOCK_TOGGLE } > > > }, /* > > > rotate-lock key release */ > > > > How are those events sent? When pressing and releasing the key, do > > you > > receive 0xC8 followed by 0xC9? Or do you receive 0xC8 when pressing > > and > > releasing the first time, and 0xC9 when pressing and releasing a > > second > > time? > > > > If the former, then it's not going to work. The release is supposed > > to > > be ignored, as you send the event with > > sparse_keymap_report_event(). > > > > If the latter, and there's an actual state, does it disable a > > device > > on-board, or activate an LED? If so, then it would need to be a > > switch, > > not a key. > > Do you think I don't test the patches before sending? Let me tell > you, it > *does* work. > > You could also read the cover letter, where you find more details, > putting the > patches in relation to each other. I guess this was so clear that Darren made the same assumption as me. I also never said that it wouldn't work, which is why I used "pretty sure", and asked questions. > Just in case its not yet clear: > The codes are emitted when pressing a button. It is a button, not a > switch. > There is no state handled in hardware. On press (as noted by the > code > comment), event code 0xc8 is emitted. On release, event code 0xc9 is > emitted. From the looks of things, some devices only send the keypresses, and never the key release, otherwise what would be the point of ignoring the key release in that table, say for the volume buttons? It's clear as mud, and there's no comments in the driver to explain that. If you don't want to answer questions, or are going to be as pissy as your replies have been, I'm happy to not continue reviewing your patches. I find patch 2 unreadable, but it might be my tiny tiny little brain.
next prev parent reply other threads:[~2017-11-10 0:39 UTC|newest] Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20171109224436.16472-1-stefan.bruens@rwth-aachen.de> 2017-11-09 22:44 ` [PATCH v2 1/5] platform/x86: intel-vbtn: support SW_TABLET_MODE Stefan Brüns 2017-11-09 22:44 ` Stefan Brüns 2017-11-09 22:44 ` [PATCH v2 2/5] platform/x86: intel-vbtn: Support separate press/release events Stefan Brüns 2017-11-09 22:44 ` Stefan Brüns 2017-11-10 1:34 ` Darren Hart 2017-11-10 1:58 ` Stefan Brüns 2017-11-10 1:58 ` Stefan Brüns 2017-11-10 2:11 ` Darren Hart 2017-11-10 2:44 ` Stefan Brüns 2017-11-10 2:44 ` Stefan Brüns 2017-11-10 2:15 ` Darren Hart 2017-12-08 20:33 ` Stefan Brüns 2017-12-08 20:33 ` Stefan Brüns 2017-11-09 22:44 ` [PATCH v2 3/5] Input: add KEY_ROTATE_LOCK_TOGGLE Stefan Brüns 2017-11-09 22:44 ` Stefan Brüns 2017-11-09 23:34 ` Bastien Nocera 2017-11-30 17:51 ` Brüns, Stefan 2017-11-30 17:51 ` Brüns, Stefan 2017-12-05 18:50 ` Jason Gerecke 2017-12-05 18:50 ` Jason Gerecke 2018-01-25 16:23 ` Jason Gerecke 2017-11-10 1:41 ` Darren Hart 2017-11-09 22:44 ` [PATCH v2 4/5] platform/x86: intel-vbtn: support KEY_ROTATE_LOCK_TOGGLE Stefan Brüns 2017-11-09 22:44 ` Stefan Brüns 2017-11-09 23:30 ` Bastien Nocera 2017-11-09 23:46 ` Darren Hart 2017-11-10 0:23 ` Stefan Brüns 2017-11-10 0:23 ` Stefan Brüns 2017-11-10 0:15 ` Stefan Brüns 2017-11-10 0:15 ` Stefan Brüns 2017-11-10 0:39 ` Bastien Nocera [this message] 2017-11-10 0:39 ` Bastien Nocera 2017-11-10 0:53 ` Darren Hart 2017-11-10 1:54 ` Darren Hart 2017-11-10 2:20 ` Stefan Brüns 2017-11-10 2:20 ` Stefan Brüns 2017-11-09 22:44 ` [PATCH v2 5/5] platform/x86: intel-vbtn: support panel front button Stefan Brüns 2017-11-09 22:44 ` Stefan Brüns 2017-11-09 23:40 ` Bastien Nocera 2017-11-10 0:21 ` Stefan Brüns 2017-11-10 0:21 ` Stefan Brüns
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1510274387.2624.78.camel@hadess.net \ --to=hadess@hadess.net \ --cc=acelan.kao@canonical.com \ --cc=andy@infradead.org \ --cc=dvhart@infradead.org \ --cc=linux-input@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=platform-driver-x86@vger.kernel.org \ --cc=stefan.bruens@rwth-aachen.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.