hid-magicmouse: Fix oops after device removal.

PATCH Superseded Michael Poole 3 participants 6 messages Mar 9, 2010 HID

lore

Share & tools

lore

Michael Poole

author

2010-03-09 00:38 UTC #0 lore raw

PATCH 1/1 hid-magicmouse: Fix oops after device removal.

Quoted 1 line

From 57c6af4c2dadcce71566995baa15593103ab33aa Mon Sep 17 00:00:00 2001

From: Michael Poole <[email protected]>
Date: Mon, 8 Mar 2010 19:09:26 -0500
Subject: [PATCH 1/1] hid-magicmouse: Fix oops after device removal.

Ask the HID core not to register an input device for the mouse. Fix an oops after removing the device, due to leaving the new input device registered.

Signed-off-by Michael Poole <[email protected]>

---
 drivers/hid/hid-magicmouse.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

drivers/hid/hid-magicmouse.c +4 -1

index 4a3a94f..2e7d701 100644
@@ -353,7 +353,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 		goto err_free;
 	}
 
-	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
 	if (ret) {
 		dev_err(&hdev->dev, "magicmouse hw start failed\n");
 		goto err_free;

@@ -409,7 +409,10 @@ err_free:
 
 static void magicmouse_remove(struct hid_device *hdev)
 {
+	struct magicmouse_sc *msc;
+	msc = hid_get_drvdata(hdev);
 	hid_hw_stop(hdev);
+	input_unregister_device(msc->input);
 	kfree(hid_get_drvdata(hdev));
 }

Signature · 1 line

1.7.0.2

Dmitry Torokhov

2010-03-09 07:48 UTC #1 lore raw

Quoted 28 lines · Michael Poole

From 57c6af4c2dadcce71566995baa15593103ab33aa Mon Sep 17 00:00:00 2001
From: Michael Poole <[email protected]>
Date: Mon, 8 Mar 2010 19:09:26 -0500
Subject: [PATCH 1/1] hid-magicmouse: Fix oops after device removal.
Ask the HID core not to register an input device for the mouse. Fix an oops after removing the device, due to leaving the new input device registered.
Signed-off-by Michael Poole <[email protected]>
---
 drivers/hid/hid-magicmouse.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/hid/hid-magicmouse.c +1 -1
index 4a3a94f..2e7d701 100644
@@ -353,7 +353,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 		goto err_free;
 	}
 
-	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
 	if (ret) {
 		dev_err(&hdev->dev, "magicmouse hw start failed\n");
 		goto err_free;

drivers/hid/hid-magicmouse.c err_free: · line 412

Quoted 4 lines

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

@@ -409,7 +409,10 @@ err_free:
 
 static void magicmouse_remove(struct hid_device *hdev)
 {
+	struct magicmouse_sc *msc;

Blank lines between variable definitions and code are always appreciated. but also coudl be written as:

	struct magicmouse_sc *msc = hid_get_drvdata(dev);

drivers/hid/hid-magicmouse.c err_free: · line 416

Quoted 4 lines

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

+	msc = hid_get_drvdata(hdev);
 	hid_hw_stop(hdev);
+	input_unregister_device(msc->input);
 	kfree(hid_get_drvdata(hdev));

You have msc already sho that should be 'kfree(msc);'.

Quoted 2 lines

drivers/hid/hid-magicmouse.c +0 -0
 }
 

Signature · 1 line

Dmitry

Michael Poole

author

2010-03-09 11:32 UTC #2 lore raw

Quoted 7 lines · Dmitry Torokhov

drivers/hid/hid-magicmouse.c err_free: · line 412
Quoted 3 lines
static void magicmouse_remove(struct hid_device *hdev) {
drivers/hid/hid-magicmouse.c +1 -0
+	struct magicmouse_sc *msc;
Blank lines between variable definitions and code are always appreciated. but also coudl be written as:

Quoted 8 lines

	struct magicmouse_sc *msc = hid_get_drvdata(dev);
drivers/hid/hid-magicmouse.c err_free: · line 416
Quoted 4 lines
drivers/hid/hid-magicmouse.c +2 -0
+	msc = hid_get_drvdata(hdev);
 	hid_hw_stop(hdev);
+	input_unregister_device(msc->input);
 	kfree(hid_get_drvdata(hdev));
You have msc already sho that should be 'kfree(msc);'.

I'll fix both of these and resubmit. Thanks for the review.

Michael

Michael Poole

author

2010-03-09 11:47 UTC #3 lore raw

PATCH v2 1/1 hid-magicmouse: Fix oops after device removal.

In reply to Dmitry Torokhov

Quoted 1 line

From 7466c15f196621c44b01e3e1f8abd61099e48989 Mon Sep 17 00:00:00 2001

From: Michael Poole <[email protected]>
Date: Tue, 9 Mar 2010 06:46:09 -0500
Subject: [PATCH v2] hid-magicmouse: Fix oops after device removal.

Ask the HID core not to register an input device for the mouse. Fix an oops after removing the device, due to leaving the new input device registered.

Signed-off-by Michael Poole <[email protected]>

---
 drivers/hid/hid-magicmouse.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

drivers/hid/hid-magicmouse.c +5 -2

index 4a3a94f..c174b64 100644
@@ -353,7 +353,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 		goto err_free;
 	}
 
-	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
 	if (ret) {
 		dev_err(&hdev->dev, "magicmouse hw start failed\n");
 		goto err_free;

@@ -409,8 +409,11 @@ err_free:
 
 static void magicmouse_remove(struct hid_device *hdev)
 {
+	struct magicmouse_sc *msc = hid_get_drvdata(hdev);
+
 	hid_hw_stop(hdev);
-	kfree(hid_get_drvdata(hdev));
+	input_unregister_device(msc->input);
+	kfree(msc);
 }
 
 static const struct hid_device_id magic_mice[] = {

Signature · 1 line

1.7.0.2

Jiri Kosina

Applied

2010-03-09 12:37 UTC #4 lore raw

Quoted 25 lines · Michael Poole

Ask the HID core not to register an input device for the mouse. Fix an oops after removing the device, due to leaving the new input device registered.

Signed-off-by Michael Poole <[email protected]>

---
 drivers/hid/hid-magicmouse.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

drivers/hid/hid-magicmouse.c +1 -1

index 4a3a94f..c174b64 100644
@@ -353,7 +353,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 		goto err_free;
 	}
 
-	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
 	if (ret) {
 		dev_err(&hdev->dev, "magicmouse hw start failed\n");
 		goto err_free;

@@ -409,8 +409,11 @@ err_free:
 
 static void magicmouse_remove(struct hid_device *hdev)

drivers/hid/hid-magicmouse.c err_free: · line 419

Quoted 10 lines

drivers/hid/hid-magicmouse.c +4 -1

 {
+	struct magicmouse_sc *msc = hid_get_drvdata(hdev);
+
 	hid_hw_stop(hdev);
-	kfree(hid_get_drvdata(hdev));
+	input_unregister_device(msc->input);
+	kfree(msc);
 }
 
 static const struct hid_device_id magic_mice[] = {

Applied, thank you Michael.

Signature · 2 lines

Jiri Kosina
SUSE Labs, Novell Inc.

Dmitry Torokhov

2010-03-09 17:36 UTC #5 lore raw

In reply to Michael Poole

Quoted 10 lines · Michael Poole

From 7466c15f196621c44b01e3e1f8abd61099e48989 Mon Sep 17 00:00:00 2001
From: Michael Poole <[email protected]>
Date: Tue, 9 Mar 2010 06:46:09 -0500
Subject: [PATCH v2] hid-magicmouse: Fix oops after device removal.
Ask the HID core not to register an input device for the mouse. Fix an oops after removing the device, due to leaving the new input device registered.
Signed-off-by Michael Poole <[email protected]>

Yep, looks good, thank you for making the change.

Quoted 33 lines

---
 drivers/hid/hid-magicmouse.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

drivers/hid/hid-magicmouse.c +5 -2

index 4a3a94f..c174b64 100644
@@ -353,7 +353,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 		goto err_free;
 	}
 
-	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
 	if (ret) {
 		dev_err(&hdev->dev, "magicmouse hw start failed\n");
 		goto err_free;

@@ -409,8 +409,11 @@ err_free:
 
 static void magicmouse_remove(struct hid_device *hdev)
 {
+	struct magicmouse_sc *msc = hid_get_drvdata(hdev);
+
 	hid_hw_stop(hdev);
-	kfree(hid_get_drvdata(hdev));
+	input_unregister_device(msc->input);
+	kfree(msc);
 }
 
 static const struct hid_device_id magic_mice[] = {

Signature · 1 line

1.7.0.2

Signature · 1 line

Dmitry