All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07
@ 2015-06-18 20:34 Hans de Goede
  2015-06-18 20:34 ` [U-Boot] [PATCH 1/2] usb: ehci: Properly deal with data toggle for interrupt endpoints Hans de Goede
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Hans de Goede @ 2015-06-18 20:34 UTC (permalink / raw)
  To: u-boot

Hi Marek,

While working on enabling the musb device-model support on more sunxi boards,
I noticed a problem with usb-keyboards when plugged into an usb-2 hub and
thus connected via the ehci code.

In the scenario of a usb-kbd connected to an ehci controller, combined
with using CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE (*), an ehci code bug
triggers which causes every other usb interrupt packet to get lost due
to a data toggle mismatch. We've had this bug for a long time, loosing
the key release packets most of the time, which did not matter, until
my recent usb-kbd keyrepeat changes.

The first patch in this series fixes this, the second patch is a
better safe then sorry patch I wrote while working on this.

Can you please queue both of these up for merging into v2015.07 ?

Thanks & Regards,

Hans


*) Which AFAIK sofar is only used by sunxi...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH 1/2] usb: ehci: Properly deal with data toggle for interrupt endpoints
  2015-06-18 20:34 [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07 Hans de Goede
@ 2015-06-18 20:34 ` Hans de Goede
  2015-06-18 20:34 ` [U-Boot] [PATCH 2/2] usb: kbd: Disable idle input reports when we do not need them Hans de Goede
  2015-06-19 12:34 ` [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07 Marek Vasut
  2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2015-06-18 20:34 UTC (permalink / raw)
  To: u-boot

Without this we loose every other interrupt packet. We never noticed this
because with keyboards the packets which we were loosing would normally
be key release packets.

But now that we do keyrepeat in software instead of relying on the hid
idle functionality, missing a release will result in key repeat triggering.

This commit fixes this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-hcd.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 1e5a6e2..bf02221 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1214,6 +1214,7 @@ static int _ehci_submit_control_msg(struct usb_device *dev, unsigned long pipe,
 
 struct int_queue {
 	int elementsize;
+	unsigned long pipe;
 	struct QH *first;
 	struct QH *current;
 	struct QH *last;
@@ -1269,7 +1270,7 @@ static struct int_queue *_ehci_create_int_queue(struct usb_device *dev,
 {
 	struct ehci_ctrl *ctrl = ehci_get_ctrl(dev);
 	struct int_queue *result = NULL;
-	int i;
+	uint32_t i, toggle;
 
 	/*
 	 * Interrupt transfers requiring several transactions are not supported
@@ -1309,6 +1310,7 @@ static struct int_queue *_ehci_create_int_queue(struct usb_device *dev,
 		goto fail1;
 	}
 	result->elementsize = elementsize;
+	result->pipe = pipe;
 	result->first = memalign(USB_DMA_MINALIGN,
 				 sizeof(struct QH) * queuesize);
 	if (!result->first) {
@@ -1326,6 +1328,8 @@ static struct int_queue *_ehci_create_int_queue(struct usb_device *dev,
 	memset(result->first, 0, sizeof(struct QH) * queuesize);
 	memset(result->tds, 0, sizeof(struct qTD) * queuesize);
 
+	toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
+
 	for (i = 0; i < queuesize; i++) {
 		struct QH *qh = result->first + i;
 		struct qTD *td = result->tds + i;
@@ -1357,7 +1361,9 @@ static struct int_queue *_ehci_create_int_queue(struct usb_device *dev,
 		td->qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 		debug("communication direction is '%s'\n",
 		      usb_pipein(pipe) ? "in" : "out");
-		td->qt_token = cpu_to_hc32((elementsize << 16) |
+		td->qt_token = cpu_to_hc32(
+			QT_TOKEN_DT(toggle) |
+			(elementsize << 16) |
 			((usb_pipein(pipe) ? 1 : 0) << 8) | /* IN/OUT token */
 			0x80); /* active */
 		td->qt_buffer[0] =
@@ -1372,6 +1378,7 @@ static struct int_queue *_ehci_create_int_queue(struct usb_device *dev,
 		    cpu_to_hc32((td->qt_buffer[0] + 0x4000) & ~0xfff);
 
 		*buf = buffer + i * elementsize;
+		toggle ^= 1;
 	}
 
 	flush_dcache_range((unsigned long)buffer,
@@ -1426,6 +1433,8 @@ static void *_ehci_poll_int_queue(struct usb_device *dev,
 {
 	struct QH *cur = queue->current;
 	struct qTD *cur_td;
+	uint32_t token, toggle;
+	unsigned long pipe = queue->pipe;
 
 	/* depleted queue */
 	if (cur == NULL) {
@@ -1436,12 +1445,15 @@ static void *_ehci_poll_int_queue(struct usb_device *dev,
 	cur_td = &queue->tds[queue->current - queue->first];
 	invalidate_dcache_range((unsigned long)cur_td,
 				ALIGN_END_ADDR(struct qTD, cur_td, 1));
-	if (QT_TOKEN_GET_STATUS(hc32_to_cpu(cur_td->qt_token)) &
-			QT_TOKEN_STATUS_ACTIVE) {
-		debug("Exit poll_int_queue with no completed intr transfer. token is %x\n",
-		      hc32_to_cpu(cur_td->qt_token));
+	token = hc32_to_cpu(cur_td->qt_token);
+	if (QT_TOKEN_GET_STATUS(token) & QT_TOKEN_STATUS_ACTIVE) {
+		debug("Exit poll_int_queue with no completed intr transfer. token is %x\n", token);
 		return NULL;
 	}
+
+	toggle = QT_TOKEN_GET_DT(token);
+	usb_settoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe), toggle);
+
 	if (!(cur->qh_link & QH_LINK_TERMINATE))
 		queue->current++;
 	else
@@ -1452,7 +1464,7 @@ static void *_ehci_poll_int_queue(struct usb_device *dev,
 					       queue->elementsize));
 
 	debug("Exit poll_int_queue with completed intr transfer. token is %x at %p (first at %p)\n",
-	      hc32_to_cpu(cur_td->qt_token), cur, queue->first);
+	      token, cur, queue->first);
 	return cur->buffer;
 }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH 2/2] usb: kbd: Disable idle input reports when we do not need them
  2015-06-18 20:34 [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07 Hans de Goede
  2015-06-18 20:34 ` [U-Boot] [PATCH 1/2] usb: ehci: Properly deal with data toggle for interrupt endpoints Hans de Goede
@ 2015-06-18 20:34 ` Hans de Goede
  2015-06-19 12:34 ` [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07 Marek Vasut
  2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2015-06-18 20:34 UTC (permalink / raw)
  To: u-boot

When we're polling and thus handling key-repeat in software, make sure
to disable idle reports, some keyboards may have these enabled by default
messing up our software keyrepeat.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 49bfc09..e2af67d 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -460,10 +460,12 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum)
 	/* We found a USB Keyboard, install it. */
 	usb_set_protocol(dev, iface->desc.bInterfaceNumber, 0);
 
+	debug("USB KBD: found set idle...\n");
 #if !defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP) && \
     !defined(CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE)
-	debug("USB KBD: found set idle...\n");
 	usb_set_idle(dev, iface->desc.bInterfaceNumber, REPEAT_RATE / 4, 0);
+#else
+	usb_set_idle(dev, iface->desc.bInterfaceNumber, 0, 0);
 #endif
 
 	debug("USB KBD: enable interrupt pipe...\n");
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07
  2015-06-18 20:34 [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07 Hans de Goede
  2015-06-18 20:34 ` [U-Boot] [PATCH 1/2] usb: ehci: Properly deal with data toggle for interrupt endpoints Hans de Goede
  2015-06-18 20:34 ` [U-Boot] [PATCH 2/2] usb: kbd: Disable idle input reports when we do not need them Hans de Goede
@ 2015-06-19 12:34 ` Marek Vasut
  2 siblings, 0 replies; 4+ messages in thread
From: Marek Vasut @ 2015-06-19 12:34 UTC (permalink / raw)
  To: u-boot

On Thursday, June 18, 2015 at 10:34:32 PM, Hans de Goede wrote:
> Hi Marek,

Hi Hans,

> While working on enabling the musb device-model support on more sunxi
> boards, I noticed a problem with usb-keyboards when plugged into an usb-2
> hub and thus connected via the ehci code.
> 
> In the scenario of a usb-kbd connected to an ehci controller, combined
> with using CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE (*), an ehci code bug
> triggers which causes every other usb interrupt packet to get lost due
> to a data toggle mismatch. We've had this bug for a long time, loosing
> the key release packets most of the time, which did not matter, until
> my recent usb-kbd keyrepeat changes.
> 
> The first patch in this series fixes this, the second patch is a
> better safe then sorry patch I wrote while working on this.
> 
> Can you please queue both of these up for merging into v2015.07 ?

Applied both, thanks!

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-06-19 12:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-18 20:34 [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07 Hans de Goede
2015-06-18 20:34 ` [U-Boot] [PATCH 1/2] usb: ehci: Properly deal with data toggle for interrupt endpoints Hans de Goede
2015-06-18 20:34 ` [U-Boot] [PATCH 2/2] usb: kbd: Disable idle input reports when we do not need them Hans de Goede
2015-06-19 12:34 ` [U-Boot] [PATCH 0/2] usb: 2 keyboard fixes for v2015.07 Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.