Re: [PATCH] usb: core: devio: add ioctls for suspend and resume

[Mayuresh Kulkarni <mkulkarni@opensource.cirrus.com>]

On Mon, 2019-06-17 at 11:55 -0400, Alan Stern wrote:
> On Mon, 17 Jun 2019, Mayuresh Kulkarni wrote:
> 
> > 
> > > 
> > > (Note: I imagine you might run into trouble because devices
> > > generally 
> > > do not get put into runtime suspend immediately.  So if you call
> > > the 
> > > USBDEVFS_SUSPEND ioctl and then the USBDEVFS_WAIT_FOR_RESUME
> > > ioctl,
> > > the 
> > > wait will return immediately because the device hasn't yet been 
> > > suspended.)
> > > 
> > Hi Alan,
> > 
> > For this particular comment, how about we add suspend-waiter similar
> > to
> > resume-waiter?
> > As per the below changes, usbfs_notify_suspend() can wake the
> > suspend-
> > waiter, if generic_suspend() is called. So, the suspend ioctl will
> > be
> > partial blocking i.e.: it will wait till suspend happens and will
> > return
> > when it is safe to do resume.
> > 
> > Will this work?
> Probably not.  Just think about it: Your program stops communicating
> with the device and tells the kernel that it's ready for the device to
> be suspended.  But the device doesn't go into suspend for several
> seconds (or longer!) and during that time your program has no idea
> what's happening to it.  I'm pretty sure that's not what you want.
> 

Right, sounds good. Thanks.

> You're right that the program needs to know when the device is about
> to 
> be suspended.  But waiting for an ioctl to return isn't a good way 
> to do it; this needs to be a callback of some sort.  That is, the 
> kernel also needs to know when the program is ready for the suspend.
> 
> I don't know what is the best approach.

This is becoming tricky now.

> 
> > 
> > The reason for asking this is - I think the suspend ioctl should
> > return
> > appropriate status to user-space indicating weather to wait-for-
> > resume
> > or not.
> > 
> > Or are you suggesting to always have a delay in suspend/resume in
> > user-
> > space?
> > 
> > Please do review my comment below in this context too.
> > 
> > In a typical SoC based system (XHCI compliant USB host controller
> > with
> > one port exposed out on PCB), wouldn't this
> > call usbfs_notify_suspend()
> > twice - first for udev of connected device and then for udev of
> > root-
> > hub?
> Yes, it would.  This wouldn't make any difference to your program, 
> since your program would have an open file reference only for the 
> connected device, not for the root hub.
> 
> > 
> > If yes, how about we call usbfs_notify_*() for root-hubs only? That
> > would be a good indication of suspend/resume since root-hubs will be
> > suspended last while resumed first.
> > 
> > Will that work?
> No.  Remember, this mechanism has to work on non-SoC systems
> too.  And 
> even on an SoC, it's possible that your device is just one of several 
> plugged into an external hub.  So your device might be suspended
> while 
> the others remain active; then the root hub would not be suspended.
> 

I think my point is - usbfs driver is actually doing nothing w.r.t USB-
2.0 L2 state, right? The root-hub's suspend will invoke the USB-2.0 L2
transitions. This will happen when all the USB devices on that port
report idle to USB-core.
I agree that usually driver's suspend/resume call-back will put the
device in its low power state. But that is not applicable to udev of
usbfs driver, right?

So, doesn't it makes sense to tell user-space about actual USB-2.0 L2
state transitions rather than suspend/resume entry call-backs of device-
driver model of kernel (which are stub in this context)?

> Alan Stern
>