Hi Marek, > On 3/23/20 8:53 AM, Lukasz Majewski wrote: > > Hi Marek, > > Hi, > > >> On 3/22/20 2:00 PM, Lukasz Majewski wrote: > >> [...] > >>> diff --git a/common/usb_storage.c b/common/usb_storage.c > >>> index 92e1e54d1c..3c2324fa1a 100644 > >>> --- a/common/usb_storage.c > >>> +++ b/common/usb_storage.c > >>> @@ -729,6 +729,7 @@ static int usb_stor_BBB_transport(struct > >>> scsi_cmd *srb, struct us_data *us) pipeout = > >>> usb_sndbulkpipe(us->pusb_dev, us->ep_out); /* DATA phase + error > >>> handling */ data_actlen = 0; > >>> + mdelay(10); /* Like linux does. */ > >> > >> Does this add delay to every single transfer ? > > > > It brings the slowdown, yes. > > > > However, without it I very often see the error that the USB address > > couldn't be assigned. > > Seems like this is hiding some real error then. > > If I do basic math, then I reach a conclusion that the comment is > bogus. Look, assume the transfer itself takes 0 time, then if you > have 10 mS delays between transfers, you can do 100 transfer per > second. If one transfer is 240 blocks * 512 bytes , then you are > limited to 12.2 MB/s. And I am positive USB 2.0 sticks in Linux can > transfer faster than that.
Theoretically USB 2.0 can have up to 60MiB/s transfer rate. The 12MiB/s
is for USB 1.x.
I cannot say why this delay helps with recognition of some devices. I
also start wondering why I do see some strange problems in U-Boot (like
not assigning address, timeouts), as those errors are not present on
Linux.
>
> >> That would mean a massive slowdown if you use short data transfers,
> >> which is needed for old/limited USB sticks.
> >>
> >>> /* no data, go immediately to the STATUS phase */
> >>> if (srb->datalen == 0)
> >>> goto st;
> >>> @@ -1023,9 +1024,32 @@ static int usb_request_sense(struct
> >>> scsi_cmd *srb, struct us_data *ss) return 0;
> >>> }
> >>>
> >>> +/*
> >>> + * This spins up the disk and also consumes the time that the
> >>> + * disk takes to become active and ready to read data.
> >>> + * Some drives (like Western Digital) can take more than 5
> >>> seconds.
> >>> + * The delay occurs on the 1st data read from the disk.
> >>> + * Extending the timeout here works better than handling the
> >>> timeout
> >>> + * as an error on a "real" read.
> >>> + */
> >>> +static int usb_spinup(struct scsi_cmd *srb, struct us_data *ss)
> >>> +{
> >>> + memset(&srb->cmd[0], 0, 12);
> >>> + srb->cmd[0] = SCSI_START_STP;
> >>> + srb->cmd[1] = srb->lun << 5;
> >>> + srb->cmd[4] = 1; /* Start spinup. */
> >>> + srb->datalen = 0;
> >>> + srb->cmdlen = 6;
> >>> + ss->pusb_dev->extra_timeout = 9876;
> >>
> >> What is this magic number ?
> >
> > This number is added to the timeout up to which ehci controller
> > waits for EHCI TD to be sent.
>
> This is generic code and has to work with OHCI/UHCI/xHCI too.
>
> > Why there is 9876 - I do guess that it was took from Linux in some
> > point in time.
>
> Please, research it.
>
> >>> + ss->transport(srb, ss);
> >>> + ss->pusb_dev->extra_timeout = 0;
> >>> + return 0;
> >>> +}
> >>
> >> [...]
> >>
> >>> diff --git a/include/usb.h b/include/usb.h
> >>> index efb67ea33f..5b0f5ce5d6 100644
> >>> --- a/include/usb.h
> >>> +++ b/include/usb.h
> >>> @@ -140,6 +140,7 @@ struct usb_device {
> >>> int act_len; /* transferred bytes
> >>> */ int maxchild; /* Number of ports if
> >>> hub */ int portnr; /* Port number, 1=first
> >>> */
> >>> + int extra_timeout; /* Add to timeout in ehci_submit_async
> >>> or spinup */
> >>
> >> Does this work with xhci too ?
> >
> > Yes, it is used in patch 5/5.
>
> Does xhci need it ?
>
It is hard to say. The original fix was for EHCI.
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
pgpYLvZ2pDDu0.pgp
Description: OpenPGP digital signature
