hp-wmi: several fixes and cleanups

hp-wmi: several fixes and cleanups

[Thomas Renninger]

Which I introduced with my last patches...
Beside the minor include cleanup.
Thus this is only needed for current platformdrivers linux-next branch.

Thanks,

    Thomas

[PATCH 1/3] hp-wmi: Fix input buffer size of perform_query

[Thomas Renninger]

The input query object has a 16 byte fixed header (signature, command,
command type, datasize) and an additional variable size of 0, 4, 128,... bytes.

This patch fixes the static wrong size (char*) and allocates the correct memory
size dynamically.
This should not have resulted in an oops, because ACPI will detect that the
buffer is too small when the query gets executed on queries needing more than
4 bytes as input buffer. Thus queries needing 128 byte size or more result in
a "exceeding buffer" acpi error or similar when the query gets executed.
This is the fix for that.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: linux-acpi@vger.kernel.or
CC: platform-driver-x86@vger.kernel.org
CC: mjg@redhat.com
---
 drivers/platform/x86/hp-wmi.c |   40 +++++++++++++++++++++++-----------------
 1 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 479ec3b..4c30dd3 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -75,14 +75,6 @@ static int __devinit hp_wmi_bios_setup(struct platform_device *device);
 static int __exit hp_wmi_bios_remove(struct platform_device *device);
 static int hp_wmi_resume_handler(struct device *device);
 
-struct bios_args {
-	u32 signature;
-	u32 command;
-	u32 commandtype;
-	u32 datasize;
-	char *data;
-};
-
 struct bios_return {
 	u32 sigpass;
 	u32 return_code;
@@ -155,17 +147,28 @@ static int hp_wmi_perform_query(int query, int write, char *buffer,
 	struct bios_return bios_return;
 	acpi_status status;
 	union acpi_object *obj;
-	struct bios_args args = {
-		.signature = 0x55434553,
-		.command = write ? 0x2 : 0x1,
-		.commandtype = query,
-		.datasize = buffersize,
-		.data = buffer,
-	};
-	struct acpi_buffer input = { sizeof(struct bios_args), &args };
+	struct acpi_buffer input;
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
 	int method_id;
 
+	u32 *command_buffer = kmalloc(buffersize + 16, GFP_KERNEL);
+	if (!command_buffer)
+		return -ENOMEM;
+
+	/* Signature */
+	*command_buffer = 0x55434553;
+	/* Command */
+	*(command_buffer + 1) = write ? 0x2 : 0x1;
+	/* Command Type */
+	*(command_buffer + 2) = query;
+	/* Datasize */
+	*(command_buffer + 3) = buffersize;
+	/* Data */
+	memcpy(command_buffer + 4, buffer, buffersize);
+
+	input.length = buffersize  + 16;
+	input.pointer = command_buffer;
+
 	/*
 	 * Each query method has a return buffer size defined. Could get
 	 * double checked against a static list of query methods and return
@@ -190,13 +193,16 @@ static int hp_wmi_perform_query(int query, int write, char *buffer,
 		method_id = 0x5;
 		break;
 	default:
-		pr_err(PREFIX "Invalid query buffer size %d\n", buffersize);
+		pr_err(PREFIX "Invalid query 0x%x with buffer size %d\n",
+		       query, buffersize);
+		kfree(command_buffer);
 		return -EINVAL;
 	};
 
 	status = wmi_evaluate_method(HPWMI_BIOS_GUID, 0,
 				     method_id, &input, &output);
 
+	kfree(command_buffer);
 	obj = output.pointer;
 
 	if (!obj)
-- 
1.6.3

[PATCH 2/3] hp-wmi: Fix mixing up of and/or directive

[Thomas Renninger]

This should have been an "and". Additionally checking for !obj
is even better.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: linux-acpi@vger.kernel.or
CC: platform-driver-x86@vger.kernel.org
CC: mjg@redhat.com
---
 drivers/platform/x86/hp-wmi.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 4c30dd3..f324c55 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -470,7 +470,9 @@ static void hp_wmi_notify(u32 value, void *context)
 
 	obj = (union acpi_object *)response.pointer;
 
-	if (obj || obj->type != ACPI_TYPE_BUFFER) {
+	if (!obj)
+		return;
+	if (obj->type != ACPI_TYPE_BUFFER) {
 		printk(KERN_INFO "hp-wmi: Unknown response received %d\n",
 		       obj->type);
 		kfree(obj);
-- 
1.6.3

[PATCH 3/3] hp-wmi: acpi_drivers.h is already included through acpi.h two lines below

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: linux-acpi@vger.kernel.or
CC: platform-driver-x86@vger.kernel.org
CC: mjg@redhat.com
---
 drivers/platform/x86/hp-wmi.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index f324c55..db792d2 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -29,7 +29,6 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/input.h>
-#include <acpi/acpi_drivers.h>
 #include <linux/platform_device.h>
 #include <linux/acpi.h>
 #include <linux/rfkill.h>
-- 
1.6.3

Re: [PATCH 1/3] hp-wmi: Fix input buffer size of perform_query

[Matthew Garrett]

Hi Thomas,

This one seems to depend on a patch I don't have. Can you recheck? I've 
applied the others.

Thanks,
-- 
Matthew Garrett | mjg59@srcf.ucam.org