Re: [PATCH 4/7] usbnet: cdc_mbim: don't recover device if suspend fails in system sleep

Alan Stern [off-list ref] writes:

On Tue, 5 Mar 2013, Bjørn Mork wrote:

quoted

Ming Lei [off-list ref] writes:

quoted

Yes, USB core will flush any outstanding URBs, but the driver still need
to deal with suspend failure carefully, for example, suppose usb_resume()
is called in suspend failure path, and the submitted URBs are killed
by USB core later. But after the device is wakeup, and the resume() will
do nothing since the suspend count is leaked. So it is what the patches
are fixing, and it is better to not depend on the default flushing URBs of
USB core.

I am starting to wonder why the USB core has combined system suspend and
runtime suspend if we are going to end up with every driver testing
PMSG_IS_AUTO(message) and selecting a completely different code path.

Mainly for historical reasons.  System suspend existed long before 
runtime suspend did.  When runtime suspend was added, it piggybacked 
off the existing code.  Furthermore, originally there was no 
requirement that system suspend always succeed; that was added later.

Also, the code paths are not completely different.  They differ mainly 
in their error handling.  But when you think about it, how serious an 
error can you encounter when you try to _stop_ using a device?

Thanks for explaining.

quoted

You are right that we will end up with problems if usbnet_resume is
called for a device usbnet hasn't suspended.  But I'd still claim that
is a bug in the USB core, which is the one that decided to ignore the
suspend error and still call resume.

I guess proper error handling here require the USB core to see the
interface driver as dead if it fails to suspend on system suspend, and
do forced rebinding on resume.

You are welcome to submit a patch to do this.  It shouldn't be hard; we
already have a flag indicating that an interface needs to be unbound at
reprobed at resume time.  You can update the kerneldoc in addition; as
you noticed, it currently does not describe the actual code completely
accurately.

I guess I saw that coming :)  Will take a shot at it when time permits.


Bjørn