From patchwork Sun Apr 6 11:17:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wolfgang Denk X-Patchwork-Id: 337255 X-Patchwork-Delegate: marek.vasut@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 9F90F1400D4 for ; Sun, 6 Apr 2014 21:18:06 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id A4B114B844; Sun, 6 Apr 2014 13:18:02 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id v9YwZlY5F9n7; Sun, 6 Apr 2014 13:18:02 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id C1D344B83C; Sun, 6 Apr 2014 13:17:59 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 148CD4B83C for ; Sun, 6 Apr 2014 13:17:55 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KwfnjONFJ85v for ; Sun, 6 Apr 2014 13:17:51 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-out.m-online.net (mail-out.m-online.net [212.18.0.9]) by theia.denx.de (Postfix) with ESMTPS id 575E84B83B for ; Sun, 6 Apr 2014 13:17:48 +0200 (CEST) Received: from frontend1.mail.m-online.net (unknown [192.168.8.180]) by mail-out.m-online.net (Postfix) with ESMTP id 3g1smq6Fvdz4KK47; Sun, 6 Apr 2014 13:17:47 +0200 (CEST) Received: from localhost (dynscan1.mnet-online.de [192.168.6.68]) by mail.m-online.net (Postfix) with ESMTP id 3g1smq64yyzbbcX; Sun, 6 Apr 2014 13:17:47 +0200 (CEST) X-Virus-Scanned: amavisd-new at mnet-online.de Received: from smtp-auth.mnet-online.de ([192.168.8.180]) by localhost (dynscan1.mail.m-online.net [192.168.6.68]) (amavisd-new, port 10024) with ESMTP id e4xi2CKkDuZN; Sun, 6 Apr 2014 13:17:46 +0200 (CEST) X-Auth-Info: LMAmpWqK0uGp29qpA7EmBPmuu+l5raSqBN3fzDdyXik= Received: from diddl.denx.de (host-80-81-18-216.customer.m-online.net [80.81.18.216]) by smtp-auth.mnet-online.de (Postfix) with ESMTPA; Sun, 6 Apr 2014 13:17:46 +0200 (CEST) Received: from gemini.denx.de (unknown [10.0.0.2]) by diddl.denx.de (Postfix) with ESMTP id 685EC1AAA1B; Sun, 6 Apr 2014 13:17:46 +0200 (CEST) Received: from gemini.denx.de (localhost [IPv6:::1]) by gemini.denx.de (Postfix) with ESMTP id 25D3538099A; Sun, 6 Apr 2014 13:17:46 +0200 (MEST) To: Adrian Cox From: Wolfgang Denk MIME-Version: 1.0 In-reply-to: <4526969.2646.1396715845133.JavaMail.adrian@Gurnard> References: <4526969.2646.1396715845133.JavaMail.adrian@Gurnard> Comments: In-reply-to Adrian Cox message dated "Sat, 05 Apr 2014 17:36:41 +0100." Date: Sun, 06 Apr 2014 13:17:46 +0200 Message-Id: <20140406111746.25D3538099A@gemini.denx.de> Cc: marex@denx.de, u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] Fix USB keyboard polling via control endpoint X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de Dear Adrian Cox, In message <4526969.2646.1396715845133.JavaMail.adrian@Gurnard> you wrote: > > USB keyboard polling failed for some keyboards on PowerPC 5020. > This was caused by requesting only 4 bytes of data from keyboards that > produce an 8 byte HID report. > > Signed-off-by: Adrian Cox > > --- > common/usb_kbd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/common/usb_kbd.c b/common/usb_kbd.c > index 1ad67ca..0f6c579 100644 > --- a/common/usb_kbd.c > +++ b/common/usb_kbd.c > @@ -341,8 +341,8 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) > struct usb_kbd_pdata *data = dev->privptr; > iface = &dev->config.if_desc[0]; > usb_get_report(dev, iface->desc.bInterfaceNumber, > - 1, 0, data->new, sizeof(data->new)); > - if (memcmp(data->old, data->new, sizeof(data->new))) > + 1, 0, data->new, 8); > + if (memcmp(data->old, data->new, 8)) > usb_kbd_irq_worker(dev); I agree that the code is wrong and needs fixing. data->new is an uint8_t pointer, so taking the size of the pointer is obviously wrong. But what you fix here is not the only place where sizeof(data->new) is used, so this patch fixes part of the problem at best. Can you please try out if the following extended version f the patch works and fixes your problem? You will note that I removed all occurrences of this magic number 8 by replacing it with USB_KBD_PDATA_SIZE so the could should also be easier to read. ------------------- snip ------------------- > From 315d78747a61330afe66b13747f03d74e60b8fcd Mon Sep 17 00:00:00 2001 > From: Wolfgang Denk Date: Sun, 6 Apr 2014 13:14:26 +0200 Subject: [PATCH] Fix USB keyboard polling via control endpoint USB keyboard polling failed for some keyboards on PowerPC 5020. This was caused by requesting only 4 bytes of data from keyboards that produce an 8 byte HID report. Signed-off-by: Adrian Cox Signed-off-by: Wolfgang Denk Cc: Marek Vasut --- common/usb_kbd.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 1ad67ca..c6692c5 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -91,6 +91,8 @@ static const unsigned char usb_kbd_arrow[] = { #define USB_KBD_LEDMASK \ (USB_KBD_NUMLOCK | USB_KBD_CAPSLOCK | USB_KBD_SCROLLLOCK) +#define USB_KBD_PDATA_SIZE 8 + struct usb_kbd_pdata { uint32_t repeat_delay; @@ -99,7 +101,7 @@ struct usb_kbd_pdata { uint8_t usb_kbd_buffer[USB_KBD_BUFFER_LEN]; uint8_t *new; - uint8_t old[8]; + uint8_t old[USB_KBD_PDATA_SIZE]; uint8_t flags; }; @@ -131,7 +133,9 @@ void usb_kbd_generic_poll(void) /* Submit a interrupt transfer request */ maxp = usb_maxpacket(usb_kbd_dev, pipe); usb_submit_int_msg(usb_kbd_dev, pipe, data->new, - maxp > 8 ? 8 : maxp, ep->bInterval); + maxp > USB_KBD_PDATA_SIZE ? + USB_KBD_PDATA_SIZE : maxp, + ep->bInterval); } /* Puts character in the queue and sets up the in and out pointer. */ @@ -266,8 +270,10 @@ static uint32_t usb_kbd_service_key(struct usb_device *dev, int i, int up) old = data->old; } - if ((old[i] > 3) && (memscan(new + 2, old[i], 6) == new + 8)) + if ((old[i] > 3) && + (memscan(new + 2, old[i], 6) == new + USB_KBD_PDATA_SIZE)) { res |= usb_kbd_translate(data, old[i], data->new[0], up); + } return res; } @@ -285,7 +291,7 @@ static int usb_kbd_irq_worker(struct usb_device *dev) else if ((data->new[0] == LEFT_CNTR) || (data->new[0] == RIGHT_CNTR)) data->flags |= USB_KBD_CTRL; - for (i = 2; i < 8; i++) { + for (i = 2; i < USB_KBD_PDATA_SIZE; i++) { res |= usb_kbd_service_key(dev, i, 0); res |= usb_kbd_service_key(dev, i, 1); } @@ -297,7 +303,7 @@ static int usb_kbd_irq_worker(struct usb_device *dev) if (res == 1) usb_kbd_setled(dev); - memcpy(data->old, data->new, 8); + memcpy(data->old, data->new, USB_KBD_PDATA_SIZE); return 1; } @@ -305,7 +311,8 @@ static int usb_kbd_irq_worker(struct usb_device *dev) /* Keyboard interrupt handler */ static int usb_kbd_irq(struct usb_device *dev) { - if ((dev->irq_status != 0) || (dev->irq_act_len != 8)) { + if ((dev->irq_status != 0) || + (dev->irq_act_len != USB_KBD_PDATA_SIZE)) { debug("USB KBD: Error %lX, len %d\n", dev->irq_status, dev->irq_act_len); return 1; @@ -333,7 +340,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) /* Submit a interrupt transfer request */ maxp = usb_maxpacket(dev, pipe); usb_submit_int_msg(dev, pipe, &data->new[0], - maxp > 8 ? 8 : maxp, ep->bInterval); + maxp > USB_KBD_PDATA_SIZE ? + USB_KBD_PDATA_SIZE : maxp, + ep->bInterval); usb_kbd_irq_worker(dev); #elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP) @@ -341,8 +350,8 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr; iface = &dev->config.if_desc[0]; usb_get_report(dev, iface->desc.bInterfaceNumber, - 1, 0, data->new, sizeof(data->new)); - if (memcmp(data->old, data->new, sizeof(data->new))) + 1, 0, data->new, USB_KBD_PDATA_SIZE); + if (memcmp(data->old, data->new, USB_KBD_PDATA_SIZE)) usb_kbd_irq_worker(dev); #endif } @@ -441,7 +450,8 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) memset(data, 0, sizeof(struct usb_kbd_pdata)); /* allocate input buffer aligned and sized to USB DMA alignment */ - data->new = memalign(USB_DMA_MINALIGN, roundup(8, USB_DMA_MINALIGN)); + data->new = memalign(USB_DMA_MINALIGN, + roundup(USB_KBD_PDATA_SIZE, USB_DMA_MINALIGN)); /* Insert private data into USB device structure */ dev->privptr = data; @@ -459,7 +469,9 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) usb_set_idle(dev, iface->desc.bInterfaceNumber, REPEAT_RATE, 0); debug("USB KBD: enable interrupt pipe...\n"); - if (usb_submit_int_msg(dev, pipe, data->new, maxp > 8 ? 8 : maxp, + if (usb_submit_int_msg(dev, pipe, data->new, + maxp > USB_KBD_PDATA_SIZE ? + USB_KBD_PDATA_SIZE : maxp, ep->bInterval) < 0) { printf("Failed to get keyboard state from device %04x:%04x\n", dev->descriptor.idVendor, dev->descriptor.idProduct);