On Wed, 6 Apr 2011, Kristen Carlson Accardi wrote: > The thing is - once you move to a threaded irq, my resume issue > is pretty much not going to impact anyone else, even if you leave > the get/put calls exactly where I have them in the patch. Basically, > unless you are a strange whacky piece of hardware like Moorestown, > you will never get an irq while you are suspended. Why do you say that? Do you think standard PC hardware (with its shared IRQs) is wacky? > If you are not > suspended, then a call to get_sync() is just going be incrementing > a ref counter, not actually scheduling a resume, and so should not > cause anyone normal any problems. Have you looked at the code? There's more to it than one might think. The overhead of a "do-nothing" get_sync call is higher than I would like, although it's not tremendous, and at the moment I can't think of any way to streamline it without violating the runtime PM requirements. > If you *are* a strange piece of > hw and getting an irq during suspend, then you are a) most likely > not sharing irqs, so this isn't a big deal and b) if you are sharing > an irq, then that is just a penalty you are going to have to take for > having dumb hardware designers. Shared IRQs are quite common with PCI hardware, which is used very extensively for USB controllers throughout the PC world. Are you saying that virtually all the PC designers throughout the last 15 years have been dumb? > I guess what I'm saying is that I feel the get/put calls are > rather insignificant once you take the leap of going to a threaded > irq. And is much cleaner to just take care of the wakeup like > this than to try to move it down into ehci or some other platform > specific host driver. I don't agree, but I'm willing to be proved wrong. Have you made timing/performance studies, comparing (for example) USB disk throughput with and without threaded interrupts? > > > You could also allow other drivers to allocate the irq (rather > > > than doing it in hcd.c), which would let them chose whether to be > > > threaded or not. > > > > That would get messy, I'd prefer to just do it all at once instead of > > waiting for the individual host controllers to have to enable it as odds > > are some never will be changed and no one will remember why they didn't. > > > > You could do somethink like add an op: > .alloc_irq > > if (op->alloc_irq) > let controller driver do allocation, which would allow them > to call request_threaded_irq and swap out the irq handler with > their own, or not - if usb_hcd_irq is made available. > else > driver doesn't have a special alloc, so just do it > the way you always have. > > Although as I said, it seems cleaner to just do the put/get > in usb_hcd_irq. I think the overhead (coming from shared IRQs) is likely to be unacceptable. Alan Stern -- 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
