public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Michał Kępień" <kernel@kempniu.pl>
To: Jonathan Woithe <jwoithe@just42.net>,
	Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/6] platform/x86: fujitsu-laptop: refactor LED registration
Date: Fri,  7 Apr 2017 15:07:09 +0200	[thread overview]
Message-ID: <20170407130713.8417-3-kernel@kempniu.pl> (raw)
In-Reply-To: <20170407130713.8417-1-kernel@kempniu.pl>

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

  parent reply	other threads:[~2017-04-07 13:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-07 13:07 [PATCH 0/6] fujitsu-laptop: LED cleanup Michał Kępień
2017-04-07 13:07 ` [PATCH 1/6] platform/x86: fujitsu-laptop: select LEDS_CLASS Michał Kępień
2017-04-07 13:07 ` Michał Kępień [this message]
2017-04-07 13:07 ` [PATCH 3/6] platform/x86: fujitsu-laptop: reorganize LED-related code Michał Kępień
2017-04-07 13:07 ` [PATCH 4/6] platform/x86: fujitsu-laptop: switch to managed LED class devices Michał Kępień
2017-04-07 13:07 ` [PATCH 5/6] platform/x86: fujitsu-laptop: do not log LED registration failures Michał Kępień
2017-04-17 16:09   ` Darren Hart
2017-04-18  8:10     ` Michał Kępień
2017-04-18 16:01       ` Darren Hart
2017-04-19  7:30         ` Jonathan Woithe
2017-04-07 13:07 ` [PATCH 6/6] platform/x86: fujitsu-laptop: simplify error handling in acpi_fujitsu_laptop_add() Michał Kępień
2017-04-13  6:49 ` [PATCH 0/6] fujitsu-laptop: LED cleanup Jonathan Woithe
2017-04-13  7:13   ` Michał Kępień
2017-04-17  9:48 ` Jonathan Woithe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170407130713.8417-3-kernel@kempniu.pl \
    --to=kernel@kempniu.pl \
    --cc=andy@infradead.org \
    --cc=dvhart@infradead.org \
    --cc=jwoithe@just42.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox