Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 8 Jul 2019 21:08:17 +0200
From:      Hans Petter Selasky <hps@selasky.org>
To:        sgk@troutmask.apl.washington.edu
Cc:        Ian Lepore <ian@freebsd.org>, freebsd-current@freebsd.org, takawata@freebsd.org
Subject:   Re: Someone broke USB
Message-ID:  <2fbea10e-ae6c-fe77-f1bc-632c361dde07@selasky.org>
In-Reply-To: <08ded31d-b159-6400-cf69-ffb711eca66c@selasky.org>
References:  <99a61cc8-495f-a333-b4bc-46fc929bae37@selasky.org> <20190707185818.GA51398@troutmask.apl.washington.edu> <20ee6a81-d513-8332-8e47-6676dbb9a159@selasky.org> <20190707203635.GA53065@troutmask.apl.washington.edu> <2ccb6b71-022d-5b66-3ba9-007e2647b3e2@selasky.org> <20190707222555.GA55146@troutmask.apl.washington.edu> <602f5c9b-e15e-d08f-f388-47fc0a560803@selasky.org> <20190708170323.GA59922@troutmask.apl.washington.edu> <ca435fbe-9942-5b02-0ffc-9d6c5962d1e3@selasky.org> <20190708172414.GA60059@troutmask.apl.washington.edu> <20190708173030.GA60139@troutmask.apl.washington.edu> <08ded31d-b159-6400-cf69-ffb711eca66c@selasky.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------A83418D0C8E28BB8D5AF1678
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi Steve,

Can you revert all prior patches and try this one instead.

--HPS

--------------A83418D0C8E28BB8D5AF1678
Content-Type: text/x-patch;
 name="usb_acpi.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="usb_acpi.diff"

Index: sys/dev/usb/usb_hub_acpi.c
===================================================================
--- sys/dev/usb/usb_hub_acpi.c	(revision 349802)
+++ sys/dev/usb/usb_hub_acpi.c	(working copy)
@@ -80,24 +80,6 @@
 #include <contrib/dev/acpica/include/accommon.h>
 #include <dev/acpica/acpivar.h>
 
-static UINT32 acpi_uhub_find_rh_cb(ACPI_HANDLE ah, UINT32 nl, void *ctx, void **status);
-static ACPI_STATUS acpi_uhub_find_rh(device_t dev, ACPI_HANDLE * ah);
-static ACPI_STATUS
-acpi_usb_hub_port_probe_cb(ACPI_HANDLE ah, UINT32 lv, void *ctx, void **rv);
-static ACPI_STATUS acpi_usb_hub_port_probe(device_t dev, ACPI_HANDLE ah);
-static int acpi_uhub_root_probe(device_t dev);
-static int acpi_uhub_probe(device_t dev);
-static int acpi_uhub_root_attach(device_t dev);
-static int acpi_uhub_attach(device_t dev);
-static int acpi_uhub_detach(device_t dev);
-static int
-acpi_uhub_read_ivar(device_t dev, device_t child, int idx,
-    uintptr_t *res);
-static int
-acpi_uhub_child_location_string(device_t parent, device_t child,
-    char *buf, size_t buflen);
-static int acpi_uhub_parse_upc(device_t dev, unsigned int port, ACPI_HANDLE ah);
-
 struct acpi_uhub_softc {
 	struct uhub_softc usc;
 	uint8_t	nports;
@@ -104,27 +86,25 @@
 	ACPI_HANDLE *porthandle;
 };
 
-UINT32
-acpi_uhub_find_rh_cb(ACPI_HANDLE ah, UINT32 nl, void *ctx, void **status){
+static UINT32
+acpi_uhub_find_rh_cb(ACPI_HANDLE ah, UINT32 nl, void *ctx, void **status)
+{
 	ACPI_DEVICE_INFO *devinfo;
-	UINT32 ret = AE_OK;
+	UINT32 ret;
 
 	*status = NULL;
 	devinfo = NULL;
 
 	ret = AcpiGetObjectInfo(ah, &devinfo);
-
-	if (ACPI_FAILURE(ret)) {
-		return ret;
+	if (ACPI_SUCCESS(ret)) {
+		if ((devinfo->Valid & ACPI_VALID_ADR) &&
+		    (devinfo->Address == 0)) {
+			ret = AE_CTRL_TERMINATE;
+			*status = ah;
+		}
+		AcpiOsFree(devinfo);
 	}
-	if ((devinfo->Valid & ACPI_VALID_ADR) &&
-	    (devinfo->Address == 0)) {
-		ret = AE_CTRL_TERMINATE;
-		*status = ah;
-	}
-	AcpiOsFree(devinfo);
-
-	return ret;
+	return (ret);
 }
 
 static int
@@ -134,14 +114,17 @@
 
 	buf.Pointer = NULL;
 	buf.Length = ACPI_ALLOCATE_BUFFER;
+
 	if (AcpiEvaluateObject(ah, "_UPC", NULL, &buf) == AE_OK) {
 		UINT64 porttypenum, conn;
 		const char *connectable;
-		const char *typelist[] = {"TypeA", "MiniAB", "Express",
+		const char *typelist[] = {
+			"TypeA", "MiniAB", "Express",
 			"USB3-A", "USB3-B", "USB-MicroB",
 			"USB3-MicroAB", "USB3-PowerB",
 			"TypeC-USB2", "TypeC-Switch",
-		"TypeC-nonSwitch"};
+			"TypeC-nonSwitch"
+		};
 		const char *porttype;
 		const int last = sizeof(typelist) / sizeof(typelist[0]);
 		ACPI_OBJECT *obj = buf.Pointer;
@@ -162,7 +145,7 @@
 	}
 	AcpiOsFree(buf.Pointer);
 
-	return 0;
+	return (0);
 }
 
 static int
@@ -172,6 +155,7 @@
 
 	buf.Pointer = NULL;
 	buf.Length = ACPI_ALLOCATE_BUFFER;
+
 	if (AcpiEvaluateObject(ah, "_PLD", NULL, &buf) == AE_OK) {
 		ACPI_OBJECT *obj;
 		unsigned char *resbuf;
@@ -235,15 +219,12 @@
 		}
 	skip:
 		AcpiOsFree(buf.Pointer);
-		
 	}
-
-
-	return 0;
+	return (0);
 }
 
-ACPI_STATUS
-acpi_uhub_find_rh(device_t dev, ACPI_HANDLE * ah)
+static ACPI_STATUS
+acpi_uhub_find_rh(device_t dev, ACPI_HANDLE *ah)
 {
 	device_t grand;
 	ACPI_HANDLE gah;
@@ -250,130 +231,133 @@
 
 	*ah = NULL;
 	grand = device_get_parent(device_get_parent(dev));
-	if ((gah = acpi_get_handle(grand)) == NULL) {
-		return AE_ERROR;
-	}
-	return AcpiWalkNamespace(ACPI_TYPE_DEVICE, gah, 1,
-	    acpi_uhub_find_rh_cb, NULL, dev, ah);
+
+	if ((gah = acpi_get_handle(grand)) == NULL)
+		return (AE_ERROR);
+
+	return (AcpiWalkNamespace(ACPI_TYPE_DEVICE, gah, 1,
+	    acpi_uhub_find_rh_cb, NULL, dev, ah));
 }
 
-ACPI_STATUS
+static ACPI_STATUS
 acpi_usb_hub_port_probe_cb(ACPI_HANDLE ah, UINT32 lv, void *ctx, void **rv)
 {
 	ACPI_DEVICE_INFO *devinfo;
 	device_t dev = ctx;
 	struct acpi_uhub_softc *sc = device_get_softc(dev);
+	UINT32 ret;
 
-	if (usb_debug)
-		device_printf(dev, "%s\n", acpi_name(ah));
-
-	AcpiGetObjectInfo(ah, &devinfo);
-	if ((devinfo->Valid & ACPI_VALID_ADR) &&
-	    (devinfo->Address > 0) &&
-	    (devinfo->Address <= (uint64_t)sc->nports)) {
-		sc->porthandle[devinfo->Address - 1] = ah;
-		acpi_uhub_parse_upc(dev, devinfo->Address, ah);
-		acpi_uhub_parse_pld(dev, devinfo->Address, ah);
-	} else {
-		device_printf(dev, "Skiping invalid devobj %s\n",
-		    acpi_name(ah));
+	ret = AcpiGetObjectInfo(ah, &devinfo);
+	if (ACPI_SUCCESS(ret)) {
+		if ((devinfo->Valid & ACPI_VALID_ADR) &&
+		    (devinfo->Address > 0) &&
+		    (devinfo->Address <= (uint64_t)sc->nports)) {
+			sc->porthandle[devinfo->Address - 1] = ah;
+			acpi_uhub_parse_upc(dev, devinfo->Address, ah);
+			acpi_uhub_parse_pld(dev, devinfo->Address, ah);
+		}
+		AcpiOsFree(devinfo);
 	}
-	AcpiOsFree(devinfo);
-	return AE_OK;
+	return (AE_OK);
 }
 
-ACPI_STATUS
+static ACPI_STATUS
 acpi_usb_hub_port_probe(device_t dev, ACPI_HANDLE ah)
 {
-	return AcpiWalkNamespace(ACPI_TYPE_DEVICE,
+	return (AcpiWalkNamespace(ACPI_TYPE_DEVICE,
 	    ah, 1,
 	    acpi_usb_hub_port_probe_cb,
-	    NULL, dev, NULL);
+	    NULL, dev, NULL));
 }
-int
+
+static int
 acpi_uhub_root_probe(device_t dev)
 {
+	ACPI_STATUS status;
 	ACPI_HANDLE ah;
-	ACPI_STATUS status;
 
-	if(acpi_disabled("usb")) {
-		return ENXIO;
-	}
+	if (acpi_disabled("usb"))
+		return (ENXIO);
+
 	status = acpi_uhub_find_rh(dev, &ah);
-	if (ACPI_SUCCESS(status)
-	    && ah != NULL
-	    && (uhub_probe(dev) <= 0)) {
-		/* success prior than non - acpi hub */
+	if (ACPI_SUCCESS(status) && ah != NULL &&
+	    uhub_probe(dev) <= 0) {
+		/* success prior than non-ACPI USB HUB */
 		return (BUS_PROBE_DEFAULT + 1);
 	}
-	return ENXIO;
+	return (ENXIO);
 }
 
-int
+static int
 acpi_uhub_probe(device_t dev)
 {
-	ACPI_HANDLE ah = acpi_get_handle(dev);
+	ACPI_HANDLE ah;
 
-	if (!acpi_disabled("usb") && ah && (uhub_probe(dev) <= 0)) {
-		/*success prior than non - acpi hub*/
-		    return (BUS_PROBE_DEFAULT + 1);
+	if (acpi_disabled("usb"))
+		return (ENXIO);
+
+	ah = acpi_get_handle(dev);
+	if (ah == NULL)
+		return (ENXIO);
+
+	if (uhub_probe(dev) <= 0) {
+		/* success prior than non-ACPI USB HUB */
+		return (BUS_PROBE_DEFAULT + 1);
 	}
 	return (ENXIO);
 }
-int
+
+static int
 acpi_uhub_root_attach(device_t dev)
 {
-	ACPI_HANDLE devhandle;
-	struct usb_hub *uh;
 	struct acpi_uhub_softc *sc = device_get_softc(dev);
+	ACPI_STATUS status;
+	ACPI_HANDLE ah;
 	int ret;
 
-	if ((ret = uhub_attach(dev)) != 0) {
-		return (ret);
-	}
-	uh = sc->usc.sc_udev->hub;
+	ret = uhub_attach(dev);
+	if (ret != 0)
+		goto done;
 
-	if (ACPI_FAILURE(acpi_uhub_find_rh(dev, &devhandle)) ||
-	    (devhandle == NULL)) {
-		return ENXIO;
+	status = acpi_uhub_find_rh(dev, &ah);
+	if (ACPI_SUCCESS(status) && ah != NULL) {
+		struct usb_hub *uh = sc->usc.sc_udev->hub;
+
+		sc->nports = uh->nports;
+		sc->porthandle = malloc(sizeof(ACPI_HANDLE) * uh->nports,
+		    M_USBDEV, M_WAITOK | M_ZERO);
+		acpi_usb_hub_port_probe(dev, ah);
 	}
-
-	sc->nports = uh->nports;
-	sc->porthandle = malloc(sizeof(ACPI_HANDLE) * uh->nports,
-	    M_USBDEV, M_WAITOK | M_ZERO);
-	acpi_usb_hub_port_probe(dev, devhandle);
-
-	return 0;
+done:
+	return (ret);
 }
 
-int
+static int
 acpi_uhub_attach(device_t dev)
 {
-	struct usb_hub *uh;
 	struct acpi_uhub_softc *sc = device_get_softc(dev);
-	ACPI_HANDLE devhandle;
+	ACPI_HANDLE ah;
 	int ret;
 
-	if ((ret = uhub_attach(dev)) != 0) {
-		return (ret);
-	}
-	uh = sc->usc.sc_udev->hub;
-	devhandle = acpi_get_handle(dev);
+	ret = uhub_attach(dev);
+	if (ret != 0)
+		goto done;
 
-	if (devhandle == NULL) {
-		return ENXIO;
+	ah = acpi_get_handle(dev);
+	if (ah != NULL) {
+		struct usb_hub *uh = sc->usc.sc_udev->hub;
+
+		sc->nports = uh->nports;
+		sc->porthandle = malloc(sizeof(ACPI_HANDLE) * uh->nports,
+		    M_USBDEV, M_WAITOK | M_ZERO);
+		acpi_usb_hub_port_probe(dev, ah);
 	}
-
-	sc->nports = uh->nports;
-	sc->porthandle = malloc(sizeof(ACPI_HANDLE) * uh->nports,
-	    M_USBDEV, M_WAITOK | M_ZERO);
-	acpi_usb_hub_port_probe(dev, acpi_get_handle(dev));
-	return 0;
+done:
+	return (ret);
 }
 
-int
-acpi_uhub_read_ivar(device_t dev, device_t child, int idx,
-    uintptr_t *res)
+static int
+acpi_uhub_read_ivar(device_t dev, device_t child, int idx, uintptr_t *res)
 {
 	struct hub_result hres;
 	struct acpi_uhub_softc *sc = device_get_softc(dev);
@@ -382,6 +366,7 @@
 	mtx_lock(&Giant);
 	uhub_find_iface_index(sc->usc.sc_udev->hub, child, &hres);
 	mtx_unlock(&Giant);
+
 	if ((idx == ACPI_IVAR_HANDLE) &&
 	    (hres.portno > 0) &&
 	    (hres.portno <= sc->nports) &&
@@ -391,16 +376,17 @@
 	}
 	return (ENXIO);
 }
+
 static int
 acpi_uhub_child_location_string(device_t parent, device_t child,
     char *buf, size_t buflen)
 {
-
 	ACPI_HANDLE ah;
 
 	uhub_child_location_string(parent, child, buf, buflen);
+
 	ah = acpi_get_handle(child);
-	if (ah) {
+	if (ah != NULL) {
 		strlcat(buf, " handle=", buflen);
 		strlcat(buf, acpi_name(ah), buflen);
 	}
@@ -407,13 +393,14 @@
 	return (0);
 }
 
-int
+static int
 acpi_uhub_detach(device_t dev)
 {
 	struct acpi_uhub_softc *sc = device_get_softc(dev);
 
 	free(sc->porthandle, M_USBDEV);
-	return uhub_detach(dev);
+
+	return (uhub_detach(dev));
 }
 
 static device_method_t acpi_uhub_methods[] = {
@@ -438,6 +425,7 @@
 static devclass_t uhub_devclass;
 extern driver_t uhub_driver;
 static kobj_class_t uhub_baseclasses[] = {&uhub_driver, NULL};
+
 static driver_t acpi_uhub_driver = {
 	.name = "uhub",
 	.methods = acpi_uhub_methods,
@@ -444,6 +432,7 @@
 	.size = sizeof(struct acpi_uhub_softc),
 	.baseclasses = uhub_baseclasses,
 };
+
 static driver_t acpi_uhub_root_driver = {
 	.name = "uhub",
 	.methods = acpi_uhub_root_methods,

--------------A83418D0C8E28BB8D5AF1678--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2fbea10e-ae6c-fe77-f1bc-632c361dde07>