usb: kbd: Fix key repeat not always using

PATCH Superseded Hans de Goede 2 participants 3 messages May 13, 2015 – May 19, 2015

Share & tools

Hans de Goede

author

2015-05-13 12:47 UTC #0 lore raw

PATCH v2 1/1 usb: kbd: Fix key repeat not always using

The usb-kbd key repeat code assumes that reports get repeated every 40 ms, this is never true when using CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP, and does not always works for CONFIG_SYS_USB_EVENT_POLL and CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE since not all usb keyboards honor the usb_set_idle() command.

For CONFIG_SYS_USB_EVENT_POLL we must use usb_set_idle() since we do a blocking wait for the hid report, so if we do not tell the keyboard to send a hid report every 40ms even if nothing changes then we will block u-boot for 1s (the default u-boot usb interrupt packet timeout). Note that in this case on keyboards which do not support usb_set_idle() we loose and we actually get 1s latencies on other u-boot activities.

For the other poll-methods this commit stops using usb_set_idle() and instead repeats the last received hid-report every 40 ms as long as no new hid-report is received. This fixes key-repeat not working at all with CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP and fixes it not working with keyboards which do not implement usb_set_idle() when using CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE.

Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:
-Change last_report type to unsigned long to match the get_timer return type

---
 common/usb_kbd.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

common/usb_kbd.c +17 -5

index 24a1a56..e8d108c 100644
@@ -31,7 +31,7 @@ int overwrite_console(void)
 #endif
 
 /* Keyboard sampling rate */
-#define REPEAT_RATE	(40 / 4)	/* 40msec -> 25cps */
+#define REPEAT_RATE	40		/* 40msec -> 25cps */
 #define REPEAT_DELAY	10		/* 10 x REPEAT_RATE = 400msec */
 
 #define NUM_LOCK	0x53

@@ -103,6 +103,7 @@ struct usb_kbd_pdata {
 	unsigned long	intpipe;
 	int		intpktsize;
 	int		intinterval;
+	unsigned long	last_report;
 	struct int_queue *intq;
 
 	uint32_t	repeat_delay;

@@ -318,15 +319,16 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev)
 			   data->intinterval);
 
 	usb_kbd_irq_worker(dev);
-#elif	defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP)
+#else
+# if	defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP)
 	struct usb_interface *iface;
 	struct usb_kbd_pdata *data = dev->privptr;
 	iface = &dev->config.if_desc[0];
 	usb_get_report(dev, iface->desc.bInterfaceNumber,
 		       1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE);
-	if (memcmp(data->old, data->new, USB_KBD_BOOT_REPORT_SIZE))
+	if (memcmp(data->old, data->new, USB_KBD_BOOT_REPORT_SIZE)) {
 		usb_kbd_irq_worker(dev);
-#elif	defined(CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE)
+# elif	defined(CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE)
 	struct usb_kbd_pdata *data = dev->privptr;
 	if (poll_int_queue(dev, data->intq)) {
 		usb_kbd_irq_worker(dev);

@@ -335,6 +337,13 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev)
 		data->intq = create_int_queue(dev, data->intpipe, 1,
 				      USB_KBD_BOOT_REPORT_SIZE, data->new,
 				      data->intinterval);
+# endif
+		data->last_report = get_timer(0);
+	/* Repeat last usb hid report every REPEAT_RATE ms for keyrepeat */
+	} else if (data->last_report != -1 &&
+		   get_timer(data->last_report) > REPEAT_RATE) {
+		usb_kbd_irq_worker(dev);
+		data->last_report = get_timer(0);
 	}
 #endif
 }

@@ -445,12 +454,15 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum)
 	data->intpktsize = min(usb_maxpacket(dev, data->intpipe),
 			       USB_KBD_BOOT_REPORT_SIZE);
 	data->intinterval = ep->bInterval;
+	data->last_report = -1;
 
 	/* We found a USB Keyboard, install it. */
 	usb_set_protocol(dev, iface->desc.bInterfaceNumber, 0);
 
+#ifdef CONFIG_SYS_USB_EVENT_POLL
 	debug("USB KBD: found set idle...\n");
-	usb_set_idle(dev, iface->desc.bInterfaceNumber, REPEAT_RATE, 0);
+	usb_set_idle(dev, iface->desc.bInterfaceNumber, REPEAT_RATE / 4, 0);
+#endif
 
 	debug("USB KBD: enable interrupt pipe...\n");
 #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE

Signature · 1 line

2.4.0

Marek Vasut

Applied

2015-05-13 20:02 UTC #1 lore raw

PATCH v2 1/1 usb: kbd: Fix key repeat not always using

Quoted 13 lines · Hans de Goede

The usb-kbd key repeat code assumes that reports get repeated every 40 ms, this is never true when using CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP, and does not always works for CONFIG_SYS_USB_EVENT_POLL and CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE since not all usb keyboards honor the usb_set_idle() command.
For CONFIG_SYS_USB_EVENT_POLL we must use usb_set_idle() since we do a blocking wait for the hid report, so if we do not tell the keyboard to send a hid report every 40ms even if nothing changes then we will block u-boot for 1s (the default u-boot usb interrupt packet timeout). Note that in this case on keyboards which do not support usb_set_idle() we loose and we actually get 1s latencies on other u-boot activities.

Quoted 8 lines

For the other poll-methods this commit stops using usb_set_idle() and instead repeats the last received hid-report every 40 ms as long as no new hid-report is received. This fixes key-repeat not working at all with CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP and fixes it not working with keyboards which do not implement usb_set_idle() when using CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE.
Signed-off-by Hans de Goede <[email protected]>

Woohoo, it's been a while since I picked some USB patches! ;-)

Applied, thanks!

Best regards,
Marek Vasut

Marek Vasut

2015-05-19 17:56 UTC #2 lore raw

PATCH v2 1/1 usb: kbd: Fix key repeat not always using

In reply to Hans de Goede

Quoted 13 lines · Hans de Goede

The usb-kbd key repeat code assumes that reports get repeated every 40 ms, this is never true when using CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP, and does not always works for CONFIG_SYS_USB_EVENT_POLL and CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE since not all usb keyboards honor the usb_set_idle() command.
For CONFIG_SYS_USB_EVENT_POLL we must use usb_set_idle() since we do a blocking wait for the hid report, so if we do not tell the keyboard to send a hid report every 40ms even if nothing changes then we will block u-boot for 1s (the default u-boot usb interrupt packet timeout). Note that in this case on keyboards which do not support usb_set_idle() we loose and we actually get 1s latencies on other u-boot activities.

Quoted 8 lines

For the other poll-methods this commit stops using usb_set_idle() and instead repeats the last received hid-report every 40 ms as long as no new hid-report is received. This fixes key-repeat not working at all with CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP and fixes it not working with keyboards which do not implement usb_set_idle() when using CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE.
Signed-off-by Hans de Goede <[email protected]>

Hi, this broke a couple of machines, quoting Tom. I'm dropping it for now.

NAK. This introduces various breakage:
12: Merge branch 'master' of git://git.denx.de/u-boot-usb

       arm:  +   VCMA9 beagle_x15 at91rm9200ek_ram peach-pi snow smdk5250

am43xx_evm dr
a7xx_evm_uart3 k2l_evm am43xx_evm_qspiboot smdk5420 dra7xx_evm smdk2410 at91rm9200ek dr
a7xx_evm_qspiboot k2hk_evm k2e_evm peach-pit
powerpc: + PIP405 MIP405
The powerpc board breakage is the same as some of the ARM ones so fix it once and they'll come along.

Best regards,
Marek Vasut