Re: [PATCH v6 part1 6/8] usb: find internal hub tier mismatch via acpi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2014-02-28 at 15:18 -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 [1] , ACPI can override the default (USB3 defined) peer port
> association for internal hubs.  External hubs follow the default peer
> association scheme.
> 
> Location data is cached as an opaque cookie in usb_port_location data.
> 
> Note that we only consider the group_token and group_position attributes
> from the _PLD data as ACPI specifies that group_token is a unique
> identifier.
> 
> When we find port location data for a port then we assume that the
> firmware will also describe its peer port.  This allows the
> implementation to only ever set the peer once.  However, once a peer is
> set via firmware data we need to recursively set the default peer for
> port below that port in the hierarchy.


Here it is with the changelog fixed up to remove this last sentence.

8<-----------
Subject: usb: find internal hub tier mismatch via acpi

From: Dan Williams <dan.j.williams@xxxxxxxxx>

ACPI identifies peer ports by setting their 'group_token' and
'group_position' _PLD data to the same value.  If a platform has tier
mismatch [1] , ACPI can override the default (USB3 defined) peer port
association for internal hubs.  External hubs follow the default peer
association scheme.

Location data is cached as an opaque cookie in usb_port_location data.

Note that we only consider the group_token and group_position attributes
from the _PLD data as ACPI specifies that group_token is a unique
identifier.

When we find port location data for a port then we assume that the
firmware will also describe its peer port.  This allows the
implementation to only ever set the peer once.  This leads to a question
about what happens when a pm runtime event occurs while the peer
associations are still resolving.  Since we only ever set the peer
information once a USB3 port needs to be prevented from suspending while
its ->peer pointer is NULL.  There is always the possibility that
firmware mis-identifies the ports, but there is not much the kernel can
do in that case.

[1]: xhci 1.1 appendix D figure 131
[2]: acpi 5 section 6.1.8

[alan]: don't do default peering when acpi data present
Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
 drivers/usb/core/hub.h      |    2 +
 drivers/usb/core/port.c     |  113 ++++++++++++++++++++++++++++++++++++++++---
 drivers/usb/core/usb-acpi.c |   32 ++++++++++--
 drivers/usb/core/usb.h      |    6 ++
 4 files changed, 139 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index d51feb68165b..6858a55eceb5 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -83,6 +83,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 +94,7 @@ struct usb_port {
 	struct dev_state *port_owner;
 	struct usb_port *peer;
 	enum usb_port_connect_type connect_type;
+	usb_port_location_t 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 8ab5999e2166..313f20fdfbf2 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -155,16 +155,27 @@ static struct device_driver usb_port_driver = {
 };
 
 /*
- * Set the default peer port for root hubs.  Assumes the primary_hcd is
- * registered first
+ * Set the default peer port for root hubs.  Platform firmware will have
+ * already set the peer if tier-mismatch is present.  Assumes the
+ * primary_hcd is registered first
  */
 static struct usb_port *find_default_peer(struct usb_hub *hub, int port1)
 {
-	struct usb_device *hdev = hub->hdev;
+	struct usb_port *port_dev = hub->ports[port1 - 1];
+	struct usb_device *hdev = hub ? hub->hdev : NULL;
 	struct usb_device *peer_hdev;
 	struct usb_port *peer = NULL;
 	struct usb_hub *peer_hub;
 
+	/*
+	 * If location data is available then we can only peer this port
+	 * by a location match, not the default peer (lest we create a
+	 * situation where we need to go back and undo a default peering
+	 * when the port is later peered by location data)
+	 */
+	if (port_dev->location)
+		return NULL;
+
 	if (!hub || hub->disconnected)
 		return NULL;
 
@@ -238,9 +249,92 @@ static void unlink_peers(struct usb_port *left, struct usb_port *right)
 	left->peer = NULL;
 }
 
+/**
+ * for_each_child_port() - invoke 'fn' on all usb_port instances beneath 'hdev'
+ * @hdev: potential hub usb_device (validated by usb_hub_to_struct_hub)
+ * @level: track recursion level to stop after reaching USB spec max depth
+ * @p: parameter to pass to 'fn'
+ * @fn: routine to invoke on each port
+ *
+ * Recursively iterate all ports (depth-first) beneath 'hdev' until 'fn'
+ * returns a non-NULL usb_port or all ports have been visited.
+ */
+static struct usb_port *for_each_child_port(struct usb_device *hdev, int level,
+		void *p, struct usb_port * (*fn)(struct usb_port *, void *))
+{
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+	int port1;
+
+#define MAX_HUB_DEPTH 5
+	if (!hub || level > MAX_HUB_DEPTH || hub->disconnected
+			|| hdev->state == USB_STATE_NOTATTACHED)
+		return NULL;
+
+	level++;
+	for (port1 = 1; port1 <= hdev->maxchild; port1++) {
+		struct usb_port *ret, *port_dev = hub->ports[port1 - 1];
+
+		if (!port_dev)
+			continue;
+		ret = fn(port_dev, p);
+		if (ret)
+			return ret;
+		ret = for_each_child_port(port_dev->child, level, p, fn);
+		if (ret)
+			return ret;
+	}
+
+	return NULL;
+}
+
+static struct usb_port *do_match_location(struct usb_port *port_dev, void *_loc)
+{
+	usb_port_location_t *loc = _loc;
+
+	if (port_dev->location == *loc)
+		return port_dev;
+	return NULL;
+}
+
+static struct usb_port *find_port_by_location(struct usb_device *hdev,
+		usb_port_location_t *loc)
+{
+	return for_each_child_port(hdev, 1, loc, do_match_location);
+}
+
+void usb_set_hub_port_location(struct usb_device *hdev, int port1,
+		usb_port_location_t location)
+{
+	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_port *peer, *port_dev;
+	struct usb_device *peer_hdev;
+
+	if (!hub)
+		return;
+
+	port_dev = hub->ports[port1 - 1];
+	port_dev->location = location;
+	if (port_dev->peer) {
+		WARN_ON(1);
+		return;
+	}
+
+	if (!peer_hcd || !peer_hcd->rh_registered)
+		return;
+
+	peer_hdev = peer_hcd->self.root_hub;
+	peer = find_port_by_location(peer_hdev, &port_dev->location);
+	if (!peer)
+		return;
+
+	link_peers(port_dev, peer);
+}
+
 int usb_hub_create_port_device(struct usb_hub *hub, int port1)
 {
-	struct usb_port *port_dev, *peer = NULL;
+	struct usb_port *port_dev;
 	int retval;
 
 	port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL);
@@ -261,10 +355,13 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
 	mutex_lock(&usb_port_peer_mutex);
 	hub->ports[port1 - 1] = port_dev;
 	retval = device_register(&port_dev->dev);
-	if (retval == 0)
-		peer = find_default_peer(hub, port1);
-	if (peer)
-		link_peers(port_dev, peer);
+	/* if we registered successfully try to assign a default peer */
+	if (retval == 0 && !port_dev->peer) {
+		struct usb_port *peer = find_default_peer(hub, port1);
+
+		if (peer)
+			link_peers(port_dev, peer);
+	}
 	mutex_unlock(&usb_port_peer_mutex);
 	if (retval)
 		goto error_register;
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 76987ba68445..e29ce4d4e6a6 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -41,6 +41,17 @@ 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
@@ -86,12 +97,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)
 {
 	enum usb_port_connect_type connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
-	struct acpi_pld_info *pld;
 	union acpi_object *upc;
 	acpi_status status;
 	int ret = 0;
@@ -107,8 +117,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);
@@ -129,7 +138,6 @@ static int usb_acpi_check_port_connect_type(struct usb_device *hdev,
 	hub->ports[port1 - 1]->connect_type = connect_type;
 
 out:
-	ACPI_FREE(pld);
 	kfree(upc);
 	return ret;
 }
@@ -165,6 +173,8 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
 		return acpi_find_child_device(adev, port1, false);
 	} else if (is_usb_port(dev)) {
 		struct usb_port *port_dev = to_usb_port(dev);
+		struct acpi_pld_info *pld;
+		acpi_status status;
 
 		/* Get the struct usb_device point of port's hub */
 		udev = to_usb_device(dev->parent->parent);
@@ -196,7 +206,17 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
 			if (!adev)
 				return NULL;
 		}
-		usb_acpi_check_port_connect_type(udev, adev->handle, port1);
+
+		status = acpi_get_physical_device_location(adev->handle, &pld);
+		if (ACPI_FAILURE(status))
+			pld = NULL;
+
+		usb_acpi_check_port_connect_type(udev, adev->handle, port1, pld);
+		usb_acpi_check_port_peer(udev, adev->handle, port1, pld);
+
+		if (pld)
+			ACPI_FREE(pld);
+
 		return adev;
 	}
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 8a1acf59d498..6d05a0a92977 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -177,11 +177,17 @@ extern void usbfs_conn_disc_event(void);
 extern int usb_devio_init(void);
 extern void usb_devio_cleanup(void);
 
+/* firmware specific cookie idenifying a port's location */
+#define USB_PORT_LOCATION_NONE 0
+typedef u32 usb_port_location_t;
+
 /* internal notify stuff */
 extern void usb_notify_add_device(struct usb_device *udev);
 extern void usb_notify_remove_device(struct usb_device *udev);
 extern void usb_notify_add_bus(struct usb_bus *ubus);
 extern void usb_notify_remove_bus(struct usb_bus *ubus);
+extern void usb_set_hub_port_location(struct usb_device *hdev, int port1,
+	usb_port_location_t location);
 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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux