Re: [PATCH v2 1/2] USB: check port changes before hub runtime suspend for some bug device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 27 Sep 2012, Ming Lei wrote:

> From: Ming Lei <tom.leiming@xxxxxxxxx>
> 
> The hub status endpoint has a long 'bInterval', which is 255ms
> for FS/LS device and 256ms for HS device according to USB 2.0 spec,
> so the device connection change may be reported later more than 255ms
> via status pipe.
> 
> The connection change in hub may have been happened already on the
> downstream ports, but no status URB completes when it is killed
> in hub_suspend(auto), so the connection change may be missed by some
> buggy hub devices, which won't generate remote wakeup signal after
> their remote wakeup is enabled and they are put into suspend state.
> 
> The problem can be observed at least on the below Genesys Logic, Inc.
> hub devices:
> 
> 	0x05e3,0x0606
> 	0x05e3,0x0608
> 
> In theory, there is no way to fix the problem completely, but we
> can make it less likely to occur by this patch.
> 
> This patch introduces one quirk of HUB_QUIRK_CHECK_PORTS_AUTOSUSPEND
> to check ports' change during hub_suspend(auto) for the buggy
> devices. If ports' change is found, terminate the auto suspend and
> return to working state.
> 
> So for the buggy hubs, if the connection change happend before
> the ports' check, it can be handled correctly. If it happens between
> the ports' check and enabling remote wakeup/entering suspend, it
> will be missed. Considered the interval is quite short, it is very
> less likely to happen during the window.

One remark and one tiny problem...

> @@ -3176,6 +3198,17 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
>  				return -EBUSY;
>  		}
>  	}
> +
> +	if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) {
> +		/* check if there are changes pending on hub ports */
> +		if (check_ports_changed(hub)) {
> +			if (!PMSG_IS_AUTO(msg) &&
> +			    device_may_wakeup(&hdev->dev))

You don't need to check device_may_wakeup(); the core has already taken
care of it.  If device_may_wakeup() returns 0 then do_remote_wakeup
will be 0 also.

> +				pm_wakeup_event(&hdev->dev, 2000);
> +			return -EBUSY;
> +		}
> +	}

The !PMSG_IS_AUTO case should not return -EBUSY, because
usb_suspend_both() will ignore the return value.  It should proceed
with the rest of the suspend.

Alan Stern

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux