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(-)
--
2.12.0
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ń <[email protected]>
---
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
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ń <[email protected]>
---
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
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
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 <[email protected]>
Reviewed-by: Jonathan Woithe <[email protected]>
Regards
jonathan
On Mon, Mar 6, 2017 at 12:57 AM, Jonathan Woithe <[email protected]> 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 <[email protected]>
> Reviewed-by: Jonathan Woithe <[email protected]>
Pushed to testing, thanks!
--
With Best Regards,
Andy Shevchenko