On Thu, 15 Feb 2007, Oliver Neukum wrote:
> > Hmm... How about using your spinlock to synchronize URB submissions with
> > the driver's suspend() method instead? That way, if an important URB is
> > in flight you simply fail the suspend call. You can check the auto_pm
> > flag in the usb_struct first, to avoid failing non-auto suspends. In a
> > way, this amounts to using the already-existing suspend() method for your
> > callback rather than introducing a new one.
>
> imho caring about the reason for a suspension is a bit messy. The current
> code is nice. It just introduces a new method.
But it's a new method that essentially duplicates the purpose of an
existing method:
The current suspend() method tells the driver that the device is
about to be suspended, gives the driver a chance to prepare, and
allows the driver to report an error (possibly aborting the
suspend).
Your new method tells the driver that the device is about to be
autosuspended, gives the driver a chance to prepare (by stopping
the output queue), and allows the driver to report an error
(aborting the autosuspend).
The only real difference is that the old method is used for all suspends
and your new method is used just for autosuspends. But the driver can
check the auto_pm flag if it wants to tell them apart. So I don't see any
point in the new method.
As for caring about the reason for a suspend... It does make sense, when
the reason is that the device has been idle for a while and the driver
knows (although usbcore doesn't) that the idleness is about to end.
> And it opens another can of worms. When may I autoresume for output?
As soon as possible. Remember, we always autoresume on demand.
> Suppose an autosuspended device gets another suspend.
It won't. When a device is suspended for any reason, usbcore won't call
the suspend() method again.
As far is the hardware is concerned, there are exactly two states: running
and suspended. There's no difference between autosuspend, suspend because
the user wrote to a sysfs file, and suspend because the system as a whole
is going to sleep. Like the hardware, the software doesn't keep track of
the reason for a suspend once the suspend has taken place.
> > It does in the usb-storage case. We raise the count whenever a I/O
> > transfer starts, and decrement it when the transfer finishes (setting the
> > DEVICE_BUSY flag as well). These are lightweight operations, without much
> > overhead other than acquiring the pm_mutex. If the timer expires while an
> > operation is in progress, usbcore sees the elevated count and restarts the
>
> Why restart? You could always restated if you lower the count again.
That's the whole point. I _don't want_ to restart the timer every time
the count gets lowered, because the count will be lowered after every I/O
transfer -- which can mean hundreds of times per second.
> > timer. An autosuspend won't occur until there are two timer expirations
> > with no I/O activity in between. You're also safe if the timer expires
> > more than once during a single long-lived transfer.
>
> Yes. I understand why you don't delete the timer once the count goes up
> or mod it when the count goes down, but why restart?
Failing to restart the timer is pretty much equivalent to deleting it.
> On the downside you now need to count timers firing and detecting idle.
We don't need to count timers firing. We do need to detect idle, but the
code for that is already written -- you're using it in your HID changes.
The overhead in usb-storage for detecting idle is minimal: a single call
to set_bit() when each transfer completes.
Alan Stern
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
