Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/23768
Change subject: libpayload/usb: Disable ports for unused devices ......................................................................
libpayload/usb: Disable ports for unused devices
USB devices that aren't controlled by libpayload due to missing driver support do have an address assigned libpayload doesn't know about. To avoid address conflicts disable the hub port if no driver is loaded after probing the device.
Change-Id: I4882ba81cc0bed8dc0001f31142dc783e1f4259d Signed-off-by: Patrick Rudolph siro@das-labor.org --- M payloads/libpayload/drivers/usb/ehci_rh.c M payloads/libpayload/drivers/usb/generic_hub.c M payloads/libpayload/drivers/usb/ohci_rh.c M payloads/libpayload/drivers/usb/uhci_rh.c M payloads/libpayload/drivers/usb/usbhub.c M payloads/libpayload/drivers/usb/xhci_rh.c 6 files changed, 57 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/23768/1
diff --git a/payloads/libpayload/drivers/usb/ehci_rh.c b/payloads/libpayload/drivers/usb/ehci_rh.c index da8d8af..4eb8dfd 100644 --- a/payloads/libpayload/drivers/usb/ehci_rh.c +++ b/payloads/libpayload/drivers/usb/ehci_rh.c @@ -149,6 +149,22 @@ RH_INST(dev)->devices[port] = usb_attach_device(dev->controller , dev->address, port, port_speed); } + + if (RH_INST(dev)->devices[port] == -1) { + // Keep software and hardware state in sync. + // Disable port to make sure that the assigned address isn't + // valid any more. + // Deassert enable, assert reset. These must change atomically. + RH_INST(dev)->ports[port] = + (RH_INST(dev)->ports[port] & ~P_PORT_ENABLE) | P_PORT_RESET; + + /* Wait a bit while reset is active (+1 to avoid Tegra race). */ + mdelay(50 + 1); // usb20 spec 7.1.7.5 (TDRSTR) + + /* Deassert reset. */ + RH_INST(dev)->ports[port] &= ~P_PORT_RESET; + } + /* RW/C register, so clear it by writing 1 */ RH_INST(dev)->ports[port] |= P_CONN_STATUS_CHANGE; } diff --git a/payloads/libpayload/drivers/usb/generic_hub.c b/payloads/libpayload/drivers/usb/generic_hub.c index 9f86705..ea2b71f 100644 --- a/payloads/libpayload/drivers/usb/generic_hub.c +++ b/payloads/libpayload/drivers/usb/generic_hub.c @@ -177,6 +177,15 @@ hub->ports[port] = usb_attach_device( dev->controller, dev->address, port, speed); } + + if ((hub->ports[port] == -1) && hub->ops->disable_port) { + // Keep software and hardware state in sync. + // Disable port to make sure that the assigned address isn't + // valid any more. + if (hub->ops->disable_port(dev, port) < 0) + return -1; + } + return 0; }
diff --git a/payloads/libpayload/drivers/usb/ohci_rh.c b/payloads/libpayload/drivers/usb/ohci_rh.c index 5d82bd5..770a563 100644 --- a/payloads/libpayload/drivers/usb/ohci_rh.c +++ b/payloads/libpayload/drivers/usb/ohci_rh.c @@ -128,6 +128,13 @@
usb_speed speed = (OHCI_INST(dev->controller)->opreg->HcRhPortStatus[port] & LowSpeedDeviceAttached) != 0; RH_INST (dev)->port[port] = usb_attach_device(dev->controller, dev->address, port, speed); + + if (RH_INST(dev)->port[port] == -1) { + // Keep software and hardware state in sync. + // Disable port to make sure that the assigned address isn't + // valid any more. + ohci_rh_disable_port(dev, port); + } }
static int diff --git a/payloads/libpayload/drivers/usb/uhci_rh.c b/payloads/libpayload/drivers/usb/uhci_rh.c index e08cce1..742d491 100644 --- a/payloads/libpayload/drivers/usb/uhci_rh.c +++ b/payloads/libpayload/drivers/usb/uhci_rh.c @@ -137,6 +137,13 @@
RH_INST (dev)->port[offset] = usb_attach_device(dev->controller, dev->address, portsc, speed); } + + if (RH_INST(dev)->port[offset] == -1) { + // Keep software and hardware state in sync. + // Disable port to make sure that the assigned address isn't + // valid any more. + uhci_rh_disable_port(dev, port); + } }
static int diff --git a/payloads/libpayload/drivers/usb/usbhub.c b/payloads/libpayload/drivers/usb/usbhub.c index 340e47a..18ed406 100644 --- a/payloads/libpayload/drivers/usb/usbhub.c +++ b/payloads/libpayload/drivers/usb/usbhub.c @@ -119,6 +119,12 @@ }
static int +usb_hub_disable_port(usbdev_t *const dev, const int port) +{ + return clear_feature(dev, port, SEL_PORT_POWER, DR_PORT); +} + +static int usb_hub_start_port_reset(usbdev_t *const dev, const int port) { return set_feature (dev, port, SEL_PORT_RESET, DR_PORT); @@ -153,7 +159,7 @@ .port_enabled = usb_hub_port_enabled, .port_speed = usb_hub_port_speed, .enable_port = usb_hub_enable_port, - .disable_port = NULL, + .disable_port = usb_hub_disable_port, .start_port_reset = usb_hub_start_port_reset, .reset_port = generic_hub_resetport, }; diff --git a/payloads/libpayload/drivers/usb/xhci_rh.c b/payloads/libpayload/drivers/usb/xhci_rh.c index d7ba82c..5650076 100644 --- a/payloads/libpayload/drivers/usb/xhci_rh.c +++ b/payloads/libpayload/drivers/usb/xhci_rh.c @@ -136,6 +136,16 @@ return 0; }
+static int +xhci_rh_disable_port(usbdev_t *const dev, int port) +{ + xhci_t *const xhci = XHCI_INST(dev->controller); + volatile u32 *const portsc = + &xhci->opreg->prs[port - 1].portsc; + + *portsc = ((*portsc & PORTSC_RW_MASK) | PORTSC_PED) & ~PORTSC_PP; + return 0; +}
static const generic_hub_ops_t xhci_rh_ops = { .hub_status_changed = xhci_rh_hub_status_changed, @@ -145,7 +155,7 @@ .port_enabled = xhci_rh_port_enabled, .port_speed = xhci_rh_port_speed, .enable_port = xhci_rh_enable_port, - .disable_port = NULL, + .disable_port = xhci_rh_disable_port, .start_port_reset = NULL, .reset_port = xhci_rh_reset_port, };
-
Patrick Rudolph (Code Review)