Re: [PATCH 1/1] HID: add have_special_driver hid module parameter

On Tue, 3 Apr 2012, Henrik Rydberg wrote:

quoted

 include/linux/hid.h |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

quoted

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 3a95da6..f771eba 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h

@@ -807,8 +807,16 @@ static inline int __must_check hid_parse(struct hid_devic> e *hdev)
 {
 	int ret;
 
-	if (hdev->status & HID_STAT_PARSED)
-		return 0;
+	if (hdev->status & HID_STAT_PARSED) {
+		/*
+		 * We want to be re-entrant to allow for dynamic driver
+		 * rebinding and still allow rdescs to be replaced and
+		 * and re-parsed once the driver has been dynamically
+		 * rebound
+		 */
+		kfree(hdev->rdesc);
+		hdev->status &= ~HID_STAT_PARSED;
+	}
 
 	ret = hdev->ll_driver->parse(hdev);
 	if (!ret)

It seems an equivalent patch would be to remove HID_STAT_PARSED
altogether, replacing it with something like this:

Yes, that's identical.

[ ... snip ... ]

which makes me wonder if something will break or be called
unnecessarily often as a result?

I don't currently see how such thing could happen. Do you have anything 
particular on your mind?

I think the main logic problem stems from viewing hid devices as being 
on the same level as usb/bt devices.  Perhaps report fixups should be 
part of the hid_ll_driver layer instead.

It probably should, yes. One of the reasons supporting this is that during 
USB reset (for example), the descriptors are reread from the device, which 
has a potential to cause a mismatch if a full rebind cycle is not 
performed. (I have a patch in my queue that tries to fix this, but moving 
the fixups to ll drivers is still something I will be considering).

Thanks,

-- 
Jiri Kosina
SUSE Labs