On Fri, May 18, 2012 at 06:33:56PM +0800, Chen Peter-B29397 wrote:
>
> >
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index f644ba9..29a9057 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -318,8 +318,7 @@ static void tdi_reset (struct ehci_hcd *ehci)
> > ehci_writel(ehci, tmp, reg_ptr);
> > }
> >
> > -/* reset a non-running (STS_HALT == 1) controller */
> > -static int ehci_reset (struct ehci_hcd *ehci)
> > +static int __ehci_reset (struct ehci_hcd *ehci)
> > {
> > int retval;
> > u32 command = ehci_readl(ehci, &ehci->regs->command);
> > @@ -352,12 +351,27 @@ static int ehci_reset (struct ehci_hcd *ehci)
> > if (ehci->debug)
> > dbgp_external_startup();
> >
> > - ehci->command = ehci_readl(ehci, &ehci->regs->command);
> > ehci->port_c_suspend = ehci->suspended_ports =
> > ehci->resuming_ports = 0;
> > return retval;
> > }
> >
> > +/* ehci hardware init */
> > +static int ehci_hw_init (struct ehci_hcd *ehci)
> > +{
> > + return __ehci_reset(ehci);
> > +}
> > +
> > +/* reset a non-running (STS_HALT == 1) controller */
> > +static int ehci_reset (struct ehci_hcd *ehci)
> > +{
> > + int val;
> > + val = __ehci_reset(ehci);
> > + ehci->command = ehci_readl(ehci, &ehci->regs->command);
> > +
> > + return val;
> > +}
> > +
> Hi Ming,
>
> I have debugged it at this afternoon, your description for this problem
> is correct, the ehci_reset override the ehci->command which is assigned
> at ehci_init, and ehci->command at ehci_init is correct.
>
> My opinion for this fix is just remove the
> " ehci->command = ehci_readl(ehci, &ehci->regs->command);" at ehci_reset.
> Even someone called ehci_reset to reset controller, it can still use command
> value assigned at ehci_init.
IMHO, ehci_reset should be the first thing in ehci_setup. I feel a
little confused, there's no standard step to initialise ehci. Some
driver call ehci_init while some driver call ehci_setup. And every
driver (except chipidea) is included to ehci-hcd.c, ehci-hcd expose
all its local functions to all other drivers.
Thanks
Richard
>
>
> > /* idle the controller (from running) */
> > static void ehci_quiesce (struct ehci_hcd *ehci)
> > {
> > @@ -836,7 +850,7 @@ static int __maybe_unused ehci_setup (struct usb_hcd
> > *hcd)
> > if (retval)
> > return retval;
> >
> > - ehci_reset(ehci);
> > + ehci_hw_init(ehci);
> >
> > return 0;
> > }
> >
> > Thanks,
> > --
> > Ming Lei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
