On Wednesday 30 January 2008, Sarah Sharp wrote:
> Hi Dave,
> 
> I've been looking at ehci_shutdown() in ehci-hcd.c with increasing puzzlement.

And you later corrected:  you mean ehci_stop() every where you wrote
ehci_shutdown.  I've corrected that below, for clarity.

Ideally that should share more code with ehci_shutdown().  The main
difference being that shutdown() can be sloppier since the kernel image
is going away ... while stop() can't leave any loose ends around which
would affect a later "modprobe ehci-hcd".

 
> Why shut off port power before attempting to stop the host controller from
> processing the periodic and async schedules?

That should indeed be done later.  In fact it should probably do what
ehci_shutdown() does ... differences being that stop() probably couldn't
avoid the msleep(), if that's even needed, and certainly can't get away
with leaving debris in misbehaving I/O queues.

Likewise ehci_shutdown() should kill those timers too, so they don't
fire before this kernel stops.


> It seems like you want to allow 
> the host to complete whatever transactions it was in the middle of.

I think ehci_stop() is committing overkill here, for the reasons I gave
before:  children already had an opportunity to cleanly shut stuff down.

Some of this code predates the usbcore cleanups which ensured that the
device tree was sanely managed, and before ehci_shutdown() existed.


> If 
> ehci_stop() is trying to stop attach/detach notifications, that will only
> work if the host controller supports port power switching.

The root hub will already have been shut down.


> ehci_stop() also calls ehci_reset() after shutting down the schedules.
> However, AFAIK the run/stop bit is never set before ehci_reset() is called.  
> The
> EHCI specification says that resetting a running HC is "will result in 
> undefined
> behavior."

See above ... this should all get cleaned up by having the first
part of ehci_stop() just call ehci_shutdown(), with ehci_shutdown()
modified by making it grab the spinlock and nuke those timers.


> ehci_quiesce() is only called from ehci_stop() so why not send 
> a halt command there?

You're wrong about ehci_quiesce() ... the hub code uses that.

- Dave

-
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

Reply via email to