[RFT PATCH v1 4/5] usb: Provide code to handle spinup of USB usb devices (mostly HDDs)

[Lukasz Majewski <lukma@denx.de>]

Hi Marek,

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

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

Why there is 9876 - I do guess that it was took from Linux in some
point in time.

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


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: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200323/d7fd11d3/attachment.sig>