Re: [PATCH 07/10] ehci-dbgp,ehci: Allow early or late use of the dbgp device

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-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/

Relevant Pages
Loading