From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bin Meng <bmeng.cn@gmail.com>
Date: Mon, 3 Jul 2017 09:20:28 +0800
Subject: [U-Boot] [PATCH 03/16] usb: Remove unnecessary work in
	usb_setup_descriptor()
In-Reply-To: <3331691.Z58oN2oheG@pebbles.site>
References: <1498211673-18715-1-git-send-email-bmeng.cn@gmail.com>
 <1498211673-18715-4-git-send-email-bmeng.cn@gmail.com>
 <CAEUhbmUnEO92wmkpN52mh-vn5tU2_8qpV2rvieYH8s0aNUe1Tg@mail.gmail.com>
 <3331691.Z58oN2oheG@pebbles.site>
Message-ID: <CAEUhbmVTcqhErH067bQqEJ1xSAqP5QV2xLtemtNbX+s3-daMmA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Stefan,

On Sun, Jul 2, 2017 at 1:29 AM,  <stefan.bruens@rwth-aachen.de> wrote:
> 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@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@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)?
>

The get_descriptor_len() was successful, so it is not caused by the
"if (err)" branch.

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