From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Fri, 30 Jun 2017 11:49:56 +0800 Subject: [U-Boot] [PATCH 03/16] usb: Remove unnecessary work in usb_setup_descriptor() In-Reply-To: <1498211673-18715-4-git-send-email-bmeng.cn@gmail.com> References: <1498211673-18715-1-git-send-email-bmeng.cn@gmail.com> <1498211673-18715-4-git-send-email-bmeng.cn@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On Fri, Jun 23, 2017 at 5:54 PM, Bin Meng wrote: > The only work we need do in usb_setup_descriptor() is to initialize > dev->descriptor.bMaxPacketSize0, when do_read is false. Other steps > are the same as do_read being true. > > While we are here, update the comment block to be within 80 cols. > > Signed-off-by: Bin Meng > --- > > common/usb.c | 40 ++++++++++++++++++---------------------- > 1 file changed, 18 insertions(+), 22 deletions(-) > > diff --git a/common/usb.c b/common/usb.c > index 15e1e4c..293d968 100644 > --- a/common/usb.c > +++ b/common/usb.c > @@ -962,37 +962,33 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) > * some more, or keeps on retransmitting the 8 byte header. > */ > > - if (dev->speed == USB_SPEED_LOW) { > - dev->descriptor.bMaxPacketSize0 = 8; > - dev->maxpacketsize = PACKET_SIZE_8; > - } else { > - dev->descriptor.bMaxPacketSize0 = 64; > - dev->maxpacketsize = PACKET_SIZE_64; > - } > - dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0; > - dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0; > - > if (do_read) { > int err; > > /* > - * Validate we've received only at least 8 bytes, not that we've > - * received the entire descriptor. The reasoning is: > - * - The code only uses fields in the first 8 bytes, so that's all we > - * need to have fetched at this stage. > - * - The smallest maxpacket size is 8 bytes. Before we know the actual > - * maxpacket the device uses, the USB controller may only accept a > - * single packet. Consequently we are only guaranteed to receive 1 > - * packet (at least 8 bytes) even in a non-error case. > + * Validate we've received only at least 8 bytes, not that > + * we've received the entire descriptor. The reasoning is: > + * - The code only uses fields in the first 8 bytes, so that's > + * all we need to have fetched at this stage. > + * - The smallest maxpacket size is 8 bytes. Before we know > + * the actual maxpacket the device uses, the USB controller > + * may only accept a single packet. Consequently we are only > + * guaranteed to receive 1 packet (at least 8 bytes) even in > + * a non-error case. > * > - * At least the DWC2 controller needs to be programmed with the number > - * of packets in addition to the number of bytes. A request for 64 > - * bytes of data with the maxpacket guessed as 64 (above) yields a > - * request for 1 packet. > + * At least the DWC2 controller needs to be programmed with > + * the number of packets in addition to the number of bytes. > + * A request for 64 bytes of data with the maxpacket guessed > + * as 64 (above) yields a request for 1 packet. > */ > err = get_descriptor_len(dev, 64, 8); > if (err) > return err; > + } else { > + if (dev->speed == USB_SPEED_LOW) > + dev->descriptor.bMaxPacketSize0 = 8; > + else > + dev->descriptor.bMaxPacketSize0 = 64; > } > > dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0; > -- For some reason that I don't understand, this patch breaks EHCI. dev->maxpacketsize is equal to zero with this patch, which leads to a 'divide error' exception in ehci_submit_async(). Not sure if anyone has some hints? For now, I will just drop this patch in v2. This needs be further revisited. Regards, Bin