[RFC] Simple but effective way of changing HID device descriptors (resent)

[RFC] Simple but effective way of changing HID device descriptors (resent)

[Marcin Tolysz]

[-- Attachment #1: Type: text/plain, Size: 2559 bytes --]

Simple but effective way of changing HID device descriptors using 
firmware loading mechanism.
Just place your new descriptor in a location specified by driver
i.e. somewhere in /lib/firmware/hid/
If it is there it will replace HID descriptor from the device.
I do not do any parsing or checking, as it is done by hid driver.

Signed-off-by: Marcin Tolysz <tolysz@gmail.com>

---
  drivers/hid/hid-core.c |   26 +++++++++++++++++++++++++-
  1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 80792d3..5d8d656 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -27,6 +27,7 @@
  #include <linux/wait.h>
  #include <linux/vmalloc.h>
  #include <linux/sched.h>
+#include <linux/firmware.h>

  #include <linux/hid.h>
  #include <linux/hiddev.h>
@@ -640,6 +641,10 @@ int hid_parse_report(struct hid_device *device, 
__u8 *start,
  	struct hid_item item;
  	__u8 *end;
  	int ret;
+	const struct firmware *fw;
+	int succ;
+	char *file;
+
  	static int (*dispatch_type[])(struct hid_parser *parser,
  				      struct hid_item *item) = {
  		hid_parser_main,
@@ -650,10 +655,27 @@ int hid_parse_report(struct hid_device *device, 
__u8 *start,

  	if (device->driver->report_fixup)
  		device->driver->report_fixup(device, start, size);
+	/* Now load a hid descriptor from a file firmware
+	ignoring this fixup thing */
+	file=kmalloc(29, GFP_KERNEL);
+ 
sprintf(file,"hid/%04X:%04X:%04X:%04X.bin",device->bus,device->vendor,device->product,device->version);
+
+	succ = request_firmware(&fw, file, &device->dev);
+
+	if (succ)
+		printk(KERN_INFO "To relace HID descriptor place it in 
/lib/firmaware/%s\n", file);
+	else{
+		start = fw->data;
+		size = fw->size;
+		printk(KERN_INFO "HID descriptor relaced with /lib/firmaware/%s\n", 
file);
+	}
+	kfree(file);

  	device->rdesc = kmalloc(size, GFP_KERNEL);
-	if (device->rdesc == NULL)
+	if (device->rdesc == NULL){
+		if(!succ)release_firmware(fw);
  		return -ENOMEM;
+	}
  	memcpy(device->rdesc, start, size);
  	device->rsize = size;

@@ -690,6 +712,7 @@ int hid_parse_report(struct hid_device *device, __u8 
*start,
  				dbg_hid("unbalanced delimiter at end of report description\n");
  				goto err;
  			}
+			if(!succ)release_firmware(fw);
  			vfree(parser);
  			return 0;
  		}
@@ -697,6 +720,7 @@ int hid_parse_report(struct hid_device *device, __u8 
*start,

  	dbg_hid("item fetching failed at offset %d\n", (int)(end - start));
  err:
+	if(!succ)release_firmware(fw);
  	vfree(parser);
  	return ret;
  }
-- 1.6.5.7

[-- Attachment #2: 0001-Simple-but-effective-way-of-chaging-HID-device-descr.patch --]
[-- Type: text/x-patch, Size: 2784 bytes --]

>From 85bc06073ad4d18f44757c2c72f9f8409ecf3ac5 Mon Sep 17 00:00:00 2001
From: Marcin Tolysz <tolysz@gmail.com>
To: <linux-input@vger.kernel.org>
Date: Thu, 24 Dec 2009 01:40:34 +0000
Subject: [RFC] Simple but effective way of chaging HID device descriptors

Simple but effective way of chaging HID device descriptorsusing firmware loading mechanism.
Just place your new descriptor in a location specified by driver
i.e. somewhere in /lib/firmware/hid/
If it is there it will replace HID descriptor from the device.
I do not do any parsing or checking, as it is done by hid driver.

Signed-off-by: Marcin Tolysz <tolysz@gmail.com>

---
 drivers/hid/hid-core.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 80792d3..5d8d656 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -27,6 +27,7 @@
 #include <linux/wait.h>
 #include <linux/vmalloc.h>
 #include <linux/sched.h>
+#include <linux/firmware.h>
 
 #include <linux/hid.h>
 #include <linux/hiddev.h>
@@ -640,6 +641,10 @@ int hid_parse_report(struct hid_device *device, __u8 *start,
 	struct hid_item item;
 	__u8 *end;
 	int ret;
+	const struct firmware *fw;
+	int succ;
+	char *file;
+
 	static int (*dispatch_type[])(struct hid_parser *parser,
 				      struct hid_item *item) = {
 		hid_parser_main,
@@ -650,10 +655,27 @@ int hid_parse_report(struct hid_device *device, __u8 *start,
 
 	if (device->driver->report_fixup)
 		device->driver->report_fixup(device, start, size);
+	/* Now load a hid descriptor from a file firmware
+	ignoring this fixup thing */
+	file=kmalloc(29, GFP_KERNEL);
+	sprintf(file,"hid/%04X:%04X:%04X:%04X.bin",device->bus,device->vendor,device->product,device->version);
+
+	succ = request_firmware(&fw, file, &device->dev);
+
+	if (succ)
+		printk(KERN_INFO "To relace HID descriptor place it in /lib/firmaware/%s\n", file);
+	else{
+		start = fw->data;
+		size = fw->size;
+		printk(KERN_INFO "HID descriptor relaced with /lib/firmaware/%s\n", file);
+	}
+	kfree(file);
 
 	device->rdesc = kmalloc(size, GFP_KERNEL);
-	if (device->rdesc == NULL)
+	if (device->rdesc == NULL){
+		if(!succ)release_firmware(fw);
 		return -ENOMEM;
+	}
 	memcpy(device->rdesc, start, size);
 	device->rsize = size;
 
@@ -690,6 +712,7 @@ int hid_parse_report(struct hid_device *device, __u8 *start,
 				dbg_hid("unbalanced delimiter at end of report description\n");
 				goto err;
 			}
+			if(!succ)release_firmware(fw);
 			vfree(parser);
 			return 0;
 		}
@@ -697,6 +720,7 @@ int hid_parse_report(struct hid_device *device, __u8 *start,
 
 	dbg_hid("item fetching failed at offset %d\n", (int)(end - start));
 err:
+	if(!succ)release_firmware(fw);
 	vfree(parser);
 	return ret;
 }
-- 
1.6.5.7

Re: [RFC] Simple but effective way of changing HID device descriptors (resent)

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 3706 bytes --]

On Thu, 24 Dec 2009 02:29:47 +0000
Marcin Tolysz <tolysz@gmail.com> wrote:

> Simple but effective way of changing HID device descriptors using 
> firmware loading mechanism.
> Just place your new descriptor in a location specified by driver
> i.e. somewhere in /lib/firmware/hid/
> If it is there it will replace HID descriptor from the device.
> I do not do any parsing or checking, as it is done by hid driver.
> 
> Signed-off-by: Marcin Tolysz <tolysz@gmail.com>
>

Only some style comments below, I haven't tested the patch. Plus, try
running the patch through scripts/checkpatch.pl to spot all the other
style issues.

> ---
>   drivers/hid/hid-core.c |   26 +++++++++++++++++++++++++-
>   1 files changed, 25 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 80792d3..5d8d656 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -27,6 +27,7 @@
>   #include <linux/wait.h>
>   #include <linux/vmalloc.h>
>   #include <linux/sched.h>
> +#include <linux/firmware.h>
> 
>   #include <linux/hid.h>
>   #include <linux/hiddev.h>
> @@ -640,6 +641,10 @@ int hid_parse_report(struct hid_device *device, 
> __u8 *start,
>   	struct hid_item item;
>   	__u8 *end;
>   	int ret;
> +	const struct firmware *fw;
> +	int succ;

success? Can't you init fw to NULL and derive this info checking
(fw != NULL) before releasing firmware?

> +	char *file;
> +
>   	static int (*dispatch_type[])(struct hid_parser *parser,
>   				      struct hid_item *item) = {
>   		hid_parser_main,
> @@ -650,10 +655,27 @@ int hid_parse_report(struct hid_device *device, 
> __u8 *start,
> 
>   	if (device->driver->report_fixup)
>   		device->driver->report_fixup(device, start, size);
> +	/* Now load a hid descriptor from a file firmware
> +	ignoring this fixup thing */
> +	file=kmalloc(29, GFP_KERNEL);
> + 
> sprintf(file,"hid/%04X:%04X:%04X:%04X.bin",device->bus,device->vendor,device->product,device->version);
> +

What about removing this 29?
#define NEW_HID_DESCRIPTOR_PATH_FMT "hid/%04X:%04X:%04X:%04X.bin"
and use it in kmalloc with strlen and in sprintf.

> +	succ = request_firmware(&fw, file, &device->dev);
> +
> +	if (succ)
> +		printk(KERN_INFO "To relace HID descriptor place it in 
> /lib/firmaware/%s\n", file);

pr_info() could be used or maybe even dev_info()

> +	else{
> +		start = fw->data;
> +		size = fw->size;
> +		printk(KERN_INFO "HID descriptor relaced with /lib/firmaware/%s\n", 
> file);
> +	}
> +	kfree(file);
> 
>   	device->rdesc = kmalloc(size, GFP_KERNEL);
> -	if (device->rdesc == NULL)
> +	if (device->rdesc == NULL){
> +		if(!succ)release_firmware(fw);
>   		return -ENOMEM;
> +	}
>   	memcpy(device->rdesc, start, size);
>   	device->rsize = size;
> 
> @@ -690,6 +712,7 @@ int hid_parse_report(struct hid_device *device, __u8 
> *start,
>   				dbg_hid("unbalanced delimiter at end of report description\n");
>   				goto err;
>   			}
> +			if(!succ)release_firmware(fw);
>   			vfree(parser);
>   			return 0;
>   		}
> @@ -697,6 +720,7 @@ int hid_parse_report(struct hid_device *device, __u8 
> *start,
> 
>   	dbg_hid("item fetching failed at offset %d\n", (int)(end - start));
>   err:
> +	if(!succ)release_firmware(fw);
>   	vfree(parser);
>   	return ret;
>   }
> -- 1.6.5.7

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [RFC] Simple but effective way of changing HID device descriptors (resent)

[Marcin Tolysz]

Just to make sure the corrected patch is: [RFC v2] [HID] allow using
external Hid Descriptors.

2009/12/24 Antonio Ospite <ospite@studenti.unina.it>:
> On Thu, 24 Dec 2009 02:29:47 +0000
> Marcin Tolysz <tolysz@gmail.com> wrote:
>
>> Simple but effective way of changing HID device descriptors using
>> firmware loading mechanism.
>> Just place your new descriptor in a location specified by driver
>> i.e. somewhere in /lib/firmware/hid/
>> If it is there it will replace HID descriptor from the device.
>> I do not do any parsing or checking, as it is done by hid driver.
>>
>> Signed-off-by: Marcin Tolysz <tolysz@gmail.com>
>>
>
> Only some style comments below, I haven't tested the patch. Plus, try
> running the patch through scripts/checkpatch.pl to spot all the other
> style issues.

Corrected most of style errors but 3 too long lines.

>> ---
>>   drivers/hid/hid-core.c |   26 +++++++++++++++++++++++++-
>>   1 files changed, 25 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 80792d3..5d8d656 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -27,6 +27,7 @@
>>   #include <linux/wait.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/sched.h>
>> +#include <linux/firmware.h>
>>
>>   #include <linux/hid.h>
>>   #include <linux/hiddev.h>
>> @@ -640,6 +641,10 @@ int hid_parse_report(struct hid_device *device,
>> __u8 *start,
>>       struct hid_item item;
>>       __u8 *end;
>>       int ret;
>> +     const struct firmware *fw;
>> +     int succ;
>
> success? Can't you init fw to NULL and derive this info checking
> (fw != NULL) before releasing firmware?
every other driver checks this value, fw is a structure thus I would
need to check fw->data;

>> +     char *file;
>> +
>>       static int (*dispatch_type[])(struct hid_parser *parser,
>>                                     struct hid_item *item) = {
>>               hid_parser_main,
>> @@ -650,10 +655,27 @@ int hid_parse_report(struct hid_device *device,
>> __u8 *start,
>>
>>       if (device->driver->report_fixup)
>>               device->driver->report_fixup(device, start, size);
>> +     /* Now load a hid descriptor from a file firmware
>> +     ignoring this fixup thing */
>> +     file=kmalloc(29, GFP_KERNEL);
>> +
>> sprintf(file,"hid/%04X:%04X:%04X:%04X.bin",device->bus,device->vendor,device->product,device->version);
>> +
>
> What about removing this 29?
> #define NEW_HID_DESCRIPTOR_PATH_FMT "hid/%04X:%04X:%04X:%04X.bin"
> and use it in kmalloc with strlen and in sprintf.

replaced by:
       file = kasprintf(GFP_KERNEL, "hid/%04X:%04X:%04X:%04X.bin",
                      device->bus, device->vendor, device->product,
device->version);
I did not putted it into define as this heavily depends on parameters,
and I do not want to separate them yet.

>> +     succ = request_firmware(&fw, file, &device->dev);
>> +
>> +     if (succ)
>> +             printk(KERN_INFO "To relace HID descriptor place it in
>> /lib/firmaware/%s\n", file);
>
> pr_info() could be used or maybe even dev_info()

I went for pr_info.

>> +     else{
>> +             start = fw->data;
>> +             size = fw->size;
>> +             printk(KERN_INFO "HID descriptor relaced with /lib/firmaware/%s\n",
>> file);
>> +     }
>> +     kfree(file);
>>
>>       device->rdesc = kmalloc(size, GFP_KERNEL);
>> -     if (device->rdesc == NULL)
>> +     if (device->rdesc == NULL){
>> +             if(!succ)release_firmware(fw);
>>               return -ENOMEM;
>> +     }
>>       memcpy(device->rdesc, start, size);
>>       device->rsize = size;
>>
>> @@ -690,6 +712,7 @@ int hid_parse_report(struct hid_device *device, __u8
>> *start,
>>                               dbg_hid("unbalanced delimiter at end of report description\n");
>>                               goto err;
>>                       }
>> +                     if(!succ)release_firmware(fw);
>>                       vfree(parser);
>>                       return 0;
>>               }
>> @@ -697,6 +720,7 @@ int hid_parse_report(struct hid_device *device, __u8
>> *start,
>>
>>       dbg_hid("item fetching failed at offset %d\n", (int)(end - start));
>>   err:
>> +     if(!succ)release_firmware(fw);
>>       vfree(parser);
>>       return ret;
>>   }
>> -- 1.6.5.7
>
> Regards,
>   Antonio

Many thanks for your help.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html