Re: usb_wwan_write() called while device still being resumed

[Alex Courbot]

On 02/15/2013 08:05 PM, BjÃrn Mork wrote:
> Maybe something like the completely untested:
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 3148b10..38e19ba 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -512,6 +512,9 @@ static int rpm_resume(struct device *dev, int rpmflags)
>   	else if (dev->power.disable_depth == 1 && dev->power.is_suspended
>   	    && dev->power.runtime_status == RPM_ACTIVE)
>   		retval = 1;
> +	else if (rpmflags & RPM_ASYNC && dev->power.request_pending &&
> +		 dev->power.request == RPM_REQ_RESUME)
> +		retval = -EINPROGRESS;
>   	else if (dev->power.disable_depth > 0)
>   		retval = -EACCES;
>   	if (retval)

Second thought: not sure this will work since power.request_pending and 
power.request are set to these values later in the same rpm_resume() 
function. However, the three lines before yours caught my attention. 
They are not in my 3.1 source tree and the conditions are very close 
from the ones I am seeing when the issue happens: disable_depth == 1, 
is_suspended == 1. Only runtime_status is not equal to RPM_ACTIVE.

Nonetheless, I have looked at the patch that introduced these 
(http://pastebin.com/PmHUjiAE ) and it details a problem that is very 
similar to mine. It also mentions a workaround to be implemented in the 
driver by saving the suspend status into a variable that is checked when 
pm_runtime_get() return -EACCES. This variable already exists in 
usb_wwan, actually it is the very variable that is checked a bit later 
in that other chunk of code you mentioned:

spin_lock_irqsave(&intfdata->susp_lock, flags);
if (intfdata->suspended) {
	usb_anchor_urb(this_urb, &portdata->delayed);
	spin_unlock_irqrestore(&intfdata->susp_lock, flags);
} else {

So it looks like this code is here for exactly that purpose. However in 
my current condition I do not see how this block could be run since 
runtime PM is disabled when intfdata->suspended is set to true.

I will try implementing the workaround suggested (checking if 
intfdata->suspended is set when -EACCES is returned and go on if it is 
the case) and see if it works (and I bet it will). In the upstream 
kernel the issue seems to have been addressed already, so this might 
just be a source-out-of-date issue.

Thanks,
Alex.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/