[GIT] HID patches for 2.6.32 merge window

[GIT] HID patches for 2.6.32 merge window

[Jiri Kosina]

Linus,

could you please pull from 'for-linus' branch of

        git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-linus

to receive HID patches accumulated for 2.6.32 merge window.

Thanks!

 drivers/hid/Kconfig             |   29 +--
 drivers/hid/Makefile            |    6 +-
 drivers/hid/hid-a4tech.c        |    4 +-
 drivers/hid/hid-apple.c         |    4 +-
 drivers/hid/hid-belkin.c        |    4 +-
 drivers/hid/hid-cherry.c        |    4 +-
 drivers/hid/hid-chicony.c       |    4 +-
 drivers/hid/hid-core.c          |   71 +++++--
 drivers/hid/hid-cypress.c       |    4 +-
 drivers/hid/hid-debug.c         |  439 ++++++++++++++++++++++++++++++++-------
 drivers/hid/hid-ezkey.c         |    4 +-
 drivers/hid/hid-gyration.c      |    4 +-
 drivers/hid/hid-ids.h           |   14 +-
 drivers/hid/hid-input.c         |   13 +-
 drivers/hid/hid-kensington.c    |    4 +-
 drivers/hid/hid-kye.c           |    4 +-
 drivers/hid/hid-lg.c            |    6 +-
 drivers/hid/hid-lgff.c          |    6 +-
 drivers/hid/hid-microsoft.c     |    4 +-
 drivers/hid/hid-monterey.c      |    4 +-
 drivers/hid/hid-ntrig.c         |   37 +++-
 drivers/hid/hid-petalynx.c      |    4 +-
 drivers/hid/hid-pl.c            |    4 +-
 drivers/hid/hid-samsung.c       |   43 ++++-
 drivers/hid/hid-sjoy.c          |    4 +-
 drivers/hid/hid-sony.c          |    4 +-
 drivers/hid/hid-sunplus.c       |    4 +-
 drivers/hid/hid-tmff.c          |   10 +-
 drivers/hid/hid-topseed.c       |    4 +-
 drivers/hid/hid-twinhan.c       |  147 +++++++++++++
 drivers/hid/hid-wacom.c         |    4 +-
 drivers/hid/hid-zpff.c          |    4 +-
 drivers/hid/usbhid/hid-core.c   |   12 +-
 drivers/hid/usbhid/hid-quirks.c |    2 +-
 drivers/hid/usbhid/hiddev.c     |    2 +-
 include/linux/hid-debug.h       |   48 +++--
 include/linux/hid.h             |   22 +--
 37 files changed, 755 insertions(+), 232 deletions(-)

Bruno Premont (1):
      HID: driver for Twinhan USB 6253:0100 remote control

H Hartley Sweeten (1):
      HID: local function should be static

Henning Glawe (1):
      HID: ignore Philips IEEE802.15.4 RF Dongle

Jan Scholz (1):
      HID: completely remove apple mightymouse from blacklist

Jarod Wilson (1):
      HID: ignore all recent SoundGraph iMON devices

Jiri Kosina (6):
      HID: use debugfs for report dumping descriptor
      HID: use debugfs for events/reports dumping
      HID: fix debugfs build with !CONFIG_DEBUG_FS
      HID: fix memory leak on error path in debug code
      HID: add force feedback support for Logitech WingMan Formula Force GP
      HID: support larger reports than 64 bytes in hiddev

Julia Lawall (2):
      HID: Drop NULL test on list_entry result
      HID: fix memory leak on error patch in debug code

Peter Huewe (1):
      HID: adding __init/__exit macros to module init/exit functions

Rafi Rubin (1):
      HID: ntrig tool separation and pen usages

Robert Schedel (1):
      HID: Support new variants of Samsung USB IR receiver (0419:0001)

Roel Kluin (1):
      HID: fix overrun in quirks initialization

Ruben Aos Garralda (1):
      HID: add rumble support for Thrustmaster Dual Trigger 3-in-1

Sergey Senozhatsky (1):
      HID: Avoid double spin_lock_init on usbhid->lock

Re: [GIT] HID patches for 2.6.32 merge window

[Marcel Holtmann]

Hi Jiri,

> could you please pull from 'for-linus' branch of
> 
>         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-linus
> 
> to receive HID patches accumulated for 2.6.32 merge window.

I am missing the patch to free the transport drivers from having to
manually disconnect hidinput and hidraw. All the hid->claimed handling
should really be hidden from the transport drivers.

Regards

Marcel

Re: [GIT] HID patches for 2.6.32 merge window

[Jiri Kosina]

On Mon, 14 Sep 2009, Marcel Holtmann wrote:

> > could you please pull from 'for-linus' branch of
> > 
> >         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-linus
> > 
> > to receive HID patches accumulated for 2.6.32 merge window.
> 
> I am missing the patch to free the transport drivers from having to
> manually disconnect hidinput and hidraw. All the hid->claimed handling
> should really be hidden from the transport drivers.

Hi Marcel,

right, thanks for spotting this. The patch below has been rotting in my 
'waiting-for-ack' queue for quite some time. I was apparently waiting for 
your Ack for the drivers/net/bluetooth part, and forgot about it when it 
didn't come.

If you're OK with that, I'll include it in the next pile. Thanks.





From: Jiri Kosina <jkosina@suse.cz>
Subject: HID: consolidate connect and disconnect into core code

HID core registers input, hidraw and hiddev devices, but leaves 
unregistering it up to the individual driver, which is not really nice. 
Let's move all the logic to the core.

Reported-by: Marcel Holtmann <marcel@holtmann.org>
Reported-by: Brian Rogers <brian@xyzw.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index f2c21d5..3b776d7 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1218,6 +1218,17 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 }
 EXPORT_SYMBOL_GPL(hid_connect);
 
+void hid_disconnect(struct hid_device *hdev)
+{
+	if (hdev->claimed & HID_CLAIMED_INPUT)
+		hidinput_disconnect(hdev);
+	if (hdev->claimed & HID_CLAIMED_HIDDEV)
+		hdev->hiddev_disconnect(hdev);
+	if (hdev->claimed & HID_CLAIMED_HIDRAW)
+		hidraw_disconnect(hdev);
+}
+EXPORT_SYMBOL_GPL(hid_disconnect);
+
 /* a list of devices for which there is a specialized driver on HID bus */
 static const struct hid_device_id hid_blacklist[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_WCP32PU) },
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 3c1fcb7..0aa180b 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1047,13 +1047,6 @@ static void usbhid_stop(struct hid_device *hid)
 
 	hid_cancel_delayed_stuff(usbhid);
 
-	if (hid->claimed & HID_CLAIMED_INPUT)
-		hidinput_disconnect(hid);
-	if (hid->claimed & HID_CLAIMED_HIDDEV)
-		hiddev_disconnect(hid);
-	if (hid->claimed & HID_CLAIMED_HIDRAW)
-		hidraw_disconnect(hid);
-
 	hid->claimed = 0;
 
 	usb_free_urb(usbhid->urbin);
@@ -1091,7 +1084,7 @@ static struct hid_ll_driver usb_hid_driver = {
 	.hidinput_input_event = usb_hidinput_input_event,
 };
 
-static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
+static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	struct usb_device *dev = interface_to_usbdev(intf);
@@ -1123,6 +1116,7 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	hid->ff_init = hid_pidff_init;
 #ifdef CONFIG_USB_HIDDEV
 	hid->hiddev_connect = hiddev_connect;
+	hid->hiddev_disconnect = hiddev_disconnect;
 	hid->hiddev_hid_event = hiddev_hid_event;
 	hid->hiddev_report_event = hiddev_report_event;
 #endif
@@ -1183,7 +1177,7 @@ err:
 	return ret;
 }
 
-static void hid_disconnect(struct usb_interface *intf)
+static void usbhid_disconnect(struct usb_interface *intf)
 {
 	struct hid_device *hid = usb_get_intfdata(intf);
 	struct usbhid_device *usbhid;
@@ -1365,8 +1359,8 @@ MODULE_DEVICE_TABLE (usb, hid_usb_ids);
 
 static struct usb_driver hid_driver = {
 	.name =		"usbhid",
-	.probe =	hid_probe,
-	.disconnect =	hid_disconnect,
+	.probe =	usbhid_probe,
+	.disconnect =	usbhid_disconnect,
 #ifdef CONFIG_PM
 	.suspend =	hid_suspend,
 	.resume =	hid_resume,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 53489fd..1c5955d 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -494,6 +494,7 @@ struct hid_device {							/* device report descriptor */
 
 	/* hiddev event handler */
 	int (*hiddev_connect)(struct hid_device *, unsigned int);
+	void (*hiddev_disconnect)(struct hid_device *);
 	void (*hiddev_hid_event) (struct hid_device *, struct hid_field *field,
 				  struct hid_usage *, __s32);
 	void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
@@ -685,6 +686,7 @@ struct hid_device *hid_allocate_device(void);
 int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
 int hid_check_keys_pressed(struct hid_device *hid);
 int hid_connect(struct hid_device *hid, unsigned int connect_mask);
+void hid_disconnect(struct hid_device *hid);
 
 /**
  * hid_map_usage - map usage input bits
@@ -794,6 +796,7 @@ static inline int __must_check hid_hw_start(struct hid_device *hdev,
  */
 static inline void hid_hw_stop(struct hid_device *hdev)
 {
+	hid_disconnect(hdev);
 	hdev->ll_driver->stop(hdev);
 }
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index b186768..6593574 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -570,11 +570,8 @@ static int hidp_session(void *arg)
 		session->input = NULL;
 	}
 
-	if (session->hid) {
-		if (session->hid->claimed & HID_CLAIMED_INPUT)
-			hidinput_disconnect(session->hid);
+	if (session->hid)
 		hid_destroy_device(session->hid);
-	}
 
 	/* Wakeup user-space polling for socket errors */
 	session->intr_sock->sk->sk_err = EUNATCH;
@@ -729,8 +726,6 @@ static void hidp_stop(struct hid_device *hid)
 	skb_queue_purge(&session->ctrl_transmit);
 	skb_queue_purge(&session->intr_transmit);
 
-	if (hid->claimed & HID_CLAIMED_INPUT)
-		hidinput_disconnect(hid);
 	hid->claimed = 0;
 }
 


-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [GIT] HID patches for 2.6.32 merge window

[Marcel Holtmann]

Hi Jiri,

> > > could you please pull from 'for-linus' branch of
> > > 
> > >         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-linus
> > > 
> > > to receive HID patches accumulated for 2.6.32 merge window.
> > 
> > I am missing the patch to free the transport drivers from having to
> > manually disconnect hidinput and hidraw. All the hid->claimed handling
> > should really be hidden from the transport drivers.
> 
> Hi Marcel,
> 
> right, thanks for spotting this. The patch below has been rotting in my 
> 'waiting-for-ack' queue for quite some time. I was apparently waiting for 
> your Ack for the drivers/net/bluetooth part, and forgot about it when it 
> didn't come.
> 
> If you're OK with that, I'll include it in the next pile. Thanks.

you need to redo it against the net-next-2.6 tree or wait until Linus
merged that one. Otherwise it will clash with a temporary solution that
I already pushed some time ago.

Regards

Marcel

Re: [GIT] HID patches for 2.6.32 merge window

[Jiri Kosina]

On Mon, 14 Sep 2009, Marcel Holtmann wrote:

> you need to redo it against the net-next-2.6 tree or wait until Linus 
> merged that one. Otherwise it will clash with a temporary solution that 
> I already pushed some time ago.

OK, I now have this in my tree (which applies on top of 364f63519d9, which 
I believe is the temporary solution you have been talking about). I'd 
appreciate your Acked-by: for the bluetooth part, so that I could push it 
during the next round.
Thanks!



From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] HID: consolidate connect and disconnect into core code

HID core registers input, hidraw and hiddev devices, but leaves
unregistering it up to the individual driver, which is not really nice.
Let's move all the logic to the core.

Reported-by: Marcel Holtmann <marcel@holtmann.org>
Reported-by: Brian Rogers <brian@xyzw.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 drivers/hid/hid-core.c        |   11 +++++++++++
 drivers/hid/usbhid/hid-core.c |   16 +++++-----------
 include/linux/hid.h           |    3 +++
 net/bluetooth/hidp/core.c     |   11 +----------
 4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index ca9bb26..be34d32 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1237,6 +1237,17 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 }
 EXPORT_SYMBOL_GPL(hid_connect);
 
+void hid_disconnect(struct hid_device *hdev)
+{
+	if (hdev->claimed & HID_CLAIMED_INPUT)
+		hidinput_disconnect(hdev);
+	if (hdev->claimed & HID_CLAIMED_HIDDEV)
+		hdev->hiddev_disconnect(hdev);
+	if (hdev->claimed & HID_CLAIMED_HIDRAW)
+		hidraw_disconnect(hdev);
+}
+EXPORT_SYMBOL_GPL(hid_disconnect);
+
 /* a list of devices for which there is a specialized driver on HID bus */
 static const struct hid_device_id hid_blacklist[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_WCP32PU) },
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1b0e07a..03bd703 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1041,13 +1041,6 @@ static void usbhid_stop(struct hid_device *hid)
 
 	hid_cancel_delayed_stuff(usbhid);
 
-	if (hid->claimed & HID_CLAIMED_INPUT)
-		hidinput_disconnect(hid);
-	if (hid->claimed & HID_CLAIMED_HIDDEV)
-		hiddev_disconnect(hid);
-	if (hid->claimed & HID_CLAIMED_HIDRAW)
-		hidraw_disconnect(hid);
-
 	hid->claimed = 0;
 
 	usb_free_urb(usbhid->urbin);
@@ -1085,7 +1078,7 @@ static struct hid_ll_driver usb_hid_driver = {
 	.hidinput_input_event = usb_hidinput_input_event,
 };
 
-static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
+static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	struct usb_device *dev = interface_to_usbdev(intf);
@@ -1117,6 +1110,7 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	hid->ff_init = hid_pidff_init;
 #ifdef CONFIG_USB_HIDDEV
 	hid->hiddev_connect = hiddev_connect;
+	hid->hiddev_disconnect = hiddev_disconnect;
 	hid->hiddev_hid_event = hiddev_hid_event;
 	hid->hiddev_report_event = hiddev_report_event;
 #endif
@@ -1177,7 +1171,7 @@ err:
 	return ret;
 }
 
-static void hid_disconnect(struct usb_interface *intf)
+static void usbhid_disconnect(struct usb_interface *intf)
 {
 	struct hid_device *hid = usb_get_intfdata(intf);
 	struct usbhid_device *usbhid;
@@ -1359,8 +1353,8 @@ MODULE_DEVICE_TABLE (usb, hid_usb_ids);
 
 static struct usb_driver hid_driver = {
 	.name =		"usbhid",
-	.probe =	hid_probe,
-	.disconnect =	hid_disconnect,
+	.probe =	usbhid_probe,
+	.disconnect =	usbhid_disconnect,
 #ifdef CONFIG_PM
 	.suspend =	hid_suspend,
 	.resume =	hid_resume,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index a0ebdac..10f6284 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -494,6 +494,7 @@ struct hid_device {							/* device report descriptor */
 
 	/* hiddev event handler */
 	int (*hiddev_connect)(struct hid_device *, unsigned int);
+	void (*hiddev_disconnect)(struct hid_device *);
 	void (*hiddev_hid_event) (struct hid_device *, struct hid_field *field,
 				  struct hid_usage *, __s32);
 	void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
@@ -691,6 +692,7 @@ struct hid_device *hid_allocate_device(void);
 int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
 int hid_check_keys_pressed(struct hid_device *hid);
 int hid_connect(struct hid_device *hid, unsigned int connect_mask);
+void hid_disconnect(struct hid_device *hid);
 
 /**
  * hid_map_usage - map usage input bits
@@ -800,6 +802,7 @@ static inline int __must_check hid_hw_start(struct hid_device *hdev,
  */
 static inline void hid_hw_stop(struct hid_device *hdev)
 {
+	hid_disconnect(hdev);
 	hdev->ll_driver->stop(hdev);
 }
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 09bedeb..75df1e1 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -576,15 +576,8 @@ static int hidp_session(void *arg)
 		session->input = NULL;
 	}
 
-	if (session->hid) {
-		if (session->hid->claimed & HID_CLAIMED_INPUT)
-			hidinput_disconnect(session->hid);
-		if (session->hid->claimed & HID_CLAIMED_HIDRAW)
-			hidraw_disconnect(session->hid);
-
+	if (session->hid)
 		hid_destroy_device(session->hid);
-		session->hid = NULL;
-	}
 
 	/* Wakeup user-space polling for socket errors */
 	session->intr_sock->sk->sk_err = EUNATCH;
@@ -747,8 +740,6 @@ static void hidp_stop(struct hid_device *hid)
 	skb_queue_purge(&session->ctrl_transmit);
 	skb_queue_purge(&session->intr_transmit);
 
-	if (hid->claimed & HID_CLAIMED_INPUT)
-		hidinput_disconnect(hid);
 	hid->claimed = 0;
 }
 
-- 
1.5.6

Re: [GIT] HID patches for 2.6.32 merge window

[Marcel Holtmann]

Hi Jiri,

> > you need to redo it against the net-next-2.6 tree or wait until Linus 
> > merged that one. Otherwise it will clash with a temporary solution that 
> > I already pushed some time ago.
> 
> OK, I now have this in my tree (which applies on top of 364f63519d9, which 
> I believe is the temporary solution you have been talking about). I'd 
> appreciate your Acked-by: for the bluetooth part, so that I could push it 
> during the next round.
> Thanks!
> 
> 
> 
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] HID: consolidate connect and disconnect into core code
> 
> HID core registers input, hidraw and hiddev devices, but leaves
> unregistering it up to the individual driver, which is not really nice.
> Let's move all the logic to the core.
> 
> Reported-by: Marcel Holtmann <marcel@holtmann.org>
> Reported-by: Brian Rogers <brian@xyzw.org>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>  drivers/hid/hid-core.c        |   11 +++++++++++
>  drivers/hid/usbhid/hid-core.c |   16 +++++-----------
>  include/linux/hid.h           |    3 +++
>  net/bluetooth/hidp/core.c     |   11 +----------
>  4 files changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index ca9bb26..be34d32 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1237,6 +1237,17 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
>  }
>  EXPORT_SYMBOL_GPL(hid_connect);
>  
> +void hid_disconnect(struct hid_device *hdev)
> +{
> +	if (hdev->claimed & HID_CLAIMED_INPUT)
> +		hidinput_disconnect(hdev);
> +	if (hdev->claimed & HID_CLAIMED_HIDDEV)
> +		hdev->hiddev_disconnect(hdev);
> +	if (hdev->claimed & HID_CLAIMED_HIDRAW)
> +		hidraw_disconnect(hdev);
> +}
> +EXPORT_SYMBOL_GPL(hid_disconnect);
> +
>  /* a list of devices for which there is a specialized driver on HID bus */
>  static const struct hid_device_id hid_blacklist[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_WCP32PU) },
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 1b0e07a..03bd703 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1041,13 +1041,6 @@ static void usbhid_stop(struct hid_device *hid)
>  
>  	hid_cancel_delayed_stuff(usbhid);
>  
> -	if (hid->claimed & HID_CLAIMED_INPUT)
> -		hidinput_disconnect(hid);
> -	if (hid->claimed & HID_CLAIMED_HIDDEV)
> -		hiddev_disconnect(hid);
> -	if (hid->claimed & HID_CLAIMED_HIDRAW)
> -		hidraw_disconnect(hid);
> -
>  	hid->claimed = 0;
>  
>  	usb_free_urb(usbhid->urbin);
> @@ -1085,7 +1078,7 @@ static struct hid_ll_driver usb_hid_driver = {
>  	.hidinput_input_event = usb_hidinput_input_event,
>  };
>  
> -static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  {
>  	struct usb_host_interface *interface = intf->cur_altsetting;
>  	struct usb_device *dev = interface_to_usbdev(intf);
> @@ -1117,6 +1110,7 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  	hid->ff_init = hid_pidff_init;
>  #ifdef CONFIG_USB_HIDDEV
>  	hid->hiddev_connect = hiddev_connect;
> +	hid->hiddev_disconnect = hiddev_disconnect;
>  	hid->hiddev_hid_event = hiddev_hid_event;
>  	hid->hiddev_report_event = hiddev_report_event;
>  #endif
> @@ -1177,7 +1171,7 @@ err:
>  	return ret;
>  }
>  
> -static void hid_disconnect(struct usb_interface *intf)
> +static void usbhid_disconnect(struct usb_interface *intf)
>  {
>  	struct hid_device *hid = usb_get_intfdata(intf);
>  	struct usbhid_device *usbhid;
> @@ -1359,8 +1353,8 @@ MODULE_DEVICE_TABLE (usb, hid_usb_ids);
>  
>  static struct usb_driver hid_driver = {
>  	.name =		"usbhid",
> -	.probe =	hid_probe,
> -	.disconnect =	hid_disconnect,
> +	.probe =	usbhid_probe,
> +	.disconnect =	usbhid_disconnect,
>  #ifdef CONFIG_PM
>  	.suspend =	hid_suspend,
>  	.resume =	hid_resume,
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index a0ebdac..10f6284 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -494,6 +494,7 @@ struct hid_device {							/* device report descriptor */
>  
>  	/* hiddev event handler */
>  	int (*hiddev_connect)(struct hid_device *, unsigned int);
> +	void (*hiddev_disconnect)(struct hid_device *);
>  	void (*hiddev_hid_event) (struct hid_device *, struct hid_field *field,
>  				  struct hid_usage *, __s32);
>  	void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
> @@ -691,6 +692,7 @@ struct hid_device *hid_allocate_device(void);
>  int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
>  int hid_check_keys_pressed(struct hid_device *hid);
>  int hid_connect(struct hid_device *hid, unsigned int connect_mask);
> +void hid_disconnect(struct hid_device *hid);
>  
>  /**
>   * hid_map_usage - map usage input bits
> @@ -800,6 +802,7 @@ static inline int __must_check hid_hw_start(struct hid_device *hdev,
>   */
>  static inline void hid_hw_stop(struct hid_device *hdev)
>  {
> +	hid_disconnect(hdev);
>  	hdev->ll_driver->stop(hdev);
>  }
>  
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 09bedeb..75df1e1 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -576,15 +576,8 @@ static int hidp_session(void *arg)
>  		session->input = NULL;
>  	}
>  
> -	if (session->hid) {
> -		if (session->hid->claimed & HID_CLAIMED_INPUT)
> -			hidinput_disconnect(session->hid);
> -		if (session->hid->claimed & HID_CLAIMED_HIDRAW)
> -			hidraw_disconnect(session->hid);
> -
> +	if (session->hid)
>  		hid_destroy_device(session->hid);
> -		session->hid = NULL;
> -	}

can you please leave the session->hid = NULL part inside the code. It is
important that it stays. Other than that

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [GIT] HID patches for 2.6.32 merge window

[Jiri Kosina]

On Wed, 16 Sep 2009, Marcel Holtmann wrote:

> > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> > index 09bedeb..75df1e1 100644
> > --- a/net/bluetooth/hidp/core.c
> > +++ b/net/bluetooth/hidp/core.c
> > @@ -576,15 +576,8 @@ static int hidp_session(void *arg)
> >  		session->input = NULL;
> >  	}
> >  
> > -	if (session->hid) {
> > -		if (session->hid->claimed & HID_CLAIMED_INPUT)
> > -			hidinput_disconnect(session->hid);
> > -		if (session->hid->claimed & HID_CLAIMED_HIDRAW)
> > -			hidraw_disconnect(session->hid);
> > -
> > +	if (session->hid)
> >  		hid_destroy_device(session->hid);
> > -		session->hid = NULL;
> > -	}
> 
> can you please leave the session->hid = NULL part inside the code. It is
> important that it stays. Other than that
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>

Thanks. I now have the patch below in the queue



From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] HID: consolidate connect and disconnect into core code

HID core registers input, hidraw and hiddev devices, but leaves
unregistering it up to the individual driver, which is not really nice.
Let's move all the logic to the core.

Reported-by: Marcel Holtmann <marcel@holtmann.org>
Reported-by: Brian Rogers <brian@xyzw.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 drivers/hid/hid-core.c        |   11 +++++++++++
 drivers/hid/usbhid/hid-core.c |   16 +++++-----------
 include/linux/hid.h           |    3 +++
 net/bluetooth/hidp/core.c     |    7 -------
 4 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index ca9bb26..be34d32 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1237,6 +1237,17 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 }
 EXPORT_SYMBOL_GPL(hid_connect);
 
+void hid_disconnect(struct hid_device *hdev)
+{
+	if (hdev->claimed & HID_CLAIMED_INPUT)
+		hidinput_disconnect(hdev);
+	if (hdev->claimed & HID_CLAIMED_HIDDEV)
+		hdev->hiddev_disconnect(hdev);
+	if (hdev->claimed & HID_CLAIMED_HIDRAW)
+		hidraw_disconnect(hdev);
+}
+EXPORT_SYMBOL_GPL(hid_disconnect);
+
 /* a list of devices for which there is a specialized driver on HID bus */
 static const struct hid_device_id hid_blacklist[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_WCP32PU) },
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1b0e07a..03bd703 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1041,13 +1041,6 @@ static void usbhid_stop(struct hid_device *hid)
 
 	hid_cancel_delayed_stuff(usbhid);
 
-	if (hid->claimed & HID_CLAIMED_INPUT)
-		hidinput_disconnect(hid);
-	if (hid->claimed & HID_CLAIMED_HIDDEV)
-		hiddev_disconnect(hid);
-	if (hid->claimed & HID_CLAIMED_HIDRAW)
-		hidraw_disconnect(hid);
-
 	hid->claimed = 0;
 
 	usb_free_urb(usbhid->urbin);
@@ -1085,7 +1078,7 @@ static struct hid_ll_driver usb_hid_driver = {
 	.hidinput_input_event = usb_hidinput_input_event,
 };
 
-static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
+static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	struct usb_device *dev = interface_to_usbdev(intf);
@@ -1117,6 +1110,7 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	hid->ff_init = hid_pidff_init;
 #ifdef CONFIG_USB_HIDDEV
 	hid->hiddev_connect = hiddev_connect;
+	hid->hiddev_disconnect = hiddev_disconnect;
 	hid->hiddev_hid_event = hiddev_hid_event;
 	hid->hiddev_report_event = hiddev_report_event;
 #endif
@@ -1177,7 +1171,7 @@ err:
 	return ret;
 }
 
-static void hid_disconnect(struct usb_interface *intf)
+static void usbhid_disconnect(struct usb_interface *intf)
 {
 	struct hid_device *hid = usb_get_intfdata(intf);
 	struct usbhid_device *usbhid;
@@ -1359,8 +1353,8 @@ MODULE_DEVICE_TABLE (usb, hid_usb_ids);
 
 static struct usb_driver hid_driver = {
 	.name =		"usbhid",
-	.probe =	hid_probe,
-	.disconnect =	hid_disconnect,
+	.probe =	usbhid_probe,
+	.disconnect =	usbhid_disconnect,
 #ifdef CONFIG_PM
 	.suspend =	hid_suspend,
 	.resume =	hid_resume,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index a0ebdac..10f6284 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -494,6 +494,7 @@ struct hid_device {							/* device report descriptor */
 
 	/* hiddev event handler */
 	int (*hiddev_connect)(struct hid_device *, unsigned int);
+	void (*hiddev_disconnect)(struct hid_device *);
 	void (*hiddev_hid_event) (struct hid_device *, struct hid_field *field,
 				  struct hid_usage *, __s32);
 	void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
@@ -691,6 +692,7 @@ struct hid_device *hid_allocate_device(void);
 int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
 int hid_check_keys_pressed(struct hid_device *hid);
 int hid_connect(struct hid_device *hid, unsigned int connect_mask);
+void hid_disconnect(struct hid_device *hid);
 
 /**
  * hid_map_usage - map usage input bits
@@ -800,6 +802,7 @@ static inline int __must_check hid_hw_start(struct hid_device *hdev,
  */
 static inline void hid_hw_stop(struct hid_device *hdev)
 {
+	hid_disconnect(hdev);
 	hdev->ll_driver->stop(hdev);
 }
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 09bedeb..49d8495 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -577,11 +577,6 @@ static int hidp_session(void *arg)
 	}
 
 	if (session->hid) {
-		if (session->hid->claimed & HID_CLAIMED_INPUT)
-			hidinput_disconnect(session->hid);
-		if (session->hid->claimed & HID_CLAIMED_HIDRAW)
-			hidraw_disconnect(session->hid);
-
 		hid_destroy_device(session->hid);
 		session->hid = NULL;
 	}
@@ -747,8 +742,6 @@ static void hidp_stop(struct hid_device *hid)
 	skb_queue_purge(&session->ctrl_transmit);
 	skb_queue_purge(&session->intr_transmit);
 
-	if (hid->claimed & HID_CLAIMED_INPUT)
-		hidinput_disconnect(hid);
 	hid->claimed = 0;
 }
 
-- 
1.5.6

Re: [GIT] HID patches for 2.6.32 merge window

[Marcel Holtmann]

Hi Jiri,

> > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> > > index 09bedeb..75df1e1 100644
> > > --- a/net/bluetooth/hidp/core.c
> > > +++ b/net/bluetooth/hidp/core.c
> > > @@ -576,15 +576,8 @@ static int hidp_session(void *arg)
> > >  		session->input = NULL;
> > >  	}
> > >  
> > > -	if (session->hid) {
> > > -		if (session->hid->claimed & HID_CLAIMED_INPUT)
> > > -			hidinput_disconnect(session->hid);
> > > -		if (session->hid->claimed & HID_CLAIMED_HIDRAW)
> > > -			hidraw_disconnect(session->hid);
> > > -
> > > +	if (session->hid)
> > >  		hid_destroy_device(session->hid);
> > > -		session->hid = NULL;
> > > -	}
> > 
> > can you please leave the session->hid = NULL part inside the code. It is
> > important that it stays. Other than that
> > 
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> Thanks. I now have the patch below in the queue
> 
> 
> 
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] HID: consolidate connect and disconnect into core code
> 
> HID core registers input, hidraw and hiddev devices, but leaves
> unregistering it up to the individual driver, which is not really nice.
> Let's move all the logic to the core.
> 
> Reported-by: Marcel Holtmann <marcel@holtmann.org>
> Reported-by: Brian Rogers <brian@xyzw.org>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>  drivers/hid/hid-core.c        |   11 +++++++++++
>  drivers/hid/usbhid/hid-core.c |   16 +++++-----------
>  include/linux/hid.h           |    3 +++
>  net/bluetooth/hidp/core.c     |    7 -------
>  4 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index ca9bb26..be34d32 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1237,6 +1237,17 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
>  }
>  EXPORT_SYMBOL_GPL(hid_connect);
>  
> +void hid_disconnect(struct hid_device *hdev)
> +{
> +	if (hdev->claimed & HID_CLAIMED_INPUT)
> +		hidinput_disconnect(hdev);
> +	if (hdev->claimed & HID_CLAIMED_HIDDEV)
> +		hdev->hiddev_disconnect(hdev);
> +	if (hdev->claimed & HID_CLAIMED_HIDRAW)
> +		hidraw_disconnect(hdev);
> +}
> +EXPORT_SYMBOL_GPL(hid_disconnect);
> +
>  /* a list of devices for which there is a specialized driver on HID bus */
>  static const struct hid_device_id hid_blacklist[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_WCP32PU) },
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 1b0e07a..03bd703 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1041,13 +1041,6 @@ static void usbhid_stop(struct hid_device *hid)
>  
>  	hid_cancel_delayed_stuff(usbhid);
>  
> -	if (hid->claimed & HID_CLAIMED_INPUT)
> -		hidinput_disconnect(hid);
> -	if (hid->claimed & HID_CLAIMED_HIDDEV)
> -		hiddev_disconnect(hid);
> -	if (hid->claimed & HID_CLAIMED_HIDRAW)
> -		hidraw_disconnect(hid);
> -
>  	hid->claimed = 0;
>  
>  	usb_free_urb(usbhid->urbin);
> @@ -1085,7 +1078,7 @@ static struct hid_ll_driver usb_hid_driver = {
>  	.hidinput_input_event = usb_hidinput_input_event,
>  };
>  
> -static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  {
>  	struct usb_host_interface *interface = intf->cur_altsetting;
>  	struct usb_device *dev = interface_to_usbdev(intf);
> @@ -1117,6 +1110,7 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  	hid->ff_init = hid_pidff_init;
>  #ifdef CONFIG_USB_HIDDEV
>  	hid->hiddev_connect = hiddev_connect;
> +	hid->hiddev_disconnect = hiddev_disconnect;
>  	hid->hiddev_hid_event = hiddev_hid_event;
>  	hid->hiddev_report_event = hiddev_report_event;
>  #endif
> @@ -1177,7 +1171,7 @@ err:
>  	return ret;
>  }
>  
> -static void hid_disconnect(struct usb_interface *intf)
> +static void usbhid_disconnect(struct usb_interface *intf)
>  {
>  	struct hid_device *hid = usb_get_intfdata(intf);
>  	struct usbhid_device *usbhid;
> @@ -1359,8 +1353,8 @@ MODULE_DEVICE_TABLE (usb, hid_usb_ids);
>  
>  static struct usb_driver hid_driver = {
>  	.name =		"usbhid",
> -	.probe =	hid_probe,
> -	.disconnect =	hid_disconnect,
> +	.probe =	usbhid_probe,
> +	.disconnect =	usbhid_disconnect,
>  #ifdef CONFIG_PM
>  	.suspend =	hid_suspend,
>  	.resume =	hid_resume,
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index a0ebdac..10f6284 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -494,6 +494,7 @@ struct hid_device {							/* device report descriptor */
>  
>  	/* hiddev event handler */
>  	int (*hiddev_connect)(struct hid_device *, unsigned int);
> +	void (*hiddev_disconnect)(struct hid_device *);
>  	void (*hiddev_hid_event) (struct hid_device *, struct hid_field *field,
>  				  struct hid_usage *, __s32);
>  	void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
> @@ -691,6 +692,7 @@ struct hid_device *hid_allocate_device(void);
>  int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
>  int hid_check_keys_pressed(struct hid_device *hid);
>  int hid_connect(struct hid_device *hid, unsigned int connect_mask);
> +void hid_disconnect(struct hid_device *hid);
>  
>  /**
>   * hid_map_usage - map usage input bits
> @@ -800,6 +802,7 @@ static inline int __must_check hid_hw_start(struct hid_device *hdev,
>   */
>  static inline void hid_hw_stop(struct hid_device *hdev)
>  {
> +	hid_disconnect(hdev);
>  	hdev->ll_driver->stop(hdev);
>  }
>  
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 09bedeb..49d8495 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -577,11 +577,6 @@ static int hidp_session(void *arg)
>  	}
>  
>  	if (session->hid) {
> -		if (session->hid->claimed & HID_CLAIMED_INPUT)
> -			hidinput_disconnect(session->hid);
> -		if (session->hid->claimed & HID_CLAIMED_HIDRAW)
> -			hidraw_disconnect(session->hid);
> -
>  		hid_destroy_device(session->hid);
>  		session->hid = NULL;
>  	}
> @@ -747,8 +742,6 @@ static void hidp_stop(struct hid_device *hid)
>  	skb_queue_purge(&session->ctrl_transmit);
>  	skb_queue_purge(&session->intr_transmit);
>  
> -	if (hid->claimed & HID_CLAIMED_INPUT)
> -		hidinput_disconnect(hid);
>  	hid->claimed = 0;
>  }

is the hid->claimed = 0 still necessary here? Not that it seems to
really matter, but it would be much cleaner if the HID transport driver
doesn't have to touch these details.

Regards

Marcel