On Thu, Feb 28, 2013 at 12:40:46PM -0500, Tony Camuso wrote:
> On 02/27/2013 07:20 PM, Sarah Sharp wrote:
> >
> >Basically, I'd like Tony to make his first patch work, rather than
> >pursuing moving the timer manipulation to xhci_bus_suspend/resume.
> >
>
> Not to add confusion, but here is a less intrusive patch that simply
> checks to see if the Compliance Mode Recovery Timer already exists
> before attempting to initialize it.
>
>
>
> ---
> drivers/usb/host/xhci.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index f1f01a8..e51db78 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -445,6 +445,11 @@ static void compliance_mode_recovery(unsigned long arg)
> */
> static void compliance_mode_recovery_timer_init(struct xhci_hcd *xhci)
> {
> + if (timer_pending(&xhci->comp_mode_recovery_timer)) {
> + xhci_dbg(xhci, "Compliance Mode Recovery Timer already active.\n");
> + return;
> + }
> +
> xhci->port_status_u0 = 0;
> init_timer(&xhci->comp_mode_recovery_timer);
Isn't this racy? There's no locking here, so what if the timer fired
just before the call to timer_pending happened? (The documentation on
the timer interface doesn't describe whether timer_pending returns true
if we're in the middle of a timer callback.)
Also, looking at the compliance timer callback brings up further
questions:
static void compliance_mode_recovery(unsigned long arg)
{
...
for (i = 0; i < xhci->num_usb3_ports; i++) {
temp = xhci_readl(xhci, xhci->usb3_ports[i]);
if ((temp & PORT_PLS_MASK) == USB_SS_PORT_LS_COMP_MOD) {
/*
* Compliance Mode Detected. Letting USB Core
* handle the Warm Reset
*/
What happens when the xHCI host controller goes into D3cold due to
runtime PM? The port status registers will read as all f's, so we will
miss any transitions to the compliance mode that happened before or
during the transition to D3cold.
This code probably needs to wake up the host controller and keep it from
suspending until all the ports can be read.
Alan, would the right way to do that be a get/put call into the runtime
PM core?
Sarah Sharp
--
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
