From 3a3c1115ce55f02e6185338808a4b7959b46e334 Mon Sep 17 00:00:00 2001 From: Michał Kępień Date: Thu, 9 Mar 2017 13:11:43 +0100 Subject: platform/x86: hp-wmi: remove sparse_keymap_free() calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As sparse_keymap_setup() now uses a managed memory allocation for the keymap copy it creates, the latter is freed automatically. Remove all calls to sparse_keymap_free(). Signed-off-by: Michał Kępień Signed-off-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 96ffda493266..7abf92d0ba81 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -673,7 +673,7 @@ static int __init hp_wmi_input_setup(void) status = wmi_install_notify_handler(HPWMI_EVENT_GUID, hp_wmi_notify, NULL); if (ACPI_FAILURE(status)) { err = -EIO; - goto err_free_keymap; + goto err_free_dev; } err = input_register_device(hp_wmi_input_dev); @@ -684,8 +684,6 @@ static int __init hp_wmi_input_setup(void) err_uninstall_notifier: wmi_remove_notify_handler(HPWMI_EVENT_GUID); - err_free_keymap: - sparse_keymap_free(hp_wmi_input_dev); err_free_dev: input_free_device(hp_wmi_input_dev); return err; @@ -694,7 +692,6 @@ static int __init hp_wmi_input_setup(void) static void hp_wmi_input_destroy(void) { wmi_remove_notify_handler(HPWMI_EVENT_GUID); - sparse_keymap_free(hp_wmi_input_dev); input_unregister_device(hp_wmi_input_dev); } -- cgit v1.2.3 From c7dfc2facbd69dad89b75e13c608da709668dcd0 Mon Sep 17 00:00:00 2001 From: Carlo Caione Date: Sun, 9 Apr 2017 15:56:07 +0200 Subject: platform/x86: hp-wmi: Fix error value for hp_wmi_tablet_state hp_wmi_tablet_state() fails to return the correct error code when hp_wmi_perform_query() returns the HP WMI query specific error code that is a positive value. Signed-off-by: Carlo Caione Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/hp-wmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 7abf92d0ba81..9db502c42b5c 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -290,7 +290,7 @@ static int hp_wmi_tablet_state(void) int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state, sizeof(state), sizeof(state)); if (ret) - return ret; + return -EINVAL; return (state & 0x4) ? 1 : 0; } -- cgit v1.2.3 From 298747b7579f5bbbced793d997b333fd10a24921 Mon Sep 17 00:00:00 2001 From: Carlo Caione Date: Sun, 9 Apr 2017 15:56:08 +0200 Subject: platform/x86: hp-wmi: Fix detection for dock and tablet mode The current driver code is not checking for the error values returned by 'hp_wmi_dock_state()' and 'hp_wmi_tablet_state()' before passing the returned values down to 'input_report_switch()'. This error code is being translated to '1' in the input subsystem, reporting the wrong status. The biggest problem caused by this issue is that several laptops are wrongly reported by the driver as docked, preventing them to be put to sleep using the LID (and in most cases they are not even dockable). With this patch we create the report switches only if we are able to read the dock and tablet mode status correctly from ACPI. Signed-off-by: Carlo Caione Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/hp-wmi.c | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 9db502c42b5c..5c5efccf9f19 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -572,10 +572,12 @@ static void hp_wmi_notify(u32 value, void *context) switch (event_id) { case HPWMI_DOCK_EVENT: - input_report_switch(hp_wmi_input_dev, SW_DOCK, - hp_wmi_dock_state()); - input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, - hp_wmi_tablet_state()); + if (test_bit(SW_DOCK, hp_wmi_input_dev->swbit)) + input_report_switch(hp_wmi_input_dev, SW_DOCK, + hp_wmi_dock_state()); + if (test_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit)) + input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, + hp_wmi_tablet_state()); input_sync(hp_wmi_input_dev); break; case HPWMI_PARK_HDD: @@ -644,6 +646,7 @@ static int __init hp_wmi_input_setup(void) { acpi_status status; int err; + int val; hp_wmi_input_dev = input_allocate_device(); if (!hp_wmi_input_dev) @@ -654,17 +657,26 @@ static int __init hp_wmi_input_setup(void) hp_wmi_input_dev->id.bustype = BUS_HOST; __set_bit(EV_SW, hp_wmi_input_dev->evbit); - __set_bit(SW_DOCK, hp_wmi_input_dev->swbit); - __set_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit); + + /* Dock */ + val = hp_wmi_dock_state(); + if (!(val < 0)) { + __set_bit(SW_DOCK, hp_wmi_input_dev->swbit); + input_report_switch(hp_wmi_input_dev, SW_DOCK, val); + } + + /* Tablet mode */ + val = hp_wmi_tablet_state(); + if (!(val < 0)) { + __set_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit); + input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, val); + } err = sparse_keymap_setup(hp_wmi_input_dev, hp_wmi_keymap, NULL); if (err) goto err_free_dev; /* Set initial hardware state */ - input_report_switch(hp_wmi_input_dev, SW_DOCK, hp_wmi_dock_state()); - input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, - hp_wmi_tablet_state()); input_sync(hp_wmi_input_dev); if (!hp_wmi_bios_2009_later() && hp_wmi_bios_2008_later()) @@ -947,10 +959,12 @@ static int hp_wmi_resume_handler(struct device *device) * changed. */ if (hp_wmi_input_dev) { - input_report_switch(hp_wmi_input_dev, SW_DOCK, - hp_wmi_dock_state()); - input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, - hp_wmi_tablet_state()); + if (test_bit(SW_DOCK, hp_wmi_input_dev->swbit)) + input_report_switch(hp_wmi_input_dev, SW_DOCK, + hp_wmi_dock_state()); + if (test_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit)) + input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, + hp_wmi_tablet_state()); input_sync(hp_wmi_input_dev); } -- cgit v1.2.3 From d313876925f3e7a480a02773fd333bcab9202d5e Mon Sep 17 00:00:00 2001 From: Carlo Caione Date: Wed, 19 Apr 2017 22:36:39 +0200 Subject: platform/x86: hp-wmi: Do not shadow error values All the helper functions (i.e. hp_wmi_dock_state, hp_wmi_tablet_state, ...) using hp_wmi_perform_query to perform an HP WMI query shadow the returned value in case of error. We return -EINVAL only when the HP WMI query returns a positive value (the specific error code) to not mix this up with the actual value returned by the helper function. Suggested-by: Andy Shevchenko Signed-off-by: Carlo Caione Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/hp-wmi.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 5c5efccf9f19..5ca9328ba296 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -248,7 +248,7 @@ static int hp_wmi_display_state(void) int ret = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, 0, &state, sizeof(state), sizeof(state)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return state; } @@ -258,7 +258,7 @@ static int hp_wmi_hddtemp_state(void) int ret = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, 0, &state, sizeof(state), sizeof(state)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return state; } @@ -268,7 +268,7 @@ static int hp_wmi_als_state(void) int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 0, &state, sizeof(state), sizeof(state)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return state; } @@ -279,7 +279,7 @@ static int hp_wmi_dock_state(void) sizeof(state), sizeof(state)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return state & 0x1; } @@ -290,7 +290,7 @@ static int hp_wmi_tablet_state(void) int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state, sizeof(state), sizeof(state)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return (state & 0x4) ? 1 : 0; } @@ -323,7 +323,7 @@ static int __init hp_wmi_enable_hotkeys(void) int ret = hp_wmi_perform_query(HPWMI_BIOS_QUERY, 1, &value, sizeof(value), 0); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return 0; } @@ -336,7 +336,7 @@ static int hp_wmi_set_block(void *data, bool blocked) ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 1, &query, sizeof(query), 0); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return 0; } @@ -428,7 +428,7 @@ static int hp_wmi_post_code_state(void) int ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, 0, &state, sizeof(state), sizeof(state)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return state; } @@ -494,7 +494,7 @@ static ssize_t set_als(struct device *dev, struct device_attribute *attr, int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 1, &tmp, sizeof(tmp), sizeof(tmp)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return count; } @@ -515,7 +515,7 @@ static ssize_t set_postcode(struct device *dev, struct device_attribute *attr, ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, 1, &tmp, sizeof(tmp), sizeof(tmp)); if (ret) - return -EINVAL; + return ret < 0 ? ret : -EINVAL; return count; } -- cgit v1.2.3 From 3bf9310a573d923483526b25c6ec32233748a989 Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Thu, 13 Apr 2017 11:05:42 -0700 Subject: platform/x86: hp-wmi: Cleanup local variable declarations Declare like types on one line. Order declarations in decreasing length where possible. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 5ca9328ba296..e772105ea967 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -346,15 +346,14 @@ static const struct rfkill_ops hp_wmi_rfkill_ops = { static bool hp_wmi_get_sw_state(enum hp_wmi_radio r) { + int mask = 0x200 << (r * 8); int wireless = 0; - int mask; + hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, &wireless, sizeof(wireless), sizeof(wireless)); /* TBD: Pass error */ - mask = 0x200 << (r * 8); - if (wireless & mask) return false; else @@ -363,15 +362,14 @@ static bool hp_wmi_get_sw_state(enum hp_wmi_radio r) static bool hp_wmi_get_hw_state(enum hp_wmi_radio r) { + int mask = 0x800 << (r * 8); int wireless = 0; - int mask; + hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, &wireless, sizeof(wireless), sizeof(wireless)); /* TBD: Pass error */ - mask = 0x800 << (r * 8); - if (wireless & mask) return false; else @@ -395,8 +393,8 @@ static const struct rfkill_ops hp_wmi_rfkill2_ops = { static int hp_wmi_rfkill2_refresh(void) { - int err, i; struct bios_rfkill2_state state; + int err, i; err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, 0, &state, 0, sizeof(state)); @@ -502,9 +500,9 @@ static ssize_t set_als(struct device *dev, struct device_attribute *attr, static ssize_t set_postcode(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { + long unsigned int tmp2; int ret; u32 tmp; - long unsigned int tmp2; ret = kstrtoul(buf, 10, &tmp2); if (ret || tmp2 != 1) @@ -530,11 +528,11 @@ static DEVICE_ATTR(postcode, S_IRUGO | S_IWUSR, show_postcode, set_postcode); static void hp_wmi_notify(u32 value, void *context) { struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL }; - union acpi_object *obj; u32 event_id, event_data; + union acpi_object *obj; int key_code = 0, ret; - u32 *location; acpi_status status; + u32 *location; status = wmi_get_event_data(value, &response); if (status != AE_OK) { @@ -645,8 +643,7 @@ static void hp_wmi_notify(u32 value, void *context) static int __init hp_wmi_input_setup(void) { acpi_status status; - int err; - int val; + int err, val; hp_wmi_input_dev = input_allocate_device(); if (!hp_wmi_input_dev) @@ -719,8 +716,7 @@ static void cleanup_sysfs(struct platform_device *device) static int __init hp_wmi_rfkill_setup(struct platform_device *device) { - int err; - int wireless = 0; + int err, wireless = 0; err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, &wireless, sizeof(wireless), sizeof(wireless)); @@ -804,8 +800,9 @@ register_wifi_error: static int __init hp_wmi_rfkill2_setup(struct platform_device *device) { - int err, i; struct bios_rfkill2_state state; + int err, i; + err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, 0, &state, 0, sizeof(state)); if (err) @@ -1002,9 +999,9 @@ static struct platform_driver hp_wmi_driver = { static int __init hp_wmi_init(void) { - int err; int event_capable = wmi_has_guid(HPWMI_EVENT_GUID); int bios_capable = wmi_has_guid(HPWMI_BIOS_GUID); + int err; if (!bios_capable && !event_capable) return -ENODEV; -- cgit v1.2.3 From d8193cff33906e24ac1830ecb2ca95833d058a3a Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Wed, 19 Apr 2017 15:42:01 -0700 Subject: platform/x86: hp-wmi: Standardize enum usage for constants Use enums consistently throughout the hp-wmi driver for groups of related constants. Use hex and align the assignment within groups. Move the *QUERY constants into an enum, create a new enum defining the READ, WRITE, and ODM constants and use them instead of 0 and 1 at the call sites. Set the command directly instead of using the ternary operator since both 1 and 3 as previously documented would result in the command being set to 0x2. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 132 +++++++++++++++++++++++------------------- 1 file changed, 74 insertions(+), 58 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index e772105ea967..2073f1e97970 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -48,41 +48,29 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45e9-BE91-3D44E2C707E4"); #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C" #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4" -#define HPWMI_DISPLAY_QUERY 0x1 -#define HPWMI_HDDTEMP_QUERY 0x2 -#define HPWMI_ALS_QUERY 0x3 -#define HPWMI_HARDWARE_QUERY 0x4 -#define HPWMI_WIRELESS_QUERY 0x5 -#define HPWMI_BIOS_QUERY 0x9 -#define HPWMI_FEATURE_QUERY 0xb -#define HPWMI_HOTKEY_QUERY 0xc -#define HPWMI_FEATURE2_QUERY 0xd -#define HPWMI_WIRELESS2_QUERY 0x1b -#define HPWMI_POSTCODEERROR_QUERY 0x2a - enum hp_wmi_radio { - HPWMI_WIFI = 0, - HPWMI_BLUETOOTH = 1, - HPWMI_WWAN = 2, - HPWMI_GPS = 3, + HPWMI_WIFI = 0x0, + HPWMI_BLUETOOTH = 0x1, + HPWMI_WWAN = 0x2, + HPWMI_GPS = 0x3, }; enum hp_wmi_event_ids { - HPWMI_DOCK_EVENT = 1, - HPWMI_PARK_HDD = 2, - HPWMI_SMART_ADAPTER = 3, - HPWMI_BEZEL_BUTTON = 4, - HPWMI_WIRELESS = 5, - HPWMI_CPU_BATTERY_THROTTLE = 6, - HPWMI_LOCK_SWITCH = 7, - HPWMI_LID_SWITCH = 8, - HPWMI_SCREEN_ROTATION = 9, - HPWMI_COOLSENSE_SYSTEM_MOBILE = 0x0A, - HPWMI_COOLSENSE_SYSTEM_HOT = 0x0B, - HPWMI_PROXIMITY_SENSOR = 0x0C, - HPWMI_BACKLIT_KB_BRIGHTNESS = 0x0D, - HPWMI_PEAKSHIFT_PERIOD = 0x0F, - HPWMI_BATTERY_CHARGE_PERIOD = 0x10, + HPWMI_DOCK_EVENT = 0x01, + HPWMI_PARK_HDD = 0x02, + HPWMI_SMART_ADAPTER = 0x03, + HPWMI_BEZEL_BUTTON = 0x04, + HPWMI_WIRELESS = 0x05, + HPWMI_CPU_BATTERY_THROTTLE = 0x06, + HPWMI_LOCK_SWITCH = 0x07, + HPWMI_LID_SWITCH = 0x08, + HPWMI_SCREEN_ROTATION = 0x09, + HPWMI_COOLSENSE_SYSTEM_MOBILE = 0x0A, + HPWMI_COOLSENSE_SYSTEM_HOT = 0x0B, + HPWMI_PROXIMITY_SENSOR = 0x0C, + HPWMI_BACKLIT_KB_BRIGHTNESS = 0x0D, + HPWMI_PEAKSHIFT_PERIOD = 0x0F, + HPWMI_BATTERY_CHARGE_PERIOD = 0x10, }; struct bios_args { @@ -93,6 +81,34 @@ struct bios_args { u32 data; }; +enum hp_wmi_commandtype { + HPWMI_DISPLAY_QUERY = 0x01, + HPWMI_HDDTEMP_QUERY = 0x02, + HPWMI_ALS_QUERY = 0x03, + HPWMI_HARDWARE_QUERY = 0x04, + HPWMI_WIRELESS_QUERY = 0x05, + HPWMI_BATTERY_QUERY = 0x07, + HPWMI_BIOS_QUERY = 0x09, + HPWMI_FEATURE_QUERY = 0x0b, + HPWMI_HOTKEY_QUERY = 0x0c, + HPWMI_FEATURE2_QUERY = 0x0d, + HPWMI_WIRELESS2_QUERY = 0x1b, + HPWMI_POSTCODEERROR_QUERY = 0x2a, +}; + +enum hp_wmi_command { + HPWMI_READ = 0x01, + HPWMI_WRITE = 0x02, + HPWMI_ODM = 0x03, +}; + +#define BIOS_ARGS_INIT(write, ctype, size) \ + (struct bios_args) { .signature = 0x55434553, \ + .command = (write) ? 0x2 : 0x1, \ + .commandtype = (ctype), \ + .datasize = (size), \ + .data = 0 } + struct bios_return { u32 sigpass; u32 return_code; @@ -170,8 +186,8 @@ static struct rfkill2_device rfkill2[HPWMI_MAX_RFKILL2_DEVICES]; /* * hp_wmi_perform_query * - * query: The commandtype -> What should be queried - * write: The command -> 0 read, 1 write, 3 ODM specific + * query: The commandtype (enum hp_wmi_commandtype) + * write: The command (enum hp_wmi_command) * buffer: Buffer used as input and/or output * insize: Size of input buffer * outsize: Size of output buffer @@ -182,20 +198,20 @@ static struct rfkill2_device rfkill2[HPWMI_MAX_RFKILL2_DEVICES]; * -EINVAL if the output buffer size exceeds buffersize * * Note: The buffersize must at least be the maximum of the input and output - * size. E.g. Battery info query (0x7) is defined to have 1 byte input + * size. E.g. Battery info query is defined to have 1 byte input * and 128 byte output. The caller would do: * buffer = kzalloc(128, GFP_KERNEL); - * ret = hp_wmi_perform_query(0x7, 0, buffer, 1, 128) + * ret = hp_wmi_perform_query(HPWMI_BATTERY_QUERY, HPWMI_READ, buffer, 1, 128) */ -static int hp_wmi_perform_query(int query, int write, void *buffer, - int insize, int outsize) +static int hp_wmi_perform_query(int query, enum hp_wmi_command command, + void *buffer, int insize, int outsize) { struct bios_return *bios_return; int actual_outsize; union acpi_object *obj; struct bios_args args = { .signature = 0x55434553, - .command = write ? 0x2 : 0x1, + .command = command, .commandtype = query, .datasize = insize, .data = 0, @@ -245,7 +261,7 @@ static int hp_wmi_perform_query(int query, int write, void *buffer, static int hp_wmi_display_state(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (ret) return ret < 0 ? ret : -EINVAL; @@ -255,7 +271,7 @@ static int hp_wmi_display_state(void) static int hp_wmi_hddtemp_state(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (ret) return ret < 0 ? ret : -EINVAL; @@ -265,7 +281,7 @@ static int hp_wmi_hddtemp_state(void) static int hp_wmi_als_state(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (ret) return ret < 0 ? ret : -EINVAL; @@ -275,7 +291,7 @@ static int hp_wmi_als_state(void) static int hp_wmi_dock_state(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (ret) @@ -287,7 +303,7 @@ static int hp_wmi_dock_state(void) static int hp_wmi_tablet_state(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (ret) return ret < 0 ? ret : -EINVAL; @@ -298,7 +314,7 @@ static int hp_wmi_tablet_state(void) static int __init hp_wmi_bios_2008_later(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_FEATURE_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_FEATURE_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (!ret) return 1; @@ -309,7 +325,7 @@ static int __init hp_wmi_bios_2008_later(void) static int __init hp_wmi_bios_2009_later(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_FEATURE2_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_FEATURE2_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (!ret) return 1; @@ -320,7 +336,7 @@ static int __init hp_wmi_bios_2009_later(void) static int __init hp_wmi_enable_hotkeys(void) { int value = 0x6e; - int ret = hp_wmi_perform_query(HPWMI_BIOS_QUERY, 1, &value, + int ret = hp_wmi_perform_query(HPWMI_BIOS_QUERY, HPWMI_WRITE, &value, sizeof(value), 0); if (ret) return ret < 0 ? ret : -EINVAL; @@ -333,7 +349,7 @@ static int hp_wmi_set_block(void *data, bool blocked) int query = BIT(r + 8) | ((!blocked) << r); int ret; - ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 1, + ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_WRITE, &query, sizeof(query), 0); if (ret) return ret < 0 ? ret : -EINVAL; @@ -349,7 +365,7 @@ static bool hp_wmi_get_sw_state(enum hp_wmi_radio r) int mask = 0x200 << (r * 8); int wireless = 0; - hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, + hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless, sizeof(wireless), sizeof(wireless)); /* TBD: Pass error */ @@ -365,7 +381,7 @@ static bool hp_wmi_get_hw_state(enum hp_wmi_radio r) int mask = 0x800 << (r * 8); int wireless = 0; - hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, + hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless, sizeof(wireless), sizeof(wireless)); /* TBD: Pass error */ @@ -381,7 +397,7 @@ static int hp_wmi_rfkill2_set_block(void *data, bool blocked) int rfkill_id = (int)(long)data; char buffer[4] = { 0x01, 0x00, rfkill_id, !blocked }; - if (hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, 1, + if (hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_WRITE, buffer, sizeof(buffer), 0)) return -EINVAL; return 0; @@ -396,7 +412,7 @@ static int hp_wmi_rfkill2_refresh(void) struct bios_rfkill2_state state; int err, i; - err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, 0, &state, + err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state, 0, sizeof(state)); if (err) return err; @@ -423,7 +439,7 @@ static int hp_wmi_rfkill2_refresh(void) static int hp_wmi_post_code_state(void) { int state = 0; - int ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, 0, &state, + int ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, HPWMI_READ, &state, sizeof(state), sizeof(state)); if (ret) return ret < 0 ? ret : -EINVAL; @@ -489,7 +505,7 @@ static ssize_t set_als(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { u32 tmp = simple_strtoul(buf, NULL, 10); - int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 1, &tmp, + int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_WRITE, &tmp, sizeof(tmp), sizeof(tmp)); if (ret) return ret < 0 ? ret : -EINVAL; @@ -510,7 +526,7 @@ static ssize_t set_postcode(struct device *dev, struct device_attribute *attr, /* Clear the POST error code. It is kept until until cleared. */ tmp = (u32) tmp2; - ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, 1, &tmp, + ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, HPWMI_WRITE, &tmp, sizeof(tmp), sizeof(tmp)); if (ret) return ret < 0 ? ret : -EINVAL; @@ -583,7 +599,7 @@ static void hp_wmi_notify(u32 value, void *context) case HPWMI_SMART_ADAPTER: break; case HPWMI_BEZEL_BUTTON: - ret = hp_wmi_perform_query(HPWMI_HOTKEY_QUERY, 0, + ret = hp_wmi_perform_query(HPWMI_HOTKEY_QUERY, HPWMI_READ, &key_code, sizeof(key_code), sizeof(key_code)); @@ -718,12 +734,12 @@ static int __init hp_wmi_rfkill_setup(struct platform_device *device) { int err, wireless = 0; - err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, &wireless, + err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless, sizeof(wireless), sizeof(wireless)); if (err) return err; - err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 1, &wireless, + err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_WRITE, &wireless, sizeof(wireless), 0); if (err) return err; @@ -803,7 +819,7 @@ static int __init hp_wmi_rfkill2_setup(struct platform_device *device) struct bios_rfkill2_state state; int err, i; - err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, 0, &state, + err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state, 0, sizeof(state)); if (err) return err; -- cgit v1.2.3 From ea621d9fe372b4a6ca2b513c13a37c24a668c35f Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Wed, 19 Apr 2017 16:26:06 -0700 Subject: platform/x86: hp-wmi: Refactor redundant HPWMI_READ functions Several functions perform the same WMI read int with different query arguments. Refactor this into a single hp_wmi_read_int function. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 82 +++++++++++++------------------------------ 1 file changed, 24 insertions(+), 58 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 2073f1e97970..eb6d0a0e1392 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -258,55 +258,35 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command, return 0; } -static int hp_wmi_display_state(void) +static int hp_wmi_read_int(int query) { - int state = 0; - int ret = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, HPWMI_READ, &state, - sizeof(state), sizeof(state)); - if (ret) - return ret < 0 ? ret : -EINVAL; - return state; -} + int val = 0, ret; -static int hp_wmi_hddtemp_state(void) -{ - int state = 0; - int ret = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, HPWMI_READ, &state, - sizeof(state), sizeof(state)); - if (ret) - return ret < 0 ? ret : -EINVAL; - return state; -} + ret = hp_wmi_perform_query(query, HPWMI_READ, &val, + sizeof(val), sizeof(val)); -static int hp_wmi_als_state(void) -{ - int state = 0; - int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_READ, &state, - sizeof(state), sizeof(state)); if (ret) return ret < 0 ? ret : -EINVAL; - return state; + + return val; } static int hp_wmi_dock_state(void) { - int state = 0; - int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, HPWMI_READ, &state, - sizeof(state), sizeof(state)); + int state = hp_wmi_read_int(HPWMI_HARDWARE_QUERY); - if (ret) - return ret < 0 ? ret : -EINVAL; + if (state < 0) + return state; return state & 0x1; } static int hp_wmi_tablet_state(void) { - int state = 0; - int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, HPWMI_READ, &state, - sizeof(state), sizeof(state)); - if (ret) - return ret < 0 ? ret : -EINVAL; + int state = hp_wmi_read_int(HPWMI_HARDWARE_QUERY); + + if (state < 0) + return state; return (state & 0x4) ? 1 : 0; } @@ -436,20 +416,10 @@ static int hp_wmi_rfkill2_refresh(void) return 0; } -static int hp_wmi_post_code_state(void) -{ - int state = 0; - int ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, HPWMI_READ, &state, - sizeof(state), sizeof(state)); - if (ret) - return ret < 0 ? ret : -EINVAL; - return state; -} - static ssize_t show_display(struct device *dev, struct device_attribute *attr, char *buf) { - int value = hp_wmi_display_state(); + int value = hp_wmi_read_int(HPWMI_DISPLAY_QUERY); if (value < 0) return -EINVAL; return sprintf(buf, "%d\n", value); @@ -458,7 +428,7 @@ static ssize_t show_display(struct device *dev, struct device_attribute *attr, static ssize_t show_hddtemp(struct device *dev, struct device_attribute *attr, char *buf) { - int value = hp_wmi_hddtemp_state(); + int value = hp_wmi_read_int(HPWMI_HDDTEMP_QUERY); if (value < 0) return -EINVAL; return sprintf(buf, "%d\n", value); @@ -467,7 +437,7 @@ static ssize_t show_hddtemp(struct device *dev, struct device_attribute *attr, static ssize_t show_als(struct device *dev, struct device_attribute *attr, char *buf) { - int value = hp_wmi_als_state(); + int value = hp_wmi_read_int(HPWMI_ALS_QUERY); if (value < 0) return -EINVAL; return sprintf(buf, "%d\n", value); @@ -495,7 +465,7 @@ static ssize_t show_postcode(struct device *dev, struct device_attribute *attr, char *buf) { /* Get the POST error code of previous boot failure. */ - int value = hp_wmi_post_code_state(); + int value = hp_wmi_read_int(HPWMI_POSTCODEERROR_QUERY); if (value < 0) return -EINVAL; return sprintf(buf, "0x%x\n", value); @@ -546,9 +516,9 @@ static void hp_wmi_notify(u32 value, void *context) struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL }; u32 event_id, event_data; union acpi_object *obj; - int key_code = 0, ret; acpi_status status; u32 *location; + int key_code; status = wmi_get_event_data(value, &response); if (status != AE_OK) { @@ -599,11 +569,8 @@ static void hp_wmi_notify(u32 value, void *context) case HPWMI_SMART_ADAPTER: break; case HPWMI_BEZEL_BUTTON: - ret = hp_wmi_perform_query(HPWMI_HOTKEY_QUERY, HPWMI_READ, - &key_code, - sizeof(key_code), - sizeof(key_code)); - if (ret) + key_code = hp_wmi_read_int(HPWMI_HOTKEY_QUERY); + if (key_code < 0) break; if (!sparse_keymap_report_event(hp_wmi_input_dev, @@ -732,12 +699,11 @@ static void cleanup_sysfs(struct platform_device *device) static int __init hp_wmi_rfkill_setup(struct platform_device *device) { - int err, wireless = 0; + int err, wireless; - err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless, - sizeof(wireless), sizeof(wireless)); - if (err) - return err; + wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY); + if (wireless) + return wireless; err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_WRITE, &wireless, sizeof(wireless), 0); -- cgit v1.2.3 From c7ef144c12033052c956ca9d80b2dfc19c73d939 Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Wed, 19 Apr 2017 18:00:01 -0700 Subject: platform/x86: hp-wmi: Cleanup wireless get_(hw|sw)state functions Use the new hp_wmi_read_int() function and add a WARN_ONCE() to the TBD regarding passing the error through. These are used in a null return function unfortunately. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index eb6d0a0e1392..577805987d35 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -343,33 +343,25 @@ static const struct rfkill_ops hp_wmi_rfkill_ops = { static bool hp_wmi_get_sw_state(enum hp_wmi_radio r) { int mask = 0x200 << (r * 8); - int wireless = 0; - hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, - &wireless, sizeof(wireless), - sizeof(wireless)); + int wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY); + /* TBD: Pass error */ + WARN_ONCE(wireless < 0, "error executing HPWMI_WIRELESS_QUERY"); - if (wireless & mask) - return false; - else - return true; + return !(wireless & mask); } static bool hp_wmi_get_hw_state(enum hp_wmi_radio r) { int mask = 0x800 << (r * 8); - int wireless = 0; - hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, - &wireless, sizeof(wireless), - sizeof(wireless)); + int wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY); + /* TBD: Pass error */ + WARN_ONCE(wireless < 0, "error executing HPWMI_WIRELESS_QUERY"); - if (wireless & mask) - return false; - else - return true; + return !(wireless & mask); } static int hp_wmi_rfkill2_set_block(void *data, bool blocked) -- cgit v1.2.3 From f9cf3b2880cc41ec2b44aa24fa156bffe934f9c7 Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Wed, 19 Apr 2017 17:03:38 -0700 Subject: platform/x86: hp-wmi: Refactor dock and tablet state fetchers Both dock and tablet use the HPWMI_HARDWARE_QUERY, but require different masks. Rather than using two functions with magic masks, define the masks, and use a common accessor. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 577805987d35..9b71829843ad 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -102,6 +102,11 @@ enum hp_wmi_command { HPWMI_ODM = 0x03, }; +enum hp_wmi_hardware_mask { + HPWMI_DOCK_MASK = 0x01, + HPWMI_TABLET_MASK = 0x04, +}; + #define BIOS_ARGS_INIT(write, ctype, size) \ (struct bios_args) { .signature = 0x55434553, \ .command = (write) ? 0x2 : 0x1, \ @@ -271,7 +276,7 @@ static int hp_wmi_read_int(int query) return val; } -static int hp_wmi_dock_state(void) +static int hp_wmi_hw_state(int mask) { int state = hp_wmi_read_int(HPWMI_HARDWARE_QUERY); @@ -281,16 +286,6 @@ static int hp_wmi_dock_state(void) return state & 0x1; } -static int hp_wmi_tablet_state(void) -{ - int state = hp_wmi_read_int(HPWMI_HARDWARE_QUERY); - - if (state < 0) - return state; - - return (state & 0x4) ? 1 : 0; -} - static int __init hp_wmi_bios_2008_later(void) { int state = 0; @@ -438,7 +433,7 @@ static ssize_t show_als(struct device *dev, struct device_attribute *attr, static ssize_t show_dock(struct device *dev, struct device_attribute *attr, char *buf) { - int value = hp_wmi_dock_state(); + int value = hp_wmi_hw_state(HPWMI_DOCK_MASK); if (value < 0) return -EINVAL; return sprintf(buf, "%d\n", value); @@ -447,7 +442,7 @@ static ssize_t show_dock(struct device *dev, struct device_attribute *attr, static ssize_t show_tablet(struct device *dev, struct device_attribute *attr, char *buf) { - int value = hp_wmi_tablet_state(); + int value = hp_wmi_hw_state(HPWMI_TABLET_MASK); if (value < 0) return -EINVAL; return sprintf(buf, "%d\n", value); @@ -550,10 +545,10 @@ static void hp_wmi_notify(u32 value, void *context) case HPWMI_DOCK_EVENT: if (test_bit(SW_DOCK, hp_wmi_input_dev->swbit)) input_report_switch(hp_wmi_input_dev, SW_DOCK, - hp_wmi_dock_state()); + hp_wmi_hw_state(HPWMI_DOCK_MASK)); if (test_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit)) input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, - hp_wmi_tablet_state()); + hp_wmi_hw_state(HPWMI_TABLET_MASK)); input_sync(hp_wmi_input_dev); break; case HPWMI_PARK_HDD: @@ -631,14 +626,14 @@ static int __init hp_wmi_input_setup(void) __set_bit(EV_SW, hp_wmi_input_dev->evbit); /* Dock */ - val = hp_wmi_dock_state(); + val = hp_wmi_hw_state(HPWMI_DOCK_MASK); if (!(val < 0)) { __set_bit(SW_DOCK, hp_wmi_input_dev->swbit); input_report_switch(hp_wmi_input_dev, SW_DOCK, val); } /* Tablet mode */ - val = hp_wmi_tablet_state(); + val = hp_wmi_hw_state(HPWMI_TABLET_MASK); if (!(val < 0)) { __set_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit); input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, val); @@ -932,10 +927,10 @@ static int hp_wmi_resume_handler(struct device *device) if (hp_wmi_input_dev) { if (test_bit(SW_DOCK, hp_wmi_input_dev->swbit)) input_report_switch(hp_wmi_input_dev, SW_DOCK, - hp_wmi_dock_state()); + hp_wmi_hw_state(HPWMI_DOCK_MASK)); if (test_bit(SW_TABLET_MODE, hp_wmi_input_dev->swbit)) input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE, - hp_wmi_tablet_state()); + hp_wmi_hw_state(HPWMI_TABLET_MASK)); input_sync(hp_wmi_input_dev); } -- cgit v1.2.3 From c21ee12f460c33fcc12fe162c9ab98f3c58073aa Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Wed, 19 Apr 2017 17:26:38 -0700 Subject: platform/x86: hp-wmi: Use DEVICE_ATTR_(RO|RW) helper macros Use the DEVICE_ATTR_(RO|RW) macros, ranaming the show and store functions accordingly. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 9b71829843ad..d1e3af23a147 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -403,7 +403,7 @@ static int hp_wmi_rfkill2_refresh(void) return 0; } -static ssize_t show_display(struct device *dev, struct device_attribute *attr, +static ssize_t display_show(struct device *dev, struct device_attribute *attr, char *buf) { int value = hp_wmi_read_int(HPWMI_DISPLAY_QUERY); @@ -412,7 +412,7 @@ static ssize_t show_display(struct device *dev, struct device_attribute *attr, return sprintf(buf, "%d\n", value); } -static ssize_t show_hddtemp(struct device *dev, struct device_attribute *attr, +static ssize_t hddtemp_show(struct device *dev, struct device_attribute *attr, char *buf) { int value = hp_wmi_read_int(HPWMI_HDDTEMP_QUERY); @@ -421,7 +421,7 @@ static ssize_t show_hddtemp(struct device *dev, struct device_attribute *attr, return sprintf(buf, "%d\n", value); } -static ssize_t show_als(struct device *dev, struct device_attribute *attr, +static ssize_t als_show(struct device *dev, struct device_attribute *attr, char *buf) { int value = hp_wmi_read_int(HPWMI_ALS_QUERY); @@ -430,7 +430,7 @@ static ssize_t show_als(struct device *dev, struct device_attribute *attr, return sprintf(buf, "%d\n", value); } -static ssize_t show_dock(struct device *dev, struct device_attribute *attr, +static ssize_t dock_show(struct device *dev, struct device_attribute *attr, char *buf) { int value = hp_wmi_hw_state(HPWMI_DOCK_MASK); @@ -439,8 +439,8 @@ static ssize_t show_dock(struct device *dev, struct device_attribute *attr, return sprintf(buf, "%d\n", value); } -static ssize_t show_tablet(struct device *dev, struct device_attribute *attr, - char *buf) +static ssize_t tablet_show(struct device *dev, struct device_attribute *attr, + char *buf) { int value = hp_wmi_hw_state(HPWMI_TABLET_MASK); if (value < 0) @@ -448,8 +448,8 @@ static ssize_t show_tablet(struct device *dev, struct device_attribute *attr, return sprintf(buf, "%d\n", value); } -static ssize_t show_postcode(struct device *dev, struct device_attribute *attr, - char *buf) +static ssize_t postcode_show(struct device *dev, struct device_attribute *attr, + char *buf) { /* Get the POST error code of previous boot failure. */ int value = hp_wmi_read_int(HPWMI_POSTCODEERROR_QUERY); @@ -458,8 +458,8 @@ static ssize_t show_postcode(struct device *dev, struct device_attribute *attr, return sprintf(buf, "0x%x\n", value); } -static ssize_t set_als(struct device *dev, struct device_attribute *attr, - const char *buf, size_t count) +static ssize_t als_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) { u32 tmp = simple_strtoul(buf, NULL, 10); int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_WRITE, &tmp, @@ -470,8 +470,8 @@ static ssize_t set_als(struct device *dev, struct device_attribute *attr, return count; } -static ssize_t set_postcode(struct device *dev, struct device_attribute *attr, - const char *buf, size_t count) +static ssize_t postcode_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) { long unsigned int tmp2; int ret; @@ -491,12 +491,12 @@ static ssize_t set_postcode(struct device *dev, struct device_attribute *attr, return count; } -static DEVICE_ATTR(display, S_IRUGO, show_display, NULL); -static DEVICE_ATTR(hddtemp, S_IRUGO, show_hddtemp, NULL); -static DEVICE_ATTR(als, S_IRUGO | S_IWUSR, show_als, set_als); -static DEVICE_ATTR(dock, S_IRUGO, show_dock, NULL); -static DEVICE_ATTR(tablet, S_IRUGO, show_tablet, NULL); -static DEVICE_ATTR(postcode, S_IRUGO | S_IWUSR, show_postcode, set_postcode); +static DEVICE_ATTR_RO(display); +static DEVICE_ATTR_RO(hddtemp); +static DEVICE_ATTR_RW(als); +static DEVICE_ATTR_RO(dock); +static DEVICE_ATTR_RO(tablet); +static DEVICE_ATTR_RW(postcode); static void hp_wmi_notify(u32 value, void *context) { -- cgit v1.2.3 From a055f9ecb5b4d0cb8b33ba664363a2ed7804a6c7 Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Wed, 19 Apr 2017 18:11:33 -0700 Subject: platform/x86: hp-wmi: Do not shadow errors in sysfs show functions The new hp_wmi_read_int function returns a negative value in case of error, pass this on directly rather than always replacing it with -EINVAL. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index d1e3af23a147..e685018670c0 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -408,7 +408,7 @@ static ssize_t display_show(struct device *dev, struct device_attribute *attr, { int value = hp_wmi_read_int(HPWMI_DISPLAY_QUERY); if (value < 0) - return -EINVAL; + return value; return sprintf(buf, "%d\n", value); } @@ -417,7 +417,7 @@ static ssize_t hddtemp_show(struct device *dev, struct device_attribute *attr, { int value = hp_wmi_read_int(HPWMI_HDDTEMP_QUERY); if (value < 0) - return -EINVAL; + return value; return sprintf(buf, "%d\n", value); } @@ -426,7 +426,7 @@ static ssize_t als_show(struct device *dev, struct device_attribute *attr, { int value = hp_wmi_read_int(HPWMI_ALS_QUERY); if (value < 0) - return -EINVAL; + return value; return sprintf(buf, "%d\n", value); } @@ -435,7 +435,7 @@ static ssize_t dock_show(struct device *dev, struct device_attribute *attr, { int value = hp_wmi_hw_state(HPWMI_DOCK_MASK); if (value < 0) - return -EINVAL; + return value; return sprintf(buf, "%d\n", value); } @@ -444,7 +444,7 @@ static ssize_t tablet_show(struct device *dev, struct device_attribute *attr, { int value = hp_wmi_hw_state(HPWMI_TABLET_MASK); if (value < 0) - return -EINVAL; + return value; return sprintf(buf, "%d\n", value); } @@ -454,7 +454,7 @@ static ssize_t postcode_show(struct device *dev, struct device_attribute *attr, /* Get the POST error code of previous boot failure. */ int value = hp_wmi_read_int(HPWMI_POSTCODEERROR_QUERY); if (value < 0) - return -EINVAL; + return value; return sprintf(buf, "0x%x\n", value); } -- cgit v1.2.3 From 527376c89caf59537d8e5c9f14d7caa940bd563b Mon Sep 17 00:00:00 2001 From: "Darren Hart (VMware)" Date: Wed, 19 Apr 2017 18:54:45 -0700 Subject: platform/x86: hp-wmi: Cleanup exit paths Several exit paths were more complex than they needed to be. Remove superfluous conditionals, use labels common cleanup, do not shadow negative error codes. Signed-off-by: Darren Hart (VMware) Tested-by: Carlo Caione Reviewed-by: Andy Shevchenko --- drivers/platform/x86/hp-wmi.c | 63 ++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 30 deletions(-) (limited to 'drivers/platform/x86/hp-wmi.c') diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index e685018670c0..0df4209648d1 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -223,7 +223,7 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command, }; struct acpi_buffer input = { sizeof(struct bios_args), &args }; struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; - u32 rc; + int ret = 0; if (WARN_ON(insize > sizeof(args.data))) return -EINVAL; @@ -235,32 +235,32 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command, if (!obj) return -EINVAL; - else if (obj->type != ACPI_TYPE_BUFFER) { - kfree(obj); - return -EINVAL; + + if (obj->type != ACPI_TYPE_BUFFER) { + ret = -EINVAL; + goto out_free; } bios_return = (struct bios_return *)obj->buffer.pointer; - rc = bios_return->return_code; + ret = bios_return->return_code; - if (rc) { - if (rc != HPWMI_RET_UNKNOWN_CMDTYPE) - pr_warn("query 0x%x returned error 0x%x\n", query, rc); - kfree(obj); - return rc; + if (ret) { + if (ret != HPWMI_RET_UNKNOWN_CMDTYPE) + pr_warn("query 0x%x returned error 0x%x\n", query, ret); + goto out_free; } - if (!outsize) { - /* ignore output data */ - kfree(obj); - return 0; - } + /* Ignore output data of zero size */ + if (!outsize) + goto out_free; actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return))); memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize); memset(buffer + actual_outsize, 0, outsize - actual_outsize); + +out_free: kfree(obj); - return 0; + return ret; } static int hp_wmi_read_int(int query) @@ -313,9 +313,8 @@ static int __init hp_wmi_enable_hotkeys(void) int value = 0x6e; int ret = hp_wmi_perform_query(HPWMI_BIOS_QUERY, HPWMI_WRITE, &value, sizeof(value), 0); - if (ret) - return ret < 0 ? ret : -EINVAL; - return 0; + + return ret <= 0 ? ret : -EINVAL; } static int hp_wmi_set_block(void *data, bool blocked) @@ -326,9 +325,8 @@ static int hp_wmi_set_block(void *data, bool blocked) ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_WRITE, &query, sizeof(query), 0); - if (ret) - return ret < 0 ? ret : -EINVAL; - return 0; + + return ret <= 0 ? ret : -EINVAL; } static const struct rfkill_ops hp_wmi_rfkill_ops = { @@ -363,11 +361,12 @@ static int hp_wmi_rfkill2_set_block(void *data, bool blocked) { int rfkill_id = (int)(long)data; char buffer[4] = { 0x01, 0x00, rfkill_id, !blocked }; + int ret; - if (hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_WRITE, - buffer, sizeof(buffer), 0)) - return -EINVAL; - return 0; + ret = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_WRITE, + buffer, sizeof(buffer), 0); + + return ret <= 0 ? ret : -EINVAL; } static const struct rfkill_ops hp_wmi_rfkill2_ops = { @@ -478,13 +477,17 @@ static ssize_t postcode_store(struct device *dev, struct device_attribute *attr, u32 tmp; ret = kstrtoul(buf, 10, &tmp2); - if (ret || tmp2 != 1) - return -EINVAL; + if (!ret && tmp2 != 1) + ret = -EINVAL; + if (ret) + goto out; /* Clear the POST error code. It is kept until until cleared. */ tmp = (u32) tmp2; ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, HPWMI_WRITE, &tmp, sizeof(tmp), sizeof(tmp)); + +out: if (ret) return ret < 0 ? ret : -EINVAL; @@ -689,7 +692,7 @@ static int __init hp_wmi_rfkill_setup(struct platform_device *device) int err, wireless; wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY); - if (wireless) + if (wireless < 0) return wireless; err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_WRITE, &wireless, @@ -775,7 +778,7 @@ static int __init hp_wmi_rfkill2_setup(struct platform_device *device) err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state, 0, sizeof(state)); if (err) - return err; + return err < 0 ? err : -EINVAL; if (state.count > HPWMI_MAX_RFKILL2_DEVICES) { pr_warn("unable to parse 0x1b query output\n"); -- cgit v1.2.3