Skip to contentSkip to conversation

HID: hid-sensor-hub: Remove setting hdev->claimed before calling hid_hw_start()

PATCH RESEND In review Axel Lin 3 participants 4 messages Sep 18, 2012 – Sep 19, 2012

Subsystems (2)
HID HID sensor hub drivers
lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH RESEND 1/2 HID: hid-sensor-hub: Remove setting hdev->claimed before calling hid_hw_start()

Current implementation of hid_hw_start() allows connect_mask to be 0. Setting hdev->claimed = HID_CLAIMED_INPUT before calling hid_hw_start() is not necessary. Remove it.

Signed-off-by Axel Lin <[email protected]>
---
 drivers/hid/hid-sensor-hub.c |    2 --
 1 file changed, 2 deletions(-)
drivers/hid/hid-sensor-hub.c +0 -2
index 4ac759c..1faacf2 100644
@@ -539,7 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	}
 	INIT_LIST_HEAD(&hdev->inputs);
 
-	hdev->claimed = HID_CLAIMED_INPUT;
 	ret = hid_hw_start(hdev, 0);
 	if (ret) {
 		hid_err(hdev, "hw start failed\n");

@@ -627,7 +626,6 @@ static void sensor_hub_remove(struct hid_device *hdev)
 	int i;
 
 	hid_dbg(hdev, " hardware removed\n");
-	hdev->claimed &= ~HID_CLAIMED_INPUT;
 	hid_hw_stop(hdev);
 	hid_hw_close(hdev);
 	spin_lock_irqsave(&data->lock, flags);
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH v2 2/2 HID: hid-sensor-hub: Fix sensor_hub_probe error handling

Fix below issues:
1. In the case of goto err_close, hid_hw_stop(hdev) is called twice. Fix it.
2. If fails to allocate MFD device name, we also need to free all successfully allocated names in previous iterations.
3. In sensor_hub_remove(), Call hid_hw_close() before hid_hw_stop().
4. Adjust unnecessary change lines for hid_err.

Signed-off-by Axel Lin <[email protected]>
Acked-by Jiri Kosina <[email protected]>
---
 drivers/hid/hid-sensor-hub.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/hid/hid-sensor-hub.c +4 -8
index 1faacf2..b9ce38e 100644
@@ -564,8 +564,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
 						sizeof(struct mfd_cell),
 						GFP_KERNEL);
 	if (sd->hid_sensor_hub_client_devs == NULL) {
-		hid_err(hdev,
-			"Failed to allocate memory for mfd cells\n");
+		hid_err(hdev, "Failed to allocate memory for mfd cells\n");
 			ret = -ENOMEM;
 			goto err_close;
 	}

@@ -577,10 +576,9 @@ static int sensor_hub_probe(struct hid_device *hdev,
 			name = kasprintf(GFP_KERNEL, "HID-SENSOR-%x",
 						field->physical);
 			if (name  == NULL) {
-				hid_err(hdev,
-					"Failed MFD device name\n");
+				hid_err(hdev, "Failed MFD device name\n");
 					ret = -ENOMEM;
-					goto err_free_cells;
+					goto err_free_names;
 			}
 			sd->hid_sensor_hub_client_devs[
 				sd->hid_sensor_client_cnt].name = name;

@@ -604,10 +602,8 @@ static int sensor_hub_probe(struct hid_device *hdev,
 err_free_names:
 	for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
 		kfree(sd->hid_sensor_hub_client_devs[i].name);
-err_free_cells:
 	kfree(sd->hid_sensor_hub_client_devs);
 err_close:
-	hid_hw_stop(hdev);
 	hid_hw_close(hdev);
 err_stop_hw:
 	hid_hw_stop(hdev);

@@ -626,8 +622,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
 	int i;
 
 	hid_dbg(hdev, " hardware removed\n");
-	hid_hw_stop(hdev);
 	hid_hw_close(hdev);
+	hid_hw_stop(hdev);
 	spin_lock_irqsave(&data->lock, flags);
 	if (data->pending.status)
 		complete(&data->pending.ready);
Signature · 1 line
1.7.9.5
#2 lore raw
In reply to Axel Lin
Quoted 5 lines · Axel Lin

Current implementation of hid_hw_start() allows connect_mask to be 0. Setting hdev->claimed = HID_CLAIMED_INPUT before calling hid_hw_start() is not necessary. Remove it.

Signed-off-by Axel Lin <[email protected]>
Signed-off-by Jiri Kosina <[email protected]>

Jonathan, please take it on top of the other changes.

Thanks.

Quoted 29 lines
---
 drivers/hid/hid-sensor-hub.c |    2 --
 1 file changed, 2 deletions(-)
drivers/hid/hid-sensor-hub.c +0 -2
index 4ac759c..1faacf2 100644
@@ -539,7 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	}
 	INIT_LIST_HEAD(&hdev->inputs);
 
-	hdev->claimed = HID_CLAIMED_INPUT;
 	ret = hid_hw_start(hdev, 0);
 	if (ret) {
 		hid_err(hdev, "hw start failed\n");

@@ -627,7 +626,6 @@ static void sensor_hub_remove(struct hid_device *hdev)
 	int i;
 
 	hid_dbg(hdev, " hardware removed\n");
-	hdev->claimed &= ~HID_CLAIMED_INPUT;
 	hid_hw_stop(hdev);
 	hid_hw_close(hdev);
 	spin_lock_irqsave(&data->lock, flags);
Signature · 1 line
1.7.9.5
Signature · 2 lines
Jiri Kosina
SUSE Labs
#3 lore raw

I felt subject line was too long. Minor change in subject line. Tested and added my ack for both of these patches.

Thanks,
Srinivas

-----Original Message-----
From: Jiri Kosina [mailto:[email protected]]
Sent: Wednesday, September 19, 2012 4:46 AM
To: Axel Lin
Cc: Jonathan Cameron; Pandruvada, Srinivas; [email protected]
Subject: Re: [PATCH RESEND 1/2] HID: hid-sensor-hub: Remove setting hdev->claimed before calling hid_hw_start()

Quoted 5 lines · Axel Lin

Current implementation of hid_hw_start() allows connect_mask to be 0. Setting hdev->claimed = HID_CLAIMED_INPUT before calling hid_hw_start() is not necessary. Remove it.

Signed-off-by Axel Lin <[email protected]>
Signed-off-by Jiri Kosina <[email protected]>

Jonathan, please take it on top of the other changes.

Thanks.

Quoted 25 lines
---
 drivers/hid/hid-sensor-hub.c |    2 --
 1 file changed, 2 deletions(-)
(quoted diff) +0 -0

b/drivers/hid/hid-sensor-hub.c index 4ac759c..1faacf2 100644

drivers/hid/hid-sensor-hub.c +0 -1
@@ -539,7 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	}
 	INIT_LIST_HEAD(&hdev->inputs);
 
-	hdev->claimed = HID_CLAIMED_INPUT;
 	ret = hid_hw_start(hdev, 0);
 	if (ret) {
 		hid_err(hdev, "hw start failed\n"); @@ -627,7 +626,6 @@ static void

sensor_hub_remove(struct hid_device *hdev)

drivers/hid/hid-sensor-hub.c +0 -2
 	int i;
 
 	hid_dbg(hdev, " hardware removed\n");
-	hdev->claimed &= ~HID_CLAIMED_INPUT;
 	hid_hw_stop(hdev);
 	hid_hw_close(hdev);
 	spin_lock_irqsave(&data->lock, flags);
--
Quoted 4 lines

1.7.9.5

--
Jiri Kosina
SUSE Labs