Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1556303 > unrolled thread
| Started by | Michał Kępień <kernel@kempniu.pl> |
|---|---|
| First post | 2017-01-11 10:10 +0100 |
| Last post | 2017-01-13 23:10 +0100 |
| Articles | 10 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup Michał Kępień <kernel@kempniu.pl> - 2017-01-11 10:10 +0100
[PATCH 3/4] platform/x86: fujitsu-laptop: break up complex loop condition Michał Kępień <kernel@kempniu.pl> - 2017-01-11 10:10 +0100
Re: [PATCH 3/4] platform/x86: fujitsu-laptop: break up complex loop condition Jonathan Woithe <jwoithe@just42.net> - 2017-01-13 13:50 +0100
[PATCH 2/4] platform/x86: fujitsu-laptop: move keycode processing to separate functions Michał Kępień <kernel@kempniu.pl> - 2017-01-11 10:10 +0100
Re: [PATCH 2/4] platform/x86: fujitsu-laptop: move keycode processing to separate functions Jonathan Woithe <jwoithe@just42.net> - 2017-01-13 14:00 +0100
Re: [PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup Jonathan Woithe <jwoithe@just42.net> - 2017-01-11 13:00 +0100
Re: [PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup Michał Kępień <kernel@kempniu.pl> - 2017-01-11 13:30 +0100
Re: [PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup Jonathan Woithe <jwoithe@just42.net> - 2017-01-11 13:50 +0100
Re: [PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup Darren Hart <dvhart@infradead.org> - 2017-01-13 23:00 +0100
Re: [PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup Darren Hart <dvhart@infradead.org> - 2017-01-13 23:10 +0100
| From | Michał Kępień <kernel@kempniu.pl> |
|---|---|
| Date | 2017-01-11 10:10 +0100 |
| Subject | [PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup |
| Message-ID | <sYmVc-4JW-11@gated-at.bofh.it> |
I am currently preparing a patch series which makes fujitsu-laptop use a sparse keymap for hotkey handling. Before that will happen, though, acpi_fujitsu_hotkey_notify() could use a revamp because it is pretty hard to read as it is. To avoid posting everything at once, here are a few patches which IMHO make that function easier to read. Some of these changes might be a matter of taste, so feel free to NACK them or suggest a preferred alternative. drivers/platform/x86/fujitsu-laptop.c | 174 ++++++++++++++++++---------------- 1 file changed, 92 insertions(+), 82 deletions(-) -- 2.11.0
[toc] | [next] | [standalone]
| From | Michał Kępień <kernel@kempniu.pl> |
|---|---|
| Date | 2017-01-11 10:10 +0100 |
| Subject | [PATCH 3/4] platform/x86: fujitsu-laptop: break up complex loop condition |
| Message-ID | <sYn4S-52M-43@gated-at.bofh.it> |
| In reply to | #1556303 |
The loop condition in acpi_fujitsu_hotkey_release() includes an
assignment, a four-argument function call and a comparison, making it
hard to read. Separate the assignment from the comparison to improve
readability.
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
drivers/platform/x86/fujitsu-laptop.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index e57d3724d2ce..06653a8594ed 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -1054,11 +1054,13 @@ static void acpi_fujitsu_hotkey_release(void)
struct input_dev *input = fujitsu_hotkey->input;
int keycode, status;
- while ((status = kfifo_out_locked(&fujitsu_hotkey->fifo,
+ while (true) {
+ status = kfifo_out_locked(&fujitsu_hotkey->fifo,
(unsigned char *)&keycode,
sizeof(keycode),
- &fujitsu_hotkey->fifo_lock))
- == sizeof(keycode)) {
+ &fujitsu_hotkey->fifo_lock);
+ if (status != sizeof(keycode))
+ return;
input_report_key(input, keycode, 0);
input_sync(input);
vdbg_printk(FUJLAPTOP_DBG_TRACE,
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Woithe <jwoithe@just42.net> |
|---|---|
| Date | 2017-01-13 13:50 +0100 |
| Subject | Re: [PATCH 3/4] platform/x86: fujitsu-laptop: break up complex loop condition |
| Message-ID | <sZ9sS-Vi-19@gated-at.bofh.it> |
| In reply to | #1556306 |
On Wed, Jan 11, 2017 at 09:59:32AM +0100, Micha?? K??pie?? wrote:
> The loop condition in acpi_fujitsu_hotkey_release() includes an
> assignment, a four-argument function call and a comparison, making it
> hard to read. Separate the assignment from the comparison to improve
> readability.
>
> Signed-off-by: Micha?? K??pie?? <kernel@kempniu.pl>
Acked-by: Jonathan Woithe <jwoithe@just42.net>
> ---
> drivers/platform/x86/fujitsu-laptop.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index e57d3724d2ce..06653a8594ed 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -1054,11 +1054,13 @@ static void acpi_fujitsu_hotkey_release(void)
> struct input_dev *input = fujitsu_hotkey->input;
> int keycode, status;
>
> - while ((status = kfifo_out_locked(&fujitsu_hotkey->fifo,
> + while (true) {
> + status = kfifo_out_locked(&fujitsu_hotkey->fifo,
> (unsigned char *)&keycode,
> sizeof(keycode),
> - &fujitsu_hotkey->fifo_lock))
> - == sizeof(keycode)) {
> + &fujitsu_hotkey->fifo_lock);
> + if (status != sizeof(keycode))
> + return;
> input_report_key(input, keycode, 0);
> input_sync(input);
> vdbg_printk(FUJLAPTOP_DBG_TRACE,
> --
> 2.11.0
--
[toc] | [prev] | [next] | [standalone]
| From | Michał Kępień <kernel@kempniu.pl> |
|---|---|
| Date | 2017-01-11 10:10 +0100 |
| Subject | [PATCH 2/4] platform/x86: fujitsu-laptop: move keycode processing to separate functions |
| Message-ID | <sYn4S-52M-49@gated-at.bofh.it> |
| In reply to | #1556303 |
acpi_fujitsu_hotkey_notify() is pretty deeply nested, which hurts
readability. Move the keycode processing part to two separate functions
to make the code easier to understand and save a few line breaks.
Rename variable keycode_r to keycode as there is no longer any need to
differentiate between the two. Tweak indentations to make checkpatch
happy.
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
drivers/platform/x86/fujitsu-laptop.c | 76 ++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 33 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index c2022f8af51b..e57d3724d2ce 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -1030,12 +1030,48 @@ static int acpi_fujitsu_hotkey_remove(struct acpi_device *device)
return 0;
}
+static void acpi_fujitsu_hotkey_press(int keycode)
+{
+ struct input_dev *input = fujitsu_hotkey->input;
+ int status;
+
+ vdbg_printk(FUJLAPTOP_DBG_TRACE,
+ "Push keycode into ringbuffer [%d]\n", keycode);
+ status = kfifo_in_locked(&fujitsu_hotkey->fifo,
+ (unsigned char *)&keycode, sizeof(keycode),
+ &fujitsu_hotkey->fifo_lock);
+ if (status != sizeof(keycode)) {
+ vdbg_printk(FUJLAPTOP_DBG_WARN,
+ "Could not push keycode [0x%x]\n", keycode);
+ } else {
+ input_report_key(input, keycode, 1);
+ input_sync(input);
+ }
+}
+
+static void acpi_fujitsu_hotkey_release(void)
+{
+ struct input_dev *input = fujitsu_hotkey->input;
+ int keycode, status;
+
+ while ((status = kfifo_out_locked(&fujitsu_hotkey->fifo,
+ (unsigned char *)&keycode,
+ sizeof(keycode),
+ &fujitsu_hotkey->fifo_lock))
+ == sizeof(keycode)) {
+ input_report_key(input, keycode, 0);
+ input_sync(input);
+ vdbg_printk(FUJLAPTOP_DBG_TRACE,
+ "Pop keycode from ringbuffer [%d]\n", keycode);
+ }
+}
+
static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
{
struct input_dev *input;
- int keycode, keycode_r;
+ int keycode;
unsigned int irb = 1;
- int i, status;
+ int i;
input = fujitsu_hotkey->input;
@@ -1083,37 +1119,11 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
keycode = -1;
break;
}
- if (keycode > 0) {
- vdbg_printk(FUJLAPTOP_DBG_TRACE,
- "Push keycode into ringbuffer [%d]\n",
- keycode);
- status = kfifo_in_locked(&fujitsu_hotkey->fifo,
- (unsigned char *)&keycode,
- sizeof(keycode),
- &fujitsu_hotkey->fifo_lock);
- if (status != sizeof(keycode)) {
- vdbg_printk(FUJLAPTOP_DBG_WARN,
- "Could not push keycode [0x%x]\n",
- keycode);
- } else {
- input_report_key(input, keycode, 1);
- input_sync(input);
- }
- } else if (keycode == 0) {
- while ((status =
- kfifo_out_locked(
- &fujitsu_hotkey->fifo,
- (unsigned char *) &keycode_r,
- sizeof(keycode_r),
- &fujitsu_hotkey->fifo_lock))
- == sizeof(keycode_r)) {
- input_report_key(input, keycode_r, 0);
- input_sync(input);
- vdbg_printk(FUJLAPTOP_DBG_TRACE,
- "Pop keycode from ringbuffer [%d]\n",
- keycode_r);
- }
- }
+
+ if (keycode > 0)
+ acpi_fujitsu_hotkey_press(keycode);
+ else if (keycode == 0)
+ acpi_fujitsu_hotkey_release();
}
/* On some models (first seen on the Skylake-based Lifebook
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Woithe <jwoithe@just42.net> |
|---|---|
| Date | 2017-01-13 14:00 +0100 |
| Subject | Re: [PATCH 2/4] platform/x86: fujitsu-laptop: move keycode processing to separate functions |
| Message-ID | <sZ9Cy-Ys-1@gated-at.bofh.it> |
| In reply to | #1556307 |
On Wed, Jan 11, 2017 at 09:59:31AM +0100, Micha?? K??pie?? wrote:
> acpi_fujitsu_hotkey_notify() is pretty deeply nested, which hurts
> readability. Move the keycode processing part to two separate functions
> to make the code easier to understand and save a few line breaks.
> Rename variable keycode_r to keycode as there is no longer any need to
> differentiate between the two. Tweak indentations to make checkpatch
> happy.
>
> Signed-off-by: Micha?? K??pie?? <kernel@kempniu.pl>
Acked-by: Jonathan Woithe <jwoithe@just42.net>
> ---
> drivers/platform/x86/fujitsu-laptop.c | 76 ++++++++++++++++++++---------------
> 1 file changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index c2022f8af51b..e57d3724d2ce 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -1030,12 +1030,48 @@ static int acpi_fujitsu_hotkey_remove(struct acpi_device *device)
> return 0;
> }
>
> +static void acpi_fujitsu_hotkey_press(int keycode)
> +{
> + struct input_dev *input = fujitsu_hotkey->input;
> + int status;
> +
> + vdbg_printk(FUJLAPTOP_DBG_TRACE,
> + "Push keycode into ringbuffer [%d]\n", keycode);
> + status = kfifo_in_locked(&fujitsu_hotkey->fifo,
> + (unsigned char *)&keycode, sizeof(keycode),
> + &fujitsu_hotkey->fifo_lock);
> + if (status != sizeof(keycode)) {
> + vdbg_printk(FUJLAPTOP_DBG_WARN,
> + "Could not push keycode [0x%x]\n", keycode);
> + } else {
> + input_report_key(input, keycode, 1);
> + input_sync(input);
> + }
> +}
> +
> +static void acpi_fujitsu_hotkey_release(void)
> +{
> + struct input_dev *input = fujitsu_hotkey->input;
> + int keycode, status;
> +
> + while ((status = kfifo_out_locked(&fujitsu_hotkey->fifo,
> + (unsigned char *)&keycode,
> + sizeof(keycode),
> + &fujitsu_hotkey->fifo_lock))
> + == sizeof(keycode)) {
> + input_report_key(input, keycode, 0);
> + input_sync(input);
> + vdbg_printk(FUJLAPTOP_DBG_TRACE,
> + "Pop keycode from ringbuffer [%d]\n", keycode);
> + }
> +}
> +
> static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
> {
> struct input_dev *input;
> - int keycode, keycode_r;
> + int keycode;
> unsigned int irb = 1;
> - int i, status;
> + int i;
>
> input = fujitsu_hotkey->input;
>
> @@ -1083,37 +1119,11 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
> keycode = -1;
> break;
> }
> - if (keycode > 0) {
> - vdbg_printk(FUJLAPTOP_DBG_TRACE,
> - "Push keycode into ringbuffer [%d]\n",
> - keycode);
> - status = kfifo_in_locked(&fujitsu_hotkey->fifo,
> - (unsigned char *)&keycode,
> - sizeof(keycode),
> - &fujitsu_hotkey->fifo_lock);
> - if (status != sizeof(keycode)) {
> - vdbg_printk(FUJLAPTOP_DBG_WARN,
> - "Could not push keycode [0x%x]\n",
> - keycode);
> - } else {
> - input_report_key(input, keycode, 1);
> - input_sync(input);
> - }
> - } else if (keycode == 0) {
> - while ((status =
> - kfifo_out_locked(
> - &fujitsu_hotkey->fifo,
> - (unsigned char *) &keycode_r,
> - sizeof(keycode_r),
> - &fujitsu_hotkey->fifo_lock))
> - == sizeof(keycode_r)) {
> - input_report_key(input, keycode_r, 0);
> - input_sync(input);
> - vdbg_printk(FUJLAPTOP_DBG_TRACE,
> - "Pop keycode from ringbuffer [%d]\n",
> - keycode_r);
> - }
> - }
> +
> + if (keycode > 0)
> + acpi_fujitsu_hotkey_press(keycode);
> + else if (keycode == 0)
> + acpi_fujitsu_hotkey_release();
> }
>
> /* On some models (first seen on the Skylake-based Lifebook
> --
> 2.11.0
--
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Woithe <jwoithe@just42.net> |
|---|---|
| Date | 2017-01-11 13:00 +0100 |
| Message-ID | <sYpJo-6v8-23@gated-at.bofh.it> |
| In reply to | #1556303 |
On Wed, Jan 11, 2017 at 09:59:29AM +0100, Micha?? K??pie?? wrote: > I am currently preparing a patch series which makes fujitsu-laptop use a > sparse keymap for hotkey handling. Before that will happen, though, > acpi_fujitsu_hotkey_notify() could use a revamp because it is pretty > hard to read as it is. To avoid posting everything at once, here are a > few patches which IMHO make that function easier to read. Some of these > changes might be a matter of taste, so feel free to NACK them or suggest > a preferred alternative. This patch series provides a significant clean up to the functions it focuses on. As such I have no real objections to them. However, because my Fujitsu laptop doesn't have any of the hotkeys of later models I am unable to test these patches with real hardware. Have you been able to do so? If they have been verified I have no problem acking these. Otherwise I will have to do as much as I can (given no access to relevant hardware) to ensure the overall behaviour isn't changed. Regards jonathan
[toc] | [prev] | [next] | [standalone]
| From | Michał Kępień <kernel@kempniu.pl> |
|---|---|
| Date | 2017-01-11 13:30 +0100 |
| Message-ID | <sYqcp-6Ty-1@gated-at.bofh.it> |
| In reply to | #1556425 |
> On Wed, Jan 11, 2017 at 09:59:29AM +0100, Micha?? K??pie?? wrote:
> > I am currently preparing a patch series which makes fujitsu-laptop use a
> > sparse keymap for hotkey handling. Before that will happen, though,
> > acpi_fujitsu_hotkey_notify() could use a revamp because it is pretty
> > hard to read as it is. To avoid posting everything at once, here are a
> > few patches which IMHO make that function easier to read. Some of these
> > changes might be a matter of taste, so feel free to NACK them or suggest
> > a preferred alternative.
>
> This patch series provides a significant clean up to the functions it
> focuses on. As such I have no real objections to them. However, because my
> Fujitsu laptop doesn't have any of the hotkeys of later models I am unable
> to test these patches with real hardware. Have you been able to do so? If
> they have been verified I have no problem acking these. Otherwise I will
> have to do as much as I can (given no access to relevant hardware) to ensure
> the overall behaviour isn't changed.
I tested these on a Lifebook E744, which is capable of generating
KEY4_CODE ("ECO on/off button") and KEY5_CODE ("Wireless/Bluetooth
on/off button"). I checked that these hotkeys still work fine with this
patch series applied. By temporarily reversing some logical conditions,
I also did my best to ensure that unexpected behaviors (unknown ACPI
event code, kfifo failures) are still handled in the same way as
previously (apart from the "Push keycode into ringbuffer" debug message,
which is now only printed upon a successful push due to the last patch).
--
Best regards,
Michał Kępień
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Woithe <jwoithe@just42.net> |
|---|---|
| Date | 2017-01-11 13:50 +0100 |
| Message-ID | <sYqvM-6ZO-7@gated-at.bofh.it> |
| In reply to | #1556444 |
On Wed, Jan 11, 2017 at 01:26:49PM +0100, Micha?? K??pie?? wrote:
> > On Wed, Jan 11, 2017 at 09:59:29AM +0100, Micha?? K??pie?? wrote:
> > > I am currently preparing a patch series which makes fujitsu-laptop use a
> > > sparse keymap for hotkey handling. Before that will happen, though,
> > > acpi_fujitsu_hotkey_notify() could use a revamp because it is pretty
> > > hard to read as it is. To avoid posting everything at once, here are a
> > > few patches which IMHO make that function easier to read. Some of these
> > > changes might be a matter of taste, so feel free to NACK them or suggest
> > > a preferred alternative.
> >
> > This patch series provides a significant clean up to the functions it
> > focuses on. As such I have no real objections to them. However, because my
> > Fujitsu laptop doesn't have any of the hotkeys of later models I am unable
> > to test these patches with real hardware. Have you been able to do so? If
> > they have been verified I have no problem acking these. Otherwise I will
> > have to do as much as I can (given no access to relevant hardware) to ensure
> > the overall behaviour isn't changed.
>
> I tested these on a Lifebook E744, which is capable of generating
> KEY4_CODE ("ECO on/off button") and KEY5_CODE ("Wireless/Bluetooth
> on/off button"). I checked that these hotkeys still work fine with this
> patch series applied. By temporarily reversing some logical conditions,
> I also did my best to ensure that unexpected behaviors (unknown ACPI
> event code, kfifo failures) are still handled in the same way as
> previously (apart from the "Push keycode into ringbuffer" debug message,
> which is now only printed upon a successful push due to the last patch).
Thanks for clarifying. It may be worth adding a comment to the effect that
the patches were tested on a Lifebook E744. That aside, I'm happy with
these clean ups.
Acked-by: Jonathan Woithe <jwoithe@just42.net>
Darren: do you want me to explicitly ack all 4 parts, or the above
sufficient for your processes?
Regards
jonathan
[toc] | [prev] | [next] | [standalone]
| From | Darren Hart <dvhart@infradead.org> |
|---|---|
| Date | 2017-01-13 23:00 +0100 |
| Message-ID | <sZi38-636-17@gated-at.bofh.it> |
| In reply to | #1556459 |
On Wed, Jan 11, 2017 at 11:18:13PM +1030, Jonathan Woithe wrote:
> On Wed, Jan 11, 2017 at 01:26:49PM +0100, Micha?? K??pie?? wrote:
> > > On Wed, Jan 11, 2017 at 09:59:29AM +0100, Micha?? K??pie?? wrote:
> > > > I am currently preparing a patch series which makes fujitsu-laptop use a
> > > > sparse keymap for hotkey handling. Before that will happen, though,
> > > > acpi_fujitsu_hotkey_notify() could use a revamp because it is pretty
> > > > hard to read as it is. To avoid posting everything at once, here are a
> > > > few patches which IMHO make that function easier to read. Some of these
> > > > changes might be a matter of taste, so feel free to NACK them or suggest
> > > > a preferred alternative.
> > >
> > > This patch series provides a significant clean up to the functions it
> > > focuses on. As such I have no real objections to them. However, because my
> > > Fujitsu laptop doesn't have any of the hotkeys of later models I am unable
> > > to test these patches with real hardware. Have you been able to do so? If
> > > they have been verified I have no problem acking these. Otherwise I will
> > > have to do as much as I can (given no access to relevant hardware) to ensure
> > > the overall behaviour isn't changed.
> >
> > I tested these on a Lifebook E744, which is capable of generating
> > KEY4_CODE ("ECO on/off button") and KEY5_CODE ("Wireless/Bluetooth
> > on/off button"). I checked that these hotkeys still work fine with this
> > patch series applied. By temporarily reversing some logical conditions,
> > I also did my best to ensure that unexpected behaviors (unknown ACPI
> > event code, kfifo failures) are still handled in the same way as
> > previously (apart from the "Push keycode into ringbuffer" debug message,
> > which is now only printed upon a successful push due to the last patch).
>
> Thanks for clarifying. It may be worth adding a comment to the effect that
> the patches were tested on a Lifebook E744. That aside, I'm happy with
> these clean ups.
>
> Acked-by: Jonathan Woithe <jwoithe@just42.net>
>
> Darren: do you want me to explicitly ack all 4 parts, or the above
> sufficient for your processes?
The above is sufficient as far as I'm concerned.
--
Darren Hart
Intel Open Source Technology Center
[toc] | [prev] | [next] | [standalone]
| From | Darren Hart <dvhart@infradead.org> |
|---|---|
| Date | 2017-01-13 23:10 +0100 |
| Message-ID | <sZicO-6lp-19@gated-at.bofh.it> |
| In reply to | #1556459 |
On Wed, Jan 11, 2017 at 11:18:13PM +1030, Jonathan Woithe wrote:
> On Wed, Jan 11, 2017 at 01:26:49PM +0100, Micha?? K??pie?? wrote:
> > > On Wed, Jan 11, 2017 at 09:59:29AM +0100, Micha?? K??pie?? wrote:
> > > > I am currently preparing a patch series which makes fujitsu-laptop use a
> > > > sparse keymap for hotkey handling. Before that will happen, though,
> > > > acpi_fujitsu_hotkey_notify() could use a revamp because it is pretty
> > > > hard to read as it is. To avoid posting everything at once, here are a
> > > > few patches which IMHO make that function easier to read. Some of these
> > > > changes might be a matter of taste, so feel free to NACK them or suggest
> > > > a preferred alternative.
> > >
> > > This patch series provides a significant clean up to the functions it
> > > focuses on. As such I have no real objections to them. However, because my
> > > Fujitsu laptop doesn't have any of the hotkeys of later models I am unable
> > > to test these patches with real hardware. Have you been able to do so? If
> > > they have been verified I have no problem acking these. Otherwise I will
> > > have to do as much as I can (given no access to relevant hardware) to ensure
> > > the overall behaviour isn't changed.
> >
> > I tested these on a Lifebook E744, which is capable of generating
> > KEY4_CODE ("ECO on/off button") and KEY5_CODE ("Wireless/Bluetooth
> > on/off button"). I checked that these hotkeys still work fine with this
> > patch series applied. By temporarily reversing some logical conditions,
> > I also did my best to ensure that unexpected behaviors (unknown ACPI
> > event code, kfifo failures) are still handled in the same way as
> > previously (apart from the "Push keycode into ringbuffer" debug message,
> > which is now only printed upon a successful push due to the last patch).
>
> Thanks for clarifying. It may be worth adding a comment to the effect that
> the patches were tested on a Lifebook E744. That aside, I'm happy with
> these clean ups.
>
> Acked-by: Jonathan Woithe <jwoithe@just42.net>
Queued to testing, thanks!
--
Darren Hart
Intel Open Source Technology Center
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web