* [PATCH v2 1/2] platform/x86: fujitsu-laptop: decrease indentation in acpi_fujitsu_bl_notify()
2017-03-01 6:42 [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup Michał Kępień
@ 2017-03-01 6:42 ` Michał Kępień
2017-03-01 6:42 ` [PATCH v2 2/2] platform/x86: fujitsu-laptop: simplify brightness key event generation logic Michał Kępień
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Michał Kępień @ 2017-03-01 6:42 UTC (permalink / raw)
To: Jonathan Woithe, Darren Hart, Andy Shevchenko
Cc: platform-driver-x86, linux-kernel
acpi_fujitsu_bl_notify() is pretty deeply nested, which hurts
readability. Strip off one level of indentation by returning early when
the event code supplied as argument is not ACPI_FUJITSU_NOTIFY_CODE1.
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
Changes introduced by this patch are best viewed when whitespace changes
are ignored.
drivers/platform/x86/fujitsu-laptop.c | 64 ++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 31 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index e12cc3504d48..b19f6e1c0173 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -795,40 +795,42 @@ static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
input = fujitsu_bl->input;
- switch (event) {
- case ACPI_FUJITSU_NOTIFY_CODE1:
- keycode = 0;
- oldb = fujitsu_bl->brightness_level;
- get_lcd_level();
- newb = fujitsu_bl->brightness_level;
-
- vdbg_printk(FUJLAPTOP_DBG_TRACE,
- "brightness button event [%i -> %i (%i)]\n",
- oldb, newb, fujitsu_bl->brightness_changed);
-
- if (oldb < newb) {
- if (disable_brightness_adjust != 1) {
- if (use_alt_lcd_levels)
- set_lcd_level_alt(newb);
- else
- set_lcd_level(newb);
- }
- keycode = KEY_BRIGHTNESSUP;
- } else if (oldb > newb) {
- if (disable_brightness_adjust != 1) {
- if (use_alt_lcd_levels)
- set_lcd_level_alt(newb);
- else
- set_lcd_level(newb);
- }
- keycode = KEY_BRIGHTNESSDOWN;
- }
- break;
- default:
+ if (event != ACPI_FUJITSU_NOTIFY_CODE1) {
keycode = KEY_UNKNOWN;
vdbg_printk(FUJLAPTOP_DBG_WARN,
"unsupported event [0x%x]\n", event);
- break;
+ input_report_key(input, keycode, 1);
+ input_sync(input);
+ input_report_key(input, keycode, 0);
+ input_sync(input);
+ return;
+ }
+
+ keycode = 0;
+ oldb = fujitsu_bl->brightness_level;
+ get_lcd_level();
+ newb = fujitsu_bl->brightness_level;
+
+ vdbg_printk(FUJLAPTOP_DBG_TRACE,
+ "brightness button event [%i -> %i (%i)]\n",
+ oldb, newb, fujitsu_bl->brightness_changed);
+
+ if (oldb < newb) {
+ if (disable_brightness_adjust != 1) {
+ if (use_alt_lcd_levels)
+ set_lcd_level_alt(newb);
+ else
+ set_lcd_level(newb);
+ }
+ keycode = KEY_BRIGHTNESSUP;
+ } else if (oldb > newb) {
+ if (disable_brightness_adjust != 1) {
+ if (use_alt_lcd_levels)
+ set_lcd_level_alt(newb);
+ else
+ set_lcd_level(newb);
+ }
+ keycode = KEY_BRIGHTNESSDOWN;
}
if (keycode != 0) {
--
2.12.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] platform/x86: fujitsu-laptop: simplify brightness key event generation logic
2017-03-01 6:42 [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup Michał Kępień
2017-03-01 6:42 ` [PATCH v2 1/2] platform/x86: fujitsu-laptop: decrease indentation in acpi_fujitsu_bl_notify() Michał Kępień
@ 2017-03-01 6:42 ` Michał Kępień
2017-03-01 22:34 ` [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup Jonathan Woithe
2017-03-05 22:57 ` Jonathan Woithe
3 siblings, 0 replies; 6+ messages in thread
From: Michał Kępień @ 2017-03-01 6:42 UTC (permalink / raw)
To: Jonathan Woithe, Darren Hart, Andy Shevchenko
Cc: platform-driver-x86, linux-kernel
Returning early when there is no brightness change allows removal of a
duplicate code block, makes the purpose of the following code clearer
and allows the condition surrounding key event generation to be removed.
Local integer variables can also be declared in a single line.
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
drivers/platform/x86/fujitsu-laptop.c | 40 +++++++++++++----------------------
1 file changed, 15 insertions(+), 25 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index b19f6e1c0173..4a5e7b60a672 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -790,8 +790,7 @@ static int acpi_fujitsu_bl_remove(struct acpi_device *device)
static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
{
struct input_dev *input;
- int keycode;
- int oldb, newb;
+ int oldb, newb, keycode;
input = fujitsu_bl->input;
@@ -806,7 +805,6 @@ static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
return;
}
- keycode = 0;
oldb = fujitsu_bl->brightness_level;
get_lcd_level();
newb = fujitsu_bl->brightness_level;
@@ -815,30 +813,22 @@ static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
"brightness button event [%i -> %i (%i)]\n",
oldb, newb, fujitsu_bl->brightness_changed);
- if (oldb < newb) {
- if (disable_brightness_adjust != 1) {
- if (use_alt_lcd_levels)
- set_lcd_level_alt(newb);
- else
- set_lcd_level(newb);
- }
- keycode = KEY_BRIGHTNESSUP;
- } else if (oldb > newb) {
- if (disable_brightness_adjust != 1) {
- if (use_alt_lcd_levels)
- set_lcd_level_alt(newb);
- else
- set_lcd_level(newb);
- }
- keycode = KEY_BRIGHTNESSDOWN;
- }
+ if (oldb == newb)
+ return;
- if (keycode != 0) {
- input_report_key(input, keycode, 1);
- input_sync(input);
- input_report_key(input, keycode, 0);
- input_sync(input);
+ if (disable_brightness_adjust != 1) {
+ if (use_alt_lcd_levels)
+ set_lcd_level_alt(newb);
+ else
+ set_lcd_level(newb);
}
+
+ keycode = oldb < newb ? KEY_BRIGHTNESSUP : KEY_BRIGHTNESSDOWN;
+
+ input_report_key(input, keycode, 1);
+ input_sync(input);
+ input_report_key(input, keycode, 0);
+ input_sync(input);
}
/* ACPI device for hotkey handling */
--
2.12.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup
2017-03-01 6:42 [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup Michał Kępień
2017-03-01 6:42 ` [PATCH v2 1/2] platform/x86: fujitsu-laptop: decrease indentation in acpi_fujitsu_bl_notify() Michał Kępień
2017-03-01 6:42 ` [PATCH v2 2/2] platform/x86: fujitsu-laptop: simplify brightness key event generation logic Michał Kępień
@ 2017-03-01 22:34 ` Jonathan Woithe
2017-03-05 22:57 ` Jonathan Woithe
3 siblings, 0 replies; 6+ messages in thread
From: Jonathan Woithe @ 2017-03-01 22:34 UTC (permalink / raw)
To: Micha?? K??pie??
Cc: Darren Hart, Andy Shevchenko, platform-driver-x86, linux-kernel
On Wed, Mar 01, 2017 at 07:42:52AM +0100, Micha?? K??pie?? wrote:
> Here are two minor cleanups for acpi_fujitsu_bl_notify() that I came up
> with while preparing the sparse keymap migration.
These both look innoculous at first glance. I will review them and test on
hardware within the next 48 hours.
Regards
jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup
2017-03-01 6:42 [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup Michał Kępień
` (2 preceding siblings ...)
2017-03-01 22:34 ` [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup Jonathan Woithe
@ 2017-03-05 22:57 ` Jonathan Woithe
2017-03-13 16:01 ` Andy Shevchenko
3 siblings, 1 reply; 6+ messages in thread
From: Jonathan Woithe @ 2017-03-05 22:57 UTC (permalink / raw)
To: Micha?? K??pie??
Cc: Darren Hart, Andy Shevchenko, platform-driver-x86, linux-kernel
On Wed, Mar 01, 2017 at 07:42:52AM +0100, Micha?? K??pie?? wrote:
> Here are two minor cleanups for acpi_fujitsu_bl_notify() that I came up
> with while preparing the sparse keymap migration.
>
> Changes from v1:
>
> - Rebase on top of reworked Alan Jenkins' cleanup patch series.
>
> - Join integer variable declarations into a single line in patch 2/2.
>
> drivers/platform/x86/fujitsu-laptop.c | 64 +++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 36 deletions(-)
These two clean ups, as their descriptions indicate, improve the clarity of
the driver code and permit some minor optimisations. No regressions are
evident when tested on S7020 hardware. Please apply.
Tested-by: Jonathan Woithe <jwoithe@just42.net>
Reviewed-by: Jonathan Woithe <jwoithe@just42.net>
Regards
jonathan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/2] fujitsu-laptop: acpi_fujitsu_bl_notify() cleanup
2017-03-05 22:57 ` Jonathan Woithe
@ 2017-03-13 16:01 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2017-03-13 16:01 UTC (permalink / raw)
To: Jonathan Woithe
Cc: Micha?? K??pie??,
Darren Hart, Andy Shevchenko, Platform Driver, linux-kernel
On Mon, Mar 6, 2017 at 12:57 AM, Jonathan Woithe <jwoithe@just42.net> wrote:
> On Wed, Mar 01, 2017 at 07:42:52AM +0100, Micha?? K??pie?? wrote:
>> Here are two minor cleanups for acpi_fujitsu_bl_notify() that I came up
>> with while preparing the sparse keymap migration.
>>
>> Changes from v1:
>>
>> - Rebase on top of reworked Alan Jenkins' cleanup patch series.
>>
>> - Join integer variable declarations into a single line in patch 2/2.
>>
>> drivers/platform/x86/fujitsu-laptop.c | 64 +++++++++++++++--------------------
>> 1 file changed, 28 insertions(+), 36 deletions(-)
>
> These two clean ups, as their descriptions indicate, improve the clarity of
> the driver code and permit some minor optimisations. No regressions are
> evident when tested on S7020 hardware. Please apply.
>
> Tested-by: Jonathan Woithe <jwoithe@just42.net>
> Reviewed-by: Jonathan Woithe <jwoithe@just42.net>
Pushed to testing, thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread