[PATCH] usb: Keep track of actual LED state

Subsystems: hid core layer, the rest, usb hid/hidbp drivers (usb keyboards, mice, remote controls, ...)

STALE5417d

2 messages, 2 authors, 2011-10-10 · open the first message on its own page

[PATCH] usb: Keep track of actual LED state

From: Matthew Garrett <hidden>
Date: 2011-10-10 15:55:00

The usbhid autosuspend code refuses to suspend if there are any LEDs lit,
in order to avoid situations where this would draw more power than
available when suspended. The current implementation is a counter that's
incremented on LED on and decremented on LED off. However, if a system
suspend takes place, the input core will resynchronise state on resume by
sending LED on and off requests. This may cause the counter to underflow,
resulting in autosuspend being disabled for the rest of system uptime.
This patch simply replaces the counter with a bitmask of the actual LED
state in order to avoid this.

Signed-off-by: Matthew Garrett <redacted>
---
 drivers/hid/usbhid/hid-core.c |    6 +++---
 drivers/hid/usbhid/usbhid.h   |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index ad978f5..fd96d38 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -598,11 +598,11 @@ static int usb_hidinput_input_event(struct input_dev *dev, unsigned int type, un
 	hid_set_field(field, offset, value);
 	if (value) {
 		spin_lock_irqsave(&usbhid->lock, flags);
-		usbhid->ledcount++;
+		usbhid->ledstate |= (1 << code);
 		spin_unlock_irqrestore(&usbhid->lock, flags);
 	} else {
 		spin_lock_irqsave(&usbhid->lock, flags);
-		usbhid->ledcount--;
+		usbhid->ledstate &= ~(1 << code);
 		spin_unlock_irqrestore(&usbhid->lock, flags);
 	}
 	usbhid_submit_report(hid, field->report, USB_DIR_OUT);
@@ -1339,7 +1339,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
 		    && !test_bit(HID_OUT_RUNNING, &usbhid->iofl)
 		    && !test_bit(HID_CTRL_RUNNING, &usbhid->iofl)
 		    && !test_bit(HID_KEYS_PRESSED, &usbhid->iofl)
-		    && (!usbhid->ledcount || ignoreled))
+		    && (!usbhid->ledstate || ignoreled))
 		{
 			set_bit(HID_REPORTED_IDLE, &usbhid->iofl);
 			spin_unlock_irq(&usbhid->lock);
diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
index 1673cac..1d0e2bd 100644
--- a/drivers/hid/usbhid/usbhid.h
+++ b/drivers/hid/usbhid/usbhid.h
@@ -96,7 +96,7 @@ struct usbhid_device {
 	unsigned int retry_delay;                                       /* Delay length in ms */
 	struct work_struct reset_work;                                  /* Task context for resets */
 	wait_queue_head_t wait;						/* For sleeping */
-	int ledcount;							/* counting the number of active leds */
+	int ledstate;							/* Bitmask of active LEDs */
 };
 
 #define	hid_to_usb_dev(hid_dev) \
-- 
1.7.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: Keep track of actual LED state

From: Henrik Rydberg <hidden>
Date: 2011-10-10 17:06:37

Hi Matthew,
quoted hunk
The usbhid autosuspend code refuses to suspend if there are any LEDs lit,
in order to avoid situations where this would draw more power than
available when suspended. The current implementation is a counter that's
incremented on LED on and decremented on LED off. However, if a system
suspend takes place, the input core will resynchronise state on resume by
sending LED on and off requests. This may cause the counter to underflow,
resulting in autosuspend being disabled for the rest of system uptime.
This patch simply replaces the counter with a bitmask of the actual LED
state in order to avoid this.

Signed-off-by: Matthew Garrett <redacted>
---
 drivers/hid/usbhid/hid-core.c |    6 +++---
 drivers/hid/usbhid/usbhid.h   |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index ad978f5..fd96d38 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -598,11 +598,11 @@ static int usb_hidinput_input_event(struct input_dev *dev, unsigned int type, un
 	hid_set_field(field, offset, value);
 	if (value) {
 		spin_lock_irqsave(&usbhid->lock, flags);
-		usbhid->ledcount++;
+		usbhid->ledstate |= (1 << code);
To be consistent, ledstate should be a bitmask the same size as in
input_dev, but it aint pretty...
 		spin_unlock_irqrestore(&usbhid->lock, flags);
 	} else {
 		spin_lock_irqsave(&usbhid->lock, flags);
-		usbhid->ledcount--;
+		usbhid->ledstate &= ~(1 << code);
Could simply check for ledcount > 0 here instead, but it is even uglier.
quoted hunk
 		spin_unlock_irqrestore(&usbhid->lock, flags);
 	}
 	usbhid_submit_report(hid, field->report, USB_DIR_OUT);
@@ -1339,7 +1339,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
 		    && !test_bit(HID_OUT_RUNNING, &usbhid->iofl)
 		    && !test_bit(HID_CTRL_RUNNING, &usbhid->iofl)
 		    && !test_bit(HID_KEYS_PRESSED, &usbhid->iofl)
-		    && (!usbhid->ledcount || ignoreled))
+		    && (!usbhid->ledstate || ignoreled))
Why not test the actual input_dev state here instead? Adding a set/get
led state function to input core could resolve this cleanly.
quoted hunk
 		{
 			set_bit(HID_REPORTED_IDLE, &usbhid->iofl);
 			spin_unlock_irq(&usbhid->lock);
diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
index 1673cac..1d0e2bd 100644
--- a/drivers/hid/usbhid/usbhid.h
+++ b/drivers/hid/usbhid/usbhid.h
@@ -96,7 +96,7 @@ struct usbhid_device {
 	unsigned int retry_delay;                                       /* Delay length in ms */
 	struct work_struct reset_work;                                  /* Task context for resets */
 	wait_queue_head_t wait;						/* For sleeping */
-	int ledcount;							/* counting the number of active leds */
+	int ledstate;							/* Bitmask of active LEDs */
 };
 
 #define	hid_to_usb_dev(hid_dev) \
-- 
1.7.6.4
Cheers,
Henrik
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help