* [PATCH 1/9] platform/x86: hp-wmi: Cleanup local variable declarations
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 2:25 ` [PATCH 2/9] platform/x86: hp-wmi: Add bios_args initializer Darren Hart
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
Declare like types on one line. Order declarations in decreasing length
where possible.
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 5ca9328..e772105 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 @@ static int __init hp_wmi_rfkill_setup(struct platform_device *device)
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;
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/9] platform/x86: hp-wmi: Add bios_args initializer
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
2017-04-20 2:25 ` [PATCH 1/9] platform/x86: hp-wmi: Cleanup local variable declarations Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 7:37 ` Andy Shevchenko
2017-04-20 2:25 ` [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants Darren Hart
` (8 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
Cleanup the hp_wmi_perform_query function some by providing a bios_args
initializer. No functional changes.
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index e772105..aa9d99c 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -93,6 +93,13 @@ struct bios_args {
u32 data;
};
+#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;
@@ -190,18 +197,12 @@ static struct rfkill2_device rfkill2[HPWMI_MAX_RFKILL2_DEVICES];
static int hp_wmi_perform_query(int query, int write, 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,
- .commandtype = query,
- .datasize = insize,
- .data = 0,
- };
+ struct bios_args args = BIOS_ARGS_INIT(write, query, insize);
struct acpi_buffer input = { sizeof(struct bios_args), &args };
struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+ struct bios_return *bios_return;
+ union acpi_object *obj;
+ int actual_outsize;
u32 rc;
if (WARN_ON(insize > sizeof(args.data)))
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/9] platform/x86: hp-wmi: Add bios_args initializer
2017-04-20 2:25 ` [PATCH 2/9] platform/x86: hp-wmi: Add bios_args initializer Darren Hart
@ 2017-04-20 7:37 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2017-04-20 7:37 UTC (permalink / raw)
To: Darren Hart; +Cc: Andy Shevchenko, Platform Driver, linux-kernel, Carlo Caione
On Thu, Apr 20, 2017 at 5:25 AM, Darren Hart <dvhart@infradead.org> wrote:
> From: "Darren Hart (VMware)" <dvhart@infradead.org>
>
> Cleanup the hp_wmi_perform_query function some by providing a bios_args
> initializer. No functional changes.
Is it going to be used only once?
If so, I would go with current code.
>
> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> ---
> drivers/platform/x86/hp-wmi.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index e772105..aa9d99c 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
> @@ -93,6 +93,13 @@ struct bios_args {
> u32 data;
> };
>
> +#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;
> @@ -190,18 +197,12 @@ static struct rfkill2_device rfkill2[HPWMI_MAX_RFKILL2_DEVICES];
> static int hp_wmi_perform_query(int query, int write, 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,
> - .commandtype = query,
> - .datasize = insize,
> - .data = 0,
> - };
> + struct bios_args args = BIOS_ARGS_INIT(write, query, insize);
> struct acpi_buffer input = { sizeof(struct bios_args), &args };
> struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> + struct bios_return *bios_return;
> + union acpi_object *obj;
> + int actual_outsize;
> u32 rc;
>
> if (WARN_ON(insize > sizeof(args.data)))
> --
> 2.9.3
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
2017-04-20 2:25 ` [PATCH 1/9] platform/x86: hp-wmi: Cleanup local variable declarations Darren Hart
2017-04-20 2:25 ` [PATCH 2/9] platform/x86: hp-wmi: Add bios_args initializer Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 7:19 ` Andy Shevchenko
2017-04-20 20:31 ` Darren Hart
2017-04-20 2:25 ` [PATCH 4/9] platform/x86: hp-wmi: Refactor redundant HPWMI_READ functions Darren Hart
` (7 subsequent siblings)
10 siblings, 2 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
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.
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 119 +++++++++++++++++++++++-------------------
1 file changed, 64 insertions(+), 55 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index aa9d99c..60d1e4c 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,27 @@ 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 = 0x00,
+ HPWMI_WRITE = 0x01,
+ HPWMI_ODM = 0x03,
+};
+
#define BIOS_ARGS_INIT(write, ctype, size) \
(struct bios_args) { .signature = 0x55434553, \
.command = (write) ? 0x2 : 0x1, \
@@ -177,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
@@ -189,10 +198,10 @@ 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)
@@ -246,7 +255,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;
@@ -256,7 +265,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;
@@ -266,7 +275,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;
@@ -276,7 +285,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)
@@ -288,7 +297,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;
@@ -299,7 +308,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;
@@ -310,7 +319,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;
@@ -321,7 +330,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;
@@ -334,7 +343,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;
@@ -350,7 +359,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 */
@@ -366,7 +375,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 */
@@ -382,7 +391,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;
@@ -397,7 +406,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;
@@ -424,7 +433,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;
@@ -490,7 +499,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;
@@ -511,7 +520,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;
@@ -584,7 +593,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));
@@ -719,12 +728,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;
@@ -804,7 +813,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;
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants
2017-04-20 2:25 ` [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants Darren Hart
@ 2017-04-20 7:19 ` Andy Shevchenko
2017-04-20 20:31 ` Darren Hart
1 sibling, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2017-04-20 7:19 UTC (permalink / raw)
To: Darren Hart; +Cc: Andy Shevchenko, Platform Driver, linux-kernel, carlo
On Thu, Apr 20, 2017 at 5:25 AM, Darren Hart <dvhart@infradead.org> wrote:
> From: "Darren Hart (VMware)" <dvhart@infradead.org>
>
> 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.
>
Looks good to me:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> ---
> drivers/platform/x86/hp-wmi.c | 119 +++++++++++++++++++++++-------------------
> 1 file changed, 64 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index aa9d99c..60d1e4c 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,27 @@ 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 = 0x00,
> + HPWMI_WRITE = 0x01,
> + HPWMI_ODM = 0x03,
> +};
> +
> #define BIOS_ARGS_INIT(write, ctype, size) \
> (struct bios_args) { .signature = 0x55434553, \
> .command = (write) ? 0x2 : 0x1, \
> @@ -177,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
> @@ -189,10 +198,10 @@ 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)
> @@ -246,7 +255,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;
> @@ -256,7 +265,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;
> @@ -266,7 +275,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;
> @@ -276,7 +285,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)
> @@ -288,7 +297,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;
> @@ -299,7 +308,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;
> @@ -310,7 +319,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;
> @@ -321,7 +330,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;
> @@ -334,7 +343,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;
> @@ -350,7 +359,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 */
> @@ -366,7 +375,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 */
> @@ -382,7 +391,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;
> @@ -397,7 +406,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;
> @@ -424,7 +433,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;
> @@ -490,7 +499,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;
> @@ -511,7 +520,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;
> @@ -584,7 +593,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));
> @@ -719,12 +728,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;
> @@ -804,7 +813,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;
> --
> 2.9.3
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants
2017-04-20 2:25 ` [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants Darren Hart
2017-04-20 7:19 ` Andy Shevchenko
@ 2017-04-20 20:31 ` Darren Hart
1 sibling, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 20:31 UTC (permalink / raw)
To: andy, platform-driver-x86, linux-kernel, carlo
On Wed, Apr 19, 2017 at 07:25:15PM -0700, Darren Hart wrote:
> From: "Darren Hart (VMware)" <dvhart@infradead.org>
>
> 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.
>
> Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
> ---
> drivers/platform/x86/hp-wmi.c | 119 +++++++++++++++++++++++-------------------
> 1 file changed, 64 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index aa9d99c..60d1e4c 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
...
> +enum hp_wmi_command {
> + HPWMI_READ = 0x00,
> + HPWMI_WRITE = 0x01,
> + HPWMI_ODM = 0x03,
> +};
> +
> #define BIOS_ARGS_INIT(write, ctype, size) \
> (struct bios_args) { .signature = 0x55434553, \
> .command = (write) ? 0x2 : 0x1, \
> @@ -177,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)
Eeek, I failed to use the enum directly and was still using the ternary operator
in the BIOS_ARGS_INIT above. Luckily, with read as 0 and write as non-zero in
the new ENUMs, and no usage of ODM, things happened to just work. Updated patch
below.
I've now:
* dropped the BIOS_ARGS_INIT per Andy's recommendation.
* Renumbered the hp_wmi_command to match the ternary assignment:
HPWMI_READ = 0x01,
HPWMI_WRITE = 0x02,
* Assigned bios_args.command with one of the enums directly
* replaced the "int write" argument with "enum hp_wmi_command command"
As follows:
Queued to testing, please let me know if you have any concerns.
>From d8193cff33906e24ac1830ecb2ca95833d058a3a Mon Sep 17 00:00:00 2001
Message-Id: <d8193cff33906e24ac1830ecb2ca95833d058a3a.1492719993.git.dvhart@infradead.org>
From: "Darren Hart (VMware)" <dvhart@infradead.org>
Date: Wed, 19 Apr 2017 15:42:01 -0700
Subject: [PATCH] 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) <dvhart@infradead.org>
Tested-by: Carlo Caione <carlo@endlessm.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
drivers/platform/x86/hp-wmi.c | 132 +++++++++++++++++++++++-------------------
1 file changed, 74 insertions(+), 58 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index e772105..2073f1e 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;
--
2.9.3
--
Darren Hart
VMware Open Source Technology Center
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/9] platform/x86: hp-wmi: Refactor redundant HPWMI_READ functions
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (2 preceding siblings ...)
2017-04-20 2:25 ` [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 2:25 ` [PATCH 5/9] platform/x86: hp-wmi: Cleanup wireless get_(hw|sw)state functions Darren Hart
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
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) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 82 +++++++++++++------------------------------
1 file changed, 24 insertions(+), 58 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 60d1e4c..758c229 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -252,55 +252,35 @@ static int hp_wmi_perform_query(int query, int write, void *buffer,
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;
}
@@ -430,20 +410,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);
@@ -452,7 +422,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);
@@ -461,7 +431,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);
@@ -489,7 +459,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);
@@ -540,9 +510,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) {
@@ -593,11 +563,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,
@@ -726,12 +693,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);
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/9] platform/x86: hp-wmi: Cleanup wireless get_(hw|sw)state functions
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (3 preceding siblings ...)
2017-04-20 2:25 ` [PATCH 4/9] platform/x86: hp-wmi: Refactor redundant HPWMI_READ functions Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 2:25 ` [PATCH 6/9] platform/x86: hp-wmi: Refactor dock and tablet state fetchers Darren Hart
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
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) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 758c229..e46b61c 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -337,33 +337,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)
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/9] platform/x86: hp-wmi: Refactor dock and tablet state fetchers
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (4 preceding siblings ...)
2017-04-20 2:25 ` [PATCH 5/9] platform/x86: hp-wmi: Cleanup wireless get_(hw|sw)state functions Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 2:25 ` [PATCH 7/9] platform/x86: hp-wmi: Use DEVICE_ATTR_(RO|RW) helper macros Darren Hart
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
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) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index e46b61c..89d6278 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, \
@@ -265,7 +270,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);
@@ -275,16 +280,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;
@@ -432,7 +427,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);
@@ -441,7 +436,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);
@@ -544,10 +539,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:
@@ -625,14 +620,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);
@@ -926,10 +921,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);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 7/9] platform/x86: hp-wmi: Use DEVICE_ATTR_(RO|RW) helper macros
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (5 preceding siblings ...)
2017-04-20 2:25 ` [PATCH 6/9] platform/x86: hp-wmi: Refactor dock and tablet state fetchers Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 2:25 ` [PATCH 8/9] platform/x86: hp-wmi: Do not shadow errors in sysfs show functions Darren Hart
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
Use the DEVICE_ATTR_(RO|RW) macros, ranaming the show and store
functions accordingly.
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 89d6278..ccacd1a 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -397,7 +397,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);
@@ -406,7 +406,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);
@@ -415,7 +415,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);
@@ -424,7 +424,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);
@@ -433,8 +433,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)
@@ -442,8 +442,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);
@@ -452,8 +452,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,
@@ -464,8 +464,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;
@@ -485,12 +485,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)
{
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 8/9] platform/x86: hp-wmi: Do not shadow errors in sysfs show functions
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (6 preceding siblings ...)
2017-04-20 2:25 ` [PATCH 7/9] platform/x86: hp-wmi: Use DEVICE_ATTR_(RO|RW) helper macros Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 2:25 ` [PATCH 9/9] platform/x86: hp-wmi: Cleanup exit paths Darren Hart
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
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) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index ccacd1a..90b8652 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -402,7 +402,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);
}
@@ -411,7 +411,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);
}
@@ -420,7 +420,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);
}
@@ -429,7 +429,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);
}
@@ -438,7 +438,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);
}
@@ -448,7 +448,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);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 9/9] platform/x86: hp-wmi: Cleanup exit paths
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (7 preceding siblings ...)
2017-04-20 2:25 ` [PATCH 8/9] platform/x86: hp-wmi: Do not shadow errors in sysfs show functions Darren Hart
@ 2017-04-20 2:25 ` Darren Hart
2017-04-20 7:38 ` [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Andy Shevchenko
2017-04-20 9:06 ` Carlo Caione
10 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 2:25 UTC (permalink / raw)
To: dvhart; +Cc: andy, platform-driver-x86, linux-kernel, carlo
From: "Darren Hart (VMware)" <dvhart@infradead.org>
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) <dvhart@infradead.org>
---
drivers/platform/x86/hp-wmi.c | 63 ++++++++++++++++++++++---------------------
1 file changed, 33 insertions(+), 30 deletions(-)
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 90b8652..effb5d5 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -217,7 +217,7 @@ static int hp_wmi_perform_query(int query, int write, void *buffer,
struct bios_return *bios_return;
union acpi_object *obj;
int actual_outsize;
- u32 rc;
+ int ret = 0;
if (WARN_ON(insize > sizeof(args.data)))
return -EINVAL;
@@ -229,32 +229,32 @@ static int hp_wmi_perform_query(int query, int write, void *buffer,
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)
@@ -307,9 +307,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)
@@ -320,9 +319,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 = {
@@ -357,11 +355,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 = {
@@ -472,13 +471,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;
@@ -683,7 +686,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,
@@ -769,7 +772,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");
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (8 preceding siblings ...)
2017-04-20 2:25 ` [PATCH 9/9] platform/x86: hp-wmi: Cleanup exit paths Darren Hart
@ 2017-04-20 7:38 ` Andy Shevchenko
2017-04-20 20:19 ` Darren Hart
2017-04-20 9:06 ` Carlo Caione
10 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2017-04-20 7:38 UTC (permalink / raw)
To: Darren Hart; +Cc: Andy Shevchenko, Platform Driver, linux-kernel, Carlo Caione
On Thu, Apr 20, 2017 at 5:25 AM, Darren Hart <dvhart@infradead.org> wrote:
> From: "Darren Hart (VMware)" <dvhart@infradead.org>
>
> This series factors out some redundant code, cleans up a number of style issues,
> modernizes the sysfs usage, and cleans up the return paths. All told, the driver
> is reduced in size by 37 lines (3.6%).
>
> I do not have an HP laptop, so I'm hoping Carlo can help out with some testing.
> In particular we need to verify that hotkeys and sysfs continue to work as
> before.
>
Series looks good to me except patch 2. So,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
with above exception.
> This series is also available here for convenience:
> git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git hp-wmi
>
>
> Darren Hart (VMware) (9):
> platform/x86: hp-wmi: Cleanup local variable declarations
> platform/x86: hp-wmi: Add bios_args initializer
> platform/x86: hp-wmi: Standardize enum usage for constants
> platform/x86: hp-wmi: Refactor redundant HPWMI_READ functions
> platform/x86: hp-wmi: Cleanup wireless get_(hw|sw)state functions
> platform/x86: hp-wmi: Refactor dock and tablet state fetchers
> platform/x86: hp-wmi: Use DEVICE_ATTR_(RO|RW) helper macros
> platform/x86: hp-wmi: Do not shadow errors in sysfs show functions
> platform/x86: hp-wmi: Cleanup exit paths
>
> drivers/platform/x86/hp-wmi.c | 385 +++++++++++++++++++-----------------------
> 1 file changed, 174 insertions(+), 211 deletions(-)
>
> --
> 2.9.3
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups
2017-04-20 7:38 ` [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Andy Shevchenko
@ 2017-04-20 20:19 ` Darren Hart
0 siblings, 0 replies; 16+ messages in thread
From: Darren Hart @ 2017-04-20 20:19 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Platform Driver, linux-kernel, Carlo Caione
On Thu, Apr 20, 2017 at 10:38:56AM +0300, Andy Shevchenko wrote:
> On Thu, Apr 20, 2017 at 5:25 AM, Darren Hart <dvhart@infradead.org> wrote:
> > From: "Darren Hart (VMware)" <dvhart@infradead.org>
> >
> > This series factors out some redundant code, cleans up a number of style issues,
> > modernizes the sysfs usage, and cleans up the return paths. All told, the driver
> > is reduced in size by 37 lines (3.6%).
> >
> > I do not have an HP laptop, so I'm hoping Carlo can help out with some testing.
> > In particular we need to verify that hotkeys and sysfs continue to work as
> > before.
> >
>
> Series looks good to me except patch 2. So,
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> with above exception.
I was on the fence with this one, which is why I separated it out. I'll drop it.
Thanks for the review.
--
Darren Hart
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups
2017-04-20 2:25 [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Darren Hart
` (9 preceding siblings ...)
2017-04-20 7:38 ` [PATCH v1 0/9] platform/x86: hp-wmi: Driver refactoring and cleanups Andy Shevchenko
@ 2017-04-20 9:06 ` Carlo Caione
10 siblings, 0 replies; 16+ messages in thread
From: Carlo Caione @ 2017-04-20 9:06 UTC (permalink / raw)
To: Darren Hart; +Cc: andy, platform-driver-x86, open list, Linux Upstreaming Team
On Thu, Apr 20, 2017 at 4:25 AM, Darren Hart <dvhart@infradead.org> wrote:
>
> From: "Darren Hart (VMware)" <dvhart@infradead.org>
>
> This series factors out some redundant code, cleans up a number of style issues,
> modernizes the sysfs usage, and cleans up the return paths. All told, the driver
> is reduced in size by 37 lines (3.6%).
>
> I do not have an HP laptop, so I'm hoping Carlo can help out with some testing.
> In particular we need to verify that hotkeys and sysfs continue to work as
> before.
On my HP 240 G5:
Tested-by: Carlo Caione <carlo@endlessm.com>
Cheers,
--
Carlo Caione
^ permalink raw reply [flat|nested] 16+ messages in thread