On Wed, 6 Mar 2013, Don Zickus wrote:
> On Tue, Mar 05, 2013 at 03:14:10PM -0800, Sarah Sharp wrote:
> > > */
> > > static void compliance_mode_recovery_timer_init(struct xhci_hcd *xhci)
> > > {
> > > + if (timer_pending(&xhci->comp_mode_recovery_timer)) {
> > > + xhci_dbg(xhci, "Compliance Mode Recovery Timer already
> > > active.\n");
> > > + return;
> > > + }
> > > +
> > > xhci->port_status_u0 = 0;
> > > init_timer(&xhci->comp_mode_recovery_timer);
> >
> > Isn't this racy? There's no locking here, so what if the timer fired
> > just before the call to timer_pending happened? (The documentation on
> > the timer interface doesn't describe whether timer_pending returns true
> > if we're in the middle of a timer callback.)
>
> Ah, yes. I think I misled Tony here. I thought the timer_pending() check
> just checked to see if the timer was on the linked list, not realizing the
> timer is removed from that list when executed. Instead I thought the
> expires was set to -1 (or something large that it never expired) but it
> stayed on the list.
>
> We might have to dumb it down and just use a global init variable that is
> set/cleared by the compliance functions with the assumption they are
> called serially (as not to race).
>
> Though this all seems very hackish and ugly. There has to be a better way
> to do this.. :-(
The init_timer call should occur only once, during the probe routine.
Later calls should use mod_timer. It's okay to call mod_timer while
the timer routine is running; in fact, many timer routines call
mod_timer themselves.
> > Also, looking at the compliance timer callback brings up further
> > questions:
> >
> > static void compliance_mode_recovery(unsigned long arg)
> > {
> > ...
> > for (i = 0; i < xhci->num_usb3_ports; i++) {
> > temp = xhci_readl(xhci, xhci->usb3_ports[i]);
> > if ((temp & PORT_PLS_MASK) == USB_SS_PORT_LS_COMP_MOD) {
> > /*
> > * Compliance Mode Detected. Letting USB Core
> > * handle the Warm Reset
> > */
> >
> > What happens when the xHCI host controller goes into D3cold due to
> > runtime PM? The port status registers will read as all f's, so we will
> > miss any transitions to the compliance mode that happened before or
> > during the transition to D3cold.
> >
> > This code probably needs to wake up the host controller and keep it from
> > suspending until all the ports can be read.
>
> Fun... I take it there is no document that describes the various paths PM
> can take during each of these phases?
Certainly there is: Documentation/power/devices.txt. Of course, this
describes the various paths in the PM core, not in the PM
implementations of specific drivers such as xhci-hcd.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
