These patches changes the *{get, set} and *available functions default
return type, changes the printed messages of the supported features,
removes some unnedded checks and bumps the driver version to 0.23.
Changes since v2:
- Introduced a new patch to remove unnecessary checks instead of
cramming them in the patch series
- Updated patch descriptions
- Reverted some changes on the previous version to avoid compiler
warnings
Changes since v1:
- Fixed typos in patch 01 description and use newly created
*led_registered variables to unregister leds
- Adapt *{lcd, video}_proc functions in patch 03 to ensure we are not
breaking userspace and updated patch description
Azael Avalos (5):
toshiba_acpi: Change *available functions return type
toshiba_acpi: Remove "*not supported" feature prints
toshiba_acpi: Refactor *{get, set} functions return value
toshiba_acpi: Remove unnecessary checks and returns in HCI/SCI
functions
toshiba_acpi: Bump driver version to 0.23
drivers/platform/x86/toshiba_acpi.c | 531 +++++++++++++++++-------------------
1 file changed, 248 insertions(+), 283 deletions(-)
--
2.4.6
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.
Also, the function toshiba_acceleremoter_supported was renamed to
toshiba_accelerometer_available to maintain the naming consistency on
the driver.
Signed-off-by: Azael Avalos <[email protected]>
---
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 f722898..d983dc4 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,26 @@ 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) {
+ 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 +513,42 @@ 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) {
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;
+ } 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,12 +669,15 @@ 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");
@@ -691,10 +698,8 @@ static int toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE)
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 +739,28 @@ 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) || out[0] == TOS_INPUT_DATA_ERROR)
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) {
+ 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) {
+ 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,
@@ -787,7 +790,6 @@ static void toshiba_usb_sleep_charge_available(struct toshiba_acpi_dev *dev)
u32 out[TCI_WORDS];
acpi_status status;
- /* Set the feature to "not supported" in case of error */
dev->usb_sleep_charge_supported = 0;
if (!sci_open(dev))
@@ -808,25 +810,17 @@ static void toshiba_usb_sleep_charge_available(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)) {
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,
@@ -2639,13 +2633,13 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
backlight_device_unregister(dev->backlight_dev);
- if (dev->illumination_supported)
+ if (dev->illumination_led_registered)
led_classdev_unregister(&dev->led_dev);
if (dev->kbd_led_registered)
led_classdev_unregister(&dev->kbd_led);
- if (dev->eco_supported)
+ if (dev->eco_led_registered)
led_classdev_unregister(&dev->eco_led);
if (toshiba_acpi)
@@ -2727,25 +2721,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
@@ -2756,14 +2752,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.6
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 <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 72 +++++++++++++++++++++++--------------
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index d983dc4..66b596a 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -459,7 +459,7 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
if (ACPI_FAILURE(status))
pr_err("ACPI call to query Illumination support failed\n");
else if (out[0] == TOS_NOT_SUPPORTED)
- pr_info("Illumination device not available\n");
+ return;
else if (out[0] == TOS_SUCCESS)
dev->illumination_supported = 1;
}
@@ -483,7 +483,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
pr_err("ACPI call for illumination failed\n");
return;
} else if (result == TOS_NOT_SUPPORTED) {
- pr_info("Illumination not supported\n");
return;
}
}
@@ -505,7 +504,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;
}
@@ -530,7 +528,7 @@ static void toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
pr_err("ACPI call to query kbd illumination support failed\n");
} else if (out[0] == TOS_NOT_SUPPORTED) {
- pr_info("Keyboard illumination not available\n");
+ return;
} else if (out[0] == TOS_SUCCESS) {
/*
* Check for keyboard backlight timeout max value,
@@ -564,7 +562,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;
}
@@ -584,7 +581,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;
}
@@ -603,7 +599,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;
}
@@ -624,7 +619,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;
}
}
@@ -758,7 +752,7 @@ static void toshiba_accelerometer_available(struct toshiba_acpi_dev *dev)
out[0] == TOS_NOT_INITIALIZED)
pr_err("Accelerometer not initialized\n");
else if (out[0] == TOS_NOT_SUPPORTED)
- pr_info("Accelerometer not supported\n");
+ return;
else if (out[0] == TOS_SUCCESS)
dev->accelerometer_supported = 1;
}
@@ -801,7 +795,6 @@ static void toshiba_usb_sleep_charge_available(struct toshiba_acpi_dev *dev)
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) {
@@ -814,7 +807,7 @@ static void toshiba_usb_sleep_charge_available(struct toshiba_acpi_dev *dev)
if (ACPI_FAILURE(status)) {
pr_err("ACPI call to get USB Sleep and Charge mode failed\n");
} else if (out[0] == TOS_NOT_SUPPORTED) {
- pr_info("USB Sleep and Charge not supported\n");
+ return;
} else if (out[0] == TOS_SUCCESS) {
dev->usbsc_bat_level = out[2];
/* Flag as supported */
@@ -837,7 +830,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;
@@ -860,7 +852,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;
@@ -886,7 +877,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;
@@ -915,7 +905,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;
@@ -942,7 +931,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;
}
@@ -969,7 +957,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;
@@ -991,7 +978,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;
@@ -1013,7 +999,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;
@@ -1036,7 +1021,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;
}
@@ -1056,7 +1040,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;
}
@@ -1077,7 +1060,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;
@@ -1099,7 +1081,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;
@@ -1122,7 +1103,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;
@@ -1144,7 +1124,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;
@@ -1166,7 +1145,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;
}
@@ -2609,6 +2587,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);
@@ -2780,6 +2798,8 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
ret = get_fan_status(dev, &dummy);
dev->fan_supported = !ret;
+ print_supported_features(dev);
+
/*
* Enable the "Special Functions" mode only if they are
* supported and if they are activated.
--
2.4.6
This patch refactors the return value of the driver *{get, set}
functions, 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 and some other were not checking)
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 now check for *FAILURE on
*set, and check for TOS_FAILURE and TOS_SUCCESS on *get with their
"default" return value set to LED_OFF.
Also the {lcd, video}_proc* functions were adapted to reflect these
changes to their parent HCI functions.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 177 +++++++++++++++++++++---------------
1 file changed, 104 insertions(+), 73 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 66b596a..7b16d8d 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
@@ -469,7 +470,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))
@@ -503,7 +505,7 @@ static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
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) {
+ } else if (result != TOS_SUCCESS) {
return LED_OFF;
}
@@ -565,7 +567,7 @@ static int toshiba_kbd_illum_status_set(struct toshiba_acpi_dev *dev, u32 time)
return -ENODEV;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
@@ -584,21 +586,22 @@ static int toshiba_kbd_illum_status_get(struct toshiba_acpi_dev *dev, u32 *time)
return -ENODEV;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -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;
+ u32 result;
+ u32 state;
/* Check the keyboard backlight state */
result = hci_read(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) {
+ } else if (result != TOS_SUCCESS) {
return LED_OFF;
}
@@ -610,7 +613,8 @@ 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;
+ u32 result;
+ u32 state;
/* Set the keyboard backlight state */
state = brightness ? 1 : 0;
@@ -640,7 +644,7 @@ static int toshiba_touchpad_set(struct toshiba_acpi_dev *dev, u32 state)
return -ENODEV;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
@@ -659,7 +663,7 @@ static int toshiba_touchpad_get(struct toshiba_acpi_dev *dev, u32 *state)
return -ENODEV;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
/* Eco Mode support */
@@ -709,6 +713,8 @@ toshiba_eco_mode_get_status(struct led_classdev *cdev)
if (ACPI_FAILURE(status) || out[0] == TOS_INPUT_DATA_ERROR) {
pr_err("ACPI call to get ECO led failed\n");
return LED_OFF;
+ } else if (out[0] != TOS_SUCCESS) {
+ return LED_OFF;
}
return out[2] ? LED_FULL : LED_OFF;
@@ -769,12 +775,15 @@ static int toshiba_accelerometer_get(struct toshiba_acpi_dev *dev,
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] == 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 */
@@ -835,7 +844,7 @@ static int toshiba_usb_sleep_charge_get(struct toshiba_acpi_dev *dev,
return -EIO;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int toshiba_usb_sleep_charge_set(struct toshiba_acpi_dev *dev,
@@ -857,7 +866,7 @@ static int toshiba_usb_sleep_charge_set(struct toshiba_acpi_dev *dev,
return -EIO;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int toshiba_sleep_functions_status_get(struct toshiba_acpi_dev *dev,
@@ -880,11 +889,12 @@ static int toshiba_sleep_functions_status_get(struct toshiba_acpi_dev *dev,
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,
@@ -910,7 +920,7 @@ static int toshiba_sleep_functions_status_set(struct toshiba_acpi_dev *dev,
return -EIO;
}
- return 0;
+ return out[0] == TOS_SUCCESS ? 0 : -EIO;
}
static int toshiba_usb_rapid_charge_get(struct toshiba_acpi_dev *dev,
@@ -932,11 +942,12 @@ static int toshiba_usb_rapid_charge_get(struct toshiba_acpi_dev *dev,
} else if (out[0] == TOS_NOT_SUPPORTED ||
out[0] == TOS_INPUT_DATA_ERROR) {
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,
@@ -962,7 +973,7 @@ static int toshiba_usb_rapid_charge_set(struct toshiba_acpi_dev *dev,
return -EIO;
}
- return 0;
+ return (out[0] == TOS_SUCCESS || out[0] == TOS_SUCCESS2) ? 0 : -EIO;
}
static int toshiba_usb_sleep_music_get(struct toshiba_acpi_dev *dev, u32 *state)
@@ -983,7 +994,7 @@ static int toshiba_usb_sleep_music_get(struct toshiba_acpi_dev *dev, u32 *state)
return -EIO;
}
- return 0;
+ return result = TOS_SUCCESS ? 0 : -EIO;
}
static int toshiba_usb_sleep_music_set(struct toshiba_acpi_dev *dev, u32 state)
@@ -1004,7 +1015,7 @@ static int toshiba_usb_sleep_music_set(struct toshiba_acpi_dev *dev, u32 state)
return -EIO;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
/* Keyboard function keys */
@@ -1024,7 +1035,7 @@ static int toshiba_function_keys_get(struct toshiba_acpi_dev *dev, u32 *mode)
return -ENODEV;
}
- return 0;
+ return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
}
static int toshiba_function_keys_set(struct toshiba_acpi_dev *dev, u32 mode)
@@ -1043,7 +1054,7 @@ static int toshiba_function_keys_set(struct toshiba_acpi_dev *dev, u32 mode)
return -ENODEV;
}
- return 0;
+ return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
}
/* Panel Power ON */
@@ -1065,7 +1076,7 @@ static int toshiba_panel_power_on_get(struct toshiba_acpi_dev *dev, u32 *state)
return -EIO;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int toshiba_panel_power_on_set(struct toshiba_acpi_dev *dev, u32 state)
@@ -1086,7 +1097,7 @@ static int toshiba_panel_power_on_set(struct toshiba_acpi_dev *dev, u32 state)
return -EIO;
}
- return 0;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
/* USB Three */
@@ -1108,7 +1119,7 @@ static int toshiba_usb_three_get(struct toshiba_acpi_dev *dev, u32 *state)
return -EIO;
}
- return 0;
+ return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
}
static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
@@ -1129,7 +1140,7 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
return -EIO;
}
- return 0;
+ return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
}
/* Hotkey Event type */
@@ -1146,26 +1157,37 @@ static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
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);
+
+ if (result == TOS_FAILURE)
+ pr_err("ACPI call to get Transflective Backlight failed\n");
+ else if (result == TOS_NOT_SUPPORTED)
+ return -ENODEV;
- return hci_result == TOS_SUCCESS ? 0 : -EIO;
+ return result == TOS_SUCCESS ? 0 : -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);
+
+ if (result == TOS_FAILURE)
+ pr_err("ACPI call to set Transflective Backlight failed\n");
+ else if (result == TOS_NOT_SUPPORTED)
+ return -ENODEV;
- return hci_result == TOS_SUCCESS ? 0 : -EIO;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static struct proc_dir_entry *toshiba_proc_dir;
@@ -1173,7 +1195,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;
@@ -1187,8 +1209,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;
+ if (result == TOS_SUCCESS)
return brightness + (value >> HCI_LCD_BRIGHTNESS_SHIFT);
return -EIO;
@@ -1204,8 +1230,8 @@ static int get_lcd_brightness(struct backlight_device *bd)
static int lcd_proc_show(struct seq_file *m, void *v)
{
struct toshiba_acpi_dev *dev = m->private;
- int value;
int levels;
+ int value;
if (!dev->backlight_dev)
return -ENODEV;
@@ -1219,6 +1245,7 @@ static int lcd_proc_show(struct seq_file *m, void *v)
}
pr_err("Error reading LCD brightness\n");
+
return -EIO;
}
@@ -1229,7 +1256,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);
@@ -1241,8 +1268,13 @@ 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;
+
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int set_lcd_status(struct backlight_device *bd)
@@ -1258,24 +1290,22 @@ static ssize_t lcd_proc_write(struct file *file, const char __user *buf,
struct toshiba_acpi_dev *dev = PDE_DATA(file_inode(file));
char cmd[42];
size_t len;
- int value;
- int ret;
int levels = dev->backlight_dev->props.max_brightness + 1;
+ int value;
len = min(count, sizeof(cmd) - 1);
if (copy_from_user(cmd, buf, len))
return -EFAULT;
cmd[len] = '\0';
- if (sscanf(cmd, " brightness : %i", &value) == 1 &&
- value >= 0 && value < levels) {
- ret = set_lcd_brightness(dev, value);
- if (ret == 0)
- ret = count;
- } else {
- ret = -EINVAL;
- }
- return ret;
+ if (sscanf(cmd, " brightness : %i", &value) != 1 &&
+ value < 0 && value > levels)
+ return -EINVAL;
+
+ if (set_lcd_brightness(dev, value))
+ return -EIO;
+
+ return count;
}
static const struct file_operations lcd_proc_fops = {
@@ -1287,22 +1317,25 @@ 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);
+
+ if (result == TOS_FAILURE)
+ pr_err("ACPI call to get Video-Out failed\n");
+ else if (result == TOS_NOT_SUPPORTED)
+ return -ENODEV;
- hci_result = hci_read(dev, HCI_VIDEO_OUT, status);
- return hci_result == TOS_SUCCESS ? 0 : -EIO;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int video_proc_show(struct seq_file *m, void *v)
{
struct toshiba_acpi_dev *dev = m->private;
u32 value;
- int ret;
- ret = get_video_status(dev, &value);
- if (!ret) {
+ if (!get_video_status(dev, &value)) {
int is_lcd = (value & HCI_VIDEO_OUT_LCD) ? 1 : 0;
int is_crt = (value & HCI_VIDEO_OUT_CRT) ? 1 : 0;
int is_tv = (value & HCI_VIDEO_OUT_TV) ? 1 : 0;
@@ -1310,9 +1343,10 @@ static int video_proc_show(struct seq_file *m, void *v)
seq_printf(m, "lcd_out: %d\n", is_lcd);
seq_printf(m, "crt_out: %d\n", is_crt);
seq_printf(m, "tv_out: %d\n", is_tv);
+ return 0;
}
- return ret;
+ return -EIO;
}
static int video_proc_open(struct inode *inode, struct file *file)
@@ -1324,13 +1358,14 @@ static ssize_t video_proc_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
{
struct toshiba_acpi_dev *dev = PDE_DATA(file_inode(file));
- char *cmd, *buffer;
- int ret;
- int value;
+ char *buffer;
+ char *cmd;
int remain = count;
int lcd_out = -1;
int crt_out = -1;
int tv_out = -1;
+ int value;
+ int ret;
u32 video_out;
cmd = kmalloc(count + 1, GFP_KERNEL);
@@ -1382,7 +1417,7 @@ static ssize_t video_proc_write(struct file *file, const char __user *buf,
ret = write_acpi_int(METHOD_VIDEO_OUT, new_video_out);
}
- return ret ? ret : count;
+ return ret ? -EIO : count;
}
static const struct file_operations video_proc_fops = {
@@ -1403,10 +1438,8 @@ static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
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;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int set_fan_status(struct toshiba_acpi_dev *dev, u32 status)
@@ -1417,10 +1450,8 @@ static int set_fan_status(struct toshiba_acpi_dev *dev, u32 status)
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;
+ return result == TOS_SUCCESS ? 0 : -EIO;
}
static int fan_proc_show(struct seq_file *m, void *v)
--
2.4.6
A previous patch added explicit feature checks for support, *SUCCESS*
and *FAILURE to the HCI/SCI *{get, set} functions.
This patch removes some unnedded checks to the driver HCI/SCI
functions given that the default error return value is now set to
-EIO, there is no need to check for other error values other than
the ones currently checking for.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 169 ++++++++++--------------------------
1 file changed, 44 insertions(+), 125 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 7b16d8d..4802fd7 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -459,8 +459,6 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
sci_close(dev);
if (ACPI_FAILURE(status))
pr_err("ACPI call to query Illumination support failed\n");
- else if (out[0] == TOS_NOT_SUPPORTED)
- return;
else if (out[0] == TOS_SUCCESS)
dev->illumination_supported = 1;
}
@@ -481,12 +479,8 @@ 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");
- return;
- } else if (result == TOS_NOT_SUPPORTED) {
- return;
- }
}
static enum led_brightness toshiba_illumination_get(struct led_classdev *cdev)
@@ -502,7 +496,7 @@ 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_SUCCESS) {
@@ -527,10 +521,8 @@ static void toshiba_kbd_illum_available(struct toshiba_acpi_dev *dev)
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");
- } else if (out[0] == TOS_NOT_SUPPORTED) {
- return;
} else if (out[0] == TOS_SUCCESS) {
/*
* Check for keyboard backlight timeout max value,
@@ -560,12 +552,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -579,12 +569,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -598,7 +586,7 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
/* 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_SUCCESS) {
@@ -619,12 +607,8 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
/* 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 */
@@ -637,12 +621,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -656,12 +638,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -679,8 +659,6 @@ static void toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
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
@@ -693,7 +671,7 @@ static void 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)
dev->eco_supported = 1;
@@ -710,7 +688,7 @@ 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) {
@@ -732,10 +710,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 */
@@ -752,19 +728,14 @@ static void toshiba_accelerometer_available(struct toshiba_acpi_dev *dev)
* 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");
- else if (out[0] == TOS_DATA_NOT_AVAILABLE ||
- out[0] == TOS_NOT_INITIALIZED)
- pr_err("Accelerometer not initialized\n");
- else if (out[0] == TOS_NOT_SUPPORTED)
- return;
else if (out[0] == TOS_SUCCESS)
dev->accelerometer_supported = 1;
}
static int toshiba_accelerometer_get(struct toshiba_acpi_dev *dev,
- u32 *xy, u32 *z)
+ u32 *xy, u32 *z)
{
u32 in[TCI_WORDS] = { HCI_GET, HCI_ACCELEROMETER, 0, 1, 0, 0 };
u32 out[TCI_WORDS];
@@ -772,7 +743,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] == 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) {
@@ -815,8 +786,6 @@ static void toshiba_usb_sleep_charge_available(struct toshiba_acpi_dev *dev)
sci_close(dev);
if (ACPI_FAILURE(status)) {
pr_err("ACPI call to get USB Sleep and Charge mode failed\n");
- } else if (out[0] == TOS_NOT_SUPPORTED) {
- return;
} else if (out[0] == TOS_SUCCESS) {
dev->usbsc_bat_level = out[2];
/* Flag as supported */
@@ -835,14 +804,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -857,14 +822,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -884,11 +845,8 @@ 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;
@@ -911,14 +869,10 @@ 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;
- }
return out[0] == TOS_SUCCESS ? 0 : -EIO;
}
@@ -938,9 +892,7 @@ 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];
@@ -964,14 +916,10 @@ 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;
- }
return (out[0] == TOS_SUCCESS || out[0] == TOS_SUCCESS2) ? 0 : -EIO;
}
@@ -985,14 +933,10 @@ 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;
- }
return result = TOS_SUCCESS ? 0 : -EIO;
}
@@ -1006,14 +950,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1028,12 +968,10 @@ 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;
- }
return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
}
@@ -1047,12 +985,10 @@ 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;
- }
return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
}
@@ -1067,14 +1003,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1088,14 +1020,10 @@ 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;
- }
return result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1110,14 +1038,10 @@ 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;
- }
return (result == TOS_SUCCESS || result == TOS_SUCCESS2) ? 0 : -EIO;
}
@@ -1131,14 +1055,10 @@ 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;
- }
return (result == TOS_SUCCESS || result == TOS_SUCCESS2) 0 : -EIO;
}
@@ -1154,7 +1074,6 @@ 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) {
--
2.4.6
Given that some features were added (/dev/toshiba_acpi device), some
clean-ups and minor (cosmetic) changes all over the driver code, bump
the driver version to 0.23 to reflect these overall changes.
Signed-off-by: Azael Avalos <[email protected]>
---
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 4802fd7..1645bc4 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.6
On Fri, Jul 31, 2015 at 09:58:13PM -0600, Azael Avalos wrote:
> 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 <[email protected]>
> ---
> drivers/platform/x86/toshiba_acpi.c | 72 +++++++++++++++++++++++--------------
> 1 file changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index d983dc4..66b596a 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -459,7 +459,7 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
> if (ACPI_FAILURE(status))
> pr_err("ACPI call to query Illumination support failed\n");
> else if (out[0] == TOS_NOT_SUPPORTED)
> - pr_info("Illumination device not available\n");
> + return;
> else if (out[0] == TOS_SUCCESS)
> dev->illumination_supported = 1;
> }
> @@ -483,7 +483,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
> pr_err("ACPI call for illumination failed\n");
> return;
> } else if (result == TOS_NOT_SUPPORTED) {
> - pr_info("Illumination not supported\n");
> return;
> }
I mentioned this in the previous review. For several of these, we have an if
statement that checks for a condition, and then returns, which is exactly what
would happen if we didn't have the if statement at all.
If the context is important, a comment should be sufficient. Is there a
compelling reason to add the redundant check?
--
Darren Hart
Intel Open Source Technology Center
> @@ -1131,14 +1055,10 @@ 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;
> - }
>
> return (result == TOS_SUCCESS || result == TOS_SUCCESS2) 0 : -EIO;
Hrm... the above line cause patch application failure via git (note the
missing ? before the '0 : -EIO;'). This never existed upstream so far as
I can determine.
It applied with some fuzz manually, but I'm concerned about how this
happened. Did you have a dirty tree when you prepared these patches
perhaps?
--
Darren Hart
Intel Open Source Technology Center
Hi Darren,
2015-08-05 3:38 GMT-06:00 Darren Hart <[email protected]>:
> On Fri, Jul 31, 2015 at 09:58:13PM -0600, Azael Avalos wrote:
>> 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 <[email protected]>
>> ---
>> drivers/platform/x86/toshiba_acpi.c | 72 +++++++++++++++++++++++--------------
>> 1 file changed, 46 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index d983dc4..66b596a 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -459,7 +459,7 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
>> if (ACPI_FAILURE(status))
>> pr_err("ACPI call to query Illumination support failed\n");
>> else if (out[0] == TOS_NOT_SUPPORTED)
>> - pr_info("Illumination device not available\n");
>> + return;
>> else if (out[0] == TOS_SUCCESS)
>> dev->illumination_supported = 1;
>> }
>> @@ -483,7 +483,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
>> pr_err("ACPI call for illumination failed\n");
>> return;
>> } else if (result == TOS_NOT_SUPPORTED) {
>> - pr_info("Illumination not supported\n");
>> return;
>> }
>
> I mentioned this in the previous review. For several of these, we have an if
> statement that checks for a condition, and then returns, which is exactly what
> would happen if we didn't have the if statement at all.
>
> If the context is important, a comment should be sufficient. Is there a
> compelling reason to add the redundant check?
The "offending" lines are removed by patch 04, that's why I didn't included
a comment or removed the lines on this patch, as I was trying to "abstract"
what each patch do, which in this patch, only removes the pr_info.
>
> --
> Darren Hart
> Intel Open Source Technology Center
Cheers
Azael
--
-- El mundo apesta y vosotros apestais tambien --
On Wed, Aug 05, 2015 at 04:15:13PM -0600, Azael Avalos wrote:
> Hi Darren,
>
> 2015-08-05 3:38 GMT-06:00 Darren Hart <[email protected]>:
> > On Fri, Jul 31, 2015 at 09:58:13PM -0600, Azael Avalos wrote:
> >> 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 <[email protected]>
> >> ---
> >> drivers/platform/x86/toshiba_acpi.c | 72 +++++++++++++++++++++++--------------
> >> 1 file changed, 46 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> >> index d983dc4..66b596a 100644
> >> --- a/drivers/platform/x86/toshiba_acpi.c
> >> +++ b/drivers/platform/x86/toshiba_acpi.c
> >> @@ -459,7 +459,7 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
> >> if (ACPI_FAILURE(status))
> >> pr_err("ACPI call to query Illumination support failed\n");
> >> else if (out[0] == TOS_NOT_SUPPORTED)
> >> - pr_info("Illumination device not available\n");
> >> + return;
> >> else if (out[0] == TOS_SUCCESS)
> >> dev->illumination_supported = 1;
> >> }
> >> @@ -483,7 +483,6 @@ static void toshiba_illumination_set(struct led_classdev *cdev,
> >> pr_err("ACPI call for illumination failed\n");
> >> return;
> >> } else if (result == TOS_NOT_SUPPORTED) {
> >> - pr_info("Illumination not supported\n");
> >> return;
> >> }
> >
> > I mentioned this in the previous review. For several of these, we have an if
> > statement that checks for a condition, and then returns, which is exactly what
> > would happen if we didn't have the if statement at all.
> >
> > If the context is important, a comment should be sufficient. Is there a
> > compelling reason to add the redundant check?
>
> The "offending" lines are removed by patch 04, that's why I didn't included
> a comment or removed the lines on this patch, as I was trying to "abstract"
> what each patch do, which in this patch, only removes the pr_info.
Apologies, I missed that. OK, we're good on this one.
--
Darren Hart
Intel Open Source Technology Center
Hi Darren,
2015-08-05 14:21 GMT-06:00 Darren Hart <[email protected]>:
>> @@ -1131,14 +1055,10 @@ 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;
>> - }
>>
>> return (result == TOS_SUCCESS || result == TOS_SUCCESS2) 0 : -EIO;
>
> Hrm... the above line cause patch application failure via git (note the
> missing ? before the '0 : -EIO;'). This never existed upstream so far as
> I can determine.
I've spotted that while compile-checking my changes locally, but I might
have sent you the wrong patch here, I'll double check in the future to avoid
these embarrassments :-(
>
> It applied with some fuzz manually, but I'm concerned about how this
> happened. Did you have a dirty tree when you prepared these patches
> perhaps?
This is weird, all these patches applied cleanly on my local copy, I'll fetch
a new copy from your "for-next" tree and check w/ it.
In the mean time, thanks for your observations, I'll try to keep a closer look
on future patches.
>
> --
> Darren Hart
> Intel Open Source Technology Center
Cheers
Azael
--
-- El mundo apesta y vosotros apestais tambien --
On Wed, Aug 05, 2015 at 04:23:49PM -0600, Azael Avalos wrote:
> Hi Darren,
>
> 2015-08-05 14:21 GMT-06:00 Darren Hart <[email protected]>:
> >> @@ -1131,14 +1055,10 @@ 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;
> >> - }
> >>
> >> return (result == TOS_SUCCESS || result == TOS_SUCCESS2) 0 : -EIO;
> >
> > Hrm... the above line cause patch application failure via git (note the
> > missing ? before the '0 : -EIO;'). This never existed upstream so far as
> > I can determine.
>
> I've spotted that while compile-checking my changes locally, but I might
> have sent you the wrong patch here, I'll double check in the future to avoid
> these embarrassments :-(
>
> >
> > It applied with some fuzz manually, but I'm concerned about how this
> > happened. Did you have a dirty tree when you prepared these patches
> > perhaps?
>
> This is weird, all these patches applied cleanly on my local copy, I'll fetch
> a new copy from your "for-next" tree and check w/ it.
Please verify what I have in "testing", if that's right, then we're good. It has
already passed my checks and 0day's.
--
Darren Hart
Intel Open Source Technology Center
On Thu, Aug 06, 2015 at 10:21:15AM -0600, Azael Avalos wrote:
> Hi Darren,
>
> 2015-08-05 17:21 GMT-06:00 Darren Hart <[email protected]>:
> > On Wed, Aug 05, 2015 at 04:23:49PM -0600, Azael Avalos wrote:
> >> Hi Darren,
> >>
> >> 2015-08-05 14:21 GMT-06:00 Darren Hart <[email protected]>:
> >> >> @@ -1131,14 +1055,10 @@ 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;
> >> >> - }
> >> >>
> >> >> return (result == TOS_SUCCESS || result == TOS_SUCCESS2) 0 : -EIO;
> >> >
> >> > Hrm... the above line cause patch application failure via git (note the
> >> > missing ? before the '0 : -EIO;'). This never existed upstream so far as
> >> > I can determine.
> >>
> >> I've spotted that while compile-checking my changes locally, but I might
> >> have sent you the wrong patch here, I'll double check in the future to avoid
> >> these embarrassments :-(
> >>
> >> >
> >> > It applied with some fuzz manually, but I'm concerned about how this
> >> > happened. Did you have a dirty tree when you prepared these patches
> >> > perhaps?
> >>
> >> This is weird, all these patches applied cleanly on my local copy, I'll fetch
> >> a new copy from your "for-next" tree and check w/ it.
> >
> > Please verify what I have in "testing", if that's right, then we're good. It has
> > already passed my checks and 0day's.
>
> I just checked it, and it's good, sorry for all the fuzz :-)
Great, these are all queued to for-next.
--
Darren Hart
Intel Open Source Technology Center
Hi Darren,
2015-08-05 17:21 GMT-06:00 Darren Hart <[email protected]>:
> On Wed, Aug 05, 2015 at 04:23:49PM -0600, Azael Avalos wrote:
>> Hi Darren,
>>
>> 2015-08-05 14:21 GMT-06:00 Darren Hart <[email protected]>:
>> >> @@ -1131,14 +1055,10 @@ 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;
>> >> - }
>> >>
>> >> return (result == TOS_SUCCESS || result == TOS_SUCCESS2) 0 : -EIO;
>> >
>> > Hrm... the above line cause patch application failure via git (note the
>> > missing ? before the '0 : -EIO;'). This never existed upstream so far as
>> > I can determine.
>>
>> I've spotted that while compile-checking my changes locally, but I might
>> have sent you the wrong patch here, I'll double check in the future to avoid
>> these embarrassments :-(
>>
>> >
>> > It applied with some fuzz manually, but I'm concerned about how this
>> > happened. Did you have a dirty tree when you prepared these patches
>> > perhaps?
>>
>> This is weird, all these patches applied cleanly on my local copy, I'll fetch
>> a new copy from your "for-next" tree and check w/ it.
>
> Please verify what I have in "testing", if that's right, then we're good. It has
> already passed my checks and 0day's.
I just checked it, and it's good, sorry for all the fuzz :-)
>
> --
> Darren Hart
> Intel Open Source Technology Center
Cheers
Azael
--
-- El mundo apesta y vosotros apestais tambien --