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

Groups > linux.kernel > #1618779 > unrolled thread

[PATCH 0/6] fujitsu-laptop: LED cleanup

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] fujitsu-laptop: LED cleanup Michał Kępień <kernel@kempniu.pl> - 2017-04-07 15:10 +0200
    [PATCH 2/6] platform/x86: fujitsu-laptop: refactor LED registration Michał Kępień <kernel@kempniu.pl> - 2017-04-07 15:10 +0200
    [PATCH 5/6] platform/x86: fujitsu-laptop: do not log LED registration failures Michał Kępień <kernel@kempniu.pl> - 2017-04-07 15:10 +0200

#1618779 — [PATCH 0/6] fujitsu-laptop: LED cleanup

This patch series cleans up parts of fujitsu-laptop responsible for
handling LED class devices.  It was tested on a Lifebook E744.  It
depends on the previous patch series I submitted for fujitsu-laptop and
should be applied on top of the backlight cleanup series.

 drivers/platform/x86/Kconfig          |   2 +-
 drivers/platform/x86/fujitsu-laptop.c | 402 +++++++++++++++-------------------
 2 files changed, 176 insertions(+), 228 deletions(-)

-- 
2.12.2

[toc] | [next] | [standalone]

#1618781 — [PATCH 2/6] platform/x86: fujitsu-laptop: refactor LED registration

Move a long section of code responsible for registering LEDs out of
acpi_fujitsu_laptop_add() to improve readability and ensure proper
cleanup of platform device and kfifo e.g. when two supported LEDs are
detected, the first one gets registered successfully but the second one
does not.  This makes the result variable in acpi_fujitsu_laptop_add()
redundant, so remove it.  Adjust whitespace to make checkpatch happy.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/fujitsu-laptop.c | 128 +++++++++++++++++++---------------
 1 file changed, 70 insertions(+), 58 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 555de246b994..4d13cc03d3b1 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -754,9 +754,72 @@ static void fujitsu_laptop_platform_remove(void)
 	platform_device_unregister(fujitsu_laptop->pf_device);
 }
 
-static int acpi_fujitsu_laptop_add(struct acpi_device *device)
+static int acpi_fujitsu_laptop_leds_register(void)
 {
 	int result = 0;
+
+	if (call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
+		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
+					       &logolamp_led);
+		if (result == 0) {
+			fujitsu_laptop->logolamp_registered = 1;
+		} else {
+			pr_err("Could not register LED handler for logo lamp, error %i\n",
+			       result);
+		}
+	}
+
+	if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) &&
+	    (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) {
+		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
+					       &kblamps_led);
+		if (result == 0) {
+			fujitsu_laptop->kblamps_registered = 1;
+		} else {
+			pr_err("Could not register LED handler for keyboard lamps, error %i\n",
+			       result);
+		}
+	}
+
+	/*
+	 * BTNI bit 24 seems to indicate the presence of a radio toggle
+	 * button in place of a slide switch, and all such machines appear
+	 * to also have an RF LED.  Therefore use bit 24 as an indicator
+	 * that an RF LED is present.
+	 */
+	if (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) & BIT(24)) {
+		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
+					       &radio_led);
+		if (result == 0) {
+			fujitsu_laptop->radio_led_registered = 1;
+		} else {
+			pr_err("Could not register LED handler for radio LED, error %i\n",
+			       result);
+		}
+	}
+
+	/* Support for eco led is not always signaled in bit corresponding
+	 * to the bit used to control the led. According to the DSDT table,
+	 * bit 14 seems to indicate presence of said led as well.
+	 * Confirm by testing the status.
+	 */
+	if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & BIT(14)) &&
+	    (call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) {
+		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
+					       &eco_led);
+		if (result == 0) {
+			fujitsu_laptop->eco_led_registered = 1;
+		} else {
+			pr_err("Could not register LED handler for eco LED, error %i\n",
+			       result);
+		}
+	}
+
+	return result;
+}
+
+static int acpi_fujitsu_laptop_add(struct acpi_device *device)
+{
 	int state = 0;
 	int error;
 	int i;
@@ -837,65 +900,14 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 	if (error)
 		goto err_free_fifo;
 
-	if (call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
-		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
-						&logolamp_led);
-		if (result == 0) {
-			fujitsu_laptop->logolamp_registered = 1;
-		} else {
-			pr_err("Could not register LED handler for logo lamp, error %i\n",
-			       result);
-		}
-	}
-
-	if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) &&
-	   (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) {
-		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
-						&kblamps_led);
-		if (result == 0) {
-			fujitsu_laptop->kblamps_registered = 1;
-		} else {
-			pr_err("Could not register LED handler for keyboard lamps, error %i\n",
-			       result);
-		}
-	}
-
-	/*
-	 * BTNI bit 24 seems to indicate the presence of a radio toggle
-	 * button in place of a slide switch, and all such machines appear
-	 * to also have an RF LED.  Therefore use bit 24 as an indicator
-	 * that an RF LED is present.
-	 */
-	if (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) & BIT(24)) {
-		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
-						&radio_led);
-		if (result == 0) {
-			fujitsu_laptop->radio_led_registered = 1;
-		} else {
-			pr_err("Could not register LED handler for radio LED, error %i\n",
-			       result);
-		}
-	}
-
-	/* Support for eco led is not always signaled in bit corresponding
-	 * to the bit used to control the led. According to the DSDT table,
-	 * bit 14 seems to indicate presence of said led as well.
-	 * Confirm by testing the status.
-	*/
-	if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & BIT(14)) &&
-	   (call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) {
-		result = led_classdev_register(&fujitsu_laptop->pf_device->dev,
-						&eco_led);
-		if (result == 0) {
-			fujitsu_laptop->eco_led_registered = 1;
-		} else {
-			pr_err("Could not register LED handler for eco LED, error %i\n",
-			       result);
-		}
-	}
+	error = acpi_fujitsu_laptop_leds_register();
+	if (error)
+		goto err_remove_platform_device;
 
-	return result;
+	return 0;
 
+err_remove_platform_device:
+	fujitsu_laptop_platform_remove();
 err_free_fifo:
 	kfifo_free(&fujitsu_laptop->fifo);
 err_stop:
-- 
2.12.2

[toc] | [prev] | [next] | [standalone]

#1618783 — [PATCH 5/6] platform/x86: fujitsu-laptop: do not log LED registration failures

If acpi_fujitsu_laptop_leds_register() returns an error, the latter will
become the return value of acpi_fujitsu_laptop_add(), which in turn will
be reported by driver core.  Simplify code by replacing pr_err() calls
with return statements.  Return 0 instead of result when no errors occur
in order to make the code easier to read.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/fujitsu-laptop.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index ce658789e748..177b9b57ac2f 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -739,22 +739,20 @@ static struct led_classdev eco_led = {
 
 static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 {
-	int result = 0;
+	int result;
 
 	if (call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
 		result = devm_led_classdev_register(&device->dev,
 						    &logolamp_led);
 		if (result)
-			pr_err("Could not register LED handler for logo lamp, error %i\n",
-			       result);
+			return result;
 	}
 
 	if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) &&
 	    (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) {
 		result = devm_led_classdev_register(&device->dev, &kblamps_led);
 		if (result)
-			pr_err("Could not register LED handler for keyboard lamps, error %i\n",
-			       result);
+			return result;
 	}
 
 	/*
@@ -766,8 +764,7 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 	if (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) & BIT(24)) {
 		result = devm_led_classdev_register(&device->dev, &radio_led);
 		if (result)
-			pr_err("Could not register LED handler for radio LED, error %i\n",
-			       result);
+			return result;
 	}
 
 	/* Support for eco led is not always signaled in bit corresponding
@@ -779,11 +776,10 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 	    (call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) {
 		result = devm_led_classdev_register(&device->dev, &eco_led);
 		if (result)
-			pr_err("Could not register LED handler for eco LED, error %i\n",
-			       result);
+			return result;
 	}
 
-	return result;
+	return 0;
 }
 
 static int acpi_fujitsu_laptop_add(struct acpi_device *device)
-- 
2.12.2

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web