RE: [PATCH] usb: host: ehci: skip STS_HALT check for aspeed platform

[Neal Liu]

> -----Original Message-----
> From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Sent: Thursday, August 26, 2021 11:28 PM
> To: Neal Liu <neal_liu@xxxxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Tony Prisk
> <linux@xxxxxxxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Tao Ren
> <rentao.bupt@xxxxxxxxx>; BMC-SW <BMC-SW@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH] usb: host: ehci: skip STS_HALT check for aspeed platform
> 
> On Thu, Aug 26, 2021 at 03:15:25PM +0800, neal_liu wrote:
> > STS_HALT also depends on ASS/PSS status for apseed.
> > Skip this check on startup.
> >
> > Signed-off-by: neal_liu <neal_liu@xxxxxxxxxxxxxx>
> > ---
> >  drivers/usb/host/ehci-hcd.c      | 10 +++++++++-
> >  drivers/usb/host/ehci-platform.c |  6 ++++++
> >  drivers/usb/host/ehci.h          |  1 +
> >  3 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index 10b0365f3439..a539e11502ef 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -634,7 +634,15 @@ static int ehci_run (struct usb_hcd *hcd)
> >  	/* Wait until HC become operational */
> >  	ehci_readl(ehci, &ehci->regs->command);	/* unblock posted writes */
> >  	msleep(5);
> > -	rc = ehci_handshake(ehci, &ehci->regs->status, STS_HALT, 0, 100 * 1000);
> > +
> > +	/* For Aspeed, STS_HALT also depends on ASS/PSS status.
> > +	 * Skip this check on startup.
> > +	 */
> > +	if (ehci->is_aspeed)
> > +		rc = 0;
> > +	else
> > +		rc = ehci_handshake(ehci, &ehci->regs->status, STS_HALT,
> > +				    0, 100 * 1000);
> 
> You must not do this.  You are assuming the controller will always begin
> operating within 5 ms of the time it is told to start.  What happens if you are
> wrong?
> 
> There must be some way for the aspeed controller's host computer to tell
> when the controller has started running.  Find out what it is and use it.
> 
> Alan Stern

We can check the command RS bit to tell the controller has started running.
I'll sent next patch within this change.
Thanks

-Neal Liu