On Monday 29 November 2004 08:27, Alan Stern wrote: > On Sun, 28 Nov 2004, David Brownell wrote: > > > > > > I suspect that root-hub polling shouldn't be controlled by flags within > > > the hcd structure. The state transitions are under the control of the > > > driver, so it would be wrong to make the core responsible for following > > > those transitions and adjusting the polling to match. > > > > It works today though ... why wouldn't a flag "use root hub timer" > > suffice? > > Suppose the flag was clear so the timer wasn't running, but then the > driver decides that it wants to start polling. Setting a flag won't be > enough to start the timer...
I see. OHCI doesn't look like it will need anything more than a static flag, turning off the root hub timer. > And don't say this should never happen! Intel UHCI controllers send IRQs > for port-change events when the controller is suspended, but not when it > is running. But clearly, UHCI can't use such a solution! > > Having a functional API to enable/disable the polling would > > be fine, if the calls were accessible in_irq(). > > Come to think of it, since the timer itself is directly accessible in the > hcd structure, we don't need these API calls at all. The driver can > simply call mod_timer, del_timer, and del_timer_sync. That might be a better solution all around; though I'm not currently clear on when they'd be called. > > > Is this outline appropriate for other HCs as well as UHCI? Should it > > > replace the collection of states you have already set up in the hcd > > > structure? Or should it remain in the private uhci_hcd structure? > > > > If you really need them -- then internal to UHCI. But I don't see > > why you should need new states ... CORE_SUSPENDED would imply > > that root_dev->state == USB_STATE_SUSPENDED, RH_SUSPENDED should > > be invisible to usbcore, and I think RUNNING and DEAD are clear > > matches for the existing RUNNING and HALT. > > It's a problem of synchronization. root_hub->state doesn't get changed to > USB_STATE_SUSPENDED at the right time (i.e., while holding the spinlock I > described above). I'm concerned about races occurring during the actual > transitions, because there are at least three independent pathways for > changing the state: > > HCD root-hub autosuspend triggered by an HCD-private timer, ... or maybe the one provided by usbcore ... > HCD root-hub autoresume because of non-zero status during > rh status check (triggered by poll timer or IRQ), and > > usbcore suspend/resume running in process context with the > root hub locked. If all the hcd->state changes are spinlocked by the HCD, then usbcore can ignore the issues. > You are correct that RH_SUSPENDED should be invisible to usbcore. Since > drivers will need to know about it, Why is that? > and thought the hcd structure might > be a good common place to store it. But never mind, I'll make all this > stuff private to uhci-hcd. OHCI has a formal state model (reused by, of all things, the isp1x6x chips) that's likewise not known to usbcore. - Dave > Alan Stern > > ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://productguide.itmanagersjournal.com/ _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
