2018-02-20 05:28:24

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups

This patch series contains miscellaneous cleanups which I think are
worth getting done before splitting fujitsu-laptop into two separate
modules.

Changes from v1:

- [6/7] Rename BACKLIGHT_POWER to BACKLIGHT_PARAM_POWER.

- [6/7] Use BACKLIGHT_OFF instead of a numeric constant.

- [7/7] Include <linux/bitops.h> due to the use of BIT().

drivers/platform/x86/fujitsu-laptop.c | 180 +++++++++++++++++-----------------
1 file changed, 91 insertions(+), 89 deletions(-)

--
2.16.1



2018-02-20 05:26:42

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 5/7] platform/x86: fujitsu-laptop: Do not include linux/slab.h

Do not include linux/slab.h as all module code now uses managed memory
allocations and thus neither k*alloc() nor kfree() is used.

Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 7888b779d6c5..17779b8b7f30 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -61,7 +61,6 @@
#include <linux/kfifo.h>
#include <linux/leds.h>
#include <linux/platform_device.h>
-#include <linux/slab.h>
#include <acpi/video.h>

#define FUJITSU_DRIVER_VERSION "0.6.0"
--
2.16.1


2018-02-20 05:26:42

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 7/7] platform/x86: fujitsu-laptop: Clean up constants

Align all constant values defined in the module to a common indentation.
Rename ACPI_FUJITSU_NOTIFY_CODE1 to ACPI_FUJITSU_NOTIFY_CODE as there is
only one ACPI notification code used throughout the driver. Define all
bitmasks using the BIT() macro. Clean up comments.

Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 66 ++++++++++++++++++-----------------
1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 6b0484cbdcf2..13bcdfea5349 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -53,6 +53,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/acpi.h>
+#include <linux/bitops.h>
#include <linux/dmi.h>
#include <linux/backlight.h>
#include <linux/fb.h>
@@ -63,9 +64,9 @@
#include <linux/platform_device.h>
#include <acpi/video.h>

-#define FUJITSU_DRIVER_VERSION "0.6.0"
+#define FUJITSU_DRIVER_VERSION "0.6.0"

-#define FUJITSU_LCD_N_LEVELS 8
+#define FUJITSU_LCD_N_LEVELS 8

#define ACPI_FUJITSU_CLASS "fujitsu"
#define ACPI_FUJITSU_BL_HID "FUJ02B1"
@@ -75,46 +76,47 @@
#define ACPI_FUJITSU_LAPTOP_DRIVER_NAME "Fujitsu laptop FUJ02E3 ACPI hotkeys driver"
#define ACPI_FUJITSU_LAPTOP_DEVICE_NAME "Fujitsu FUJ02E3"

-#define ACPI_FUJITSU_NOTIFY_CODE1 0x80
+#define ACPI_FUJITSU_NOTIFY_CODE 0x80

/* FUNC interface - command values */
-#define FUNC_FLAGS 0x1000
-#define FUNC_LEDS 0x1001
-#define FUNC_BUTTONS 0x1002
-#define FUNC_BACKLIGHT 0x1004
+#define FUNC_FLAGS BIT(12)
+#define FUNC_LEDS (BIT(12) | BIT(0))
+#define FUNC_BUTTONS (BIT(12) | BIT(1))
+#define FUNC_BACKLIGHT (BIT(12) | BIT(2))

/* FUNC interface - responses */
-#define UNSUPPORTED_CMD 0x80000000
+#define UNSUPPORTED_CMD BIT(31)

/* FUNC interface - status flags */
-#define FLAG_RFKILL 0x020
-#define FLAG_LID 0x100
-#define FLAG_DOCK 0x200
+#define FLAG_RFKILL BIT(5)
+#define FLAG_LID BIT(8)
+#define FLAG_DOCK BIT(9)

/* FUNC interface - LED control */
-#define FUNC_LED_OFF 0x1
-#define FUNC_LED_ON 0x30001
-#define KEYBOARD_LAMPS 0x100
-#define LOGOLAMP_POWERON 0x2000
-#define LOGOLAMP_ALWAYS 0x4000
-#define RADIO_LED_ON 0x20
-#define ECO_LED 0x10000
-#define ECO_LED_ON 0x80000
+#define FUNC_LED_OFF BIT(0)
+#define FUNC_LED_ON (BIT(0) | BIT(16) | BIT(17))
+#define LOGOLAMP_POWERON BIT(13)
+#define LOGOLAMP_ALWAYS BIT(14)
+#define KEYBOARD_LAMPS BIT(8)
+#define RADIO_LED_ON BIT(5)
+#define ECO_LED BIT(16)
+#define ECO_LED_ON BIT(19)

/* FUNC interface - backlight power control */
-#define BACKLIGHT_PARAM_POWER 0x4
-#define BACKLIGHT_OFF 0x3
-#define BACKLIGHT_ON 0x0
+#define BACKLIGHT_PARAM_POWER BIT(2)
+#define BACKLIGHT_OFF (BIT(0) | BIT(1))
+#define BACKLIGHT_ON 0

-/* Hotkey details */
-#define KEY1_CODE 0x410 /* codes for the keys in the GIRB register */
-#define KEY2_CODE 0x411
-#define KEY3_CODE 0x412
-#define KEY4_CODE 0x413
-#define KEY5_CODE 0x420
+/* Scancodes read from the GIRB register */
+#define KEY1_CODE 0x410
+#define KEY2_CODE 0x411
+#define KEY3_CODE 0x412
+#define KEY4_CODE 0x413
+#define KEY5_CODE 0x420

-#define MAX_HOTKEY_RINGBUFFER_SIZE 100
-#define RINGBUFFERSIZE 40
+/* Hotkey ringbuffer limits */
+#define MAX_HOTKEY_RINGBUFFER_SIZE 100
+#define RINGBUFFERSIZE 40

/* Module parameters */
static int use_alt_lcd_levels = -1;
@@ -428,7 +430,7 @@ static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
struct fujitsu_bl *priv = acpi_driver_data(device);
int oldb, newb;

- if (event != ACPI_FUJITSU_NOTIFY_CODE1) {
+ if (event != ACPI_FUJITSU_NOTIFY_CODE) {
acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
event);
sparse_keymap_report_event(priv->input, -1, 1, true);
@@ -902,7 +904,7 @@ static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
int scancode, i = 0;
unsigned int irb;

- if (event != ACPI_FUJITSU_NOTIFY_CODE1) {
+ if (event != ACPI_FUJITSU_NOTIFY_CODE) {
acpi_handle_info(device->handle, "Unsupported event [0x%x]\n",
event);
sparse_keymap_report_event(priv->input, -1, 1, true);
--
2.16.1


2018-02-20 05:26:49

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 6/7] platform/x86: fujitsu-laptop: Define constants for backlight power control

Use named constants instead of integers in call_fext_func() invocations
related to backlight power control in order to more clearly convey the
intent of each call.

Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 17779b8b7f30..6b0484cbdcf2 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -101,6 +101,11 @@
#define ECO_LED 0x10000
#define ECO_LED_ON 0x80000

+/* FUNC interface - backlight power control */
+#define BACKLIGHT_PARAM_POWER 0x4
+#define BACKLIGHT_OFF 0x3
+#define BACKLIGHT_ON 0x0
+
/* Hotkey details */
#define KEY1_CODE 0x410 /* codes for the keys in the GIRB register */
#define KEY2_CODE 0x411
@@ -257,9 +262,11 @@ static int bl_update_status(struct backlight_device *b)

if (fext) {
if (b->props.power == FB_BLANK_POWERDOWN)
- call_fext_func(fext, FUNC_BACKLIGHT, 0x1, 0x4, 0x3);
+ call_fext_func(fext, FUNC_BACKLIGHT, 0x1,
+ BACKLIGHT_PARAM_POWER, BACKLIGHT_OFF);
else
- call_fext_func(fext, FUNC_BACKLIGHT, 0x1, 0x4, 0x0);
+ call_fext_func(fext, FUNC_BACKLIGHT, 0x1,
+ BACKLIGHT_PARAM_POWER, BACKLIGHT_ON);
}

return set_lcd_level(device, b->props.brightness);
@@ -818,7 +825,8 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
/* Sync backlight power status */
if (fujitsu_bl && fujitsu_bl->bl_device &&
acpi_video_get_backlight_type() == acpi_backlight_vendor) {
- if (call_fext_func(fext, FUNC_BACKLIGHT, 0x2, 0x4, 0x0) == 3)
+ if (call_fext_func(fext, FUNC_BACKLIGHT, 0x2,
+ BACKLIGHT_PARAM_POWER, 0x0) == BACKLIGHT_OFF)
fujitsu_bl->bl_device->props.power = FB_BLANK_POWERDOWN;
else
fujitsu_bl->bl_device->props.power = FB_BLANK_UNBLANK;
--
2.16.1


2018-02-20 05:27:14

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 2/7] platform/x86: fujitsu-laptop: Defer input device registration

Only register input devices after the device-specific data structures
they access are fully initialized.

Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index b5f782807bfa..7f30a427a16c 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -399,10 +399,6 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
strcpy(acpi_device_class(device), ACPI_FUJITSU_CLASS);
device->driver_data = priv;

- ret = acpi_fujitsu_bl_input_setup(device);
- if (ret)
- return ret;
-
pr_info("ACPI: %s [%s]\n",
acpi_device_name(device), acpi_device_bid(device));

@@ -410,6 +406,10 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
priv->max_brightness = FUJITSU_LCD_N_LEVELS;
get_lcd_level(device);

+ ret = acpi_fujitsu_bl_input_setup(device);
+ if (ret)
+ return ret;
+
ret = fujitsu_backlight_register(device);
if (ret)
return ret;
@@ -795,10 +795,6 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
goto err_stop;
}

- ret = acpi_fujitsu_laptop_input_setup(device);
- if (ret)
- goto err_free_fifo;
-
pr_info("ACPI: %s [%s]\n",
acpi_device_name(device), acpi_device_bid(device));

@@ -833,6 +829,10 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
fujitsu_bl->bl_device->props.power = FB_BLANK_UNBLANK;
}

+ ret = acpi_fujitsu_laptop_input_setup(device);
+ if (ret)
+ goto err_free_fifo;
+
ret = acpi_fujitsu_laptop_leds_register(device);
if (ret)
goto err_free_fifo;
--
2.16.1


2018-02-20 05:27:25

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 4/7] platform/x86: fujitsu-laptop: Clearly distinguish module parameters

In order to improve code clarity, move declarations of variables that
are module parameters higher up and add a comment.

Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 94ff7f86fa8f..7888b779d6c5 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -112,6 +112,10 @@
#define MAX_HOTKEY_RINGBUFFER_SIZE 100
#define RINGBUFFERSIZE 40

+/* Module parameters */
+static int use_alt_lcd_levels = -1;
+static bool disable_brightness_adjust;
+
/* Device controlling the backlight and associated keys */
struct fujitsu_bl {
struct input_dev *input;
@@ -122,8 +126,6 @@ struct fujitsu_bl {
};

static struct fujitsu_bl *fujitsu_bl;
-static int use_alt_lcd_levels = -1;
-static bool disable_brightness_adjust;

/* Device used to access hotkeys and other features on the laptop */
struct fujitsu_laptop {
--
2.16.1


2018-02-20 05:28:12

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 3/7] platform/x86: fujitsu-laptop: Simplify error paths

Replace the last few lines of acpi_fujitsu_bl_add() with a simple return
in order to improve code readability without changing the logic.

As acpi_fujitsu_laptop_add() uses a managed memory allocation for
device-specific data, it is fine to just return immediately upon kfifo
allocation failure. Do that instead of jumping to the end of the
function to improve code readability. Running out of memory while
allocating the kfifo does not seem probable enough to warrant logging an
error message, so do not do it.

Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 7f30a427a16c..94ff7f86fa8f 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -410,11 +410,7 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
if (ret)
return ret;

- ret = fujitsu_backlight_register(device);
- if (ret)
- return ret;
-
- return 0;
+ return fujitsu_backlight_register(device);
}

/* Brightness notify */
@@ -790,10 +786,8 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
spin_lock_init(&priv->fifo_lock);
ret = kfifo_alloc(&priv->fifo, RINGBUFFERSIZE * sizeof(int),
GFP_KERNEL);
- if (ret) {
- pr_err("kfifo_alloc failed\n");
- goto err_stop;
- }
+ if (ret)
+ return ret;

pr_info("ACPI: %s [%s]\n",
acpi_device_name(device), acpi_device_bid(device));
@@ -845,7 +839,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)

err_free_fifo:
kfifo_free(&priv->fifo);
-err_stop:
+
return ret;
}

--
2.16.1


2018-02-20 05:29:36

by Michał Kępień

[permalink] [raw]
Subject: [PATCH v2 1/7] platform/x86: fujitsu-laptop: Unify local variable naming

Different functions in the module use varying names (error, result,
status) for a local variable storing the return value of a function call
that has to be checked for errors. Use a common name (ret) for all
these local variables to improve code consistency. Merge integer
variable declarations in acpi_fujitsu_laptop_add() into one line.

Signed-off-by: Michał Kępień <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 85 +++++++++++++++++------------------
1 file changed, 41 insertions(+), 44 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 2cfbd3fa5136..b5f782807bfa 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -385,7 +385,7 @@ static int fujitsu_backlight_register(struct acpi_device *device)
static int acpi_fujitsu_bl_add(struct acpi_device *device)
{
struct fujitsu_bl *priv;
- int error;
+ int ret;

if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
return -ENODEV;
@@ -399,9 +399,9 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
strcpy(acpi_device_class(device), ACPI_FUJITSU_CLASS);
device->driver_data = priv;

- error = acpi_fujitsu_bl_input_setup(device);
- if (error)
- return error;
+ ret = acpi_fujitsu_bl_input_setup(device);
+ if (ret)
+ return ret;

pr_info("ACPI: %s [%s]\n",
acpi_device_name(device), acpi_device_bid(device));
@@ -410,9 +410,9 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
priv->max_brightness = FUJITSU_LCD_N_LEVELS;
get_lcd_level(device);

- error = fujitsu_backlight_register(device);
- if (error)
- return error;
+ ret = fujitsu_backlight_register(device);
+ if (ret)
+ return ret;

return 0;
}
@@ -693,7 +693,7 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
{
struct fujitsu_laptop *priv = acpi_driver_data(device);
struct led_classdev *led;
- int result;
+ int ret;

if (call_fext_func(device,
FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
@@ -704,9 +704,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
led->name = "fujitsu::logolamp";
led->brightness_set_blocking = logolamp_set;
led->brightness_get = logolamp_get;
- result = devm_led_classdev_register(&device->dev, led);
- if (result)
- return result;
+ ret = devm_led_classdev_register(&device->dev, led);
+ if (ret)
+ return ret;
}

if ((call_fext_func(device,
@@ -719,9 +719,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
led->name = "fujitsu::kblamps";
led->brightness_set_blocking = kblamps_set;
led->brightness_get = kblamps_get;
- result = devm_led_classdev_register(&device->dev, led);
- if (result)
- return result;
+ ret = devm_led_classdev_register(&device->dev, led);
+ if (ret)
+ return ret;
}

/*
@@ -742,9 +742,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
led->brightness_set_blocking = radio_led_set;
led->brightness_get = radio_led_get;
led->default_trigger = "rfkill-any";
- result = devm_led_classdev_register(&device->dev, led);
- if (result)
- return result;
+ ret = devm_led_classdev_register(&device->dev, led);
+ if (ret)
+ return ret;
}

/* Support for eco led is not always signaled in bit corresponding
@@ -762,9 +762,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
led->name = "fujitsu::eco_led";
led->brightness_set_blocking = eco_led_set;
led->brightness_get = eco_led_get;
- result = devm_led_classdev_register(&device->dev, led);
- if (result)
- return result;
+ ret = devm_led_classdev_register(&device->dev, led);
+ if (ret)
+ return ret;
}

return 0;
@@ -773,8 +773,7 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
static int acpi_fujitsu_laptop_add(struct acpi_device *device)
{
struct fujitsu_laptop *priv;
- int error;
- int i;
+ int ret, i = 0;

priv = devm_kzalloc(&device->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -789,23 +788,22 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)

/* kfifo */
spin_lock_init(&priv->fifo_lock);
- error = kfifo_alloc(&priv->fifo, RINGBUFFERSIZE * sizeof(int),
- GFP_KERNEL);
- if (error) {
+ ret = kfifo_alloc(&priv->fifo, RINGBUFFERSIZE * sizeof(int),
+ GFP_KERNEL);
+ if (ret) {
pr_err("kfifo_alloc failed\n");
goto err_stop;
}

- error = acpi_fujitsu_laptop_input_setup(device);
- if (error)
+ ret = acpi_fujitsu_laptop_input_setup(device);
+ if (ret)
goto err_free_fifo;

pr_info("ACPI: %s [%s]\n",
acpi_device_name(device), acpi_device_bid(device));

- i = 0;
- while (call_fext_func(device, FUNC_BUTTONS, 0x1, 0x0, 0x0) != 0
- && (i++) < MAX_HOTKEY_RINGBUFFER_SIZE)
+ while (call_fext_func(device, FUNC_BUTTONS, 0x1, 0x0, 0x0) != 0 &&
+ i++ < MAX_HOTKEY_RINGBUFFER_SIZE)
; /* No action, result is discarded */
acpi_handle_debug(device->handle, "Discarded %i ringbuffer entries\n",
i);
@@ -835,12 +833,12 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
fujitsu_bl->bl_device->props.power = FB_BLANK_UNBLANK;
}

- error = acpi_fujitsu_laptop_leds_register(device);
- if (error)
+ ret = acpi_fujitsu_laptop_leds_register(device);
+ if (ret)
goto err_free_fifo;

- error = fujitsu_laptop_platform_add(device);
- if (error)
+ ret = fujitsu_laptop_platform_add(device);
+ if (ret)
goto err_free_fifo;

return 0;
@@ -848,7 +846,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
err_free_fifo:
kfifo_free(&priv->fifo);
err_stop:
- return error;
+ return ret;
}

static int acpi_fujitsu_laptop_remove(struct acpi_device *device)
@@ -865,11 +863,11 @@ static int acpi_fujitsu_laptop_remove(struct acpi_device *device)
static void acpi_fujitsu_laptop_press(struct acpi_device *device, int scancode)
{
struct fujitsu_laptop *priv = acpi_driver_data(device);
- int status;
+ int ret;

- status = kfifo_in_locked(&priv->fifo, (unsigned char *)&scancode,
- sizeof(scancode), &priv->fifo_lock);
- if (status != sizeof(scancode)) {
+ ret = kfifo_in_locked(&priv->fifo, (unsigned char *)&scancode,
+ sizeof(scancode), &priv->fifo_lock);
+ if (ret != sizeof(scancode)) {
dev_info(&priv->input->dev, "Could not push scancode [0x%x]\n",
scancode);
return;
@@ -882,13 +880,12 @@ static void acpi_fujitsu_laptop_press(struct acpi_device *device, int scancode)
static void acpi_fujitsu_laptop_release(struct acpi_device *device)
{
struct fujitsu_laptop *priv = acpi_driver_data(device);
- int scancode, status;
+ int scancode, ret;

while (true) {
- status = kfifo_out_locked(&priv->fifo,
- (unsigned char *)&scancode,
- sizeof(scancode), &priv->fifo_lock);
- if (status != sizeof(scancode))
+ ret = kfifo_out_locked(&priv->fifo, (unsigned char *)&scancode,
+ sizeof(scancode), &priv->fifo_lock);
+ if (ret != sizeof(scancode))
return;
sparse_keymap_report_event(priv->input, scancode, 0, false);
dev_dbg(&priv->input->dev,
--
2.16.1


2018-02-20 06:29:18

by Jonathan Woithe

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups

On Tue, Feb 20, 2018 at 06:24:47AM +0100, Micha?? K??pie?? wrote:
> This patch series contains miscellaneous cleanups which I think are
> worth getting done before splitting fujitsu-laptop into two separate
> modules.
>
> Changes from v1:
>
> - [6/7] Rename BACKLIGHT_POWER to BACKLIGHT_PARAM_POWER.
>
> - [6/7] Use BACKLIGHT_OFF instead of a numeric constant.
>
> - [7/7] Include <linux/bitops.h> due to the use of BIT().

Thanks for the revision; the result looks good to me. Collectively these
changes improve the readability of the code and make its intent clearer.

Reviewed-by: Jonathan Woithe <[email protected]>

Regards
jonathan

2018-02-22 20:56:40

by Darren Hart

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups

On Tue, Feb 20, 2018 at 06:24:47AM +0100, Michał Kępień wrote:
> This patch series contains miscellaneous cleanups which I think are
> worth getting done before splitting fujitsu-laptop into two separate
> modules.
>
> Changes from v1:
>
> - [6/7] Rename BACKLIGHT_POWER to BACKLIGHT_PARAM_POWER.
>
> - [6/7] Use BACKLIGHT_OFF instead of a numeric constant.
>
> - [7/7] Include <linux/bitops.h> due to the use of BIT().
>
> drivers/platform/x86/fujitsu-laptop.c | 180 +++++++++++++++++-----------------
> 1 file changed, 91 insertions(+), 89 deletions(-)

Queued for 4.17, thanks.

--
Darren Hart
VMware Open Source Technology Center