On Mon, 9 May 2005, David Brownell wrote:

> > Do we really need to throttle down traffic from within usbcore? 
> 
> Sure looked like it last time I checked this issue out in detail.

I'm pretty sure UHCI will work without external throttling.  Of course 
OHCI and EHCI may have more stringent requirements.

> > Won't it 
> > be enough to rely on the HCDs to reject submissions when they are unable
> > to accept them?  (Plus the fact that thanks to all the cleanups made in
> > the last couple of years, hardly any submissions are made at inappropriate
> > times.)
> 
> Thing is there can be a window of around 10 msec there, unless this
> is the autosuspend case.  During those 10 msec, something needs to
> prevent other code from using that device (e.g. on SMP).  For now,
> that something is hcd->state.  Maybe it can be improved, but I don't
> have timte to do it.

You're talking about the case where the driver is being unloaded, right?  
So the HC needs to be shut down cleanly.  Doesn't unregistering the root
hub do everything you want?  It recursively removes all the devices on
that bus, and it waits for all the endpoints to be fully disabled and gone
from the hardware.  By the time hcd->stop is called there should be
nothing remaining in the schedule or in the controller.  And since there
are no registered usb_devices remaining on the bus (and no way to add a
new one), even on SMP there shouldn't be anything calling into the HCD.

> > If you really want to do this from within usbcore, consider restricting 
> > hcd->state to only three possible values:
> > 
> >     Okay to submit and unlink (running normally);
> >     Okay to unlink but not to submit (quiescing);
> >     Not okay to unlink or submit (any other state).
> 
> That is, two bits to control submit and unlink capabilities,
> rather than the various other ones that now compose hcd->state?

Yes, although 3 states makes slightly more sense than 2 bits since I can't 
imagine a situation where you would allow submit and not unlink.


> > Any additional information needed by hcd-pci.c or the HCDs (such as
> > whether IRQs are enabled) can be stored in a separate new field.
> 
> Sounds plausible.  As I recall, the reason usbcore touches hcd->state
> is primarily since the HCDs couldn't previously be relied on to do it
> right in all the relevant cases.

I hope by now the code has improved!

> > Apart from the PCI bus glue, all of that stuff is private to the HCDs.  
> > It shouldn't be folded in along with a field used elsewhere in usbcore.  
> 
> Most of that "superset" is the root hub timer, and after your timer
> updates get merged (post-2.6.12) it becomes more reasonable for those
> bits to be managed that way.

Okay, I don't mind waiting.

>  Re the [1]..[4] cases, I'd have to look
> up the snipped bits to see if I agree with applying that argument.

Your original message is here:

http://marc.theaimsgroup.com/?l=linux-usb-devel&m=111524542215119&w=2

[1] was pci_dev->current_state, which is already stored elsewhere.

[2] was non-PM PCI ("legacy") devices, where DMA and IRQ may be disabled 
even while remaining in D0.  Likely this will require a new field.

[3] was non-PCI SOC controllers.  Again, they will require something extra
-- probably platform-specific and maybe already present in the code.

[4] was non-PCI controller chips like sl811h or isp116x.  They look to be
pretty much the same as [3] as far as representing the controller's state.


> > And even the PCI glue info should be kept separate from things used only
> > by HCDs.  Right now hcd->state mixes all these different categories
> > together in a confusing and error-prone way.
> 
> It's called "evolution in action".  ;)

As you wish...  :-)

Alan "Natural Selection" Stern



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to