Possible bug in xhci_resume()

[Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>]

Hi Andiry,

I'm seeing an unreachable code path in xhci_resume().  Can you please
explain what you meant to do here?

In xhci_resume, you save the host controller's state in the variable
old_state.  But then you only use it on an unreachable code path:

int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
{
	u32			command, temp = 0;
	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
	int	old_state, retval;

	old_state = hcd->state;
	...

	if (!hibernated) {
		/* restore registers */
	}

	if ((temp & STS_SRE) || hibernated) {
		/* restart the host controller */

		if (!retval)
			set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
		hcd->state = HC_STATE_SUSPENDED;
		return retval;

		/* Note the return here - this means if hibernated is
		 * true, we will always return out of this function
		 * here.
		 */
	}

	...
	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
	if (!hibernated)
		hcd->state = old_state;
	else
		hcd->state = HC_STATE_SUSPENDED;
	...
}

Did you mean to set the hcd->state back to the old value when the host
was resumed after a power loss?  If so, perhaps the if statement that
tests for hibernation or a power loss could be changed to:

	if ((temp & STS_SRE) || hibernated) {
		/* restart the host controller */
		if (!retval)
			set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
		if (temp & STS_SRE)
			hcd->state = old_state;
		else
			hcd->state = HC_STATE_SUSPENDED;
		return retval;
	}

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