[PATCH v2 0/3] Return codes cleanup

[PATCH v2 0/3] Return codes cleanup

[Azael Avalos]

Up for review.

This series of patches are a cleanup to the Toshiba
configuration interface return codes (unification),
since we are now using both the HCI and the SCI, as
well as changing the returned type of the HCI/SCI
read/write functions from acpi_status to u32, since
the "status" was never checked on most of the functions.

Changes since v1:
- Be a bit more verbose on patch 1 about the Toshiba
  configuration interface
- Merged patches 3 and 4 into a same patch

Azael Avalos (3):
  toshiba_acpi: Rename hci_raw to tci_raw
  toshiba_acpi: Unify return codes prefix from HCI/SCI to TOS
  toshiba_acpi: Change HCI/SCI functions return code type

 drivers/platform/x86/toshiba_acpi.c | 369 ++++++++++++++++++------------------
 1 file changed, 183 insertions(+), 186 deletions(-)

-- 
2.0.0

[PATCH v2 1/3] toshiba_acpi: Rename hci_raw to tci_raw

[Azael Avalos]

The function name hci_raw was used before to reflect
a raw (read/write) call to Toshiba's Hardware
Configuration Interface (HCI), however, since the
introduction of the System Configuration Interface
(SCI), that "name" no longer applies.

This patch changes the name of that function to
tci_raw (for Toshiba Configuration Interface), and
change the comments about it.

Also, the HCI_WORDS definition was changed to TCI_RAW,
to better reflect that we're no longer using pure HCI
calls, but a combination of HCI and SCI, which form
part of the Toshiba Configuration Interface.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_acpi.c | 119 ++++++++++++++++++------------------
 1 file changed, 60 insertions(+), 59 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index edd8f3d..ed3671c 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -71,7 +71,8 @@ MODULE_LICENSE("GPL");
 /* Toshiba ACPI method paths */
 #define METHOD_VIDEO_OUT	"\\_SB_.VALX.DSSX"
 
-/* Toshiba HCI interface definitions
+/* The Toshiba configuration interface is composed of the HCI and the SCI,
+ * which are defined as follows:
  *
  * HCI is Toshiba's "Hardware Control Interface" which is supposed to
  * be uniform across all their models.  Ideally we would just call
@@ -84,7 +85,7 @@ MODULE_LICENSE("GPL");
  * conceal differences in hardware between different models.
  */
 
-#define HCI_WORDS			6
+#define TCI_WORDS			6
 
 /* operations */
 #define HCI_SET				0xff00
@@ -274,22 +275,22 @@ static int write_acpi_int(const char *methodName, int val)
 	return (status == AE_OK) ? 0 : -EIO;
 }
 
-/* Perform a raw HCI call.  Here we don't care about input or output buffer
- * format.
+/* Perform a raw configuration call.  Here we don't care about input or output
+ * buffer format.
  */
-static acpi_status hci_raw(struct toshiba_acpi_dev *dev,
-			   const u32 in[HCI_WORDS], u32 out[HCI_WORDS])
+static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
+			   const u32 in[TCI_WORDS], u32 out[TCI_WORDS])
 {
 	struct acpi_object_list params;
-	union acpi_object in_objs[HCI_WORDS];
+	union acpi_object in_objs[TCI_WORDS];
 	struct acpi_buffer results;
-	union acpi_object out_objs[HCI_WORDS + 1];
+	union acpi_object out_objs[TCI_WORDS + 1];
 	acpi_status status;
 	int i;
 
-	params.count = HCI_WORDS;
+	params.count = TCI_WORDS;
 	params.pointer = in_objs;
-	for (i = 0; i < HCI_WORDS; ++i) {
+	for (i = 0; i < TCI_WORDS; ++i) {
 		in_objs[i].type = ACPI_TYPE_INTEGER;
 		in_objs[i].integer.value = in[i];
 	}
@@ -300,7 +301,7 @@ static acpi_status hci_raw(struct toshiba_acpi_dev *dev,
 	status = acpi_evaluate_object(dev->acpi_dev->handle,
 				      (char *)dev->method_hci, &params,
 				      &results);
-	if ((status == AE_OK) && (out_objs->package.count <= HCI_WORDS)) {
+	if ((status == AE_OK) && (out_objs->package.count <= TCI_WORDS)) {
 		for (i = 0; i < out_objs->package.count; ++i) {
 			out[i] = out_objs->package.elements[i].integer.value;
 		}
@@ -318,9 +319,9 @@ static acpi_status hci_raw(struct toshiba_acpi_dev *dev,
 static acpi_status hci_write1(struct toshiba_acpi_dev *dev, u32 reg,
 			      u32 in1, u32 *result)
 {
-	u32 in[HCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
-	u32 out[HCI_WORDS];
-	acpi_status status = hci_raw(dev, in, out);
+	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
+	u32 out[TCI_WORDS];
+	acpi_status status = tci_raw(dev, in, out);
 	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
 	return status;
 }
@@ -328,9 +329,9 @@ static acpi_status hci_write1(struct toshiba_acpi_dev *dev, u32 reg,
 static acpi_status hci_read1(struct toshiba_acpi_dev *dev, u32 reg,
 			     u32 *out1, u32 *result)
 {
-	u32 in[HCI_WORDS] = { HCI_GET, reg, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
-	acpi_status status = hci_raw(dev, in, out);
+	u32 in[TCI_WORDS] = { HCI_GET, reg, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
+	acpi_status status = tci_raw(dev, in, out);
 	*out1 = out[2];
 	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
 	return status;
@@ -339,9 +340,9 @@ static acpi_status hci_read1(struct toshiba_acpi_dev *dev, u32 reg,
 static acpi_status hci_write2(struct toshiba_acpi_dev *dev, u32 reg,
 			      u32 in1, u32 in2, u32 *result)
 {
-	u32 in[HCI_WORDS] = { HCI_SET, reg, in1, in2, 0, 0 };
-	u32 out[HCI_WORDS];
-	acpi_status status = hci_raw(dev, in, out);
+	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, in2, 0, 0 };
+	u32 out[TCI_WORDS];
+	acpi_status status = tci_raw(dev, in, out);
 	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
 	return status;
 }
@@ -349,9 +350,9 @@ static acpi_status hci_write2(struct toshiba_acpi_dev *dev, u32 reg,
 static acpi_status hci_read2(struct toshiba_acpi_dev *dev, u32 reg,
 			     u32 *out1, u32 *out2, u32 *result)
 {
-	u32 in[HCI_WORDS] = { HCI_GET, reg, *out1, *out2, 0, 0 };
-	u32 out[HCI_WORDS];
-	acpi_status status = hci_raw(dev, in, out);
+	u32 in[TCI_WORDS] = { HCI_GET, reg, *out1, *out2, 0, 0 };
+	u32 out[TCI_WORDS];
+	acpi_status status = tci_raw(dev, in, out);
 	*out1 = out[2];
 	*out2 = out[3];
 	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
@@ -363,11 +364,11 @@ static acpi_status hci_read2(struct toshiba_acpi_dev *dev, u32 reg,
 
 static int sci_open(struct toshiba_acpi_dev *dev)
 {
-	u32 in[HCI_WORDS] = { SCI_OPEN, 0, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { SCI_OPEN, 0, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if  (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
 		pr_err("ACPI call to open SCI failed\n");
 		return 0;
@@ -387,11 +388,11 @@ static int sci_open(struct toshiba_acpi_dev *dev)
 
 static void sci_close(struct toshiba_acpi_dev *dev)
 {
-	u32 in[HCI_WORDS] = { SCI_CLOSE, 0, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { SCI_CLOSE, 0, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
 		pr_err("ACPI call to close SCI failed\n");
 		return;
@@ -408,9 +409,9 @@ static void sci_close(struct toshiba_acpi_dev *dev)
 static acpi_status sci_read(struct toshiba_acpi_dev *dev, u32 reg,
 			    u32 *out1, u32 *result)
 {
-	u32 in[HCI_WORDS] = { SCI_GET, reg, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
-	acpi_status status = hci_raw(dev, in, out);
+	u32 in[TCI_WORDS] = { SCI_GET, reg, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
+	acpi_status status = tci_raw(dev, in, out);
 	*out1 = out[2];
 	*result = (ACPI_SUCCESS(status)) ? out[0] : HCI_FAILURE;
 	return status;
@@ -419,9 +420,9 @@ static acpi_status sci_read(struct toshiba_acpi_dev *dev, u32 reg,
 static acpi_status sci_write(struct toshiba_acpi_dev *dev, u32 reg,
 			     u32 in1, u32 *result)
 {
-	u32 in[HCI_WORDS] = { SCI_SET, reg, in1, 0, 0, 0 };
-	u32 out[HCI_WORDS];
-	acpi_status status = hci_raw(dev, in, out);
+	u32 in[TCI_WORDS] = { SCI_SET, reg, in1, 0, 0, 0 };
+	u32 out[TCI_WORDS];
+	acpi_status status = tci_raw(dev, in, out);
 	*result = (ACPI_SUCCESS(status)) ? out[0] : HCI_FAILURE;
 	return status;
 }
@@ -429,14 +430,14 @@ static acpi_status sci_write(struct toshiba_acpi_dev *dev, u32 reg,
 /* Illumination support */
 static int toshiba_illumination_available(struct toshiba_acpi_dev *dev)
 {
-	u32 in[HCI_WORDS] = { SCI_GET, SCI_ILLUMINATION, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { SCI_GET, SCI_ILLUMINATION, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
 	if (!sci_open(dev))
 		return 0;
 
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	sci_close(dev);
 	if (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
 		pr_err("ACPI call to query Illumination support failed\n");
@@ -502,14 +503,14 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
 /* KBD Illumination */
 static int toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
 {
-	u32 in[HCI_WORDS] = { SCI_GET, SCI_KBD_ILLUM_STATUS, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { SCI_GET, SCI_KBD_ILLUM_STATUS, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
 	if (!sci_open(dev))
 		return 0;
 
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	sci_close(dev);
 	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to query kbd illumination support failed\n");
@@ -663,10 +664,10 @@ static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
 static int toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
 {
 	acpi_status status;
-	u32 in[HCI_WORDS] = { HCI_GET, HCI_ECO_MODE, 0, 1, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { HCI_GET, HCI_ECO_MODE, 0, 1, 0, 0 };
+	u32 out[TCI_WORDS];
 
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
 		pr_info("ACPI call to get ECO led failed\n");
 		return 0;
@@ -679,11 +680,11 @@ static enum led_brightness toshiba_eco_mode_get_status(struct led_classdev *cdev
 {
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, eco_led);
-	u32 in[HCI_WORDS] = { HCI_GET, HCI_ECO_MODE, 0, 1, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { HCI_GET, HCI_ECO_MODE, 0, 1, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to get ECO led failed\n");
 		return LED_OFF;
@@ -697,13 +698,13 @@ static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
 {
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, eco_led);
-	u32 in[HCI_WORDS] = { HCI_SET, HCI_ECO_MODE, 0, 1, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { HCI_SET, HCI_ECO_MODE, 0, 1, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
 	/* Switch the Eco Mode led on/off */
 	in[2] = (brightness) ? 1 : 0;
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to set ECO led failed\n");
 		return;
@@ -713,14 +714,14 @@ static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
 /* Accelerometer support */
 static int toshiba_accelerometer_supported(struct toshiba_acpi_dev *dev)
 {
-	u32 in[HCI_WORDS] = { HCI_GET, HCI_ACCELEROMETER2, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { HCI_GET, HCI_ACCELEROMETER2, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
 	/* Check if the accelerometer call exists,
 	 * this call also serves as initialization
 	 */
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to query the accelerometer failed\n");
 		return -EIO;
@@ -739,12 +740,12 @@ static int toshiba_accelerometer_supported(struct toshiba_acpi_dev *dev)
 static int toshiba_accelerometer_get(struct toshiba_acpi_dev *dev,
 				      u32 *xy, u32 *z)
 {
-	u32 in[HCI_WORDS] = { HCI_GET, HCI_ACCELEROMETER, 0, 1, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { HCI_GET, HCI_ACCELEROMETER, 0, 1, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
 	/* Check the Accelerometer status */
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to query the accelerometer failed\n");
 		return -EIO;
@@ -925,8 +926,8 @@ static int lcd_proc_open(struct inode *inode, struct file *file)
 
 static int set_lcd_brightness(struct toshiba_acpi_dev *dev, int value)
 {
-	u32 in[HCI_WORDS] = { HCI_SET, HCI_LCD_BRIGHTNESS, 0, 0, 0, 0 };
-	u32 out[HCI_WORDS];
+	u32 in[TCI_WORDS] = { HCI_SET, HCI_LCD_BRIGHTNESS, 0, 0, 0, 0 };
+	u32 out[TCI_WORDS];
 	acpi_status status;
 
 	if (dev->tr_backlight_supported) {
@@ -939,7 +940,7 @@ static int set_lcd_brightness(struct toshiba_acpi_dev *dev, int value)
 	}
 
 	in[2] = value << HCI_LCD_BRIGHTNESS_SHIFT;
-	status = hci_raw(dev, in, out);
+	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
 		pr_err("ACPI call to set brightness failed");
 		return -EIO;
-- 
2.0.0

[PATCH v2 2/3] toshiba_acpi: Unify return codes prefix from HCI/SCI to TOS

[Azael Avalos]

The return codes are split in between HCI/SCI prefixes,
but they are shared (used) by both interfaces, mixing
hci_read/write calls with SCI_* return codes, and
sci_read/write calls with HCI_* ones.

This patch changes the prefix of the return codes
definitions, dropping the HCI/SCI naming and instead
replacing it with TOS (for TOShiba).

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_acpi.c | 143 ++++++++++++++++++------------------
 1 file changed, 72 insertions(+), 71 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index ed3671c..589a858 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -96,17 +96,18 @@ MODULE_LICENSE("GPL");
 #define SCI_SET				0xf400
 
 /* return codes */
-#define HCI_SUCCESS			0x0000
-#define HCI_FAILURE			0x1000
-#define HCI_NOT_SUPPORTED		0x8000
-#define HCI_EMPTY			0x8c00
-#define HCI_DATA_NOT_AVAILABLE		0x8d20
-#define HCI_NOT_INITIALIZED		0x8d50
-#define SCI_OPEN_CLOSE_OK		0x0044
-#define SCI_ALREADY_OPEN		0x8100
-#define SCI_NOT_OPENED			0x8200
-#define SCI_INPUT_DATA_ERROR		0x8300
-#define SCI_NOT_PRESENT			0x8600
+#define TOS_SUCCESS			0x0000
+#define TOS_OPEN_CLOSE_OK		0x0044
+#define TOS_FAILURE			0x1000
+#define TOS_NOT_SUPPORTED		0x8000
+#define TOS_ALREADY_OPEN		0x8100
+#define TOS_NOT_OPENED			0x8200
+#define TOS_INPUT_DATA_ERROR		0x8300
+#define TOS_WRITE_PROTECTED		0x8400
+#define TOS_NOT_PRESENT			0x8600
+#define TOS_FIFO_EMPTY			0x8c00
+#define TOS_DATA_NOT_AVAILABLE		0x8d20
+#define TOS_NOT_INITIALIZED		0x8d50
 
 /* registers */
 #define HCI_FAN				0x0004
@@ -322,7 +323,7 @@ static acpi_status hci_write1(struct toshiba_acpi_dev *dev, u32 reg,
 	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
-	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
+	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
 	return status;
 }
 
@@ -333,7 +334,7 @@ static acpi_status hci_read1(struct toshiba_acpi_dev *dev, u32 reg,
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
 	*out1 = out[2];
-	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
+	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
 	return status;
 }
 
@@ -343,7 +344,7 @@ static acpi_status hci_write2(struct toshiba_acpi_dev *dev, u32 reg,
 	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, in2, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
-	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
+	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
 	return status;
 }
 
@@ -355,7 +356,7 @@ static acpi_status hci_read2(struct toshiba_acpi_dev *dev, u32 reg,
 	acpi_status status = tci_raw(dev, in, out);
 	*out1 = out[2];
 	*out2 = out[3];
-	*result = (status == AE_OK) ? out[0] : HCI_FAILURE;
+	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
 	return status;
 }
 
@@ -369,17 +370,17 @@ static int sci_open(struct toshiba_acpi_dev *dev)
 	acpi_status status;
 
 	status = tci_raw(dev, in, out);
-	if  (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
+	if  (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
 		pr_err("ACPI call to open SCI failed\n");
 		return 0;
 	}
 
-	if (out[0] == SCI_OPEN_CLOSE_OK) {
+	if (out[0] == TOS_OPEN_CLOSE_OK) {
 		return 1;
-	} else if (out[0] == SCI_ALREADY_OPEN) {
+	} else if (out[0] == TOS_ALREADY_OPEN) {
 		pr_info("Toshiba SCI already opened\n");
 		return 1;
-	} else if (out[0] == SCI_NOT_PRESENT) {
+	} else if (out[0] == TOS_NOT_PRESENT) {
 		pr_info("Toshiba SCI is not present\n");
 	}
 
@@ -393,16 +394,16 @@ static void sci_close(struct toshiba_acpi_dev *dev)
 	acpi_status status;
 
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
 		pr_err("ACPI call to close SCI failed\n");
 		return;
 	}
 
-	if (out[0] == SCI_OPEN_CLOSE_OK)
+	if (out[0] == TOS_OPEN_CLOSE_OK)
 		return;
-	else if (out[0] == SCI_NOT_OPENED)
+	else if (out[0] == TOS_NOT_OPENED)
 		pr_info("Toshiba SCI not opened\n");
-	else if (out[0] == SCI_NOT_PRESENT)
+	else if (out[0] == TOS_NOT_PRESENT)
 		pr_info("Toshiba SCI is not present\n");
 }
 
@@ -413,7 +414,7 @@ static acpi_status sci_read(struct toshiba_acpi_dev *dev, u32 reg,
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
 	*out1 = out[2];
-	*result = (ACPI_SUCCESS(status)) ? out[0] : HCI_FAILURE;
+	*result = (ACPI_SUCCESS(status)) ? out[0] : TOS_FAILURE;
 	return status;
 }
 
@@ -423,7 +424,7 @@ static acpi_status sci_write(struct toshiba_acpi_dev *dev, u32 reg,
 	u32 in[TCI_WORDS] = { SCI_SET, reg, in1, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
-	*result = (ACPI_SUCCESS(status)) ? out[0] : HCI_FAILURE;
+	*result = (ACPI_SUCCESS(status)) ? out[0] : TOS_FAILURE;
 	return status;
 }
 
@@ -439,10 +440,10 @@ static int toshiba_illumination_available(struct toshiba_acpi_dev *dev)
 
 	status = tci_raw(dev, in, out);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
 		pr_err("ACPI call to query Illumination support failed\n");
 		return 0;
-	} else if (out[0] == HCI_NOT_SUPPORTED) {
+	} else if (out[0] == TOS_NOT_SUPPORTED) {
 		pr_info("Illumination device not available\n");
 		return 0;
 	}
@@ -469,7 +470,7 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call for illumination failed\n");
 		return;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		pr_info("Illumination not supported\n");
 		return;
 	}
@@ -489,10 +490,10 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
 	/* Check the illumination */
 	status = sci_read(dev, SCI_ILLUMINATION, &state, &result);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || result == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call for illumination failed\n");
 		return LED_OFF;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		pr_info("Illumination not supported\n");
 		return LED_OFF;
 	}
@@ -512,10 +513,10 @@ static int toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
 
 	status = tci_raw(dev, in, out);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to query kbd illumination support failed\n");
 		return 0;
-	} else if (out[0] == HCI_NOT_SUPPORTED) {
+	} else if (out[0] == TOS_NOT_SUPPORTED) {
 		pr_info("Keyboard illumination not available\n");
 		return 0;
 	}
@@ -547,10 +548,10 @@ static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
 
 	status = sci_write(dev, SCI_KBD_ILLUM_STATUS, time, &result);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || result == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to set KBD backlight status failed\n");
 		return -EIO;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		pr_info("Keyboard backlight status not supported\n");
 		return -ENODEV;
 	}
@@ -568,10 +569,10 @@ static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
 
 	status = sci_read(dev, SCI_KBD_ILLUM_STATUS, time, &result);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || result == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to get KBD backlight status failed\n");
 		return -EIO;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		pr_info("Keyboard backlight status not supported\n");
 		return -ENODEV;
 	}
@@ -588,10 +589,10 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
 
 	/* Check the keyboard backlight state */
 	status = hci_read1(dev, HCI_KBD_ILLUMINATION, &state, &result);
-	if (ACPI_FAILURE(status) || result == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to get the keyboard backlight failed\n");
 		return LED_OFF;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		pr_info("Keyboard backlight not supported\n");
 		return LED_OFF;
 	}
@@ -610,10 +611,10 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
 	/* Set the keyboard backlight state */
 	state = brightness ? 1 : 0;
 	status = hci_write1(dev, HCI_KBD_ILLUMINATION, state, &result);
-	if (ACPI_FAILURE(status) || result == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to set KBD Illumination mode failed\n");
 		return;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		pr_info("Keyboard backlight not supported\n");
 		return;
 	}
@@ -633,7 +634,7 @@ static int toshiba_touchpad_set(struct toshiba_acpi_dev *dev, u32 state)
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to set the touchpad failed\n");
 		return -EIO;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		return -ENODEV;
 	}
 
@@ -653,7 +654,7 @@ static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to query the touchpad failed\n");
 		return -EIO;
-	} else if (result == HCI_NOT_SUPPORTED) {
+	} else if (result == TOS_NOT_SUPPORTED) {
 		return -ENODEV;
 	}
 
@@ -668,7 +669,7 @@ static int toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
 	u32 out[TCI_WORDS];
 
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
 		pr_info("ACPI call to get ECO led failed\n");
 		return 0;
 	}
@@ -685,7 +686,7 @@ static enum led_brightness toshiba_eco_mode_get_status(struct led_classdev *cdev
 	acpi_status status;
 
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to get ECO led failed\n");
 		return LED_OFF;
 	}
@@ -705,7 +706,7 @@ static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
 	/* Switch the Eco Mode led on/off */
 	in[2] = (brightness) ? 1 : 0;
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to set ECO led failed\n");
 		return;
 	}
@@ -722,14 +723,14 @@ static int toshiba_accelerometer_supported(struct toshiba_acpi_dev *dev)
 	 * this call also serves as initialization
 	 */
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to query the accelerometer failed\n");
 		return -EIO;
-	} else if (out[0] == HCI_DATA_NOT_AVAILABLE ||
-		   out[0] == HCI_NOT_INITIALIZED) {
+	} else if (out[0] == TOS_DATA_NOT_AVAILABLE ||
+		   out[0] == TOS_NOT_INITIALIZED) {
 		pr_err("Accelerometer not initialized\n");
 		return -EIO;
-	} else if (out[0] == HCI_NOT_SUPPORTED) {
+	} else if (out[0] == TOS_NOT_SUPPORTED) {
 		pr_info("Accelerometer not supported\n");
 		return -ENODEV;
 	}
@@ -746,7 +747,7 @@ static int toshiba_accelerometer_get(struct toshiba_acpi_dev *dev,
 
 	/* Check the Accelerometer status */
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == SCI_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to query the accelerometer failed\n");
 		return -EIO;
 	}
@@ -767,7 +768,7 @@ static u32 hci_get_bt_present(struct toshiba_acpi_dev *dev, bool *present)
 	value = 0;
 	value2 = 0;
 	hci_read2(dev, HCI_WIRELESS, &value, &value2, &hci_result);
-	if (hci_result == HCI_SUCCESS)
+	if (hci_result == TOS_SUCCESS)
 		*present = (value & HCI_WIRELESS_BT_PRESENT) ? true : false;
 
 	return hci_result;
@@ -797,7 +798,7 @@ static int bt_rfkill_set_block(void *data, bool blocked)
 	value = (blocked == false);
 
 	mutex_lock(&dev->mutex);
-	if (hci_get_radio_state(dev, &radio_state) != HCI_SUCCESS) {
+	if (hci_get_radio_state(dev, &radio_state) != TOS_SUCCESS) {
 		err = -EIO;
 		goto out;
 	}
@@ -810,7 +811,7 @@ static int bt_rfkill_set_block(void *data, bool blocked)
 	hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_POWER, &result1);
 	hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_ATTACH, &result2);
 
-	if (result1 != HCI_SUCCESS || result2 != HCI_SUCCESS)
+	if (result1 != TOS_SUCCESS || result2 != TOS_SUCCESS)
 		err = -EIO;
 	else
 		err = 0;
@@ -829,7 +830,7 @@ static void bt_rfkill_poll(struct rfkill *rfkill, void *data)
 	mutex_lock(&dev->mutex);
 
 	hci_result = hci_get_radio_state(dev, &value);
-	if (hci_result != HCI_SUCCESS) {
+	if (hci_result != TOS_SUCCESS) {
 		/* Can't do anything useful */
 		mutex_unlock(&dev->mutex);
 		return;
@@ -855,7 +856,7 @@ static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, bool *enabled)
 
 	hci_read1(dev, HCI_TR_BACKLIGHT, &status, &hci_result);
 	*enabled = !status;
-	return hci_result == HCI_SUCCESS ? 0 : -EIO;
+	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
 
 static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
@@ -864,7 +865,7 @@ static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
 	u32 value = !enable;
 
 	hci_write1(dev, HCI_TR_BACKLIGHT, value, &hci_result);
-	return hci_result == HCI_SUCCESS ? 0 : -EIO;
+	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
 
 static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
@@ -886,7 +887,7 @@ static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
 	}
 
 	hci_read1(dev, HCI_LCD_BRIGHTNESS, &value, &hci_result);
-	if (hci_result == HCI_SUCCESS)
+	if (hci_result == TOS_SUCCESS)
 		return brightness + (value >> HCI_LCD_BRIGHTNESS_SHIFT);
 
 	return -EIO;
@@ -941,18 +942,18 @@ static int set_lcd_brightness(struct toshiba_acpi_dev *dev, int value)
 
 	in[2] = value << HCI_LCD_BRIGHTNESS_SHIFT;
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
+	if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
 		pr_err("ACPI call to set brightness failed");
 		return -EIO;
 	}
 	/* Extra check for "incomplete" backlight method, where the AML code
-	 * doesn't check for HCI_SET or HCI_GET and returns HCI_SUCCESS,
+	 * doesn't check for HCI_SET or HCI_GET and returns TOS_SUCCESS,
 	 * the actual brightness, and in some cases the max brightness.
 	 */
 	if (out[2] > 0  || out[3] == 0xE000)
 		return -ENODEV;
 
-	return out[0] == HCI_SUCCESS ? 0 : -EIO;
+	return out[0] == TOS_SUCCESS ? 0 : -EIO;
 }
 
 static int set_lcd_status(struct backlight_device *bd)
@@ -1001,7 +1002,7 @@ static int get_video_status(struct toshiba_acpi_dev *dev, u32 *status)
 	u32 hci_result;
 
 	hci_read1(dev, HCI_VIDEO_OUT, status, &hci_result);
-	return hci_result == HCI_SUCCESS ? 0 : -EIO;
+	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
 
 static int video_proc_show(struct seq_file *m, void *v)
@@ -1105,7 +1106,7 @@ static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
 	u32 hci_result;
 
 	hci_read1(dev, HCI_FAN, status, &hci_result);
-	return hci_result == HCI_SUCCESS ? 0 : -EIO;
+	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
 
 static int fan_proc_show(struct seq_file *m, void *v)
@@ -1145,7 +1146,7 @@ static ssize_t fan_proc_write(struct file *file, const char __user *buf,
 	if (sscanf(cmd, " force_on : %i", &value) == 1 &&
 	    value >= 0 && value <= 1) {
 		hci_write1(dev, HCI_FAN, value, &hci_result);
-		if (hci_result != HCI_SUCCESS)
+		if (hci_result != TOS_SUCCESS)
 			return -EIO;
 		else
 			dev->force_fan = value;
@@ -1173,12 +1174,12 @@ static int keys_proc_show(struct seq_file *m, void *v)
 
 	if (!dev->key_event_valid && dev->system_event_supported) {
 		hci_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
-		if (hci_result == HCI_SUCCESS) {
+		if (hci_result == TOS_SUCCESS) {
 			dev->key_event_valid = 1;
 			dev->last_key_event = value;
-		} else if (hci_result == HCI_EMPTY) {
+		} else if (hci_result == TOS_FIFO_EMPTY) {
 			/* better luck next time */
-		} else if (hci_result == HCI_NOT_SUPPORTED) {
+		} else if (hci_result == TOS_NOT_SUPPORTED) {
 			/* This is a workaround for an unresolved issue on
 			 * some machines where system events sporadically
 			 * become disabled. */
@@ -1677,7 +1678,7 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 		dev->info_supported = 1;
 	else {
 		hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
-		if (hci_result == HCI_SUCCESS)
+		if (hci_result == TOS_SUCCESS)
 			dev->system_event_supported = 1;
 	}
 
@@ -1857,7 +1858,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 		goto error;
 
 	/* Register rfkill switch for Bluetooth */
-	if (hci_get_bt_present(dev, &bt_present) == HCI_SUCCESS && bt_present) {
+	if (hci_get_bt_present(dev, &bt_present) == TOS_SUCCESS && bt_present) {
 		dev->bt_rfk = rfkill_alloc("Toshiba Bluetooth",
 					   &acpi_dev->dev,
 					   RFKILL_TYPE_BLUETOOTH,
@@ -1962,10 +1963,10 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
 		do {
 			hci_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
 			switch (hci_result) {
-			case HCI_SUCCESS:
+			case TOS_SUCCESS:
 				toshiba_acpi_report_hotkey(dev, (int)value);
 				break;
-			case HCI_NOT_SUPPORTED:
+			case TOS_NOT_SUPPORTED:
 				/*
 				 * This is a workaround for an unresolved
 				 * issue on some machines where system events
@@ -1979,7 +1980,7 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
 				retries--;
 				break;
 			}
-		} while (retries && hci_result != HCI_EMPTY);
+		} while (retries && hci_result != TOS_FIFO_EMPTY);
 	}
 }
 
-- 
2.0.0

[PATCH v2 3/3] toshiba_acpi: Change HCI/SCI functions return code type

[Azael Avalos]

Currently the HCI/SCI read/write functions are returning
the status of the ACPI call and also assigning the
returned value of the HCI/SCI function, however, only
the HCI/SCI status is being checked.

This patch changes such functions, returning the value
of the HCI/SCI function instead of the ACPI call status,
eliminating one parameter, and returning something
useful that indeed is being checked.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_acpi.c | 129 +++++++++++++++++-------------------
 1 file changed, 62 insertions(+), 67 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 589a858..5d509ea 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -317,47 +317,49 @@ static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
  * may be useful (such as "not supported").
  */
 
-static acpi_status hci_write1(struct toshiba_acpi_dev *dev, u32 reg,
-			      u32 in1, u32 *result)
+static u32 hci_write1(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
 {
 	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
-	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
-	return status;
+
+	return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
 }
 
-static acpi_status hci_read1(struct toshiba_acpi_dev *dev, u32 reg,
-			     u32 *out1, u32 *result)
+static u32 hci_read1(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
 {
 	u32 in[TCI_WORDS] = { HCI_GET, reg, 0, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
+	if (ACPI_FAILURE(status))
+		return TOS_FAILURE;
+
 	*out1 = out[2];
-	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
-	return status;
+
+	return out[0];
 }
 
-static acpi_status hci_write2(struct toshiba_acpi_dev *dev, u32 reg,
-			      u32 in1, u32 in2, u32 *result)
+static u32 hci_write2(struct toshiba_acpi_dev *dev, u32 reg, u32 in1, u32 in2)
 {
 	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, in2, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
-	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
-	return status;
+
+	return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
 }
 
-static acpi_status hci_read2(struct toshiba_acpi_dev *dev, u32 reg,
-			     u32 *out1, u32 *out2, u32 *result)
+static u32 hci_read2(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1, u32 *out2)
 {
 	u32 in[TCI_WORDS] = { HCI_GET, reg, *out1, *out2, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
+	if (ACPI_FAILURE(status))
+		return TOS_FAILURE;
+
 	*out1 = out[2];
 	*out2 = out[3];
-	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
-	return status;
+
+	return out[0];
 }
 
 /* common sci tasks
@@ -407,25 +409,26 @@ static void sci_close(struct toshiba_acpi_dev *dev)
 		pr_info("Toshiba SCI is not present\n");
 }
 
-static acpi_status sci_read(struct toshiba_acpi_dev *dev, u32 reg,
-			    u32 *out1, u32 *result)
+static u32 sci_read(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
 {
 	u32 in[TCI_WORDS] = { SCI_GET, reg, 0, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
+	if (ACPI_FAILURE(status))
+		return TOS_FAILURE;
+
 	*out1 = out[2];
-	*result = (ACPI_SUCCESS(status)) ? out[0] : TOS_FAILURE;
-	return status;
+
+	return out[0];
 }
 
-static acpi_status sci_write(struct toshiba_acpi_dev *dev, u32 reg,
-			     u32 in1, u32 *result)
+static u32 sci_write(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
 {
 	u32 in[TCI_WORDS] = { SCI_SET, reg, in1, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status = tci_raw(dev, in, out);
-	*result = (ACPI_SUCCESS(status)) ? out[0] : TOS_FAILURE;
-	return status;
+
+	return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
 }
 
 /* Illumination support */
@@ -457,7 +460,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, led_dev);
 	u32 state, result;
-	acpi_status status;
 
 	/* First request : initialize communication. */
 	if (!sci_open(dev))
@@ -465,9 +467,9 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
 
 	/* Switch the illumination on/off */
 	state = brightness ? 1 : 0;
-	status = sci_write(dev, SCI_ILLUMINATION, state, &result);
+	result = sci_write(dev, SCI_ILLUMINATION, state);
 	sci_close(dev);
-	if (ACPI_FAILURE(status)) {
+	if (result == TOS_FAILURE) {
 		pr_err("ACPI call for illumination failed\n");
 		return;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -481,16 +483,15 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, led_dev);
 	u32 state, result;
-	acpi_status status;
 
 	/* First request : initialize communication. */
 	if (!sci_open(dev))
 		return LED_OFF;
 
 	/* Check the illumination */
-	status = sci_read(dev, SCI_ILLUMINATION, &state, &result);
+	result = sci_read(dev, SCI_ILLUMINATION, &state);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call for illumination failed\n");
 		return LED_OFF;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -541,14 +542,13 @@ static int toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
 static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
 {
 	u32 result;
-	acpi_status status;
 
 	if (!sci_open(dev))
 		return -EIO;
 
-	status = sci_write(dev, SCI_KBD_ILLUM_STATUS, time, &result);
+	result = sci_write(dev, SCI_KBD_ILLUM_STATUS, time);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to set KBD backlight status failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -562,14 +562,13 @@ static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
 static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
 {
 	u32 result;
-	acpi_status status;
 
 	if (!sci_open(dev))
 		return -EIO;
 
-	status = sci_read(dev, SCI_KBD_ILLUM_STATUS, time, &result);
+	result = sci_read(dev, SCI_KBD_ILLUM_STATUS, time);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to get KBD backlight status failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -585,11 +584,10 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, kbd_led);
 	u32 state, result;
-	acpi_status status;
 
 	/* Check the keyboard backlight state */
-	status = hci_read1(dev, HCI_KBD_ILLUMINATION, &state, &result);
-	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
+	result = hci_read1(dev, HCI_KBD_ILLUMINATION, &state);
+	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to get the keyboard backlight failed\n");
 		return LED_OFF;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -606,12 +604,11 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
 	struct toshiba_acpi_dev *dev = container_of(cdev,
 			struct toshiba_acpi_dev, kbd_led);
 	u32 state, result;
-	acpi_status status;
 
 	/* Set the keyboard backlight state */
 	state = brightness ? 1 : 0;
-	status = hci_write1(dev, HCI_KBD_ILLUMINATION, state, &result);
-	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
+	result = hci_write1(dev, HCI_KBD_ILLUMINATION, state);
+	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
 		pr_err("ACPI call to set KBD Illumination mode failed\n");
 		return;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -624,14 +621,13 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
 static int toshiba_touchpad_set(struct toshiba_acpi_dev *dev, u32 state)
 {
 	u32 result;
-	acpi_status status;
 
 	if (!sci_open(dev))
 		return -EIO;
 
-	status = sci_write(dev, SCI_TOUCHPAD, state, &result);
+	result = sci_write(dev, SCI_TOUCHPAD, state);
 	sci_close(dev);
-	if (ACPI_FAILURE(status)) {
+	if (result == TOS_FAILURE) {
 		pr_err("ACPI call to set the touchpad failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -644,14 +640,13 @@ static int toshiba_touchpad_set(struct toshiba_acpi_dev *dev, u32 state)
 static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
 {
 	u32 result;
-	acpi_status status;
 
 	if (!sci_open(dev))
 		return -EIO;
 
-	status = sci_read(dev, SCI_TOUCHPAD, state, &result);
+	result = sci_read(dev, SCI_TOUCHPAD, state);
 	sci_close(dev);
-	if (ACPI_FAILURE(status)) {
+	if (result == TOS_FAILURE) {
 		pr_err("ACPI call to query the touchpad failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
@@ -767,7 +762,7 @@ static u32 hci_get_bt_present(struct toshiba_acpi_dev *dev, bool *present)
 
 	value = 0;
 	value2 = 0;
-	hci_read2(dev, HCI_WIRELESS, &value, &value2, &hci_result);
+	hci_result = hci_read2(dev, HCI_WIRELESS, &value, &value2);
 	if (hci_result == TOS_SUCCESS)
 		*present = (value & HCI_WIRELESS_BT_PRESENT) ? true : false;
 
@@ -781,7 +776,7 @@ static u32 hci_get_radio_state(struct toshiba_acpi_dev *dev, bool *radio_state)
 
 	value = 0;
 	value2 = 0x0001;
-	hci_read2(dev, HCI_WIRELESS, &value, &value2, &hci_result);
+	hci_result = hci_read2(dev, HCI_WIRELESS, &value, &value2);
 
 	*radio_state = value & HCI_WIRELESS_KILL_SWITCH;
 	return hci_result;
@@ -808,8 +803,8 @@ static int bt_rfkill_set_block(void *data, bool blocked)
 		goto out;
 	}
 
-	hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_POWER, &result1);
-	hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_ATTACH, &result2);
+	result1 = hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_POWER);
+	result2 = hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_ATTACH);
 
 	if (result1 != TOS_SUCCESS || result2 != TOS_SUCCESS)
 		err = -EIO;
@@ -854,7 +849,7 @@ static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, bool *enabled)
 	u32 hci_result;
 	u32 status;
 
-	hci_read1(dev, HCI_TR_BACKLIGHT, &status, &hci_result);
+	hci_result = hci_read1(dev, HCI_TR_BACKLIGHT, &status);
 	*enabled = !status;
 	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
@@ -864,7 +859,7 @@ static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
 	u32 hci_result;
 	u32 value = !enable;
 
-	hci_write1(dev, HCI_TR_BACKLIGHT, value, &hci_result);
+	hci_result = hci_write1(dev, HCI_TR_BACKLIGHT, value);
 	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
 
@@ -886,7 +881,7 @@ static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
 		brightness++;
 	}
 
-	hci_read1(dev, HCI_LCD_BRIGHTNESS, &value, &hci_result);
+	hci_result = hci_read1(dev, HCI_LCD_BRIGHTNESS, &value);
 	if (hci_result == TOS_SUCCESS)
 		return brightness + (value >> HCI_LCD_BRIGHTNESS_SHIFT);
 
@@ -1001,7 +996,7 @@ static int get_video_status(struct toshiba_acpi_dev *dev, u32 *status)
 {
 	u32 hci_result;
 
-	hci_read1(dev, HCI_VIDEO_OUT, status, &hci_result);
+	hci_result = hci_read1(dev, HCI_VIDEO_OUT, status);
 	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
 
@@ -1105,7 +1100,7 @@ static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
 {
 	u32 hci_result;
 
-	hci_read1(dev, HCI_FAN, status, &hci_result);
+	hci_result = hci_read1(dev, HCI_FAN, status);
 	return hci_result == TOS_SUCCESS ? 0 : -EIO;
 }
 
@@ -1145,7 +1140,7 @@ static ssize_t fan_proc_write(struct file *file, const char __user *buf,
 
 	if (sscanf(cmd, " force_on : %i", &value) == 1 &&
 	    value >= 0 && value <= 1) {
-		hci_write1(dev, HCI_FAN, value, &hci_result);
+		hci_result = hci_write1(dev, HCI_FAN, value);
 		if (hci_result != TOS_SUCCESS)
 			return -EIO;
 		else
@@ -1173,7 +1168,7 @@ static int keys_proc_show(struct seq_file *m, void *v)
 	u32 value;
 
 	if (!dev->key_event_valid && dev->system_event_supported) {
-		hci_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
+		hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
 		if (hci_result == TOS_SUCCESS) {
 			dev->key_event_valid = 1;
 			dev->last_key_event = value;
@@ -1183,7 +1178,7 @@ static int keys_proc_show(struct seq_file *m, void *v)
 			/* This is a workaround for an unresolved issue on
 			 * some machines where system events sporadically
 			 * become disabled. */
-			hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
+			hci_result = hci_write1(dev, HCI_SYSTEM_EVENT, 1);
 			pr_notice("Re-enabled hotkeys\n");
 		} else {
 			pr_err("Error reading hotkey status\n");
@@ -1677,7 +1672,7 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 	if (acpi_has_method(dev->acpi_dev->handle, "INFO"))
 		dev->info_supported = 1;
 	else {
-		hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
+		hci_result = hci_write1(dev, HCI_SYSTEM_EVENT, 1);
 		if (hci_result == TOS_SUCCESS)
 			dev->system_event_supported = 1;
 	}
@@ -1700,7 +1695,7 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 		goto err_remove_filter;
 	}
 
-	hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, &hci_result);
+	hci_result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
 	return 0;
 
  err_remove_filter:
@@ -1961,7 +1956,7 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
 			toshiba_acpi_report_hotkey(dev, scancode);
 	} else if (dev->system_event_supported) {
 		do {
-			hci_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
+			hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
 			switch (hci_result) {
 			case TOS_SUCCESS:
 				toshiba_acpi_report_hotkey(dev, (int)value);
@@ -1972,8 +1967,8 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
 				 * issue on some machines where system events
 				 * sporadically become disabled.
 				 */
-				hci_write1(dev, HCI_SYSTEM_EVENT, 1,
-					   &hci_result);
+				hci_result =
+					hci_write1(dev, HCI_SYSTEM_EVENT, 1);
 				pr_notice("Re-enabled hotkeys\n");
 				/* fall through */
 			default:
@@ -1991,7 +1986,7 @@ static int toshiba_acpi_suspend(struct device *device)
 	u32 result;
 
 	if (dev->hotkey_dev)
-		hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE, &result);
+		result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE);
 
 	return 0;
 }
@@ -2008,7 +2003,7 @@ static int toshiba_acpi_resume(struct device *device)
 		if (ACPI_FAILURE(status))
 			pr_info("Unable to re-enable hotkeys\n");
 
-		hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, &result);
+		result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
 	}
 
 	return 0;
-- 
2.0.0

Re: [PATCH v2 0/3] Return codes cleanup

[Darren Hart]

On Mon, Sep 29, 2014 at 08:40:06PM -0600, Azael Avalos wrote:
> Up for review.
> 
> This series of patches are a cleanup to the Toshiba
> configuration interface return codes (unification),
> since we are now using both the HCI and the SCI, as
> well as changing the returned type of the HCI/SCI
> read/write functions from acpi_status to u32, since
> the "status" was never checked on most of the functions.
> 
> Changes since v1:
> - Be a bit more verbose on patch 1 about the Toshiba
>   configuration interface
> - Merged patches 3 and 4 into a same patch
> 
> Azael Avalos (3):
>   toshiba_acpi: Rename hci_raw to tci_raw
>   toshiba_acpi: Unify return codes prefix from HCI/SCI to TOS
>   toshiba_acpi: Change HCI/SCI functions return code type
> 
>  drivers/platform/x86/toshiba_acpi.c | 369 ++++++++++++++++++------------------
>  1 file changed, 183 insertions(+), 186 deletions(-)

Queued to the testing branch. As we are at rc7 for 3.17 at this point, and this is a
cleanup, it is going to have to wait for the 3.18 merge window.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH v2 3/3] toshiba_acpi: Change HCI/SCI functions return code type

[Darren Hart]

On Mon, Sep 29, 2014 at 08:40:09PM -0600, Azael Avalos wrote:
> Currently the HCI/SCI read/write functions are returning
> the status of the ACPI call and also assigning the
> returned value of the HCI/SCI function, however, only
> the HCI/SCI status is being checked.
> 
> This patch changes such functions, returning the value
> of the HCI/SCI function instead of the ACPI call status,
> eliminating one parameter, and returning something
> useful that indeed is being checked.
> 
> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>

Cc linux-acpi

Rafael,

This follows a couple patches renaming interfaces and error codes. While
there is some information to be had in checking for ACPI specific errors, I
don't think it's significant to warrant asking Azael to go the other way and
check for them specifically and add errorcodes to the interface rather than
cleanup the functionality as it stands today and simplifiy the code as he does
here.

Any objection?

> ---
>  drivers/platform/x86/toshiba_acpi.c | 129 +++++++++++++++++-------------------
>  1 file changed, 62 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 589a858..5d509ea 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -317,47 +317,49 @@ static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
>   * may be useful (such as "not supported").
>   */
>  
> -static acpi_status hci_write1(struct toshiba_acpi_dev *dev, u32 reg,
> -			      u32 in1, u32 *result)
> +static u32 hci_write1(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
>  {
>  	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
>  	u32 out[TCI_WORDS];
>  	acpi_status status = tci_raw(dev, in, out);
> -	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
> -	return status;
> +
> +	return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
>  }
>  
> -static acpi_status hci_read1(struct toshiba_acpi_dev *dev, u32 reg,
> -			     u32 *out1, u32 *result)
> +static u32 hci_read1(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
>  {
>  	u32 in[TCI_WORDS] = { HCI_GET, reg, 0, 0, 0, 0 };
>  	u32 out[TCI_WORDS];
>  	acpi_status status = tci_raw(dev, in, out);
> +	if (ACPI_FAILURE(status))
> +		return TOS_FAILURE;
> +
>  	*out1 = out[2];
> -	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
> -	return status;
> +
> +	return out[0];
>  }
>  
> -static acpi_status hci_write2(struct toshiba_acpi_dev *dev, u32 reg,
> -			      u32 in1, u32 in2, u32 *result)
> +static u32 hci_write2(struct toshiba_acpi_dev *dev, u32 reg, u32 in1, u32 in2)
>  {
>  	u32 in[TCI_WORDS] = { HCI_SET, reg, in1, in2, 0, 0 };
>  	u32 out[TCI_WORDS];
>  	acpi_status status = tci_raw(dev, in, out);
> -	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
> -	return status;
> +
> +	return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
>  }
>  
> -static acpi_status hci_read2(struct toshiba_acpi_dev *dev, u32 reg,
> -			     u32 *out1, u32 *out2, u32 *result)
> +static u32 hci_read2(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1, u32 *out2)
>  {
>  	u32 in[TCI_WORDS] = { HCI_GET, reg, *out1, *out2, 0, 0 };
>  	u32 out[TCI_WORDS];
>  	acpi_status status = tci_raw(dev, in, out);
> +	if (ACPI_FAILURE(status))
> +		return TOS_FAILURE;
> +
>  	*out1 = out[2];
>  	*out2 = out[3];
> -	*result = (status == AE_OK) ? out[0] : TOS_FAILURE;
> -	return status;
> +
> +	return out[0];
>  }
>  
>  /* common sci tasks
> @@ -407,25 +409,26 @@ static void sci_close(struct toshiba_acpi_dev *dev)
>  		pr_info("Toshiba SCI is not present\n");
>  }
>  
> -static acpi_status sci_read(struct toshiba_acpi_dev *dev, u32 reg,
> -			    u32 *out1, u32 *result)
> +static u32 sci_read(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
>  {
>  	u32 in[TCI_WORDS] = { SCI_GET, reg, 0, 0, 0, 0 };
>  	u32 out[TCI_WORDS];
>  	acpi_status status = tci_raw(dev, in, out);
> +	if (ACPI_FAILURE(status))
> +		return TOS_FAILURE;
> +
>  	*out1 = out[2];
> -	*result = (ACPI_SUCCESS(status)) ? out[0] : TOS_FAILURE;
> -	return status;
> +
> +	return out[0];
>  }
>  
> -static acpi_status sci_write(struct toshiba_acpi_dev *dev, u32 reg,
> -			     u32 in1, u32 *result)
> +static u32 sci_write(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
>  {
>  	u32 in[TCI_WORDS] = { SCI_SET, reg, in1, 0, 0, 0 };
>  	u32 out[TCI_WORDS];
>  	acpi_status status = tci_raw(dev, in, out);
> -	*result = (ACPI_SUCCESS(status)) ? out[0] : TOS_FAILURE;
> -	return status;
> +
> +	return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
>  }
>  
>  /* Illumination support */
> @@ -457,7 +460,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
>  	struct toshiba_acpi_dev *dev = container_of(cdev,
>  			struct toshiba_acpi_dev, led_dev);
>  	u32 state, result;
> -	acpi_status status;
>  
>  	/* First request : initialize communication. */
>  	if (!sci_open(dev))
> @@ -465,9 +467,9 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
>  
>  	/* Switch the illumination on/off */
>  	state = brightness ? 1 : 0;
> -	status = sci_write(dev, SCI_ILLUMINATION, state, &result);
> +	result = sci_write(dev, SCI_ILLUMINATION, state);
>  	sci_close(dev);
> -	if (ACPI_FAILURE(status)) {
> +	if (result == TOS_FAILURE) {
>  		pr_err("ACPI call for illumination failed\n");
>  		return;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -481,16 +483,15 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
>  	struct toshiba_acpi_dev *dev = container_of(cdev,
>  			struct toshiba_acpi_dev, led_dev);
>  	u32 state, result;
> -	acpi_status status;
>  
>  	/* First request : initialize communication. */
>  	if (!sci_open(dev))
>  		return LED_OFF;
>  
>  	/* Check the illumination */
> -	status = sci_read(dev, SCI_ILLUMINATION, &state, &result);
> +	result = sci_read(dev, SCI_ILLUMINATION, &state);
>  	sci_close(dev);
> -	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
> +	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
>  		pr_err("ACPI call for illumination failed\n");
>  		return LED_OFF;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -541,14 +542,13 @@ static int toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
>  static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
>  {
>  	u32 result;
> -	acpi_status status;
>  
>  	if (!sci_open(dev))
>  		return -EIO;
>  
> -	status = sci_write(dev, SCI_KBD_ILLUM_STATUS, time, &result);
> +	result = sci_write(dev, SCI_KBD_ILLUM_STATUS, time);
>  	sci_close(dev);
> -	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
> +	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
>  		pr_err("ACPI call to set KBD backlight status failed\n");
>  		return -EIO;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -562,14 +562,13 @@ static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
>  static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
>  {
>  	u32 result;
> -	acpi_status status;
>  
>  	if (!sci_open(dev))
>  		return -EIO;
>  
> -	status = sci_read(dev, SCI_KBD_ILLUM_STATUS, time, &result);
> +	result = sci_read(dev, SCI_KBD_ILLUM_STATUS, time);
>  	sci_close(dev);
> -	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
> +	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
>  		pr_err("ACPI call to get KBD backlight status failed\n");
>  		return -EIO;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -585,11 +584,10 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
>  	struct toshiba_acpi_dev *dev = container_of(cdev,
>  			struct toshiba_acpi_dev, kbd_led);
>  	u32 state, result;
> -	acpi_status status;
>  
>  	/* Check the keyboard backlight state */
> -	status = hci_read1(dev, HCI_KBD_ILLUMINATION, &state, &result);
> -	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
> +	result = hci_read1(dev, HCI_KBD_ILLUMINATION, &state);
> +	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
>  		pr_err("ACPI call to get the keyboard backlight failed\n");
>  		return LED_OFF;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -606,12 +604,11 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
>  	struct toshiba_acpi_dev *dev = container_of(cdev,
>  			struct toshiba_acpi_dev, kbd_led);
>  	u32 state, result;
> -	acpi_status status;
>  
>  	/* Set the keyboard backlight state */
>  	state = brightness ? 1 : 0;
> -	status = hci_write1(dev, HCI_KBD_ILLUMINATION, state, &result);
> -	if (ACPI_FAILURE(status) || result == TOS_INPUT_DATA_ERROR) {
> +	result = hci_write1(dev, HCI_KBD_ILLUMINATION, state);
> +	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
>  		pr_err("ACPI call to set KBD Illumination mode failed\n");
>  		return;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -624,14 +621,13 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
>  static int toshiba_touchpad_set(struct toshiba_acpi_dev *dev, u32 state)
>  {
>  	u32 result;
> -	acpi_status status;
>  
>  	if (!sci_open(dev))
>  		return -EIO;
>  
> -	status = sci_write(dev, SCI_TOUCHPAD, state, &result);
> +	result = sci_write(dev, SCI_TOUCHPAD, state);
>  	sci_close(dev);
> -	if (ACPI_FAILURE(status)) {
> +	if (result == TOS_FAILURE) {
>  		pr_err("ACPI call to set the touchpad failed\n");
>  		return -EIO;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -644,14 +640,13 @@ static int toshiba_touchpad_set(struct toshiba_acpi_dev *dev, u32 state)
>  static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
>  {
>  	u32 result;
> -	acpi_status status;
>  
>  	if (!sci_open(dev))
>  		return -EIO;
>  
> -	status = sci_read(dev, SCI_TOUCHPAD, state, &result);
> +	result = sci_read(dev, SCI_TOUCHPAD, state);
>  	sci_close(dev);
> -	if (ACPI_FAILURE(status)) {
> +	if (result == TOS_FAILURE) {
>  		pr_err("ACPI call to query the touchpad failed\n");
>  		return -EIO;
>  	} else if (result == TOS_NOT_SUPPORTED) {
> @@ -767,7 +762,7 @@ static u32 hci_get_bt_present(struct toshiba_acpi_dev *dev, bool *present)
>  
>  	value = 0;
>  	value2 = 0;
> -	hci_read2(dev, HCI_WIRELESS, &value, &value2, &hci_result);
> +	hci_result = hci_read2(dev, HCI_WIRELESS, &value, &value2);
>  	if (hci_result == TOS_SUCCESS)
>  		*present = (value & HCI_WIRELESS_BT_PRESENT) ? true : false;
>  
> @@ -781,7 +776,7 @@ static u32 hci_get_radio_state(struct toshiba_acpi_dev *dev, bool *radio_state)
>  
>  	value = 0;
>  	value2 = 0x0001;
> -	hci_read2(dev, HCI_WIRELESS, &value, &value2, &hci_result);
> +	hci_result = hci_read2(dev, HCI_WIRELESS, &value, &value2);
>  
>  	*radio_state = value & HCI_WIRELESS_KILL_SWITCH;
>  	return hci_result;
> @@ -808,8 +803,8 @@ static int bt_rfkill_set_block(void *data, bool blocked)
>  		goto out;
>  	}
>  
> -	hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_POWER, &result1);
> -	hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_ATTACH, &result2);
> +	result1 = hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_POWER);
> +	result2 = hci_write2(dev, HCI_WIRELESS, value, HCI_WIRELESS_BT_ATTACH);
>  
>  	if (result1 != TOS_SUCCESS || result2 != TOS_SUCCESS)
>  		err = -EIO;
> @@ -854,7 +849,7 @@ static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, bool *enabled)
>  	u32 hci_result;
>  	u32 status;
>  
> -	hci_read1(dev, HCI_TR_BACKLIGHT, &status, &hci_result);
> +	hci_result = hci_read1(dev, HCI_TR_BACKLIGHT, &status);
>  	*enabled = !status;
>  	return hci_result == TOS_SUCCESS ? 0 : -EIO;
>  }
> @@ -864,7 +859,7 @@ static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
>  	u32 hci_result;
>  	u32 value = !enable;
>  
> -	hci_write1(dev, HCI_TR_BACKLIGHT, value, &hci_result);
> +	hci_result = hci_write1(dev, HCI_TR_BACKLIGHT, value);
>  	return hci_result == TOS_SUCCESS ? 0 : -EIO;
>  }
>  
> @@ -886,7 +881,7 @@ static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
>  		brightness++;
>  	}
>  
> -	hci_read1(dev, HCI_LCD_BRIGHTNESS, &value, &hci_result);
> +	hci_result = hci_read1(dev, HCI_LCD_BRIGHTNESS, &value);
>  	if (hci_result == TOS_SUCCESS)
>  		return brightness + (value >> HCI_LCD_BRIGHTNESS_SHIFT);
>  
> @@ -1001,7 +996,7 @@ static int get_video_status(struct toshiba_acpi_dev *dev, u32 *status)
>  {
>  	u32 hci_result;
>  
> -	hci_read1(dev, HCI_VIDEO_OUT, status, &hci_result);
> +	hci_result = hci_read1(dev, HCI_VIDEO_OUT, status);
>  	return hci_result == TOS_SUCCESS ? 0 : -EIO;
>  }
>  
> @@ -1105,7 +1100,7 @@ static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
>  {
>  	u32 hci_result;
>  
> -	hci_read1(dev, HCI_FAN, status, &hci_result);
> +	hci_result = hci_read1(dev, HCI_FAN, status);
>  	return hci_result == TOS_SUCCESS ? 0 : -EIO;
>  }
>  
> @@ -1145,7 +1140,7 @@ static ssize_t fan_proc_write(struct file *file, const char __user *buf,
>  
>  	if (sscanf(cmd, " force_on : %i", &value) == 1 &&
>  	    value >= 0 && value <= 1) {
> -		hci_write1(dev, HCI_FAN, value, &hci_result);
> +		hci_result = hci_write1(dev, HCI_FAN, value);
>  		if (hci_result != TOS_SUCCESS)
>  			return -EIO;
>  		else
> @@ -1173,7 +1168,7 @@ static int keys_proc_show(struct seq_file *m, void *v)
>  	u32 value;
>  
>  	if (!dev->key_event_valid && dev->system_event_supported) {
> -		hci_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
> +		hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
>  		if (hci_result == TOS_SUCCESS) {
>  			dev->key_event_valid = 1;
>  			dev->last_key_event = value;
> @@ -1183,7 +1178,7 @@ static int keys_proc_show(struct seq_file *m, void *v)
>  			/* This is a workaround for an unresolved issue on
>  			 * some machines where system events sporadically
>  			 * become disabled. */
> -			hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
> +			hci_result = hci_write1(dev, HCI_SYSTEM_EVENT, 1);
>  			pr_notice("Re-enabled hotkeys\n");
>  		} else {
>  			pr_err("Error reading hotkey status\n");
> @@ -1677,7 +1672,7 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
>  	if (acpi_has_method(dev->acpi_dev->handle, "INFO"))
>  		dev->info_supported = 1;
>  	else {
> -		hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
> +		hci_result = hci_write1(dev, HCI_SYSTEM_EVENT, 1);
>  		if (hci_result == TOS_SUCCESS)
>  			dev->system_event_supported = 1;
>  	}
> @@ -1700,7 +1695,7 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
>  		goto err_remove_filter;
>  	}
>  
> -	hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, &hci_result);
> +	hci_result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
>  	return 0;
>  
>   err_remove_filter:
> @@ -1961,7 +1956,7 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
>  			toshiba_acpi_report_hotkey(dev, scancode);
>  	} else if (dev->system_event_supported) {
>  		do {
> -			hci_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
> +			hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
>  			switch (hci_result) {
>  			case TOS_SUCCESS:
>  				toshiba_acpi_report_hotkey(dev, (int)value);
> @@ -1972,8 +1967,8 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
>  				 * issue on some machines where system events
>  				 * sporadically become disabled.
>  				 */
> -				hci_write1(dev, HCI_SYSTEM_EVENT, 1,
> -					   &hci_result);
> +				hci_result =
> +					hci_write1(dev, HCI_SYSTEM_EVENT, 1);
>  				pr_notice("Re-enabled hotkeys\n");
>  				/* fall through */
>  			default:
> @@ -1991,7 +1986,7 @@ static int toshiba_acpi_suspend(struct device *device)
>  	u32 result;
>  
>  	if (dev->hotkey_dev)
> -		hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE, &result);
> +		result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE);
>  
>  	return 0;
>  }
> @@ -2008,7 +2003,7 @@ static int toshiba_acpi_resume(struct device *device)
>  		if (ACPI_FAILURE(status))
>  			pr_info("Unable to re-enable hotkeys\n");
>  
> -		hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, &result);
> +		result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
>  	}
>  
>  	return 0;
> -- 
> 2.0.0
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH v2 3/3] toshiba_acpi: Change HCI/SCI functions return code type

[Rafael J. Wysocki]

On Tuesday, September 30, 2014 01:58:14 PM Darren Hart wrote:
> On Mon, Sep 29, 2014 at 08:40:09PM -0600, Azael Avalos wrote:
> > Currently the HCI/SCI read/write functions are returning
> > the status of the ACPI call and also assigning the
> > returned value of the HCI/SCI function, however, only
> > the HCI/SCI status is being checked.
> > 
> > This patch changes such functions, returning the value
> > of the HCI/SCI function instead of the ACPI call status,
> > eliminating one parameter, and returning something
> > useful that indeed is being checked.
> > 
> > Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> 
> Cc linux-acpi
> 
> Rafael,
> 
> This follows a couple patches renaming interfaces and error codes. While
> there is some information to be had in checking for ACPI specific errors, I
> don't think it's significant to warrant asking Azael to go the other way and
> check for them specifically and add errorcodes to the interface rather than
> cleanup the functionality as it stands today and simplifiy the code as he does
> here.
> 
> Any objection?

Nope.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.