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

RFC In review Marcin Tolysz 2 participants 3 messages Dec 24, 2009 – Dec 31, 2009 HID

Share & tools

Marcin Tolysz

author

2009-12-24 02:29 UTC #0 lore raw

RFC 1/1 Simple but effective way of changing HID device descriptors (resent)

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 <[email protected]>

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

drivers/hid/hid-core.c +25 -1

index 80792d3..5d8d656 100644
@@ -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

Quoted 1 line

From 85bc06073ad4d18f44757c2c72f9f8409ecf3ac5 Mon Sep 17 00:00:00 2001

From: Marcin Tolysz <[email protected]>
To: <[email protected]>
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 <[email protected]>

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

drivers/hid/hid-core.c +25 -1

index 80792d3..5d8d656 100644
@@ -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;
 }

Signature · 1 line

1.6.5.7

Antonio Ospite

2009-12-24 08:39 UTC #1 lore raw

Quoted 9 lines · On Thu, 24 Dec 2009 02:29:47 +0000 Marcin Tolysz <[email protected]> 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 <[email protected]>

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.

Quoted 17 lines

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

drivers/hid/hid-core.c +1 -0

index 80792d3..5d8d656 100644
@@ -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>

drivers/hid/hid-core.c int hid_parse_report(struct hid_device *device, · line 646

Quoted 6 lines

drivers/hid/hid-core.c +2 -0

@@ -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?

Quoted 6 lines

drivers/hid/hid-core.c +2 -0

+	char *file;
+
  	static int (*dispatch_type[])(struct hid_parser *parser,
  				      struct hid_item *item) = {

hid_parser_main,

drivers/hid/hid-core.c int hid_parse_report(struct hid_device *device, · line 664

Quoted 10 lines

drivers/hid/hid-core.c +5 -0

@@ -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.

drivers/hid/hid-core.c int hid_parse_report(struct hid_device *device, __u8 *start,

Quoted 5 lines

drivers/hid/hid-core.c +4 -0

+	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()

Quoted 34 lines

drivers/hid/hid-core.c +10 -1

+	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;

drivers/hid/hid-core.c +1 -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;

Quoted 2 lines

}
-- 1.6.5.7

Regards,
Antonio

Signature · 10 lines

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?

Marcin Tolysz

author

2009-12-31 17:29 UTC #2 lore raw

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

2009/12/24 Antonio Ospite <[email protected]>:

Quoted 10 lines

Quoted 7 lines · On Thu, 24 Dec 2009 02:29:47 +0000 Marcin Tolysz <[email protected]> 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.

Quoted 6 lines

Quoted 2 lines
Signed-off-by Marcin Tolysz <[email protected]>
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.

Quoted 14 lines

---
drivers/hid/hid-core.c | 26 +++++++++++++++++++++++++-

  1 files changed, 25 insertions(+), 1 deletions(-)

drivers/hid/hid-core.c +1 -0

index 80792d3..5d8d656 100644
@@ -27,6 +27,7 @@
   #include <linux/wait.h>
   #include <linux/vmalloc.h>
   #include <linux/sched.h>
+#include <linux/firmware.h>

Quoted 12 lines

drivers/hid/hid-core.c int hid_parse_report(struct hid_device *device, · line 646
Quoted 9 lines
drivers/hid/hid-core.c +2 -0
   #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;

Quoted 8 lines

drivers/hid/hid-core.c +2 -0

+     char *file;
+
       static int (*dispatch_type[])(struct hid_parser *parser,
                                     struct hid_item *item) = {

hid_parser_main,

drivers/hid/hid-core.c +0 -0

@@ -650,10 +655,27 @@ int hid_parse_report(struct hid_device *device,
 __u8 *start,

Quoted 12 lines

drivers/hid/hid-core.c int hid_parse_report(struct hid_device *device, __u8 *start, · line 664
Quoted 8 lines
drivers/hid/hid-core.c +5 -0
       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.

Quoted 7 lines

drivers/hid/hid-core.c int hid_parse_report(struct hid_device *device, __u8 *start,
Quoted 5 lines
drivers/hid/hid-core.c +4 -0
+     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.

Quoted 29 lines

drivers/hid/hid-core.c +8 -1

+     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;

drivers/hid/hid-core.c +1 -0

@@ -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;

}

drivers/hid/hid-core.c +0 -0

@@ -697,6 +720,7 @@ int hid_parse_report(struct hid_device *device, __u8
 *start,

Quoted 10 lines

Quoted 7 lines
drivers/hid/hid-core.c +0 -0
       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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html