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. >> 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 ? -- Best regards, Marek Vasut