Change in coreboot[master]: drivers/usb/pci_xhci: Don't return ACPI names for missing ports - coreboot-gerrit

10 Jul 2020

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43354 )
Change subject: drivers/usb/pci_xhci: Don't return ACPI names for missing ports
......................................................................
drivers/usb/pci_xhci: Don't return ACPI names for missing ports
We only want to return an ACPI name for a USB port if the controller
physically has the port. This has the desired side effect of making the
usb_acpi driver skip generating an ACPI node for a device which has
no port. This prevents writing an invalid SSDT table which the OS then
complains about.
BUG=b:154756391, b:158096224
TEST=Boot picasso trembyle and verify HS05, HS06 and SS05 are no longer
generated. Also checked the logs and saw the devices being ignored.
_SB.PCI0.LPCB.EC0.CREC.TUN0: Cros EC I2C Tunnel at GENERIC: 0.0
_SB.PCI0.LPCB.EC0.CREC.MSTH: Cros EC I2C Tunnel at GENERIC: 1.0
_SB.PCI0.LPCB.EC0.CREC.ECA0: Cros EC audio codec at GENERIC: 0.0
_SB.PCI0.PBRA.XHC0.RHUB.HS01: Left Type-C Port at USB2 port 0
_SB.PCI0.PBRA.XHC0.RHUB.HS02: Left Type-A Port at USB2 port 1
_SB.PCI0.PBRA.XHC0.RHUB.HS03: Right Type-A Port at USB2 port 2
_SB.PCI0.PBRA.XHC0.RHUB.HS04: Right Type-C Port at USB2 port 3
xhci_acpi_name: USB2 port 4 does not exist on xHC PCI: 03:00.3
xhci_acpi_name: USB2 port 5 does not exist on xHC PCI: 03:00.3
_SB.PCI0.PBRA.XHC0.RHUB.SS01: Left Type-C Port at USB3 port 0
_SB.PCI0.PBRA.XHC0.RHUB.SS02: Left Type-A Port at USB3 port 1
_SB.PCI0.PBRA.XHC0.RHUB.SS03: Right Type-A Port at USB3 port 2
_SB.PCI0.PBRA.XHC0.RHUB.SS04: Right Type-C Port at USB3 port 3
xhci_acpi_name: USB3 port 4 does not exist on xHC PCI: 03:00.3
Signed-off-by: Raul E Rangel rrangel@chromium.org
Change-Id: Ia645380bea74f39fd94e2f9cbca3fcd4d18a878e
---
M src/drivers/usb/pci_xhci/pci_xhci.c
1 file changed, 96 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/43354/1

diff --git a/src/drivers/usb/pci_xhci/pci_xhci.c b/src/drivers/usb/pci_xhci/pci_xhci.c
index edee03a..b787127 100644
--- a/src/drivers/usb/pci_xhci/pci_xhci.c
+++ b/src/drivers/usb/pci_xhci/pci_xhci.c
@@ -24,11 +24,72 @@
    return CB_SUCCESS;
 }
+
+static void xhci_count_ports(void *context, const struct xhci_supported_protocol *data)
+{
+	struct port_counts *counts = context;
+
+	switch (data->major_rev) {
+	case 3:
+		counts->super_speed += data->port_count;
+		return;
+	case 2:
+		counts->high_speed += data->port_count;
+		return;
+	default:
+		printk(BIOS_INFO, "%s: Unknown USB Version: %#x\n", __func__, data->major_rev);
+		return;
+	}
+}
+
+static bool xhci_port_exists(const struct device *dev, const struct usb_path *path)
+{
+	/* Cache the counts so we don't have to iterate on each invocation. */
+	static struct {
+		const struct device *dev;
+		struct port_counts counts;
+	} cache;
+
+	if (cache.dev != dev) {
+		cache.counts.high_speed = 0;
+		cache.counts.super_speed = 0;
+		cache.dev = dev;
+
+		xhci_for_each_supported_usb_cap(dev, &cache.counts, xhci_count_ports);
+	}
+
+	/* port_ids are 0 based */
+	switch (path->port_type) {
+	case 3:
+		return path->port_id < cache.counts.super_speed;
+	case 2:
+		return path->port_id < cache.counts.high_speed;
+	default:
+		printk(BIOS_INFO, "%s: Unknown USB Version: %#x\n", __func__, path->port_type);
+		return false;
+	}
+}
+
+static const struct device *get_xhci_dev(const struct device *dev)
+{
+	while (dev && dev->ops != &xhci_pci_ops) {
+		if (dev->path.type == DEVICE_PATH_ROOT)
+			return NULL;
+
+		dev = dev->bus->dev;
+	}
+
+	return dev;
+}
+
 static const char *xhci_acpi_name(const struct device *dev)
 {
    char *name;
    unsigned int port_id;
+	const char *pattern;
+	const struct device *xhci_dev;
+	/* Generate ACPI names for the usb_acpi driver */
    if (dev->path.type == DEVICE_PATH_USB) {
    	/* Ports index start at 1 */
    	port_id = dev->path.usb.port_id + 1;
@@ -37,22 +98,48 @@
    	case 0:
    		return "RHUB";
    	case 2:
-			name = malloc(ACPI_NAME_BUFFER_SIZE);
-			snprintf(name, ACPI_NAME_BUFFER_SIZE, "HS%02d", port_id);
-			name[4] = '\0';
-
-			return name;
+			pattern = "HS%02d";
+			break;
    	case 3:
-			name = malloc(ACPI_NAME_BUFFER_SIZE);
-			snprintf(name, ACPI_NAME_BUFFER_SIZE, "SS%02d", port_id);
-			name[4] = '\0';
-			return name;
+			pattern = "SS%02d";
+			break;
+		default:
+			printk(BIOS_INFO, "%s: Unknown USB Version: %#x\n", __func__,
+			       dev->path.usb.port_type);
+			return NULL;
    	}
+
+		xhci_dev = get_xhci_dev(dev);
+		if (!xhci_dev)
+			die("%s: xHCI controller not found for %s\n", __func__, dev_path(dev));
+
+		/*
+		 * We only want to return an ACPI name for a USB port if the controller
+		 * physically has the port. This has the desired side effect of making the
+		 * usb_acpi driver skip generating an ACPI node for a device which has
+		 * no port. This prevents writing an invalid SSDT table which the OS then
+		 * complains about.
+		 */
+		if (!xhci_port_exists(xhci_dev, &dev->path.usb)) {
+			printk(BIOS_WARNING, "%s: %s does not exist on xHC ", __func__,
+			       dev_path(dev));
+			/* dev_path uses a static buffer :( */
+			printk(BIOS_WARNING, "%s\n", dev_path(xhci_dev));
+
+			return NULL;
+		}
+
+		name = malloc(ACPI_NAME_BUFFER_SIZE);
+		snprintf(name, ACPI_NAME_BUFFER_SIZE, pattern, port_id);
+		name[4] = '\0';
+
+		return name;
    } else if (dev->ops == &xhci_pci_ops) {
    	return dev->name;
    }
printk(BIOS_ERR, "%s: Unknown device %s\n", __func__, dev_path(dev));
+
    return NULL;
 }
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/43354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia645380bea74f39fd94e2f9cbca3fcd4d18a878e
Gerrit-Change-Number: 43354
Gerrit-PatchSet: 1
Gerrit-Owner: Raul Rangel rrangel@chromium.org
Gerrit-MessageType: newchange



    