Message ID | 1462308680-9366-4-git-send-email-marex@denx.de |
---|---|
State | Accepted |
Commit | f647bf0ba36a5236d4bc7f93d39bfacfb1cfe6c7 |
Delegated to: | Marek Vasut |
Headers | show |
On 03.05.2016 22:51, Marek Vasut wrote: > Some devices, like the SanDisk Cruzer Pop need some time to process > the Set Configuration request, so wait a little until they are ready. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Chin Liang See <clsee@altera.com> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Stefan Roese <sr@denx.de> > Cc: Stephen Warren <swarren@nvidia.com> > --- > common/usb.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/common/usb.c b/common/usb.c > index 63429d4..205041b 100644 > --- a/common/usb.c > +++ b/common/usb.c > @@ -1107,6 +1107,14 @@ int usb_select_config(struct usb_device *dev) > "len %d, status %lX\n", dev->act_len, dev->status); > return err; > } > + > + /* > + * Wait until the Set Configuration request gets processed by the > + * device. This is required by at least SanDisk Cruzer Pop USB 2.0 > + * and Kingston DT Ultimate 32GB USB 3.0 on DWC2 OTG controller. > + */ > + mdelay(10); > + > debug("new device strings: Mfr=%d, Product=%d, SerialNumber=%d\n", > dev->descriptor.iManufacturer, dev->descriptor.iProduct, > dev->descriptor.iSerialNumber); > As you might have expected, I'm not a fan of adding new delays to the common USB code. As this negatively affects all platforms. Did you test these two sticks that require this delay on other platforms than SoCFPGA? I would be very interested to know, if these keys are successfully detected without this patch on other platforms. I don't have access to these USB keys, so I can't test it on my platforms. Thanks, Stefan
On Wed, 2016-05-04 at 09:41 +0200, Stefan Roese wrote: > On 03.05.2016 22:51, Marek Vasut wrote: > > Some devices, like the SanDisk Cruzer Pop need some time to process > > the Set Configuration request, so wait a little until they are > > ready. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: Chin Liang See <clsee@altera.com> > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > Cc: Hans de Goede <hdegoede@redhat.com> > > Cc: Stefan Roese <sr@denx.de> > > Cc: Stephen Warren <swarren@nvidia.com> > > --- > > common/usb.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/common/usb.c b/common/usb.c > > index 63429d4..205041b 100644 > > --- a/common/usb.c > > +++ b/common/usb.c > > @@ -1107,6 +1107,14 @@ int usb_select_config(struct usb_device > > *dev) > > "len %d, status %lX\n", dev->act_len, dev > > ->status); > > return err; > > } > > + > > + /* > > + * Wait until the Set Configuration request gets processed > > by the > > + * device. This is required by at least SanDisk Cruzer Pop > > USB 2.0 > > + * and Kingston DT Ultimate 32GB USB 3.0 on DWC2 OTG > > controller. > > + */ > > + mdelay(10); > > + > > debug("new device strings: Mfr=%d, Product=%d, > > SerialNumber=%d\n", > > dev->descriptor.iManufacturer, dev > > ->descriptor.iProduct, > > dev->descriptor.iSerialNumber); > > > > As you might have expected, I'm not a fan of adding new delays to > the common USB code. As this negatively affects all platforms. Did > you test these two sticks that require this delay on other platforms > than SoCFPGA? I would be very interested to know, if these keys are > successfully detected without this patch on other platforms. I > don't have access to these USB keys, so I can't test it on my > platforms. > Actually this series of patches (include the delay) help for all my problematic pen drives too. It sound to me these pen drives need time to process. But at same time, its strange to me that the device didn't NAK (as I added printout for NAK) to indicate that its busy. Seems that this issue is noticed at Freescale too. http://lists.denx.de/pipermail/u-boot/2015-December/238434.html. Cc'ed them as they might can share more details on this. Chin Liang > Thanks, > Stefan
On 04.05.2016 11:45, Chin Liang See wrote: > On Wed, 2016-05-04 at 09:41 +0200, Stefan Roese wrote: >> On 03.05.2016 22:51, Marek Vasut wrote: >>> Some devices, like the SanDisk Cruzer Pop need some time to process >>> the Set Configuration request, so wait a little until they are >>> ready. >>> >>> Signed-off-by: Marek Vasut <marex@denx.de> >>> Cc: Chin Liang See <clsee@altera.com> >>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >>> Cc: Hans de Goede <hdegoede@redhat.com> >>> Cc: Stefan Roese <sr@denx.de> >>> Cc: Stephen Warren <swarren@nvidia.com> >>> --- >>> common/usb.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/common/usb.c b/common/usb.c >>> index 63429d4..205041b 100644 >>> --- a/common/usb.c >>> +++ b/common/usb.c >>> @@ -1107,6 +1107,14 @@ int usb_select_config(struct usb_device >>> *dev) >>> "len %d, status %lX\n", dev->act_len, dev >>> ->status); >>> return err; >>> } >>> + >>> + /* >>> + * Wait until the Set Configuration request gets processed >>> by the >>> + * device. This is required by at least SanDisk Cruzer Pop >>> USB 2.0 >>> + * and Kingston DT Ultimate 32GB USB 3.0 on DWC2 OTG >>> controller. >>> + */ >>> + mdelay(10); >>> + >>> debug("new device strings: Mfr=%d, Product=%d, >>> SerialNumber=%d\n", >>> dev->descriptor.iManufacturer, dev >>> ->descriptor.iProduct, >>> dev->descriptor.iSerialNumber); >>> >> >> As you might have expected, I'm not a fan of adding new delays to >> the common USB code. As this negatively affects all platforms. Did >> you test these two sticks that require this delay on other platforms >> than SoCFPGA? I would be very interested to know, if these keys are >> successfully detected without this patch on other platforms. I >> don't have access to these USB keys, so I can't test it on my >> platforms. >> > > Actually this series of patches (include the delay) help for all my > problematic pen drives too. It sound to me these pen drives need time > to process. This delay (I'm talking mainly about the 1000ms delay per USB hub in patch 7/7) does not seem to be necessary on other platforms. Chin, could you please test again without this patch 7/7 but with "usb_pgood_delay" set to 1000? And let us know if this also fixes all the problems with your problematic pen drives? That would be very helpful. Thanks, Stefan
On 05/04/2016 09:41 AM, Stefan Roese wrote: > On 03.05.2016 22:51, Marek Vasut wrote: >> Some devices, like the SanDisk Cruzer Pop need some time to process >> the Set Configuration request, so wait a little until they are ready. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Chin Liang See <clsee@altera.com> >> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >> Cc: Hans de Goede <hdegoede@redhat.com> >> Cc: Stefan Roese <sr@denx.de> >> Cc: Stephen Warren <swarren@nvidia.com> >> --- >> common/usb.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/common/usb.c b/common/usb.c >> index 63429d4..205041b 100644 >> --- a/common/usb.c >> +++ b/common/usb.c >> @@ -1107,6 +1107,14 @@ int usb_select_config(struct usb_device *dev) >> "len %d, status %lX\n", dev->act_len, dev->status); >> return err; >> } >> + >> + /* >> + * Wait until the Set Configuration request gets processed by the >> + * device. This is required by at least SanDisk Cruzer Pop USB 2.0 >> + * and Kingston DT Ultimate 32GB USB 3.0 on DWC2 OTG controller. >> + */ >> + mdelay(10); >> + >> debug("new device strings: Mfr=%d, Product=%d, SerialNumber=%d\n", >> dev->descriptor.iManufacturer, dev->descriptor.iProduct, >> dev->descriptor.iSerialNumber); >> > > As you might have expected, I'm not a fan of adding new delays to > the common USB code. As this negatively affects all platforms. Did > you test these two sticks that require this delay on other platforms > than SoCFPGA? Yes, on intel. Many microframes pass between these control EP requests, while on the dwc2 in u-boot, quite a few of these requests end in the same microframe because the code is fast, which is what the stick does not like. > I would be very interested to know, if these keys are > successfully detected without this patch on other platforms. I > don't have access to these USB keys, so I can't test it on my > platforms. > > Thanks, > Stefan
On Wed, 2016-05-04 at 12:00 +0200, Stefan Roese wrote: > On 04.05.2016 11:45, Chin Liang See wrote: > > On Wed, 2016-05-04 at 09:41 +0200, Stefan Roese wrote: > > > On 03.05.2016 22:51, Marek Vasut wrote: > > > > Some devices, like the SanDisk Cruzer Pop need some time to > > > > process > > > > the Set Configuration request, so wait a little until they are > > > > ready. > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > Cc: Chin Liang See <clsee@altera.com> > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > Cc: Hans de Goede <hdegoede@redhat.com> > > > > Cc: Stefan Roese <sr@denx.de> > > > > Cc: Stephen Warren <swarren@nvidia.com> > > > > --- > > > > common/usb.c | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/common/usb.c b/common/usb.c > > > > index 63429d4..205041b 100644 > > > > --- a/common/usb.c > > > > +++ b/common/usb.c > > > > @@ -1107,6 +1107,14 @@ int usb_select_config(struct usb_device > > > > *dev) > > > > "len %d, status %lX\n", dev > > > > ->act_len, dev > > > > ->status); > > > > return err; > > > > } > > > > + > > > > + /* > > > > + * Wait until the Set Configuration request gets > > > > processed > > > > by the > > > > + * device. This is required by at least SanDisk Cruzer > > > > Pop > > > > USB 2.0 > > > > + * and Kingston DT Ultimate 32GB USB 3.0 on DWC2 OTG > > > > controller. > > > > + */ > > > > + mdelay(10); > > > > + > > > > debug("new device strings: Mfr=%d, Product=%d, > > > > SerialNumber=%d\n", > > > > dev->descriptor.iManufacturer, dev > > > > ->descriptor.iProduct, > > > > dev->descriptor.iSerialNumber); > > > > > > > > > > As you might have expected, I'm not a fan of adding new delays to > > > the common USB code. As this negatively affects all platforms. > > > Did > > > you test these two sticks that require this delay on other > > > platforms > > > than SoCFPGA? I would be very interested to know, if these keys > > > are > > > successfully detected without this patch on other platforms. I > > > don't have access to these USB keys, so I can't test it on my > > > platforms. > > > > > > > Actually this series of patches (include the delay) help for all my > > problematic pen drives too. It sound to me these pen drives need > > time > > to process. > > This delay (I'm talking mainly about the 1000ms delay per USB hub > in patch 7/7) does not seem to be necessary on other platforms. > > Chin, could you please test again without this patch 7/7 but > with "usb_pgood_delay" set to 1000? And let us know if this > also fixes all the problems with your problematic pen drives? > That would be very helpful. > Yup, it works for my problematic pen drives. This apply to both with and without setting the usb_pgood_delay to 1000. Thanks Chin Liang > Thanks, > Stefan
>-----Original Message----- >From: Chin Liang See [mailto:clsee@altera.com] >Sent: Wednesday, May 04, 2016 3:15 PM >To: Stefan Roese <sr@denx.de>; Marek Vasut <marex@denx.de>; u- >boot@lists.denx.de >Cc: Dinh Nguyen <dinguyen@opensource.altera.com>; Hans de Goede ><hdegoede@redhat.com>; Stephen Warren <swarren@nvidia.com>; >sriram.dash@freescale.com; rajesh.bhagat@freescale.com >Subject: Re: [PATCH 4/7] usb: Wait after sending Set Configuration request > >On Wed, 2016-05-04 at 09:41 +0200, Stefan Roese wrote: >> On 03.05.2016 22:51, Marek Vasut wrote: >> > Some devices, like the SanDisk Cruzer Pop need some time to process >> > the Set Configuration request, so wait a little until they are >> > ready. >> > >> > Signed-off-by: Marek Vasut <marex@denx.de> >> > Cc: Chin Liang See <clsee@altera.com> >> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >> > Cc: Hans de Goede <hdegoede@redhat.com> >> > Cc: Stefan Roese <sr@denx.de> >> > Cc: Stephen Warren <swarren@nvidia.com> >> > --- >> > common/usb.c | 8 ++++++++ >> > 1 file changed, 8 insertions(+) >> > >> > diff --git a/common/usb.c b/common/usb.c index 63429d4..205041b >> > 100644 >> > --- a/common/usb.c >> > +++ b/common/usb.c >> > @@ -1107,6 +1107,14 @@ int usb_select_config(struct usb_device >> > *dev) >> > "len %d, status %lX\n", dev->act_len, dev >> > ->status); >> > return err; >> > } >> > + >> > + /* >> > + * Wait until the Set Configuration request gets processed >> > by the >> > + * device. This is required by at least SanDisk Cruzer Pop >> > USB 2.0 >> > + * and Kingston DT Ultimate 32GB USB 3.0 on DWC2 OTG >> > controller. >> > + */ >> > + mdelay(10); >> > + >> > debug("new device strings: Mfr=%d, Product=%d, >> > SerialNumber=%d\n", >> > dev->descriptor.iManufacturer, dev >> > ->descriptor.iProduct, >> > dev->descriptor.iSerialNumber); >> > >> >> As you might have expected, I'm not a fan of adding new delays to the >> common USB code. As this negatively affects all platforms. Did you >> test these two sticks that require this delay on other platforms than >> SoCFPGA? I would be very interested to know, if these keys are >> successfully detected without this patch on other platforms. I don't >> have access to these USB keys, so I can't test it on my platforms. >> > >Actually this series of patches (include the delay) help for all my problematic pen >drives too. It sound to me these pen drives need time to process. But at same time, >its strange to me that the device didn't NAK (as I added printout for NAK) to indicate >that its busy. > >Seems that this issue is noticed at Freescale too. >http://lists.denx.de/pipermail/u-boot/2015-December/238434.html. Cc'ed them as >they might can share more details on this. > Hello, In our case, for some USB 2.0 stick(notably : Sandisk Cruzer blade 4 GB) on USB 3.0 controller, we were getting timeouts in get descriptor string, and were not able to retrieve the manufacturer, product id , serial number. Later we discovered, they needed a little more time (~ 2 ms) after setting configuration. However, as per the usb set address delay, we choose 10 ms delay(to support even slower devices). Sriram >Chin Liang > > > >> Thanks, >> Stefan
diff --git a/common/usb.c b/common/usb.c index 63429d4..205041b 100644 --- a/common/usb.c +++ b/common/usb.c @@ -1107,6 +1107,14 @@ int usb_select_config(struct usb_device *dev) "len %d, status %lX\n", dev->act_len, dev->status); return err; } + + /* + * Wait until the Set Configuration request gets processed by the + * device. This is required by at least SanDisk Cruzer Pop USB 2.0 + * and Kingston DT Ultimate 32GB USB 3.0 on DWC2 OTG controller. + */ + mdelay(10); + debug("new device strings: Mfr=%d, Product=%d, SerialNumber=%d\n", dev->descriptor.iManufacturer, dev->descriptor.iProduct, dev->descriptor.iSerialNumber);
Some devices, like the SanDisk Cruzer Pop need some time to process the Set Configuration request, so wait a little until they are ready. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Chin Liang See <clsee@altera.com> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> Cc: Hans de Goede <hdegoede@redhat.com> Cc: Stefan Roese <sr@denx.de> Cc: Stephen Warren <swarren@nvidia.com> --- common/usb.c | 8 ++++++++ 1 file changed, 8 insertions(+)