On Fri, 31 Jul 2009, Jason Wessel wrote:
> If the EHCI debug port is initialized and in use, the EHCI host
> controller driver must follow two rules.
>
> 1) If the EHCI host driver issues a controller reset, the debug
> controller driver re-initialization must get called after the reset
> is completed.
>
> 2) The EHCI host driver should ignore any requests to the physical
> EHCI debug port when the EHCI debug port is in use.
>
> The code to check for the debug port was moved from ehci_pci_reinit()
> to ehci_pci_setup because it must get called prior to ehci_reset()
> which will clear the debug port registers.
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -816,6 +816,15 @@ static int ehci_hub_control (
> case SetPortFeature:
> selector = wIndex >> 8;
> wIndex &= 0xff;
> + if (unlikely(ehci->debug)) {
Don't you need a similar test for ClearPortFeature? Or does that not
matter?
> + /* If the debug port is active any port
> + * feature requests should get denied */
> + if ((readl(&ehci->debug->control) & DBGP_ENABLED) &&
> + wIndex == HCS_DEBUG_PORT(ehci->hcs_params)) {
The order of the two tests here should be reversed, to avoid an
unnecessary I/O access in the common case.
> + retval = -ENODEV;
> + goto error_exit;
This is the wrong return value. You should return -EPIPE, i.e., goto
error instead of error_exit.
> + }
> + }
> if (!wIndex || wIndex > ports)
> goto error;
> wIndex--;
> @@ -894,6 +903,7 @@ error:
> /* "stall" on error */
> retval = -EPIPE;
> }
> +error_exit:
And then this new label isn't needed.
> spin_unlock_irqrestore (&ehci->lock, flags);
> return retval;
> }
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
