Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1556303 > unrolled thread

[PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup

Started byMichał Kępień <kernel@kempniu.pl>
First post2017-01-11 10:10 +0100
Last post2017-01-13 23:10 +0100
Articles 10 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [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

#1556303 — [PATCH 0/4] fujitsu-laptop: acpi_fujitsu_hotkey_notify() cleanup

FromMichał Kępień <kernel@kempniu.pl>
Date2017-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]


#1556306 — [PATCH 3/4] platform/x86: fujitsu-laptop: break up complex loop condition

FromMichał Kępień <kernel@kempniu.pl>
Date2017-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]


#1558380 — Re: [PATCH 3/4] platform/x86: fujitsu-laptop: break up complex loop condition

FromJonathan Woithe <jwoithe@just42.net>
Date2017-01-13 13:50 +0100
SubjectRe: [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]


#1556307 — [PATCH 2/4] platform/x86: fujitsu-laptop: move keycode processing to separate functions

FromMichał Kępień <kernel@kempniu.pl>
Date2017-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]


#1558385 — Re: [PATCH 2/4] platform/x86: fujitsu-laptop: move keycode processing to separate functions

FromJonathan Woithe <jwoithe@just42.net>
Date2017-01-13 14:00 +0100
SubjectRe: [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]


#1556425

FromJonathan Woithe <jwoithe@just42.net>
Date2017-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]


#1556444

FromMichał Kępień <kernel@kempniu.pl>
Date2017-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]


#1556459

FromJonathan Woithe <jwoithe@just42.net>
Date2017-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]


#1558766

FromDarren Hart <dvhart@infradead.org>
Date2017-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]


#1558771

FromDarren Hart <dvhart@infradead.org>
Date2017-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