Re: HID bus

Re: HID bus

[Jiri Slaby]

Hmm, I must admit, I didn't know how exactly autoloading works. On suse,
at least, module aliases are used. So autoloading works for me after this
patch and slight modifications of the previous patches. The pro of this
is that it's in-kernel modification of modpost phase.

Now, I wonder for exactly what purpose are module.*map files in
/lib/modules/`uname -r`/ dir, i.e. what would break, if the devices won't
be listed there on systems with unpatched module-init-tools.

--

Generate aliases for usb hid device modules to support autoloading.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 include/linux/hid.h             |    7 +++++++
 include/linux/mod_devicetable.h |    8 ++++++++
 scripts/mod/file2alias.c        |   21 +++++++++++++++++++++
 3 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d951ec4..44c1b8b 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -29,11 +29,18 @@
  * Vojtech Pavlik, Simunkova 1594, Prague 8, 182 00 Czech Republic
  */
 
+#ifdef __KERNEL__
+#include <linux/mod_devicetable.h>
+#endif
+
 /*
  * USB HID (Human Interface Device) interface class code
  */
 
+/* this one is for userspace, we define this in linux/mod_devicetable.h */
+#ifndef USB_INTERFACE_CLASS_HID
 #define USB_INTERFACE_CLASS_HID		3
+#endif
 
 /*
  * USB HID interface subclass and protocol codes
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 139d49d..2227c43 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -131,6 +131,14 @@ struct usb_device_id {
 #define USB_DEVICE_ID_MATCH_INT_SUBCLASS	0x0100
 #define USB_DEVICE_ID_MATCH_INT_PROTOCOL	0x0200
 
+#define USB_INTERFACE_CLASS_HID			3
+#define HID_ANY_ID				(~0)
+
+struct hid_device_id {
+	__u32 vendor, product;
+	kernel_ulong_t driver_data;
+};
+
 /* s390 CCW devices */
 struct ccw_device_id {
 	__u16	match_flags;	/* which fields to match against */
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 769b69d..d92445e 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -199,6 +199,23 @@ static void do_usb_table(void *symval, unsigned long size,
 		do_usb_entry_multi(symval + i, mod);
 }
 
+/* Looks like: usb */
+static int do_hid_usb_entry(const char *filename,
+			     struct hid_device_id *id, char *alias)
+{
+	__u16 v = TO_NATIVE((__u16)id->vendor);
+	__u16 p = TO_NATIVE((__u16)id->product);
+
+	strcpy(alias, "usb:");
+	ADD(alias, "v", id->vendor != HID_ANY_ID, v);
+	ADD(alias, "p", id->product != HID_ANY_ID, p);
+
+	sprintf(alias + strlen(alias), "d*dc*dsc*dp*ic%02Xisc*ip*",
+			USB_INTERFACE_CLASS_HID);
+
+	return 1;
+}
+
 /* Looks like: ieee1394:venNmoNspNverN */
 static int do_ieee1394_entry(const char *filename,
 			     struct ieee1394_device_id *id, char *alias)
@@ -642,6 +659,10 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 	else if (sym_is(symname, "__mod_usb_device_table"))
 		/* special case to handle bcdDevice ranges */
 		do_usb_table(symval, sym->st_size, mod);
+	else if (sym_is(symname, "__mod_hid_usb_device_table"))
+		do_table(symval, sym->st_size,
+			 sizeof(struct hid_device_id), "hid_usb",
+			 do_hid_usb_entry, mod);
 	else if (sym_is(symname, "__mod_ieee1394_device_table"))
 		do_table(symval, sym->st_size,
 			 sizeof(struct ieee1394_device_id), "ieee1394",
-- 
1.5.4.5

Re: HID bus

[Jiri Slaby]

On 04/20/2008 12:38 AM, Jiri Slaby wrote:
> Now, I wonder for exactly what purpose are module.*map files in
> /lib/modules/`uname -r`/ dir, i.e. what would break, if the devices won't
> be listed there on systems with unpatched module-init-tools.

Well, used by /sbin/hotplug. I.e. on systems which uses that hotplugging system 
and not new enough udev. It means older RHEL, SLE* et al., and older 
distributions too...

So I suppose we still need the option to let users compile all the chosen 
drivers into hid.

Re: HID bus

[Anssi Hannula]

Jiri Slaby wrote:
> Hmm, I must admit, I didn't know how exactly autoloading works. On suse,
> at least, module aliases are used. So autoloading works for me after this
> patch and slight modifications of the previous patches. The pro of this
> is that it's in-kernel modification of modpost phase.

Indeed in current systems udev uses module aliases for autoloading.

> --
> 
> Generate aliases for usb hid device modules to support autoloading.
> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
[...]
> +/* Looks like: usb */

Looks like: "usb:vNpNd*dc*dsc*dp*ic3isc*ip*"

> +static int do_hid_usb_entry(const char *filename,
> +			     struct hid_device_id *id, char *alias)
> +{
> +	__u16 v = TO_NATIVE((__u16)id->vendor);
> +	__u16 p = TO_NATIVE((__u16)id->product);
> +
> +	strcpy(alias, "usb:");
> +	ADD(alias, "v", id->vendor != HID_ANY_ID, v);
> +	ADD(alias, "p", id->product != HID_ANY_ID, p);
> +
> +	sprintf(alias + strlen(alias), "d*dc*dsc*dp*ic%02Xisc*ip*",
> +			USB_INTERFACE_CLASS_HID);
> +
> +	return 1;
> +}

Oh, so we create a normal usb modalias entry anyway, not a custom
'usbhid:' one.

Why not just do something like

#define HID_DEVICE(vend, dev) \
	.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
		       USB_DEVICE_ID_MATCH_INT_CLASS, \
	.idVendor = (vend), \
	.idProduct = (prod), \
	.bInterfaceClass = USB_INTERFACE_CLASS_HID
(see linux/usb.h)

and use USB hotplugging?

Or do we plan to match against something else as well, such as hid
reports or something?

-- 
Anssi Hannula

Re: HID bus

[Jiri Slaby]

On 04/20/2008 06:15 PM, Anssi Hannula wrote:
> Oh, so we create a normal usb modalias entry anyway, not a custom
> 'usbhid:' one.

Arrr, thank you, this provoked me an idea. I'll repost the patch tomorrow or so.

Re: HID bus

[Jiri Kosina]

On Sun, 20 Apr 2008, Anssi Hannula wrote:

> Why not just do something like
> 
> #define HID_DEVICE(vend, dev) \
> 	.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
> 		       USB_DEVICE_ID_MATCH_INT_CLASS, \
> 	.idVendor = (vend), \
> 	.idProduct = (prod), \
> 	.bInterfaceClass = USB_INTERFACE_CLASS_HID
> (see linux/usb.h)
> 
> and use USB hotplugging?
> Or do we plan to match against something else as well, such as hid
> reports or something?

No, we plan to do matching based solely on VID/PID. The report-specific 
stuff is then handled between the specific driver and HID generic code.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: HID bus

[Dmitry Torokhov]

On Mon, Apr 21, 2008 at 11:25:43AM +0200, Jiri Kosina wrote:
> On Sun, 20 Apr 2008, Anssi Hannula wrote:
> 
> > Why not just do something like
> > 
> > #define HID_DEVICE(vend, dev) \
> > 	.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
> > 		       USB_DEVICE_ID_MATCH_INT_CLASS, \
> > 	.idVendor = (vend), \
> > 	.idProduct = (prod), \
> > 	.bInterfaceClass = USB_INTERFACE_CLASS_HID
> > (see linux/usb.h)
> > 
> > and use USB hotplugging?
> > Or do we plan to match against something else as well, such as hid
> > reports or something?
> 
> No, we plan to do matching based solely on VID/PID. The report-specific 
> stuff is then handled between the specific driver and HID generic code.
> 

Hmm, [ab]using USB modalias makes me a bit uneasy. What about bluetooth
HID devices?

-- 
Dmitry

Re: HID bus

[Jiri Slaby]

On 04/21/2008 03:36 PM, Dmitry Torokhov wrote:
> On Mon, Apr 21, 2008 at 11:25:43AM +0200, Jiri Kosina wrote:
>> On Sun, 20 Apr 2008, Anssi Hannula wrote:
>>
>>> Why not just do something like
>>>
>>> #define HID_DEVICE(vend, dev) \
>>> 	.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
>>> 		       USB_DEVICE_ID_MATCH_INT_CLASS, \
>>> 	.idVendor = (vend), \
>>> 	.idProduct = (prod), \
>>> 	.bInterfaceClass = USB_INTERFACE_CLASS_HID
>>> (see linux/usb.h)
>>>
>>> and use USB hotplugging?
>>> Or do we plan to match against something else as well, such as hid
>>> reports or something?
>> No, we plan to do matching based solely on VID/PID. The report-specific 
>> stuff is then handled between the specific driver and HID generic code.
>>
> 
> Hmm, [ab]using USB modalias makes me a bit uneasy. What about bluetooth
> HID devices?

I do agree, it won't be nice and abstract enough. Working on it, give me some 
time, please.

Re: HID bus

[Jiri Kosina]

On Sun, 20 Apr 2008, Jiri Slaby wrote:

> So I suppose we still need the option to let users compile all the 
> chosen drivers into hid.

Another possible workaround is to introduce dummy "hid" module, that will 
just depend on all other drivers with a reference to certail symbol (and 
this could of course be optional as CONFIG_HID_COMPAT, or whatever).

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: HID bus

[Jiri Slaby]

On 04/22/2008 02:00 AM, Jiri Kosina wrote:
> On Sun, 20 Apr 2008, Jiri Slaby wrote:
> 
>> So I suppose we still need the option to let users compile all the 
>> chosen drivers into hid.
> 
> Another possible workaround is to introduce dummy "hid" module, that will 
> just depend on all other drivers with a reference to certail symbol (and 
> this could of course be optional as CONFIG_HID_COMPAT, or whatever).

If I understand correctly, it would create circular dependency:
hid->dummy_hid->hid_apple->hid

Is is correct?

Thanks.

Re: HID bus

[Jiri Slaby]

On 04/22/2008 02:00 AM, Jiri Kosina wrote:
> On Sun, 20 Apr 2008, Jiri Slaby wrote:
> 
>> So I suppose we still need the option to let users compile all the 
>> chosen drivers into hid.

This won't work either. It wouldn't link due to multiple definitions of 
module_init, module_exit and some module tables.

> Another possible workaround is to introduce dummy "hid" module, that will 
> just depend on all other drivers with a reference to certail symbol (and 
> this could of course be optional as CONFIG_HID_COMPAT, or whatever).

Actually this may work if we load hid_dummy by request_module from hid. I have 
no other better idea.