On Mon, Jan 13, 2014 at 02:56:57PM -0800, Sarah Sharp wrote: > On Tue, Jan 07, 2014 at 12:29:44PM -0800, Dan Williams wrote: > > ACPI identifies peer ports by setting their 'group_token' and 'group_position' > > _PLD data to the same value. If a platform has tier mismatch (see Appendix D > > figure 131 in the xhci spec), ACPI can override the default peer port > > association (for internal hubs). > > > > Location data is cached as an opaque cookie in usb_port_location data. > > I haven't looked at this too closely, but what happens if: > - the USB 2.0 roothub is registered > - userspace immediately sets autosuspend_delay to zero and > pm_qos_no_port_poweroff to zero > - usb_acpi_find_device changes the connect_type to something other than > unknown > - before usb_acpi_check_port_peer() is called, the port is suspended Or that call finishes, but no peer port is set yet, because the USB 3.0 roothub isn't registered yet. The USB 2.0 bus can be suspended before the USB 3.0 bus has finished registering, as I've seen from this thread: http://marc.info/?l=linux-usb&m=138914518219334&w=2 > In that case, we'll potentially power off a suspended USB 2.0 port > before we know if it has a USB 3.0 peer port. It seems like you > shouldn't change the connection type until you set the peer port. > Unless you need that connection type for matching the peer port? > > Also, why only match on group token and group position? The xHCI spec > in Appendix D says, "The _UPC declarations for LS/FS/HS and SS ports > that are paired to form a USB 3.0 compatible connector. A 'pair' is > defined by two ports that declare _PLDs with identical Panel, Vertical > Position, Horizontal Position, Shape, Group Orientation and Group Token > parameter values." (Note, I know nothing about what these fields are > actually filled in with, I'm just quoting the spec here.) > > Sarah Sharp > > > > > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> > > --- > > drivers/usb/core/hub.h | 6 +++ > > drivers/usb/core/port.c | 96 +++++++++++++++++++++++++++++++++++++++++++ > > drivers/usb/core/usb-acpi.c | 35 +++++++++++++--- > > drivers/usb/core/usb.h | 2 + > > 4 files changed, 131 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h > > index b5efc713498e..2ba10798c943 100644 > > --- a/drivers/usb/core/hub.h > > +++ b/drivers/usb/core/hub.h > > @@ -76,6 +76,10 @@ struct usb_hub { > > struct usb_port **ports; > > }; > > > > +struct usb_port_location { > > + u32 cookie; > > +}; > > + > > /** > > * struct usb port - kernel's representation of a usb port > > * @child: usb device attatched to the port > > @@ -83,6 +87,7 @@ struct usb_hub { > > * @port_owner: port's owner > > * @peer: related usb2 and usb3 ports (share the same connector) > > * @connect_type: port's connect type > > + * @location: opaque representation of platform connector location > > * @portnum: port index num based one > > * @power_is_on: port's power state > > * @did_runtime_put: port has done pm_runtime_put(). > > @@ -93,6 +98,7 @@ struct usb_port { > > struct dev_state *port_owner; > > struct usb_port *peer; > > enum usb_port_connect_type connect_type; > > + struct usb_port_location location; > > u8 portnum; > > unsigned power_is_on:1; > > unsigned did_runtime_put:1; > > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c > > index 7fd22020d7ee..8fae3cd03305 100644 > > --- a/drivers/usb/core/port.c > > +++ b/drivers/usb/core/port.c > > @@ -149,11 +149,14 @@ struct device_type usb_port_device_type = { > > > > static struct usb_port *find_peer_port(struct usb_hub *hub, int port1) > > { > > - struct usb_device *hdev = hub->hdev; > > + struct usb_device *hdev = hub ? hub->hdev : NULL; > > struct usb_device *peer_hdev; > > struct usb_port *peer = NULL; > > struct usb_hub *peer_hub; > > > > + if (!hub) > > + return NULL; > > + > > /* set the default peer port for root hubs. Platform firmware > > * can later fix this up if tier-mismatch is present. Assumes > > * the primary_hcd is usb2.0 and registered first > > @@ -193,6 +196,97 @@ static struct usb_port *find_peer_port(struct usb_hub *hub, int port1) > > return peer; > > } > > > > +static void reset_peer(struct usb_port *port_dev, struct usb_port *peer) > > +{ > > + if (!peer) > > + return; > > + > > + spin_lock(&peer_lock); > > + if (port_dev->peer) > > + put_device(&port_dev->peer->dev); > > + if (peer->peer) > > + put_device(&peer->peer->dev); > > + port_dev->peer = peer; > > + peer->peer = port_dev; > > + get_device(&peer->dev); > > + get_device(&port_dev->dev); > > + spin_unlock(&peer_lock); > > +} > > + > > +/* assumes that location data is only set for external connectors and that > > + * external hubs never have tier mismatch > > + */ > > +static int redo_find_peer_port(struct device *dev, void *data) > > +{ > > + struct usb_port *port_dev = to_usb_port(dev); > > + > > + if (is_usb_port(dev)) { > > + struct usb_device *hdev = to_usb_device(dev->parent->parent); > > + struct usb_hub *hub = usb_hub_to_struct_hub(hdev); > > + int port1 = port_dev->portnum; > > + struct usb_port *peer; > > + > > + peer = find_peer_port(hub, port1); > > + reset_peer(port_dev, peer); > > + } > > + > > + return device_for_each_child(dev, NULL, redo_find_peer_port); > > +} > > + > > +static int pair_port(struct device *dev, void *data) > > +{ > > + struct usb_port *peer = data; > > + struct usb_port *port_dev = to_usb_port(dev); > > + > > + if (!is_usb_port(dev) > > + || port_dev->location.cookie != peer->location.cookie) > > + return device_for_each_child(dev, peer, pair_port); > > + > > + dev_dbg(dev->parent->parent, "port%d peer = %s port%d (by location)\n", > > + port_dev->portnum, dev_name(peer->dev.parent->parent), > > + peer->portnum); > > + if (port_dev->peer != peer) { > > + /* Sigh, tier mismatch has invalidated our ancestry. > > + * This should not be too onerous even in deep hub > > + * topologies as we will discover tier mismatch early > > + * (after platform internal hubs have been enumerated), > > + * before external hubs are probed. > > + */ > > + reset_peer(port_dev, peer); > > + device_for_each_child(dev, NULL, redo_find_peer_port); > > + } > > + > > + return true; > > +} > > + > > +void usb_set_hub_port_location(struct usb_device *hdev, int port1, > > + u32 cookie) > > +{ > > + struct usb_hub *hub = usb_hub_to_struct_hub(hdev); > > + struct usb_hcd *hcd = bus_to_hcd(hdev->bus); > > + struct usb_hcd *peer_hcd = hcd->shared_hcd; > > + struct usb_device *peer_hdev; > > + struct usb_port *port_dev; > > + > > + if (cookie == 0) > > + return; > > + > > + if (!hub) > > + return; > > + > > + port_dev = hub->ports[port1 - 1]; > > + port_dev->location.cookie = cookie; > > + > > + /* see if a port with the same location data exists in a peer > > + * usb domain > > + */ > > + if (!peer_hcd) > > + return; > > + > > + peer_hdev = peer_hcd->self.root_hub; > > + device_for_each_child(&peer_hdev->dev, port_dev, pair_port); > > +} > > + > > int usb_hub_create_port_device(struct usb_hub *hub, int port1) > > { > > struct usb_port *port_dev, *peer; > > diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c > > index 4e243c37f17f..e957c5216ebe 100644 > > --- a/drivers/usb/core/usb-acpi.c > > +++ b/drivers/usb/core/usb-acpi.c > > @@ -18,7 +18,7 @@ > > #include <linux/usb/hcd.h> > > #include <acpi/acpi_bus.h> > > > > -#include "usb.h" > > +#include "hub.h" > > > > /** > > * usb_acpi_power_manageable - check whether usb port has > > @@ -42,6 +42,19 @@ bool usb_acpi_power_manageable(struct usb_device *hdev, int index) > > } > > EXPORT_SYMBOL_GPL(usb_acpi_power_manageable); > > > > +static void usb_acpi_check_port_peer(struct usb_device *hdev, > > + acpi_handle *handle, int port1, > > + struct acpi_pld_info *pld) > > +{ > > + if (!pld) > > + return; > > + > > + #define USB_ACPI_LOCATION_VALID (1 << 31) > > + usb_set_hub_port_location(hdev, port1, USB_ACPI_LOCATION_VALID > > + | pld->group_token << 8 > > + | pld->group_position); > > +} > > + > > /** > > * usb_acpi_set_power_state - control usb port's power via acpi power > > * resource > > @@ -83,12 +96,11 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable) > > EXPORT_SYMBOL_GPL(usb_acpi_set_power_state); > > > > static int usb_acpi_check_port_connect_type(struct usb_device *hdev, > > - acpi_handle handle, int port1) > > + acpi_handle handle, int port1, struct acpi_pld_info *pld) > > { > > acpi_status status; > > struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > > union acpi_object *upc; > > - struct acpi_pld_info *pld; > > int ret = 0; > > > > /* > > @@ -99,8 +111,7 @@ static int usb_acpi_check_port_connect_type(struct usb_device *hdev, > > * a usb device is directly hard-wired to the port. If no visible and > > * no connectable, the port would be not used. > > */ > > - status = acpi_get_physical_device_location(handle, &pld); > > - if (ACPI_FAILURE(status)) > > + if (!pld) > > return -ENODEV; > > > > status = acpi_evaluate_object(handle, "_UPC", NULL, &buffer); > > @@ -122,7 +133,6 @@ static int usb_acpi_check_port_connect_type(struct usb_device *hdev, > > usb_set_hub_port_connect_type(hdev, port1, USB_PORT_NOT_USED); > > > > out: > > - ACPI_FREE(pld); > > kfree(upc); > > return ret; > > } > > @@ -179,6 +189,9 @@ static int usb_acpi_find_device(struct device *dev, acpi_handle *handle) > > return -ENODEV; > > return 0; > > } else if (is_usb_port(dev)) { > > + struct acpi_pld_info *pld; > > + acpi_status status; > > + > > sscanf(dev_name(dev), "port%d", &port_num); > > /* Get the struct usb_device point of port's hub */ > > udev = to_usb_device(dev->parent->parent); > > @@ -209,7 +222,15 @@ static int usb_acpi_find_device(struct device *dev, acpi_handle *handle) > > if (!*handle) > > return -ENODEV; > > } > > - usb_acpi_check_port_connect_type(udev, *handle, port_num); > > + > > + status = acpi_get_physical_device_location(*handle, &pld); > > + if (ACPI_FAILURE(status)) > > + pld = NULL; > > + > > + usb_acpi_check_port_connect_type(udev, *handle, port_num, pld); > > + usb_acpi_check_port_peer(udev, *handle, port_num, pld); > > + > > + ACPI_FREE(pld); > > } else > > return -ENODEV; > > > > diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h > > index c49383669cd8..b80785106b27 100644 > > --- a/drivers/usb/core/usb.h > > +++ b/drivers/usb/core/usb.h > > @@ -184,6 +184,8 @@ extern enum usb_port_connect_type > > usb_get_hub_port_connect_type(struct usb_device *hdev, int port1); > > extern void usb_set_hub_port_connect_type(struct usb_device *hdev, int port1, > > enum usb_port_connect_type type); > > +extern void usb_set_hub_port_location(struct usb_device *hdev, int port1, > > + u32 cookie); > > extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev, > > struct usb_hub_descriptor *desc); > > > > > -- > 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 -- 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
