These patches cleanup the driver from some no longer needed
functions, renames hci_{read, write}1 functions, some comment
blocks were changed, unneeded error checks removed and the driver
version was bumped to 0.22.
Changes since v1:
- Added two new patches to the series, one containing error check
removals and another bumping the driver version.
Darren:
Sorry for this, I totally forgot to include the patch to remove
the function error check :-(
Now this patchset is complete now :-)
Azael Avalos (5):
toshiba_acpi: Remove no longer needed hci_{read, write}2 functions
toshiba_acpi: Rename hci_{read, write}1 functions
toshiba_acpi: Cleanup blank lines after comment blocks
toshiba_acpi: Remove TOS_FAILURE check from some functions
toshiba_acpi: Bump driver version to 0.22
drivers/platform/x86/toshiba_acpi.c | 136 +++++++++++++++---------------------
1 file changed, 57 insertions(+), 79 deletions(-)
--
2.3.6
This patch removes the hci_{read, write}2 functions from the driver,
and the toshiba_hotkey_event_type_get function was adapted to use the
tci_raw function.
The hci_write2 function was only used by the bluetooth rfkill code,
but since its removal, it was causing build warnings, and the
hci_read2 function was only used by the toshiba_hotkey_event_type_get
function.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 39 +++++++------------------------------
1 file changed, 7 insertions(+), 32 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 8ee7c24..e593762 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -355,31 +355,6 @@ static u32 hci_read1(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
return out[0];
}
-static u32 hci_write2(struct toshiba_acpi_dev *dev, u32 reg, u32 in1, u32 in2)
-{
- u32 in[TCI_WORDS] = { HCI_SET, reg, in1, in2, 0, 0 };
- u32 out[TCI_WORDS];
- acpi_status status = tci_raw(dev, in, out);
-
- return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
-}
-
-static u32 hci_read2(struct toshiba_acpi_dev *dev,
- u32 reg, u32 *out1, u32 *out2)
-{
- u32 in[TCI_WORDS] = { HCI_GET, reg, *out1, *out2, 0, 0 };
- u32 out[TCI_WORDS];
- acpi_status status = tci_raw(dev, in, out);
-
- if (ACPI_FAILURE(status))
- return TOS_FAILURE;
-
- *out1 = out[2];
- *out2 = out[3];
-
- return out[0];
-}
-
/*
* Common sci tasks
*/
@@ -1190,20 +1165,20 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
u32 *type)
{
- u32 val1 = 0x03;
- u32 val2 = 0;
- u32 result;
+ u32 in[TCI_WORDS] = { HCI_GET, HCI_SYSTEM_INFO, 0x03, 0, 0, 0 };
+ u32 out[TCI_WORDS];
+ acpi_status status;
- result = hci_read2(dev, HCI_SYSTEM_INFO, &val1, &val2);
- if (result == TOS_FAILURE) {
+ status = tci_raw(dev, in, out);
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to get System type failed\n");
return -EIO;
- } else if (result == TOS_NOT_SUPPORTED) {
+ } else if (out[0] == TOS_NOT_SUPPORTED) {
pr_info("System type not supported\n");
return -ENODEV;
}
- *type = val2;
+ *type = out[3];
return 0;
}
--
2.3.6
This patch simply renames the hci_{read, write}1 functions to
hci_{read, write}.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 44 ++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index e593762..19efb05 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -326,13 +326,13 @@ static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
}
/*
- * Common hci tasks (get or set one or two value)
+ * Common hci tasks
*
* In addition to the ACPI status, the HCI system returns a result which
* may be useful (such as "not supported").
*/
-static u32 hci_write1(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
+static u32 hci_write(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
{
u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
u32 out[TCI_WORDS];
@@ -341,7 +341,7 @@ static u32 hci_write1(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
return ACPI_SUCCESS(status) ? out[0] : TOS_FAILURE;
}
-static u32 hci_read1(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
+static u32 hci_read(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
{
u32 in[TCI_WORDS] = { HCI_GET, reg, 0, 0, 0, 0 };
u32 out[TCI_WORDS];
@@ -596,7 +596,7 @@ static enum led_brightness toshiba_kbd_backlight_get(struct led_classdev *cdev)
u32 state, result;
/* Check the keyboard backlight state */
- result = hci_read1(dev, HCI_KBD_ILLUMINATION, &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;
@@ -617,7 +617,7 @@ static void toshiba_kbd_backlight_set(struct led_classdev *cdev,
/* Set the keyboard backlight state */
state = brightness ? 1 : 0;
- result = hci_write1(dev, HCI_KBD_ILLUMINATION, state);
+ result = hci_write(dev, HCI_KBD_ILLUMINATION, state);
if (result == TOS_FAILURE || result == TOS_INPUT_DATA_ERROR) {
pr_err("ACPI call to set KBD Illumination mode failed\n");
return;
@@ -1188,7 +1188,7 @@ static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, bool *enabled)
u32 hci_result;
u32 status;
- hci_result = hci_read1(dev, HCI_TR_BACKLIGHT, &status);
+ hci_result = hci_read(dev, HCI_TR_BACKLIGHT, &status);
*enabled = !status;
return hci_result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1198,7 +1198,7 @@ static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
u32 hci_result;
u32 value = !enable;
- hci_result = hci_write1(dev, HCI_TR_BACKLIGHT, value);
+ hci_result = hci_write(dev, HCI_TR_BACKLIGHT, value);
return hci_result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1221,7 +1221,7 @@ static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
brightness++;
}
- hci_result = hci_read1(dev, HCI_LCD_BRIGHTNESS, &value);
+ hci_result = hci_read(dev, HCI_LCD_BRIGHTNESS, &value);
if (hci_result == TOS_SUCCESS)
return brightness + (value >> HCI_LCD_BRIGHTNESS_SHIFT);
@@ -1276,7 +1276,7 @@ static int set_lcd_brightness(struct toshiba_acpi_dev *dev, int value)
}
value = value << HCI_LCD_BRIGHTNESS_SHIFT;
- hci_result = hci_write1(dev, HCI_LCD_BRIGHTNESS, value);
+ hci_result = hci_write(dev, HCI_LCD_BRIGHTNESS, value);
return hci_result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1326,7 +1326,7 @@ static int get_video_status(struct toshiba_acpi_dev *dev, u32 *status)
{
u32 hci_result;
- hci_result = hci_read1(dev, HCI_VIDEO_OUT, status);
+ hci_result = hci_read(dev, HCI_VIDEO_OUT, status);
return hci_result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1432,7 +1432,7 @@ static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
{
u32 hci_result;
- hci_result = hci_read1(dev, HCI_FAN, status);
+ hci_result = hci_read(dev, HCI_FAN, status);
return hci_result == TOS_SUCCESS ? 0 : -EIO;
}
@@ -1472,7 +1472,7 @@ static ssize_t fan_proc_write(struct file *file, const char __user *buf,
if (sscanf(cmd, " force_on : %i", &value) == 1 &&
value >= 0 && value <= 1) {
- hci_result = hci_write1(dev, HCI_FAN, value);
+ hci_result = hci_write(dev, HCI_FAN, value);
if (hci_result == TOS_SUCCESS)
dev->force_fan = value;
else
@@ -1500,7 +1500,7 @@ static int keys_proc_show(struct seq_file *m, void *v)
u32 value;
if (!dev->key_event_valid && dev->system_event_supported) {
- hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
+ hci_result = hci_read(dev, HCI_SYSTEM_EVENT, &value);
if (hci_result == TOS_SUCCESS) {
dev->key_event_valid = 1;
dev->last_key_event = value;
@@ -1512,7 +1512,7 @@ static int keys_proc_show(struct seq_file *m, void *v)
* some machines where system events sporadically
* become disabled.
*/
- hci_result = hci_write1(dev, HCI_SYSTEM_EVENT, 1);
+ hci_result = hci_write(dev, HCI_SYSTEM_EVENT, 1);
pr_notice("Re-enabled hotkeys\n");
} else {
pr_err("Error reading hotkey status\n");
@@ -1649,7 +1649,7 @@ static ssize_t fan_store(struct device *dev,
if (state != 0 && state != 1)
return -EINVAL;
- result = hci_write1(toshiba, HCI_FAN, state);
+ result = hci_write(toshiba, HCI_FAN, state);
if (result == TOS_FAILURE)
return -EIO;
else if (result == TOS_NOT_SUPPORTED)
@@ -2271,7 +2271,7 @@ static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
if (ACPI_FAILURE(status))
return -ENODEV;
- result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
+ result = hci_write(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
if (result == TOS_FAILURE)
return -EIO;
else if (result == TOS_NOT_SUPPORTED)
@@ -2288,8 +2288,8 @@ static void toshiba_acpi_enable_special_functions(struct toshiba_acpi_dev *dev)
* Re-activate the hotkeys, but this time, we are using the
* "Special Functions" mode.
*/
- result = hci_write1(dev, HCI_HOTKEY_EVENT,
- HCI_HOTKEY_SPECIAL_FUNCTIONS);
+ result = hci_write(dev, HCI_HOTKEY_EVENT,
+ HCI_HOTKEY_SPECIAL_FUNCTIONS);
if (result != TOS_SUCCESS)
pr_err("Could not enable the Special Function mode\n");
}
@@ -2370,7 +2370,7 @@ static void toshiba_acpi_process_hotkeys(struct toshiba_acpi_dev *dev)
toshiba_acpi_report_hotkey(dev, scancode);
} else if (dev->system_event_supported) {
do {
- hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
+ hci_result = hci_read(dev, HCI_SYSTEM_EVENT, &value);
switch (hci_result) {
case TOS_SUCCESS:
toshiba_acpi_report_hotkey(dev, (int)value);
@@ -2382,7 +2382,7 @@ static void toshiba_acpi_process_hotkeys(struct toshiba_acpi_dev *dev)
* sporadically become disabled.
*/
hci_result =
- hci_write1(dev, HCI_SYSTEM_EVENT, 1);
+ hci_write(dev, HCI_SYSTEM_EVENT, 1);
pr_notice("Re-enabled hotkeys\n");
/* Fall through */
default:
@@ -2459,7 +2459,7 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
if (acpi_has_method(dev->acpi_dev->handle, "INFO"))
dev->info_supported = 1;
else {
- hci_result = hci_write1(dev, HCI_SYSTEM_EVENT, 1);
+ hci_result = hci_write(dev, HCI_SYSTEM_EVENT, 1);
if (hci_result == TOS_SUCCESS)
dev->system_event_supported = 1;
}
@@ -2784,7 +2784,7 @@ static int toshiba_acpi_suspend(struct device *device)
u32 result;
if (dev->hotkey_dev)
- result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE);
+ result = hci_write(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE);
return 0;
}
--
2.3.6
This patch removes some blank lines after comment blocks, capitalizes
some comments first words and adds a few comments to the beggining
of some functions.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 19efb05..dca3dd2 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -78,10 +78,9 @@ MODULE_LICENSE("GPL");
* SCI stands for "System Configuration Interface" which aim is to
* conceal differences in hardware between different models.
*/
-
#define TCI_WORDS 6
-/* operations */
+/* Operations */
#define HCI_SET 0xff00
#define HCI_GET 0xfe00
#define SCI_OPEN 0xf100
@@ -89,7 +88,7 @@ MODULE_LICENSE("GPL");
#define SCI_GET 0xf300
#define SCI_SET 0xf400
-/* return codes */
+/* Return codes */
#define TOS_SUCCESS 0x0000
#define TOS_OPEN_CLOSE_OK 0x0044
#define TOS_FAILURE 0x1000
@@ -104,7 +103,7 @@ MODULE_LICENSE("GPL");
#define TOS_NOT_INITIALIZED 0x8d50
#define TOS_NOT_INSTALLED 0x8e00
-/* registers */
+/* Registers */
#define HCI_FAN 0x0004
#define HCI_TR_BACKLIGHT 0x0005
#define HCI_SYSTEM_EVENT 0x0016
@@ -126,7 +125,7 @@ MODULE_LICENSE("GPL");
#define SCI_TOUCHPAD 0x050e
#define SCI_KBD_FUNCTION_KEYS 0x0522
-/* field definitions */
+/* Field definitions */
#define HCI_ACCEL_MASK 0x7fff
#define HCI_HOTKEY_DISABLE 0x0b
#define HCI_HOTKEY_ENABLE 0x09
@@ -272,7 +271,6 @@ static const struct dmi_system_id toshiba_vendor_backlight_dmi[] = {
/*
* Utility
*/
-
static inline void _set_bit(u32 *word, u32 mask, int value)
{
*word = (*word & ~mask) | (mask * value);
@@ -281,7 +279,6 @@ static inline void _set_bit(u32 *word, u32 mask, int value)
/*
* ACPI interface wrappers
*/
-
static int write_acpi_int(const char *methodName, int val)
{
acpi_status status;
@@ -331,7 +328,6 @@ static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
* In addition to the ACPI status, the HCI system returns a result which
* may be useful (such as "not supported").
*/
-
static u32 hci_write(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
{
u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
@@ -358,7 +354,6 @@ static u32 hci_read(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
/*
* Common sci tasks
*/
-
static int sci_open(struct toshiba_acpi_dev *dev)
{
u32 in[TCI_WORDS] = { SCI_OPEN, 0, 0, 0, 0, 0 };
@@ -1183,6 +1178,7 @@ static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
return 0;
}
+/* Transflective Backlight */
static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, bool *enabled)
{
u32 hci_result;
@@ -1204,6 +1200,7 @@ static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
static struct proc_dir_entry *toshiba_proc_dir /*= 0*/;
+/* LCD Brightness */
static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
{
u32 hci_result;
@@ -1322,6 +1319,7 @@ 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;
@@ -1411,7 +1409,8 @@ static ssize_t video_proc_write(struct file *file, const char __user *buf,
_set_bit(&new_video_out, HCI_VIDEO_OUT_TV, tv_out);
/*
* To avoid unnecessary video disruption, only write the new
- * video setting if something changed. */
+ * video setting if something changed.
+ */
if (new_video_out != video_out)
ret = write_acpi_int(METHOD_VIDEO_OUT, new_video_out);
}
@@ -1428,6 +1427,7 @@ 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;
@@ -1583,7 +1583,6 @@ static const struct file_operations version_proc_fops = {
/*
* Proc and module init
*/
-
#define PROC_TOSHIBA "toshiba"
static void create_toshiba_proc_entries(struct toshiba_acpi_dev *dev)
@@ -2324,9 +2323,7 @@ static void toshiba_acpi_hotkey_work(struct work_struct *work)
pr_err("ACPI NTFY method execution failed\n");
}
-/*
- * Returns hotkey scancode, or < 0 on failure.
- */
+/* Returns hotkey scancode, or < 0 on failure */
static int toshiba_acpi_query_hotkey(struct toshiba_acpi_dev *dev)
{
unsigned long long value;
@@ -2488,6 +2485,9 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
return error;
}
+/*
+ * Backlight
+ */
static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
{
struct backlight_properties props;
@@ -2552,6 +2552,9 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
return 0;
}
+/*
+ * ACPI driver functions
+ */
static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
{
struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
--
2.3.6
This patch removes the check for TOS_FAILURE whenever we are using
the tci_raw function call, as that code is only returned by the
{hci, sci}_{read, write} functions and never by the tci_raw, and
thus making that check irrelevant.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_acpi.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index dca3dd2..dc38204 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -361,7 +361,7 @@ static int sci_open(struct toshiba_acpi_dev *dev)
acpi_status status;
status = tci_raw(dev, in, out);
- if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to open SCI failed\n");
return 0;
}
@@ -399,7 +399,7 @@ static void sci_close(struct toshiba_acpi_dev *dev)
acpi_status status;
status = tci_raw(dev, in, out);
- if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to close SCI failed\n");
return;
}
@@ -447,7 +447,7 @@ static int toshiba_illumination_available(struct toshiba_acpi_dev *dev)
status = tci_raw(dev, in, out);
sci_close(dev);
- if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to query Illumination support failed\n");
return 0;
} else if (out[0] == TOS_NOT_SUPPORTED) {
@@ -669,7 +669,7 @@ static int toshiba_eco_mode_available(struct toshiba_acpi_dev *dev)
u32 out[TCI_WORDS];
status = tci_raw(dev, in, out);
- if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
+ 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");
@@ -791,7 +791,7 @@ static void toshiba_usb_sleep_charge_available(struct toshiba_acpi_dev *dev)
return;
status = tci_raw(dev, in, out);
- if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to get USB Sleep and Charge mode failed\n");
sci_close(dev);
return;
@@ -805,7 +805,7 @@ 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);
- if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to get USB Sleep and Charge mode failed\n");
sci_close(dev);
return;
@@ -885,7 +885,7 @@ static int toshiba_sleep_functions_status_get(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) || out[0] == TOS_FAILURE) {
+ 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) {
@@ -914,7 +914,7 @@ 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) || out[0] == TOS_FAILURE) {
+ 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) {
@@ -940,7 +940,7 @@ static int toshiba_usb_rapid_charge_get(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) || out[0] == TOS_FAILURE) {
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to get USB Rapid Charge failed\n");
return -EIO;
} else if (out[0] == TOS_NOT_SUPPORTED ||
@@ -968,7 +968,7 @@ 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) || out[0] == TOS_FAILURE) {
+ if (ACPI_FAILURE(status)) {
pr_err("ACPI call to set USB Rapid Charge failed\n");
return -EIO;
} else if (out[0] == TOS_NOT_SUPPORTED) {
--
2.3.6
This patch simply bumps the driver version to 0.22, as significant
changeswere made to the driver, such as cleanups, updated events,
keymap handling, fixes and the bluetooth rfkill code removal.
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 dc38204..d41cf5c 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.21"
+#define TOSHIBA_ACPI_VERSION "0.22"
#define PROC_INTERFACE_VERSION 1
#include <linux/kernel.h>
--
2.3.6
On Mon, 2015-05-04 at 11:15 -0600, Azael Avalos wrote:
> This patch removes some blank lines after comment blocks, capitalizes
> some comments first words and adds a few comments to the beggining
> of some functions.
I think blank lines after comments are generally OK.
This is now inconsistent about
/*
* short comment
*/
and
/* short comment */
Not all function blocks need introductory comments.
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
[]
> @@ -78,10 +78,9 @@ MODULE_LICENSE("GPL");
> * SCI stands for "System Configuration Interface" which aim is to
> * conceal differences in hardware between different models.
> */
> -
> #define TCI_WORDS 6
It's sometimes nice to have blank lines after long
block comments.
> -/* operations */
> +/* Operations */
I think these sentence case changes aren't better.
Hi there,
2015-05-05 8:18 GMT-06:00 Joe Perches <[email protected]>:
> On Mon, 2015-05-04 at 11:15 -0600, Azael Avalos wrote:
>> This patch removes some blank lines after comment blocks, capitalizes
>> some comments first words and adds a few comments to the beggining
>> of some functions.
>
> I think blank lines after comments are generally OK.
>
> This is now inconsistent about
>
> /*
> * short comment
> */
I was using this kind of comment block to identify function blocks
that belonged to a certain feature set (eg: ACPI functions, sysfs, etc.)
and provide a bit more readability to someone unfamiliar with the code.
>
> and
>
> /* short comment */
And I was using this kind for helper functions or feature function calls.
>
> Not all function blocks need introductory comments.
I know that, but again, I was just adding this for a bit of readability and
consistency, as I've been adding this kind of comment blocks to the
beginning of each feature function call I've been added so far.
So either I remove them all (and be consistent with how the previous
functions were, basically no comments), or keep adding them (and keep
consistency of what I added), I think this is just a matter of taste.
>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> []
>> @@ -78,10 +78,9 @@ MODULE_LICENSE("GPL");
>> * SCI stands for "System Configuration Interface" which aim is to
>> * conceal differences in hardware between different models.
>> */
>> -
>> #define TCI_WORDS 6
>
> It's sometimes nice to have blank lines after long
> block comments.
OK, I can keep them as is, I have no problem either with these :-)
>
>> -/* operations */
>> +/* Operations */
>
> I think these sentence case changes aren't better.
This was just to follow up on a capitalization already done
to the rest of the comment blocks, but somehow these were
left behind, 'tho not sure if it was on purpose or simply missed.
Cheers
Azael
--
-- El mundo apesta y vosotros apestais tambien --
On Mon, May 04, 2015 at 11:15:56AM -0600, Azael Avalos wrote:
> This patch removes some blank lines after comment blocks, capitalizes
> some comments first words and adds a few comments to the beggining
> of some functions.
>
> Signed-off-by: Azael Avalos <[email protected]>
> ---
> drivers/platform/x86/toshiba_acpi.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 19efb05..dca3dd2 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -78,10 +78,9 @@ MODULE_LICENSE("GPL");
> * SCI stands for "System Configuration Interface" which aim is to
> * conceal differences in hardware between different models.
> */
> -
> #define TCI_WORDS 6
>
> -/* operations */
> +/* Operations */
I don't object - but just for the record. I care about Sentence formatting when
using sentences and multiline comments, not so much for single word or one line
phrases.
Be consistent within the file, that matters most.
> #define HCI_SET 0xff00
> #define HCI_GET 0xfe00
> #define SCI_OPEN 0xf100
> @@ -89,7 +88,7 @@ MODULE_LICENSE("GPL");
> #define SCI_GET 0xf300
> #define SCI_SET 0xf400
>
> -/* return codes */
> +/* Return codes */
> #define TOS_SUCCESS 0x0000
> #define TOS_OPEN_CLOSE_OK 0x0044
> #define TOS_FAILURE 0x1000
> @@ -104,7 +103,7 @@ MODULE_LICENSE("GPL");
> #define TOS_NOT_INITIALIZED 0x8d50
> #define TOS_NOT_INSTALLED 0x8e00
>
> -/* registers */
> +/* Registers */
> #define HCI_FAN 0x0004
> #define HCI_TR_BACKLIGHT 0x0005
> #define HCI_SYSTEM_EVENT 0x0016
> @@ -126,7 +125,7 @@ MODULE_LICENSE("GPL");
> #define SCI_TOUCHPAD 0x050e
> #define SCI_KBD_FUNCTION_KEYS 0x0522
>
> -/* field definitions */
> +/* Field definitions */
> #define HCI_ACCEL_MASK 0x7fff
> #define HCI_HOTKEY_DISABLE 0x0b
> #define HCI_HOTKEY_ENABLE 0x09
> @@ -272,7 +271,6 @@ static const struct dmi_system_id toshiba_vendor_backlight_dmi[] = {
> /*
> * Utility
> */
> -
I'd keep the space, just as you will keep a space between _set_bit and functions
that follow it.
> static inline void _set_bit(u32 *word, u32 mask, int value)
> {
> *word = (*word & ~mask) | (mask * value);
> @@ -281,7 +279,6 @@ static inline void _set_bit(u32 *word, u32 mask, int value)
> /*
> * ACPI interface wrappers
> */
> -
> static int write_acpi_int(const char *methodName, int val)
> {
> acpi_status status;
> @@ -331,7 +328,6 @@ static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
> * In addition to the ACPI status, the HCI system returns a result which
> * may be useful (such as "not supported").
> */
> -
> static u32 hci_write(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
> {
> u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
> @@ -358,7 +354,6 @@ static u32 hci_read(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
> /*
> * Common sci tasks
> */
> -
> static int sci_open(struct toshiba_acpi_dev *dev)
> {
> u32 in[TCI_WORDS] = { SCI_OPEN, 0, 0, 0, 0, 0 };
> @@ -1183,6 +1178,7 @@ static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
> return 0;
> }
>
> +/* Transflective Backlight */
> static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, bool *enabled)
> {
> u32 hci_result;
> @@ -1204,6 +1200,7 @@ static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
>
> static struct proc_dir_entry *toshiba_proc_dir /*= 0*/;
>
> +/* LCD Brightness */
> static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
> {
> u32 hci_result;
> @@ -1322,6 +1319,7 @@ 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;
> @@ -1411,7 +1409,8 @@ static ssize_t video_proc_write(struct file *file, const char __user *buf,
> _set_bit(&new_video_out, HCI_VIDEO_OUT_TV, tv_out);
> /*
> * To avoid unnecessary video disruption, only write the new
> - * video setting if something changed. */
> + * video setting if something changed.
> + */
> if (new_video_out != video_out)
> ret = write_acpi_int(METHOD_VIDEO_OUT, new_video_out);
> }
> @@ -1428,6 +1427,7 @@ 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;
> @@ -1583,7 +1583,6 @@ static const struct file_operations version_proc_fops = {
> /*
> * Proc and module init
> */
> -
> #define PROC_TOSHIBA "toshiba"
>
> static void create_toshiba_proc_entries(struct toshiba_acpi_dev *dev)
> @@ -2324,9 +2323,7 @@ static void toshiba_acpi_hotkey_work(struct work_struct *work)
> pr_err("ACPI NTFY method execution failed\n");
> }
>
> -/*
> - * Returns hotkey scancode, or < 0 on failure.
> - */
> +/* Returns hotkey scancode, or < 0 on failure */
> static int toshiba_acpi_query_hotkey(struct toshiba_acpi_dev *dev)
> {
> unsigned long long value;
> @@ -2488,6 +2485,9 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
> return error;
> }
>
> +/*
> + * Backlight
> + */
> static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
> {
> struct backlight_properties props;
> @@ -2552,6 +2552,9 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
> return 0;
> }
>
> +/*
> + * ACPI driver functions
> + */
> static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
> {
> struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
> --
> 2.3.6
>
>
--
Darren Hart
Intel Open Source Technology Center
On Mon, May 04, 2015 at 11:15:53AM -0600, Azael Avalos wrote:
> These patches cleanup the driver from some no longer needed
> functions, renames hci_{read, write}1 functions, some comment
> blocks were changed, unneeded error checks removed and the driver
> version was bumped to 0.22.
>
> Changes since v1:
> - Added two new patches to the series, one containing error check
> removals and another bumping the driver version.
>
> Darren:
> Sorry for this, I totally forgot to include the patch to remove
> the function error check :-(
> Now this patchset is complete now :-)
>
So I'm a little confused :-) I have staged the patches locally as follows
(newest first):
<top>
f6ea7d1 toshiba_acpi: Bump driver version to 0.22
dd8b682 toshiba_acpi: Remove TOS_FAILURE check from some functions
60d9349 toshiba_acpi: Cleanup blank lines after comment blocks
c62377a toshiba_acpi: Rename hci_{read, write}1 functions
cbed7e3 toshiba_acpi: Remove no longer needed hci_{read, write}2 functions
8f163a9 toshiba_acpi: Remove bluetooth rfkill code
0a4c28f toshiba_bluetooth: Change BT status message to debug
3ad2df6 toshiba_bluetooth: Adapt *_enable, *_notify and *_resume functions to rfkill
9eb4797 toshiba_bluetooth: Add RFKill handler functions
1ae8cb3 toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev
...
Is this what you had in mind?
As for the comment cleanup (blank lines, etc.). Joe raised some points, I think I did too - is there another version of that patch coming?
--
Darren Hart
Intel Open Source Technology Center
Hi Darren,
2015-05-06 0:00 GMT-06:00 Darren Hart <[email protected]>:
> On Mon, May 04, 2015 at 11:15:53AM -0600, Azael Avalos wrote:
>> These patches cleanup the driver from some no longer needed
>> functions, renames hci_{read, write}1 functions, some comment
>> blocks were changed, unneeded error checks removed and the driver
>> version was bumped to 0.22.
>>
>> Changes since v1:
>> - Added two new patches to the series, one containing error check
>> removals and another bumping the driver version.
>>
>> Darren:
>> Sorry for this, I totally forgot to include the patch to remove
>> the function error check :-(
>> Now this patchset is complete now :-)
>>
>
> So I'm a little confused :-) I have staged the patches locally as follows
> (newest first):
>
> <top>
> f6ea7d1 toshiba_acpi: Bump driver version to 0.22
> dd8b682 toshiba_acpi: Remove TOS_FAILURE check from some functions
> 60d9349 toshiba_acpi: Cleanup blank lines after comment blocks
> c62377a toshiba_acpi: Rename hci_{read, write}1 functions
> cbed7e3 toshiba_acpi: Remove no longer needed hci_{read, write}2 functions
> 8f163a9 toshiba_acpi: Remove bluetooth rfkill code
> 0a4c28f toshiba_bluetooth: Change BT status message to debug
> 3ad2df6 toshiba_bluetooth: Adapt *_enable, *_notify and *_resume functions to rfkill
> 9eb4797 toshiba_bluetooth: Add RFKill handler functions
> 1ae8cb3 toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev
> ...
>
> Is this what you had in mind?
The BT code removal from toshiba_acpi should come first, then the rfkill
to the toshiba_bluetooth driver, then all the rest :-)
>
> As for the comment cleanup (blank lines, etc.). Joe raised some points, I think I did too - is there another version of that patch coming?
Yeah, I'll send a v3 shortly, was in a meeting most of the day yesterday
and didn't had time to send it.
>
> --
> Darren Hart
> Intel Open Source Technology Center
Cheers
Azael
--
-- El mundo apesta y vosotros apestais tambien --
On Wed, May 06, 2015 at 09:08:52AM -0600, Azael Avalos wrote:
> Hi Darren,
>
> 2015-05-06 0:00 GMT-06:00 Darren Hart <[email protected]>:
> > On Mon, May 04, 2015 at 11:15:53AM -0600, Azael Avalos wrote:
> >> These patches cleanup the driver from some no longer needed
> >> functions, renames hci_{read, write}1 functions, some comment
> >> blocks were changed, unneeded error checks removed and the driver
> >> version was bumped to 0.22.
> >>
> >> Changes since v1:
> >> - Added two new patches to the series, one containing error check
> >> removals and another bumping the driver version.
> >>
> >> Darren:
> >> Sorry for this, I totally forgot to include the patch to remove
> >> the function error check :-(
> >> Now this patchset is complete now :-)
> >>
> >
> > So I'm a little confused :-) I have staged the patches locally as follows
> > (newest first):
> >
> > <top>
> > f6ea7d1 toshiba_acpi: Bump driver version to 0.22
> > dd8b682 toshiba_acpi: Remove TOS_FAILURE check from some functions
> > 60d9349 toshiba_acpi: Cleanup blank lines after comment blocks
> > c62377a toshiba_acpi: Rename hci_{read, write}1 functions
> > cbed7e3 toshiba_acpi: Remove no longer needed hci_{read, write}2 functions
> > 8f163a9 toshiba_acpi: Remove bluetooth rfkill code
> > 0a4c28f toshiba_bluetooth: Change BT status message to debug
> > 3ad2df6 toshiba_bluetooth: Adapt *_enable, *_notify and *_resume functions to rfkill
> > 9eb4797 toshiba_bluetooth: Add RFKill handler functions
> > 1ae8cb3 toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev
> > ...
> >
> > Is this what you had in mind?
>
> The BT code removal from toshiba_acpi should come first, then the rfkill
> to the toshiba_bluetooth driver, then all the rest :-)
>
OK, please have a look at:
http://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git/shortlog/refs/heads/toshiba
This should everything I have pending from you. Please review and let me know if
anything is missing. This will all got toward 4.2. Sorry about the mess.
CC [M] drivers/platform/x86/toshiba_bluetooth.o
In file included from include/linux/printk.h:275:0,
from include/linux/kernel.h:13,
from drivers/platform/x86/toshiba_bluetooth.c:17:
drivers/platform/x86/toshiba_bluetooth.c: In function ‘toshiba_bluetooth_sync_status’:
include/linux/dynamic_debug.h:64:16: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘int’ [-Wformat=]
static struct _ddebug __aligned(8) \
^
include/linux/dynamic_debug.h:76:2: note: in expansion of macro ‘DEFINE_DYNAMIC_DEBUG_METADATA’
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
^
include/linux/printk.h:281:2: note: in expansion of macro ‘dynamic_pr_debug’
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^
drivers/platform/x86/toshiba_bluetooth.c:158:2: note: in expansion of macro ‘pr_debug’
pr_debug("Bluetooth status %llu killswitch %d plugged %d powered %d\n",
^
Resolved with:
$ git diff
diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index 220645a..c5e4508 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -155,7 +155,7 @@ static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
bt_dev->plugged = (status & BT_PLUGGED_MASK) ? true : false;
bt_dev->powered = (status & BT_POWER_MASK) ? true : false;
- pr_debug("Bluetooth status %llu killswitch %d plugged %d powered %d\n",
+ pr_debug("Bluetooth status %d killswitch %d plugged %d powered %d\n",
status, bt_dev->killswitch, bt_dev->plugged, bt_dev->powered);
return 0;
If this is the change you would make, just ack this here and I'll roll it in -
I don't think I can handle another round of this series ;-)
--
Darren Hart
Intel Open Source Technology Center
Hi Darren,
2015-05-06 16:45 GMT-06:00 Darren Hart <[email protected]>:
> On Wed, May 06, 2015 at 09:08:52AM -0600, Azael Avalos wrote:
>> Hi Darren,
>>
>> 2015-05-06 0:00 GMT-06:00 Darren Hart <[email protected]>:
>> > On Mon, May 04, 2015 at 11:15:53AM -0600, Azael Avalos wrote:
>> >> These patches cleanup the driver from some no longer needed
>> >> functions, renames hci_{read, write}1 functions, some comment
>> >> blocks were changed, unneeded error checks removed and the driver
>> >> version was bumped to 0.22.
>> >>
>> >> Changes since v1:
>> >> - Added two new patches to the series, one containing error check
>> >> removals and another bumping the driver version.
>> >>
>> >> Darren:
>> >> Sorry for this, I totally forgot to include the patch to remove
>> >> the function error check :-(
>> >> Now this patchset is complete now :-)
>> >>
>> >
>> > So I'm a little confused :-) I have staged the patches locally as follows
>> > (newest first):
>> >
>> > <top>
>> > f6ea7d1 toshiba_acpi: Bump driver version to 0.22
>> > dd8b682 toshiba_acpi: Remove TOS_FAILURE check from some functions
>> > 60d9349 toshiba_acpi: Cleanup blank lines after comment blocks
>> > c62377a toshiba_acpi: Rename hci_{read, write}1 functions
>> > cbed7e3 toshiba_acpi: Remove no longer needed hci_{read, write}2 functions
>> > 8f163a9 toshiba_acpi: Remove bluetooth rfkill code
>> > 0a4c28f toshiba_bluetooth: Change BT status message to debug
>> > 3ad2df6 toshiba_bluetooth: Adapt *_enable, *_notify and *_resume functions to rfkill
>> > 9eb4797 toshiba_bluetooth: Add RFKill handler functions
>> > 1ae8cb3 toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev
>> > ...
>> >
>> > Is this what you had in mind?
>>
>> The BT code removal from toshiba_acpi should come first, then the rfkill
>> to the toshiba_bluetooth driver, then all the rest :-)
>>
>
> OK, please have a look at:
>
> http://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git/shortlog/refs/heads/toshiba
>
> This should everything I have pending from you. Please review and let me know if
> anything is missing. This will all got toward 4.2. Sorry about the mess.
>
> CC [M] drivers/platform/x86/toshiba_bluetooth.o
> In file included from include/linux/printk.h:275:0,
> from include/linux/kernel.h:13,
> from drivers/platform/x86/toshiba_bluetooth.c:17:
> drivers/platform/x86/toshiba_bluetooth.c: In function ‘toshiba_bluetooth_sync_status’:
> include/linux/dynamic_debug.h:64:16: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘int’ [-Wformat=]
> static struct _ddebug __aligned(8) \
> ^
> include/linux/dynamic_debug.h:76:2: note: in expansion of macro ‘DEFINE_DYNAMIC_DEBUG_METADATA’
> DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
> ^
> include/linux/printk.h:281:2: note: in expansion of macro ‘dynamic_pr_debug’
> dynamic_pr_debug(fmt, ##__VA_ARGS__)
> ^
> drivers/platform/x86/toshiba_bluetooth.c:158:2: note: in expansion of macro ‘pr_debug’
> pr_debug("Bluetooth status %llu killswitch %d plugged %d powered %d\n",
> ^
>
> Resolved with:
>
> $ git diff
> diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
> index 220645a..c5e4508 100644
> --- a/drivers/platform/x86/toshiba_bluetooth.c
> +++ b/drivers/platform/x86/toshiba_bluetooth.c
> @@ -155,7 +155,7 @@ static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
> bt_dev->plugged = (status & BT_PLUGGED_MASK) ? true : false;
> bt_dev->powered = (status & BT_POWER_MASK) ? true : false;
>
> - pr_debug("Bluetooth status %llu killswitch %d plugged %d powered %d\n",
> + pr_debug("Bluetooth status %d killswitch %d plugged %d powered %d\n",
> status, bt_dev->killswitch, bt_dev->plugged, bt_dev->powered);
>
> return 0;
>
> If this is the change you would make, just ack this here and I'll roll it in -
> I don't think I can handle another round of this series ;-)
Yeah, this is how it is supposed to be in, sorry about the mess too :-)
>
> --
> Darren Hart
> Intel Open Source Technology Center
Cheers
Azael
--
-- El mundo apesta y vosotros apestais tambien --