All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups
@ 2018-02-20  5:24 Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 1/7] platform/x86: fujitsu-laptop: Unify local variable naming Michał Kępień
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/7] platform/x86: fujitsu-laptop: Unify local variable naming
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
@ 2018-02-20  5:24 ` Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 2/7] platform/x86: fujitsu-laptop: Defer input device registration Michał Kępień
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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ń <kernel@kempniu.pl>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/7] platform/x86: fujitsu-laptop: Defer input device registration
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 1/7] platform/x86: fujitsu-laptop: Unify local variable naming Michał Kępień
@ 2018-02-20  5:24 ` Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 3/7] platform/x86: fujitsu-laptop: Simplify error paths Michał Kępień
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/7] platform/x86: fujitsu-laptop: Simplify error paths
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 1/7] platform/x86: fujitsu-laptop: Unify local variable naming Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 2/7] platform/x86: fujitsu-laptop: Defer input device registration Michał Kępień
@ 2018-02-20  5:24 ` Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 4/7] platform/x86: fujitsu-laptop: Clearly distinguish module parameters Michał Kępień
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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ń <kernel@kempniu.pl>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/7] platform/x86: fujitsu-laptop: Clearly distinguish module parameters
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
                   ` (2 preceding siblings ...)
  2018-02-20  5:24 ` [PATCH v2 3/7] platform/x86: fujitsu-laptop: Simplify error paths Michał Kępień
@ 2018-02-20  5:24 ` Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 5/7] platform/x86: fujitsu-laptop: Do not include linux/slab.h Michał Kępień
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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ń <kernel@kempniu.pl>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/7] platform/x86: fujitsu-laptop: Do not include linux/slab.h
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
                   ` (3 preceding siblings ...)
  2018-02-20  5:24 ` [PATCH v2 4/7] platform/x86: fujitsu-laptop: Clearly distinguish module parameters Michał Kępień
@ 2018-02-20  5:24 ` Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 6/7] platform/x86: fujitsu-laptop: Define constants for backlight power control Michał Kępień
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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ń <kernel@kempniu.pl>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 6/7] platform/x86: fujitsu-laptop: Define constants for backlight power control
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
                   ` (4 preceding siblings ...)
  2018-02-20  5:24 ` [PATCH v2 5/7] platform/x86: fujitsu-laptop: Do not include linux/slab.h Michał Kępień
@ 2018-02-20  5:24 ` Michał Kępień
  2018-02-20  5:24 ` [PATCH v2 7/7] platform/x86: fujitsu-laptop: Clean up constants Michał Kępień
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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ń <kernel@kempniu.pl>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 7/7] platform/x86: fujitsu-laptop: Clean up constants
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
                   ` (5 preceding siblings ...)
  2018-02-20  5:24 ` [PATCH v2 6/7] platform/x86: fujitsu-laptop: Define constants for backlight power control Michał Kępień
@ 2018-02-20  5:24 ` Michał Kępień
  2018-02-20  6:27 ` [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Jonathan Woithe
  2018-02-22 20:55 ` Darren Hart
  8 siblings, 0 replies; 10+ messages in thread
From: Michał Kępień @ 2018-02-20  5:24 UTC (permalink / raw)
  To: Jonathan Woithe, Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, linux-kernel

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ń <kernel@kempniu.pl>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
                   ` (6 preceding siblings ...)
  2018-02-20  5:24 ` [PATCH v2 7/7] platform/x86: fujitsu-laptop: Clean up constants Michał Kępień
@ 2018-02-20  6:27 ` Jonathan Woithe
  2018-02-22 20:55 ` Darren Hart
  8 siblings, 0 replies; 10+ messages in thread
From: Jonathan Woithe @ 2018-02-20  6:27 UTC (permalink / raw)
  To: Micha?? K??pie??
  Cc: Darren Hart, Andy Shevchenko, platform-driver-x86, linux-kernel

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 <jwoithe@just42.net>

Regards
  jonathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups
  2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
                   ` (7 preceding siblings ...)
  2018-02-20  6:27 ` [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Jonathan Woithe
@ 2018-02-22 20:55 ` Darren Hart
  8 siblings, 0 replies; 10+ messages in thread
From: Darren Hart @ 2018-02-22 20:55 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Jonathan Woithe, Andy Shevchenko, platform-driver-x86, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-02-22 20:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-20  5:24 [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Michał Kępień
2018-02-20  5:24 ` [PATCH v2 1/7] platform/x86: fujitsu-laptop: Unify local variable naming Michał Kępień
2018-02-20  5:24 ` [PATCH v2 2/7] platform/x86: fujitsu-laptop: Defer input device registration Michał Kępień
2018-02-20  5:24 ` [PATCH v2 3/7] platform/x86: fujitsu-laptop: Simplify error paths Michał Kępień
2018-02-20  5:24 ` [PATCH v2 4/7] platform/x86: fujitsu-laptop: Clearly distinguish module parameters Michał Kępień
2018-02-20  5:24 ` [PATCH v2 5/7] platform/x86: fujitsu-laptop: Do not include linux/slab.h Michał Kępień
2018-02-20  5:24 ` [PATCH v2 6/7] platform/x86: fujitsu-laptop: Define constants for backlight power control Michał Kępień
2018-02-20  5:24 ` [PATCH v2 7/7] platform/x86: fujitsu-laptop: Clean up constants Michał Kępień
2018-02-20  6:27 ` [PATCH v2 0/7] fujitsu-laptop: Miscellaneous cleanups Jonathan Woithe
2018-02-22 20:55 ` Darren Hart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.