[U-Boot] [PATCH 03/16] usb: Remove unnecessary work in usb_setup_descriptor()

[stefan.bruens at rwth-aachen.de]

On Freitag, 30. Juni 2017 05:49:56 CEST Bin Meng wrote:
> Hi,
> 
> On Fri, Jun 23, 2017 at 5:54 PM, Bin Meng <bmeng.cn at gmail.com> 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 <bmeng.cn at gmail.com>
> > ---
> > 
> >  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;
> > -

You remove the initialization of dev->maxpacketsize here ...

> > 
> >         if (do_read) {
> >         
[...]
> > 
> >                  */
> >                 
> >                 err = get_descriptor_len(dev, 64, 8);
> >                 if (err)
> >                 
> >                         return err;

... and probably return early here. Can you add some debug output to 
get_descriptor_len(...) (len, expected len, return code)?

> > 
> > +       } 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.

Kind regards,

Stefan