All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries
@ 2015-07-23  1:37 Azael Avalos
  2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Remove unused wireless defines Azael Avalos
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

This patch simply reorders the entries found in the new keymap by
ascending order, this is simply a cosmetic change, no functionality
was modified.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 649786d..b550be2 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -251,16 +251,16 @@ static const struct key_entry toshiba_acpi_keymap[] = {
 };
 
 static const struct key_entry toshiba_acpi_alt_keymap[] = {
-	{ KE_KEY, 0x157, { KEY_MUTE } },
 	{ KE_KEY, 0x102, { KEY_ZOOMOUT } },
 	{ KE_KEY, 0x103, { KEY_ZOOMIN } },
 	{ KE_KEY, 0x12c, { KEY_KBDILLUMTOGGLE } },
 	{ KE_KEY, 0x139, { KEY_ZOOMRESET } },
-	{ KE_KEY, 0x13e, { KEY_SWITCHVIDEOMODE } },
 	{ KE_KEY, 0x13c, { KEY_BRIGHTNESSDOWN } },
 	{ KE_KEY, 0x13d, { KEY_BRIGHTNESSUP } },
-	{ KE_KEY, 0x158, { KEY_WLAN } },
+	{ KE_KEY, 0x13e, { KEY_SWITCHVIDEOMODE } },
 	{ KE_KEY, 0x13f, { KEY_TOUCHPAD_TOGGLE } },
+	{ KE_KEY, 0x157, { KEY_MUTE } },
+	{ KE_KEY, 0x158, { KEY_WLAN } },
 	{ KE_END, 0 },
 };
 
-- 
2.4.5


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

* [PATCH 1/1] toshiba_acpi: Remove unused wireless defines
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Add set_fan_status function Azael Avalos
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

Commit 2b74103547b4 ("toshiba_acpi: Remove bluetooth rfkill code")
removed bluetooth related code, however, the wireless defines were
not removed and are being unused.

This patch simply removes those defines as there is no code using
them.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index b550be2..6013a11 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -113,7 +113,6 @@ MODULE_LICENSE("GPL");
 #define HCI_VIDEO_OUT			0x001c
 #define HCI_HOTKEY_EVENT		0x001e
 #define HCI_LCD_BRIGHTNESS		0x002a
-#define HCI_WIRELESS			0x0056
 #define HCI_ACCELEROMETER		0x006d
 #define HCI_KBD_ILLUMINATION		0x0095
 #define HCI_ECO_MODE			0x0097
@@ -142,10 +141,6 @@ MODULE_LICENSE("GPL");
 #define HCI_VIDEO_OUT_LCD		0x1
 #define HCI_VIDEO_OUT_CRT		0x2
 #define HCI_VIDEO_OUT_TV		0x4
-#define HCI_WIRELESS_KILL_SWITCH	0x01
-#define HCI_WIRELESS_BT_PRESENT		0x0f
-#define HCI_WIRELESS_BT_ATTACH		0x40
-#define HCI_WIRELESS_BT_POWER		0x80
 #define SCI_KBD_MODE_MASK		0x1f
 #define SCI_KBD_MODE_FNZ		0x1
 #define SCI_KBD_MODE_AUTO		0x2
-- 
2.4.5


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

* [PATCH 1/1] toshiba_acpi: Add set_fan_status function
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
  2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Remove unused wireless defines Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  2015-07-24 22:45   ` Darren Hart
  2015-07-23  1:37 ` [PATCH] toshiba_acpi: Change some variables to avoid warnings the ninja-check script Azael Avalos
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

This patch adds a new function named "set_fan_status" to complement
its get* counterpart, as well as to avoid code duplication between
"fan_proc_write" and "fan_store".

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 6013a11..08fc867 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1422,12 +1422,33 @@ static const struct file_operations video_proc_fops = {
 	.write		= video_proc_write,
 };
 
+/* Fan status */
 static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
 {
-	u32 hci_result;
+	u32 result = hci_read(dev, HCI_FAN, status);
 
-	hci_result = hci_read(dev, HCI_FAN, status);
-	return hci_result == TOS_SUCCESS ? 0 : -EIO;
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to get Fan status failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+	else if (result == TOS_SUCCESS)
+		return 0;
+
+	return -EIO;
+}
+
+static int set_fan_status(struct toshiba_acpi_dev *dev, u32 status)
+{
+	u32 result = hci_write(dev, HCI_FAN, status);
+
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to set Fan status failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+	else if (result == TOS_SUCCESS)
+		return 0;
+
+	return -EIO;
 }
 
 static int fan_proc_show(struct seq_file *m, void *v)
@@ -1457,23 +1478,22 @@ static ssize_t fan_proc_write(struct file *file, const char __user *buf,
 	char cmd[42];
 	size_t len;
 	int value;
-	u32 hci_result;
+	int ret;
 
 	len = min(count, sizeof(cmd) - 1);
 	if (copy_from_user(cmd, buf, len))
 		return -EFAULT;
 	cmd[len] = '\0';
 
-	if (sscanf(cmd, " force_on : %i", &value) == 1 &&
-	    value >= 0 && value <= 1) {
-		hci_result = hci_write(dev, HCI_FAN, value);
-		if (hci_result == TOS_SUCCESS)
-			dev->force_fan = value;
-		else
-			return -EIO;
-	} else {
+	if (sscanf(cmd, " force_on : %i", &value) != 1 &&
+	    value != 0 && value != 1)
 		return -EINVAL;
-	}
+
+	ret = set_fan_status(dev, value);
+	if (ret)
+		return ret;
+
+	dev->force_fan = value;
 
 	return count;
 }
@@ -1610,7 +1630,6 @@ static ssize_t fan_store(struct device *dev,
 			 const char *buf, size_t count)
 {
 	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
-	u32 result;
 	int state;
 	int ret;
 
@@ -1621,11 +1640,9 @@ static ssize_t fan_store(struct device *dev,
 	if (state != 0 && state != 1)
 		return -EINVAL;
 
-	result = hci_write(toshiba, HCI_FAN, state);
-	if (result == TOS_FAILURE)
-		return -EIO;
-	else if (result == TOS_NOT_SUPPORTED)
-		return -ENODEV;
+	ret = set_fan_status(toshiba, state);
+	if (ret)
+		return ret;
 
 	return count;
 }
-- 
2.4.5


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

* [PATCH] toshiba_acpi: Change some variables to avoid warnings the ninja-check script
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
  2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Remove unused wireless defines Azael Avalos
  2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Add set_fan_status function Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  2015-07-23  1:37 ` [PATCH 0/4] toshiba_acpi: Refactor *{get, set} and *available functions Azael Avalos
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

This patch changes some variables to avoid warnins in the ninja-check
script.

We are basically moving some variables inside the conditionals were
such variables are being used, and we are checking the returned
values of some others.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 08fc867..32d6d16 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1668,7 +1668,6 @@ static ssize_t kbd_backlight_mode_store(struct device *dev,
 {
 	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
 	int mode;
-	int time;
 	int ret;
 
 
@@ -1699,7 +1698,7 @@ static ssize_t kbd_backlight_mode_store(struct device *dev,
 	/* Only make a change if the actual mode has changed */
 	if (toshiba->kbd_mode != mode) {
 		/* Shift the time to "base time" (0x3c0000 == 60 seconds) */
-		time = toshiba->kbd_time << HCI_MISC_SHIFT;
+		int time = toshiba->kbd_time << HCI_MISC_SHIFT;
 
 		/* OR the "base time" to the actual method format */
 		if (toshiba->kbd_type == 1) {
@@ -2873,10 +2872,14 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
 static int toshiba_acpi_suspend(struct device *device)
 {
 	struct toshiba_acpi_dev *dev = acpi_driver_data(to_acpi_device(device));
-	u32 result;
 
-	if (dev->hotkey_dev)
+	if (dev->hotkey_dev) {
+		u32 result;
+
 		result = hci_write(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE);
+		if (result != TOS_SUCCESS)
+			pr_info("Unable to disable hotkeys\n");
+	}
 
 	return 0;
 }
@@ -2884,10 +2887,10 @@ static int toshiba_acpi_suspend(struct device *device)
 static int toshiba_acpi_resume(struct device *device)
 {
 	struct toshiba_acpi_dev *dev = acpi_driver_data(to_acpi_device(device));
-	int error;
 
 	if (dev->hotkey_dev) {
-		error = toshiba_acpi_enable_hotkeys(dev);
+		int error = toshiba_acpi_enable_hotkeys(dev);
+
 		if (error)
 			pr_info("Unable to re-enable hotkeys\n");
 	}
-- 
2.4.5


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

* [PATCH 0/4] toshiba_acpi: Refactor *{get, set} and *available functions
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
                   ` (2 preceding siblings ...)
  2015-07-23  1:37 ` [PATCH] toshiba_acpi: Change some variables to avoid warnings the ninja-check script Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  2015-07-23  1:37 ` [PATCH 1/4] toshiba_acpi: Change *available functions return type Azael Avalos
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

These patches changes the *{get, set} and available *functions,
changes the printed messages of the supported features and bumps the
driver version to 0.23.

Azael Avalos (4):
  toshiba_acpi: Change *available functions return type
  toshiba_acpi: Remove "*not supported" feature prints
  toshiba_acpi: Refactor *{get, set} functions return value
  toshiba_acpi: Bump driver version to 0.23

 drivers/platform/x86/toshiba_acpi.c | 546 ++++++++++++++++++------------------
 1 file changed, 274 insertions(+), 272 deletions(-)

-- 
2.4.5


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

* [PATCH 1/4] toshiba_acpi: Change *available functions return type
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
                   ` (3 preceding siblings ...)
  2015-07-23  1:37 ` [PATCH 0/4] toshiba_acpi: Refactor *{get, set} and *available functions Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  2015-07-23  1:37 ` [PATCH 2/4] toshiba_acpi: Remove "*not supported" feature prints Azael Avalos
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

This patch changes the *available functions return type from int to
void.

The checks for support of their respective features are done inside
such functions and there was no need to return anything as we can
flag the queried feature as supported inside these functions.

The code was adapted accordingly to these changes and two new
variables were created and another was changed from uint to bool.

While at the neighborhood, some of the functions were cleaned from
unnecessary checks, as we are explicitly checking for TOS_SUCCESS,
and also renames the function *acceleremoter_supported to
*accelerometer_supported.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 32d6d16..789a17c 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -187,7 +187,6 @@ struct toshiba_acpi_dev {
 	unsigned int info_supported:1;
 	unsigned int tr_backlight_supported:1;
 	unsigned int kbd_illum_supported:1;
-	unsigned int kbd_led_registered:1;
 	unsigned int touchpad_supported:1;
 	unsigned int eco_supported:1;
 	unsigned int accelerometer_supported:1;
@@ -198,6 +197,10 @@ struct toshiba_acpi_dev {
 	unsigned int panel_power_on_supported:1;
 	unsigned int usb_three_supported:1;
 	unsigned int sysfs_created:1;
+
+	bool kbd_led_registered;
+	bool illumination_led_registered;
+	bool eco_led_registered;
 };
 
 static struct toshiba_acpi_dev *toshiba_acpi;
@@ -439,26 +442,24 @@ static u32 sci_write(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
 }
 
 /* Illumination support */
-static int toshiba_illumination_available(struct toshiba_acpi_dev *dev)
+static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
 {
 	u32 in[TCI_WORDS] = { SCI_GET, SCI_ILLUMINATION, 0, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status;
 
+	dev->illumination_supported = 0;
+	dev->illumination_led_registered = false;
+
 	if (!sci_open(dev))
-		return 0;
+		return;
 
 	status = tci_raw(dev, in, out);
 	sci_close(dev);
-	if (ACPI_FAILURE(status)) {
+	if (ACPI_FAILURE(status))
 		pr_err("ACPI call to query Illumination support failed\n");
-		return 0;
-	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("Illumination device not available\n");
-		return 0;
-	}
-
-	return 1;
+	else if (out[0] == TOS_SUCCESS)
+		dev->illumination_supported = 1;
 }
 
 static void toshiba_illumination_set(struct led_classdev *cdev,
@@ -510,41 +511,41 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
 }
 
 /* KBD Illumination */
-static int toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
+static void toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
 {
 	u32 in[TCI_WORDS] = { SCI_GET, SCI_KBD_ILLUM_STATUS, 0, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status;
 
+	dev->kbd_illum_supported = 0;
+	dev->kbd_led_registered = false;
+
 	if (!sci_open(dev))
-		return 0;
+		return;
 
 	status = tci_raw(dev, in, out);
 	sci_close(dev);
-	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to query kbd illumination support failed\n");
-		return 0;
-	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("Keyboard illumination not available\n");
-		return 0;
+		return;
+	} else if (out[0] == TOS_SUCCESS) {
+		/*
+		 * Check for keyboard backlight timeout max value,
+		 * previous kbd backlight implementation set this to
+		 * 0x3c0003, and now the new implementation set this
+		 * to 0x3c001a, use this to distinguish between them.
+		 */
+		if (out[3] == SCI_KBD_TIME_MAX)
+			dev->kbd_type = 2;
+		else
+			dev->kbd_type = 1;
+		/* Get the current keyboard backlight mode */
+		dev->kbd_mode = out[2] & SCI_KBD_MODE_MASK;
+		/* Get the current time (1-60 seconds) */
+		dev->kbd_time = out[2] >> HCI_MISC_SHIFT;
+		/* Flag as supported */
+		dev->kbd_illum_supported = 1;
 	}
-
-	/*
-	 * Check for keyboard backlight timeout max value,
-	 * previous kbd backlight implementation set this to
-	 * 0x3c0003, and now the new implementation set this
-	 * to 0x3c001a, use this to distinguish between them.
-	 */
-	if (out[3] == SCI_KBD_TIME_MAX)
-		dev->kbd_type = 2;
-	else
-		dev->kbd_type = 1;
-	/* Get the current keyboard backlight mode */
-	dev->kbd_mode = out[2] & SCI_KBD_MODE_MASK;
-	/* Get the current time (1-60 seconds) */
-	dev->kbd_time = out[2] >> HCI_MISC_SHIFT;
-
-	return 1;
 }
 
 static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
@@ -665,17 +666,18 @@ static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
 }
 
 /* Eco Mode support */
-static int toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
+static void toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
 {
 	acpi_status status;
 	u32 in[TCI_WORDS] = { HCI_GET, HCI_ECO_MODE, 0, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 
+	dev->eco_supported = 0;
+	dev->eco_led_registered = false;
+
 	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to get ECO led failed\n");
-	} else if (out[0] == TOS_NOT_INSTALLED) {
-		pr_info("ECO led not installed");
 	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
 		/*
 		 * If we receive 0x8300 (Input Data Error), it means that the
@@ -688,13 +690,11 @@ static int toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
 		 */
 		in[3] = 1;
 		status = tci_raw(dev, in, out);
-		if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE)
+		if (ACPI_FAILURE(status))
 			pr_err("ACPI call to get ECO led failed\n");
 		else if (out[0] == TOS_SUCCESS)
-			return 1;
+			dev->eco_supported = 1;
 	}
-
-	return 0;
 }
 
 static enum led_brightness
@@ -734,30 +734,23 @@ static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
 }
 
 /* Accelerometer support */
-static int toshiba_accelerometer_supported(struct toshiba_acpi_dev *dev)
+static void toshiba_accelerometer_available(struct toshiba_acpi_dev *dev)
 {
 	u32 in[TCI_WORDS] = { HCI_GET, HCI_ACCELEROMETER2, 0, 0, 0, 0 };
 	u32 out[TCI_WORDS];
 	acpi_status status;
 
+	dev->accelerometer_supported = 0;
+
 	/*
 	 * Check if the accelerometer call exists,
 	 * this call also serves as initialization
 	 */
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status))
 		pr_err("ACPI call to query the accelerometer failed\n");
-		return -EIO;
-	} 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] == TOS_NOT_SUPPORTED) {
-		pr_info("Accelerometer not supported\n");
-		return -ENODEV;
-	}
-
-	return 0;
+	else if (out[0] == TOS_SUCCESS)
+		dev->accelerometer_supported = 1;
 }
 
 static int toshiba_accelerometer_get(struct toshiba_acpi_dev *dev,
@@ -798,35 +791,21 @@ static void toshiba_usb_sleep_charge_available(struct toshiba_acpi_dev *dev)
 		pr_err("ACPI call to get USB Sleep and Charge mode failed\n");
 		sci_close(dev);
 		return;
-	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("USB Sleep and Charge not supported\n");
-		sci_close(dev);
-		return;
 	} else if (out[0] == TOS_SUCCESS) {
 		dev->usbsc_mode_base = out[4];
 	}
 
 	in[5] = SCI_USB_CHARGE_BAT_LVL;
 	status = tci_raw(dev, in, out);
+	sci_close(dev);
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to get USB Sleep and Charge mode failed\n");
-		sci_close(dev);
-		return;
-	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("USB Sleep and Charge not supported\n");
-		sci_close(dev);
 		return;
 	} else if (out[0] == TOS_SUCCESS) {
 		dev->usbsc_bat_level = out[2];
-		/*
-		 * If we reach this point, it means that the laptop has support
-		 * for this feature and all values are initialized.
-		 * Set it as supported.
-		 */
+		/* Flag as supported */
 		dev->usb_sleep_charge_supported = 1;
 	}
-
-	sci_close(dev);
 }
 
 static int toshiba_usb_sleep_charge_get(struct toshiba_acpi_dev *dev,
@@ -2730,25 +2709,27 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	if (ret)
 		goto error;
 
-	if (toshiba_illumination_available(dev)) {
+	toshiba_illumination_available(dev);
+	if (dev->illumination_supported) {
 		dev->led_dev.name = "toshiba::illumination";
 		dev->led_dev.max_brightness = 1;
 		dev->led_dev.brightness_set = toshiba_illumination_set;
 		dev->led_dev.brightness_get = toshiba_illumination_get;
 		if (!led_classdev_register(&acpi_dev->dev, &dev->led_dev))
-			dev->illumination_supported = 1;
+			dev->illumination_led_registered = true;
 	}
 
-	if (toshiba_eco_mode_available(dev)) {
+	toshiba_eco_mode_available(dev);
+	if (dev->eco_supported) {
 		dev->eco_led.name = "toshiba::eco_mode";
 		dev->eco_led.max_brightness = 1;
 		dev->eco_led.brightness_set = toshiba_eco_mode_set_status;
 		dev->eco_led.brightness_get = toshiba_eco_mode_get_status;
 		if (!led_classdev_register(&dev->acpi_dev->dev, &dev->eco_led))
-			dev->eco_supported = 1;
+			dev->eco_led_registered = true;
 	}
 
-	dev->kbd_illum_supported = toshiba_kbd_illum_available(dev);
+	toshiba_kbd_illum_available(dev);
 	/*
 	 * Only register the LED if KBD illumination is supported
 	 * and the keyboard backlight operation mode is set to FN-Z
@@ -2759,14 +2740,13 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 		dev->kbd_led.brightness_set = toshiba_kbd_backlight_set;
 		dev->kbd_led.brightness_get = toshiba_kbd_backlight_get;
 		if (!led_classdev_register(&dev->acpi_dev->dev, &dev->kbd_led))
-			dev->kbd_led_registered = 1;
+			dev->kbd_led_registered = true;
 	}
 
 	ret = toshiba_touchpad_get(dev, &dummy);
 	dev->touchpad_supported = !ret;
 
-	ret = toshiba_accelerometer_supported(dev);
-	dev->accelerometer_supported = !ret;
+	toshiba_accelerometer_available(dev);
 
 	toshiba_usb_sleep_charge_available(dev);
 
-- 
2.4.5


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

* [PATCH 2/4] toshiba_acpi: Remove "*not supported" feature prints
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
                   ` (4 preceding siblings ...)
  2015-07-23  1:37 ` [PATCH 1/4] toshiba_acpi: Change *available functions return type Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  2015-07-23  1:37 ` [PATCH 3/4] toshiba_acpi: Refactor *{get, set} functions return value Azael Avalos
  2015-07-23  1:37 ` [PATCH 4/4] toshiba_acpi: Bump driver version to 0.23 Azael Avalos
  7 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

Currently the driver prints "*not supported" if any of the features
queried are in fact not supported, let us print the available
features instead.

This patch removes all instances pr_info printing "*not supported",
and add a new function called "print_supported_features", which will
print the available laptop features.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 789a17c..653d1fd 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -477,13 +477,10 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
 	state = brightness ? 1 : 0;
 	result = sci_write(dev, SCI_ILLUMINATION, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call for illumination failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
 		return;
-	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Illumination not supported\n");
-		return;
-	}
 }
 
 static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
@@ -503,7 +500,6 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
 		pr_err("ACPI call for illumination failed\n");
 		return LED_OFF;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Illumination not supported\n");
 		return LED_OFF;
 	}
 
@@ -561,7 +557,6 @@ static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
 		pr_err("ACPI call to set KBD backlight status failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Keyboard backlight status not supported\n");
 		return -ENODEV;
 	}
 
@@ -581,7 +576,6 @@ static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
 		pr_err("ACPI call to get KBD backlight status failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Keyboard backlight status not supported\n");
 		return -ENODEV;
 	}
 
@@ -600,7 +594,6 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
 		pr_err("ACPI call to get the keyboard backlight failed\n");
 		return LED_OFF;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Keyboard backlight not supported\n");
 		return LED_OFF;
 	}
 
@@ -621,7 +614,6 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
 		pr_err("ACPI call to set KBD Illumination mode failed\n");
 		return;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Keyboard backlight not supported\n");
 		return;
 	}
 }
@@ -822,7 +814,6 @@ static int toshiba_usb_sleep_charge_get(struct toshiba_acpi_dev *dev,
 		pr_err("ACPI call to set USB S&C mode failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("USB Sleep and Charge not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -845,7 +836,6 @@ static int toshiba_usb_sleep_charge_set(struct toshiba_acpi_dev *dev,
 		pr_err("ACPI call to set USB S&C mode failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("USB Sleep and Charge not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -871,7 +861,6 @@ static int toshiba_sleep_functions_status_get(struct toshiba_acpi_dev *dev,
 		pr_err("ACPI call to get USB S&C battery level failed\n");
 		return -EIO;
 	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("USB Sleep and Charge not supported\n");
 		return -ENODEV;
 	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -900,7 +889,6 @@ static int toshiba_sleep_functions_status_set(struct toshiba_acpi_dev *dev,
 		pr_err("ACPI call to set USB S&C battery level failed\n");
 		return -EIO;
 	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("USB Sleep and Charge not supported\n");
 		return -ENODEV;
 	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -927,7 +915,6 @@ static int toshiba_usb_rapid_charge_get(struct toshiba_acpi_dev *dev,
 		return -EIO;
 	} else if (out[0] == TOS_NOT_SUPPORTED ||
 		   out[0] == TOS_INPUT_DATA_ERROR) {
-		pr_info("USB Rapid Charge not supported\n");
 		return -ENODEV;
 	}
 
@@ -954,7 +941,6 @@ static int toshiba_usb_rapid_charge_set(struct toshiba_acpi_dev *dev,
 		pr_err("ACPI call to set USB Rapid Charge failed\n");
 		return -EIO;
 	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("USB Rapid Charge not supported\n");
 		return -ENODEV;
 	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -976,7 +962,6 @@ static int toshiba_usb_sleep_music_get(struct toshiba_acpi_dev *dev, u32 *state)
 		pr_err("ACPI call to get Sleep and Music failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Sleep and Music not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -998,7 +983,6 @@ static int toshiba_usb_sleep_music_set(struct toshiba_acpi_dev *dev, u32 state)
 		pr_err("ACPI call to set Sleep and Music failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Sleep and Music not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -1021,7 +1005,6 @@ static int toshiba_function_keys_get(struct toshiba_acpi_dev *dev, u32 *mode)
 		pr_err("ACPI call to get KBD function keys failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("KBD function keys not supported\n");
 		return -ENODEV;
 	}
 
@@ -1041,7 +1024,6 @@ static int toshiba_function_keys_set(struct toshiba_acpi_dev *dev, u32 mode)
 		pr_err("ACPI call to set KBD function keys failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("KBD function keys not supported\n");
 		return -ENODEV;
 	}
 
@@ -1062,7 +1044,6 @@ static int toshiba_panel_power_on_get(struct toshiba_acpi_dev *dev, u32 *state)
 		pr_err("ACPI call to get Panel Power ON failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Panel Power on not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -1084,7 +1065,6 @@ static int toshiba_panel_power_on_set(struct toshiba_acpi_dev *dev, u32 state)
 		pr_err("ACPI call to set Panel Power ON failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("Panel Power ON not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -1107,7 +1087,6 @@ static int toshiba_usb_three_get(struct toshiba_acpi_dev *dev, u32 *state)
 		pr_err("ACPI call to get USB 3 failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("USB 3 not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -1129,7 +1108,6 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
 		pr_err("ACPI call to set USB 3 failed\n");
 		return -EIO;
 	} else if (result == TOS_NOT_SUPPORTED) {
-		pr_info("USB 3 not supported\n");
 		return -ENODEV;
 	} else if (result == TOS_INPUT_DATA_ERROR) {
 		return -EIO;
@@ -1151,7 +1129,6 @@ static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
 		pr_err("ACPI call to get System type failed\n");
 		return -EIO;
 	} else if (out[0] == TOS_NOT_SUPPORTED) {
-		pr_info("System type not supported\n");
 		return -ENODEV;
 	}
 
@@ -2597,6 +2574,46 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
 	return 0;
 }
 
+static void print_supported_features(struct toshiba_acpi_dev *dev)
+{
+	pr_info("Supported laptop features:");
+
+	if (dev->hotkey_dev)
+		pr_cont(" hotkeys");
+	if (dev->backlight_dev)
+		pr_cont(" backlight");
+	if (dev->video_supported)
+		pr_cont(" video-out");
+	if (dev->fan_supported)
+		pr_cont(" fan");
+	if (dev->tr_backlight_supported)
+		pr_cont(" transflective-backlight");
+	if (dev->illumination_supported)
+		pr_cont(" illumination");
+	if (dev->kbd_illum_supported)
+		pr_cont(" keyboard-backlight");
+	if (dev->touchpad_supported)
+		pr_cont(" touchpad");
+	if (dev->eco_supported)
+		pr_cont(" eco-led");
+	if (dev->accelerometer_supported)
+		pr_cont(" accelerometer-axes");
+	if (dev->usb_sleep_charge_supported)
+		pr_cont(" usb-sleep-charge");
+	if (dev->usb_rapid_charge_supported)
+		pr_cont(" usb-rapid-charge");
+	if (dev->usb_sleep_music_supported)
+		pr_cont(" usb-sleep-music");
+	if (dev->kbd_function_keys_supported)
+		pr_cont(" special-function-keys");
+	if (dev->panel_power_on_supported)
+		pr_cont(" panel-power-on");
+	if (dev->usb_three_supported)
+		pr_cont(" usb3");
+
+	pr_cont("\n");
+}
+
 static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
 {
 	struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
@@ -2775,6 +2792,8 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	if (dev->kbd_function_keys_supported && special_functions)
 		toshiba_acpi_enable_special_functions(dev);
 
+	print_supported_features(dev);
+
 	ret = sysfs_create_group(&dev->acpi_dev->dev.kobj,
 				 &toshiba_attr_group);
 	if (ret) {
-- 
2.4.5


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

* [PATCH 3/4] toshiba_acpi: Refactor *{get, set} functions return value
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
                   ` (5 preceding siblings ...)
  2015-07-23  1:37 ` [PATCH 2/4] toshiba_acpi: Remove "*not supported" feature prints Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  2015-07-23  1:37 ` [PATCH 4/4] toshiba_acpi: Bump driver version to 0.23 Azael Avalos
  7 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

This patch changes the default return value of the driver *{get, set}
functions from 0 (success) to -EIO, since the driver default error
value is -EIO.

All the functions now check for TOS_FAILURE, TOS_NOT_SUPPORTED and
TOS_SUCCESS.

On TOS_FAILURE a pr_err message is printed informing the user of the
error (no change was made to this, except the check was added to the
functions not checking for this).

On TOS_NOT_SUPPORTED we now return -ENODEV immediately (some
functions were returning -EIO)

On TOS_SUCCESS we now return 0, as a side effect, a new success value
was added, since some functions return one instead of zero to
indicate success.

As a special case, the LED functions only check for TOS_FAILURE on
*set with their default return value as LED_OFF, and check for
TOS_FAILURE and TOS_SUCCESS on *get.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 653d1fd..c099137 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -93,6 +93,7 @@ MODULE_LICENSE("GPL");
 
 /* Return codes */
 #define TOS_SUCCESS			0x0000
+#define TOS_SUCCESS2			0x0001
 #define TOS_OPEN_CLOSE_OK		0x0044
 #define TOS_FAILURE			0x1000
 #define TOS_NOT_SUPPORTED		0x8000
@@ -467,7 +468,8 @@ 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;
+	u32 result;
+	u32 state;
 
 	/* First request : initialize communication. */
 	if (!sci_open(dev))
@@ -479,8 +481,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
 	sci_close(dev);
 	if (result == TOS_FAILURE)
 		pr_err("ACPI call for illumination failed\n");
-	else if (result == TOS_NOT_SUPPORTED)
-		return;
 }
 
 static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
@@ -496,14 +496,12 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
 	/* Check the illumination */
 	result = sci_read(dev, SCI_ILLUMINATION, &state);
 	sci_close(dev);
-	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call for illumination failed\n");
-		return LED_OFF;
-	} else if (result == TOS_NOT_SUPPORTED) {
-		return LED_OFF;
-	}
+	else if (result == TOS_SUCCESS)
+		return state ? LED_FULL : LED_OFF;
 
-	return state ? LED_FULL : LED_OFF;
+	return LED_OFF;
 }
 
 /* KBD Illumination */
@@ -553,14 +551,14 @@ static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
 
 	result = sci_write(dev, SCI_KBD_ILLUM_STATUS, time);
 	sci_close(dev);
-	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set KBD backlight status failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
@@ -572,50 +570,46 @@ static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
 
 	result = sci_read(dev, SCI_KBD_ILLUM_STATUS, time);
 	sci_close(dev);
-	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to get KBD backlight status failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 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;
+	struct toshiba_acpi_dev *dev;
+	u32 result;
+	u32 state;
 
+	dev = container_of(cdev, struct toshiba_acpi_dev, kbd_led);
 	/* Check the keyboard backlight state */
 	result = hci_read(dev, HCI_KBD_ILLUMINATION, &state);
-	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to get the keyboard backlight failed\n");
-		return LED_OFF;
-	} else if (result == TOS_NOT_SUPPORTED) {
-		return LED_OFF;
-	}
+	else if (result == TOS_SUCCESS)
+		return state ? LED_FULL : LED_OFF;
 
-	return state ? LED_FULL : LED_OFF;
+	return LED_OFF;
 }
 
 static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
 				     enum led_brightness brightness)
 {
-	struct toshiba_acpi_dev *dev = container_of(cdev,
-			struct toshiba_acpi_dev, kbd_led);
-	u32 state, result;
+	struct toshiba_acpi_dev *dev;
+	u32 result;
+	u32 state;
 
+	dev = container_of(cdev, struct toshiba_acpi_dev, kbd_led);
 	/* Set the keyboard backlight state */
 	state = brightness ? 1 : 0;
 	result = hci_write(dev, HCI_KBD_ILLUMINATION, state);
-	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set KBD Illumination mode failed\n");
-		return;
-	} else if (result == TOS_NOT_SUPPORTED) {
-		return;
-	}
 }
 
 /* TouchPad support */
@@ -628,14 +622,14 @@ static int toshiba_touchpad_set(struct toshiba_acpi_dev *dev, u32 state)
 
 	result = sci_write(dev, SCI_TOUCHPAD, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set the touchpad failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
@@ -647,14 +641,14 @@ static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
 
 	result = sci_read(dev, SCI_TOUCHPAD, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to query the touchpad failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 /* Eco Mode support */
@@ -699,12 +693,12 @@ toshiba_eco_mode_get_status(struct led_classdev *cdev)
 	acpi_status status;
 
 	status = tci_raw(dev, in, out);
-	if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status))
 		pr_err("ACPI call to get ECO led failed\n");
-		return LED_OFF;
-	}
+	else if (out[0] == TOS_SUCCESS)
+		return out[2] ? LED_FULL : LED_OFF;
 
-	return out[2] ? LED_FULL : LED_OFF;
+	return LED_OFF;
 }
 
 static void toshiba_eco_mode_set_status(struct led_classdev *cdev,
@@ -719,10 +713,8 @@ 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] == TOS_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status))
 		pr_err("ACPI call to set ECO led failed\n");
-		return;
-	}
 }
 
 /* Accelerometer support */
@@ -754,15 +746,17 @@ 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] == TOS_INPUT_DATA_ERROR) {
+	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to query the accelerometer failed\n");
-		return -EIO;
+	} else if (out[0] == TOS_NOT_SUPPORTED) {
+		return -ENODEV;
+	} else if (out[0] == TOS_SUCCESS) {
+		*xy = out[2];
+		*z = out[4];
+		return 0;
 	}
 
-	*xy = out[2];
-	*z = out[4];
-
-	return 0;
+	return -EIO;
 }
 
 /* Sleep (Charge and Music) utilities support */
@@ -810,16 +804,14 @@ static int toshiba_usb_sleep_charge_get(struct toshiba_acpi_dev *dev,
 
 	result = sci_read(dev, SCI_USB_SLEEP_CHARGE, mode);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set USB S&C mode failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_usb_sleep_charge_set(struct toshiba_acpi_dev *dev,
@@ -832,16 +824,14 @@ static int toshiba_usb_sleep_charge_set(struct toshiba_acpi_dev *dev,
 
 	result = sci_write(dev, SCI_USB_SLEEP_CHARGE, mode);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set USB S&C mode failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_sleep_functions_status_get(struct toshiba_acpi_dev *dev,
@@ -859,16 +849,14 @@ static int toshiba_sleep_functions_status_get(struct toshiba_acpi_dev *dev,
 	sci_close(dev);
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to get USB S&C battery level failed\n");
-		return -EIO;
 	} else if (out[0] == TOS_NOT_SUPPORTED) {
 		return -ENODEV;
-	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
+	} else if (out[0] == TOS_SUCCESS) {
+		*mode = out[2];
+		return 0;
 	}
 
-	*mode = out[2];
-
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_sleep_functions_status_set(struct toshiba_acpi_dev *dev,
@@ -885,16 +873,14 @@ static int toshiba_sleep_functions_status_set(struct toshiba_acpi_dev *dev,
 	in[5] = SCI_USB_CHARGE_BAT_LVL;
 	status = tci_raw(dev, in, out);
 	sci_close(dev);
-	if (ACPI_FAILURE(status)) {
+	if (ACPI_FAILURE(status))
 		pr_err("ACPI call to set USB S&C battery level failed\n");
-		return -EIO;
-	} else if (out[0] == TOS_NOT_SUPPORTED) {
+	else if (out[0] == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (out[0] == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_usb_rapid_charge_get(struct toshiba_acpi_dev *dev,
@@ -912,15 +898,14 @@ static int toshiba_usb_rapid_charge_get(struct toshiba_acpi_dev *dev,
 	sci_close(dev);
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to get USB Rapid Charge failed\n");
-		return -EIO;
-	} else if (out[0] == TOS_NOT_SUPPORTED ||
-		   out[0] == TOS_INPUT_DATA_ERROR) {
+	} else if (out[0] == TOS_NOT_SUPPORTED) {
 		return -ENODEV;
+	} else if (out[0] == TOS_SUCCESS || out[0] == TOS_SUCCESS2) {
+		*state = out[2];
+		return 0;
 	}
 
-	*state = out[2];
-
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_usb_rapid_charge_set(struct toshiba_acpi_dev *dev,
@@ -937,16 +922,14 @@ static int toshiba_usb_rapid_charge_set(struct toshiba_acpi_dev *dev,
 	in[5] = SCI_USB_CHARGE_RAPID_DSP;
 	status = tci_raw(dev, in, out);
 	sci_close(dev);
-	if (ACPI_FAILURE(status)) {
+	if (ACPI_FAILURE(status))
 		pr_err("ACPI call to set USB Rapid Charge failed\n");
-		return -EIO;
-	} else if (out[0] == TOS_NOT_SUPPORTED) {
+	else if (out[0] == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (out[0] == TOS_SUCCESS || out[0] == TOS_SUCCESS2)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_usb_sleep_music_get(struct toshiba_acpi_dev *dev, u32 *state)
@@ -958,16 +941,14 @@ static int toshiba_usb_sleep_music_get(struct toshiba_acpi_dev *dev, u32 *state)
 
 	result = sci_read(dev, SCI_USB_SLEEP_MUSIC, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to get Sleep and Music failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_usb_sleep_music_set(struct toshiba_acpi_dev *dev, u32 state)
@@ -979,16 +960,14 @@ static int toshiba_usb_sleep_music_set(struct toshiba_acpi_dev *dev, u32 state)
 
 	result = sci_write(dev, SCI_USB_SLEEP_MUSIC, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set Sleep and Music failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 /* Keyboard function keys */
@@ -1001,14 +980,14 @@ static int toshiba_function_keys_get(struct toshiba_acpi_dev *dev, u32 *mode)
 
 	result = sci_read(dev, SCI_KBD_FUNCTION_KEYS, mode);
 	sci_close(dev);
-	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to get KBD function keys failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	}
+	else if (result == TOS_SUCCESS || result == TOS_SUCCESS2)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_function_keys_set(struct toshiba_acpi_dev *dev, u32 mode)
@@ -1020,14 +999,14 @@ static int toshiba_function_keys_set(struct toshiba_acpi_dev *dev, u32 mode)
 
 	result = sci_write(dev, SCI_KBD_FUNCTION_KEYS, mode);
 	sci_close(dev);
-	if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set KBD function keys failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	}
+	else if (result == TOS_SUCCESS || result == TOS_SUCCESS2)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 /* Panel Power ON */
@@ -1040,16 +1019,14 @@ static int toshiba_panel_power_on_get(struct toshiba_acpi_dev *dev, u32 *state)
 
 	result = sci_read(dev, SCI_PANEL_POWER_ON, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to get Panel Power ON failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_panel_power_on_set(struct toshiba_acpi_dev *dev, u32 state)
@@ -1061,16 +1038,14 @@ static int toshiba_panel_power_on_set(struct toshiba_acpi_dev *dev, u32 state)
 
 	result = sci_write(dev, SCI_PANEL_POWER_ON, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set Panel Power ON failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 /* USB Three */
@@ -1083,16 +1058,14 @@ static int toshiba_usb_three_get(struct toshiba_acpi_dev *dev, u32 *state)
 
 	result = sci_read(dev, SCI_USB_THREE, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to get USB 3 failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS || result == TOS_SUCCESS2)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
@@ -1104,16 +1077,14 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
 
 	result = sci_write(dev, SCI_USB_THREE, state);
 	sci_close(dev);
-	if (result == TOS_FAILURE) {
+	if (result == TOS_FAILURE)
 		pr_err("ACPI call to set USB 3 failed\n");
-		return -EIO;
-	} else if (result == TOS_NOT_SUPPORTED) {
+	else if (result == TOS_NOT_SUPPORTED)
 		return -ENODEV;
-	} else if (result == TOS_INPUT_DATA_ERROR) {
-		return -EIO;
-	}
+	else if (result == TOS_SUCCESS || result == TOS_SUCCESS2)
+		return 0;
 
-	return 0;
+	return -EIO;
 }
 
 /* Hotkey Event type */
@@ -1127,29 +1098,43 @@ static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
 	status = tci_raw(dev, in, out);
 	if (ACPI_FAILURE(status)) {
 		pr_err("ACPI call to get System type failed\n");
-		return -EIO;
 	} else if (out[0] == TOS_NOT_SUPPORTED) {
 		return -ENODEV;
+	} else if (out[0] == TOS_SUCCESS) {
+		*type = out[3];
+		return 0;
 	}
 
-	*type = out[3];
-
-	return 0;
+	return -EIO;
 }
 
 /* Transflective Backlight */
 static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, u32 *status)
 {
-	u32 hci_result = hci_read(dev, HCI_TR_BACKLIGHT, status);
+	u32 result = hci_read(dev, HCI_TR_BACKLIGHT, status);
 
-	return hci_result == TOS_SUCCESS ? 0 : -EIO;
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to get Transflective Backlight failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+	else if (result == TOS_SUCCESS)
+		return 0;
+
+	return -EIO;
 }
 
 static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, u32 status)
 {
-	u32 hci_result = hci_write(dev, HCI_TR_BACKLIGHT, !status);
+	u32 result = hci_write(dev, HCI_TR_BACKLIGHT, !status);
 
-	return hci_result == TOS_SUCCESS ? 0 : -EIO;
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to set Transflective Backlight failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+	else if (result == TOS_SUCCESS)
+		return 0;
+
+	return -EIO;
 }
 
 static struct proc_dir_entry *toshiba_proc_dir;
@@ -1157,7 +1142,7 @@ static struct proc_dir_entry *toshiba_proc_dir;
 /* LCD Brightness */
 static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
 {
-	u32 hci_result;
+	u32 result;
 	u32 value;
 	int brightness = 0;
 
@@ -1171,8 +1156,12 @@ static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
 		brightness++;
 	}
 
-	hci_result = hci_read(dev, HCI_LCD_BRIGHTNESS, &value);
-	if (hci_result == TOS_SUCCESS)
+	result = hci_read(dev, HCI_LCD_BRIGHTNESS, &value);
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to get LCD Brightness failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+	else if (result == TOS_SUCCESS)
 		return brightness + (value >> HCI_LCD_BRIGHTNESS_SHIFT);
 
 	return -EIO;
@@ -1213,7 +1202,7 @@ static int lcd_proc_open(struct inode *inode, struct file *file)
 
 static int set_lcd_brightness(struct toshiba_acpi_dev *dev, int value)
 {
-	u32 hci_result;
+	u32 result;
 
 	if (dev->tr_backlight_supported) {
 		int ret = set_tr_backlight_status(dev, !value);
@@ -1225,8 +1214,15 @@ static int set_lcd_brightness(struct toshiba_acpi_dev *dev, int value)
 	}
 
 	value = value << HCI_LCD_BRIGHTNESS_SHIFT;
-	hci_result = hci_write(dev, HCI_LCD_BRIGHTNESS, value);
-	return hci_result == TOS_SUCCESS ? 0 : -EIO;
+	result = hci_write(dev, HCI_LCD_BRIGHTNESS, value);
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to set LCD Brightness failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+	else if (result == TOS_SUCCESS)
+		return 0;
+
+	return -EIO;
 }
 
 static int set_lcd_status(struct backlight_device *bd)
@@ -1271,12 +1267,19 @@ static const struct file_operations lcd_proc_fops = {
 	.write		= lcd_proc_write,
 };
 
+/* Video-Out */
 static int get_video_status(struct toshiba_acpi_dev *dev, u32 *status)
 {
-	u32 hci_result;
+	u32 result = hci_read(dev, HCI_VIDEO_OUT, status);
 
-	hci_result = hci_read(dev, HCI_VIDEO_OUT, status);
-	return hci_result == TOS_SUCCESS ? 0 : -EIO;
+	if (result == TOS_FAILURE)
+		pr_err("ACPI call to get Video-Out failed\n");
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+	else if (result == TOS_SUCCESS)
+		return 0;
+
+	return -EIO;
 }
 
 static int video_proc_show(struct seq_file *m, void *v)
-- 
2.4.5


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

* [PATCH 4/4] toshiba_acpi: Bump driver version to 0.23
  2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
                   ` (6 preceding siblings ...)
  2015-07-23  1:37 ` [PATCH 3/4] toshiba_acpi: Refactor *{get, set} functions return value Azael Avalos
@ 2015-07-23  1:37 ` Azael Avalos
  7 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-23  1:37 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

Given that some features were added (/dev/toshiba_acpi device), some
clean-ups and minor changes, bump the driver version to 0.23.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index c099137..ea1bd6d 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -31,7 +31,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#define TOSHIBA_ACPI_VERSION	"0.22"
+#define TOSHIBA_ACPI_VERSION	"0.23"
 #define PROC_INTERFACE_VERSION	1
 
 #include <linux/kernel.h>
-- 
2.4.5


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

* Re: [PATCH 1/1] toshiba_acpi: Add set_fan_status function
  2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Add set_fan_status function Azael Avalos
@ 2015-07-24 22:45   ` Darren Hart
  2015-07-24 23:40       ` Azael Avalos
  0 siblings, 1 reply; 12+ messages in thread
From: Darren Hart @ 2015-07-24 22:45 UTC (permalink / raw)
  To: Azael Avalos; +Cc: platform-driver-x86, linux-kernel

On Wed, Jul 22, 2015 at 07:37:48PM -0600, Azael Avalos wrote:
> This patch adds a new function named "set_fan_status" to complement
> its get* counterpart, as well as to avoid code duplication between
> "fan_proc_write" and "fan_store".

It also appears to change the error codes returned via sysfs?

Also adds some pr_ statements.

> 
> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> ---
>  drivers/platform/x86/toshiba_acpi.c | 55 ++++++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 6013a11..08fc867 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -1422,12 +1422,33 @@ static const struct file_operations video_proc_fops = {
>  	.write		= video_proc_write,
>  };
>  
> +/* Fan status */
>  static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
>  {
> -	u32 hci_result;
> +	u32 result = hci_read(dev, HCI_FAN, status);
>  
> -	hci_result = hci_read(dev, HCI_FAN, status);
> -	return hci_result == TOS_SUCCESS ? 0 : -EIO;
> +	if (result == TOS_FAILURE)
> +		pr_err("ACPI call to get Fan status failed\n");

s/Fan/fan/

Below too.

> +	else if (result == TOS_NOT_SUPPORTED)
> +		return -ENODEV;

This condition would have returned -EIO previously correct?

I agree this new one is the right change, but it does pose a risk to userspace.
There is a slim chance we will have to revert if someone complains and can't
work around it, so we need to be clear about this in the commit message at the
very least.
-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH 1/1] toshiba_acpi: Add set_fan_status function
  2015-07-24 22:45   ` Darren Hart
@ 2015-07-24 23:40       ` Azael Avalos
  0 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-24 23:40 UTC (permalink / raw)
  To: Darren Hart; +Cc: platform-driver-x86, linux-kernel

Hi Darren,

2015-07-24 16:45 GMT-06:00 Darren Hart <dvhart@infradead.org>:
> On Wed, Jul 22, 2015 at 07:37:48PM -0600, Azael Avalos wrote:
>> This patch adds a new function named "set_fan_status" to complement
>> its get* counterpart, as well as to avoid code duplication between
>> "fan_proc_write" and "fan_store".
>
> It also appears to change the error codes returned via sysfs?
>
> Also adds some pr_ statements.
>
>>
>> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
>> ---
>>  drivers/platform/x86/toshiba_acpi.c | 55 ++++++++++++++++++++++++-------------
>>  1 file changed, 36 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index 6013a11..08fc867 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -1422,12 +1422,33 @@ static const struct file_operations video_proc_fops = {
>>       .write          = video_proc_write,
>>  };
>>
>> +/* Fan status */
>>  static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
>>  {
>> -     u32 hci_result;
>> +     u32 result = hci_read(dev, HCI_FAN, status);
>>
>> -     hci_result = hci_read(dev, HCI_FAN, status);
>> -     return hci_result == TOS_SUCCESS ? 0 : -EIO;
>> +     if (result == TOS_FAILURE)
>> +             pr_err("ACPI call to get Fan status failed\n");
>
> s/Fan/fan/
>
> Below too.
>
>> +     else if (result == TOS_NOT_SUPPORTED)
>> +             return -ENODEV;
>
> This condition would have returned -EIO previously correct?

Yes, we were returning -EIO for all error codes, with this patch we are at
least informing userspace that such feature is not supported with
-ENODEV instead of just returnning -EIO all the time.

With the pr_ message we are informing that an ACPI error happened,
which might (or might not) be the drivers fault, and this will be
on par with the rest of the HCI/SCI functions of the driver.

>
> I agree this new one is the right change, but it does pose a risk to userspace.
> There is a slim chance we will have to revert if someone complains and can't
> work around it, so we need to be clear about this in the commit message at the
> very least.

Ok, I spotted a few "issues" on the latter patches, I'll wait for your comments
and send a v2 afterwards with an expanded commit message.

> --
> Darren Hart
> Intel Open Source Technology Center


Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --

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

* Re: [PATCH 1/1] toshiba_acpi: Add set_fan_status function
@ 2015-07-24 23:40       ` Azael Avalos
  0 siblings, 0 replies; 12+ messages in thread
From: Azael Avalos @ 2015-07-24 23:40 UTC (permalink / raw)
  To: Darren Hart; +Cc: platform-driver-x86, linux-kernel

Hi Darren,

2015-07-24 16:45 GMT-06:00 Darren Hart <dvhart@infradead.org>:
> On Wed, Jul 22, 2015 at 07:37:48PM -0600, Azael Avalos wrote:
>> This patch adds a new function named "set_fan_status" to complement
>> its get* counterpart, as well as to avoid code duplication between
>> "fan_proc_write" and "fan_store".
>
> It also appears to change the error codes returned via sysfs?
>
> Also adds some pr_ statements.
>
>>
>> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
>> ---
>>  drivers/platform/x86/toshiba_acpi.c | 55 ++++++++++++++++++++++++-------------
>>  1 file changed, 36 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index 6013a11..08fc867 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -1422,12 +1422,33 @@ static const struct file_operations video_proc_fops = {
>>       .write          = video_proc_write,
>>  };
>>
>> +/* Fan status */
>>  static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
>>  {
>> -     u32 hci_result;
>> +     u32 result = hci_read(dev, HCI_FAN, status);
>>
>> -     hci_result = hci_read(dev, HCI_FAN, status);
>> -     return hci_result == TOS_SUCCESS ? 0 : -EIO;
>> +     if (result == TOS_FAILURE)
>> +             pr_err("ACPI call to get Fan status failed\n");
>
> s/Fan/fan/
>
> Below too.
>
>> +     else if (result == TOS_NOT_SUPPORTED)
>> +             return -ENODEV;
>
> This condition would have returned -EIO previously correct?

Yes, we were returning -EIO for all error codes, with this patch we are at
least informing userspace that such feature is not supported with
-ENODEV instead of just returnning -EIO all the time.

With the pr_ message we are informing that an ACPI error happened,
which might (or might not) be the drivers fault, and this will be
on par with the rest of the HCI/SCI functions of the driver.

>
> I agree this new one is the right change, but it does pose a risk to userspace.
> There is a slim chance we will have to revert if someone complains and can't
> work around it, so we need to be clear about this in the commit message at the
> very least.

Ok, I spotted a few "issues" on the latter patches, I'll wait for your comments
and send a v2 afterwards with an expanded commit message.

> --
> Darren Hart
> Intel Open Source Technology Center


Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --

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

end of thread, other threads:[~2015-07-24 23:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-23  1:37 [PATCH 1/1] toshiba_acpi: Reorder toshiba_acpi_alt_keymap entries Azael Avalos
2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Remove unused wireless defines Azael Avalos
2015-07-23  1:37 ` [PATCH 1/1] toshiba_acpi: Add set_fan_status function Azael Avalos
2015-07-24 22:45   ` Darren Hart
2015-07-24 23:40     ` Azael Avalos
2015-07-24 23:40       ` Azael Avalos
2015-07-23  1:37 ` [PATCH] toshiba_acpi: Change some variables to avoid warnings the ninja-check script Azael Avalos
2015-07-23  1:37 ` [PATCH 0/4] toshiba_acpi: Refactor *{get, set} and *available functions Azael Avalos
2015-07-23  1:37 ` [PATCH 1/4] toshiba_acpi: Change *available functions return type Azael Avalos
2015-07-23  1:37 ` [PATCH 2/4] toshiba_acpi: Remove "*not supported" feature prints Azael Avalos
2015-07-23  1:37 ` [PATCH 3/4] toshiba_acpi: Refactor *{get, set} functions return value Azael Avalos
2015-07-23  1:37 ` [PATCH 4/4] toshiba_acpi: Bump driver version to 0.23 Azael Avalos

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.