Remove sc->sc_lock and add several splusb()'s in dwc2, to avoid a

possible lock recursion panic on transfer timeout. The lock is not
needed yet because the USB stack is not MP-safe.

ok mpi@, "makes sense" jasper@

2 files changed, 15 insertions, 88 deletions

diff --git a/sys/dev/usb/dwc2/dwc2.c b/sys/dev/usb/dwc2/dwc2.c
index d1e3319da32..b0ca00aa97c 100644
--- a/sys/dev/usb/dwc2/dwc2.c
+++ b/sys/dev/usb/dwc2/dwc2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dwc2.c,v 1.31 2015/06/28 11:48:18 jmatthew Exp $	*/
+/*	$OpenBSD: dwc2.c,v 1.32 2015/09/03 12:55:54 visa Exp $	*/
 /*	$NetBSD: dwc2.c,v 1.32 2014/09/02 23:26:20 macallan Exp $	*/
 
 /*-
@@ -354,12 +354,10 @@ dwc2_rhc(void *addr)
 	u_char *p;
 
 	DPRINTF("\n");
-	mtx_enter(&sc->sc_lock);
 	xfer = sc->sc_intrxfer;
 
 	if (xfer == NULL) {
 		/* Just ignore the change. */
-		mtx_leave(&sc->sc_lock);
 		return;
 
 	}
@@ -372,7 +370,6 @@ dwc2_rhc(void *addr)
 	xfer->status = USBD_NORMAL_COMPLETION;
 
 	usb_transfer_complete(xfer);
-	mtx_leave(&sc->sc_lock);
 }
 
 STATIC void
@@ -383,8 +380,6 @@ dwc2_softintr(void *v)
 	struct dwc2_hsotg *hsotg = sc->sc_hsotg;
 	struct dwc2_xfer *dxfer;
 
-	KASSERT(sc->sc_bus.use_polling || mtx_owned(&sc->sc_lock));
-
 	mtx_enter(&hsotg->lock);
 	while ((dxfer = TAILQ_FIRST(&sc->sc_complete)) != NULL) {
 
@@ -438,10 +433,8 @@ dwc2_waitintr(struct dwc2_softc *sc, struct usbd_xfer *xfer)
 	/* Timeout */
 	DPRINTF("timeout\n");
 
-	mtx_enter(&sc->sc_lock);
 	xfer->status = USBD_TIMEOUT;
 	usb_transfer_complete(xfer);
-	mtx_leave(&sc->sc_lock);
 }
 
 STATIC void
@@ -455,9 +448,7 @@ dwc2_timeout(void *addr)
 	DPRINTF("dxfer=%p\n", dxfer);
 
 	if (sc->sc_dying) {
-		mtx_enter(&sc->sc_lock);
-		dwc2_abort_xfer(&dxfer->xfer, USBD_TIMEOUT);
-		mtx_leave(&sc->sc_lock);
+		dwc2_timeout_task(addr);
 		return;
 	}
 
@@ -471,13 +462,13 @@ STATIC void
 dwc2_timeout_task(void *addr)
 {
 	struct usbd_xfer *xfer = addr;
- 	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
+	int s;
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	mtx_enter(&sc->sc_lock);
+	s = splusb();
 	dwc2_abort_xfer(xfer, USBD_TIMEOUT);
-	mtx_leave(&sc->sc_lock);
+	splx(s);
 }
 
 usbd_status
@@ -560,11 +551,7 @@ dwc2_poll(struct usbd_bus *bus)
 STATIC void
 dwc2_close_pipe(struct usbd_pipe *pipe)
 {
-#ifdef DIAGNOSTIC
-	struct dwc2_softc *sc = DWC2_PIPE2SC(pipe);
-#endif
-
-	KASSERT(mtx_owned(&sc->sc_lock));
+	/* nothing */
 }
 
 /*
@@ -580,10 +567,9 @@ dwc2_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
 	bool wake;
 	int err;
 
-	DPRINTF("xfer=%p\n", xfer);
+	SPLUSBCHECK;
 
-	KASSERT(mtx_owned(&sc->sc_lock));
-	//KASSERT(!cpu_intr_p() && !cpu_softintr_p());
+	DPRINTF("xfer=%p\n", xfer);
 
 	if (sc->sc_dying) {
 		xfer->status = status;
@@ -727,12 +713,9 @@ STATIC const usb_hub_descriptor_t dwc2_hubd = {
 STATIC usbd_status
 dwc2_root_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
 	usbd_status err;
 
-	mtx_enter(&sc->sc_lock);
 	err = usb_insert_transfer(xfer);
-	mtx_leave(&sc->sc_lock);
 	if (err)
 		return err;
 
@@ -746,7 +729,7 @@ dwc2_root_ctrl_start(struct usbd_xfer *xfer)
 	usb_device_request_t *req;
 	uint8_t *buf;
 	uint16_t len;
-	int value, index, l, totlen;
+	int value, index, l, s, totlen;
 	usbd_status err = USBD_IOERROR;
 
 	if (sc->sc_dying)
@@ -878,10 +861,10 @@ dwc2_root_ctrl_start(struct usbd_xfer *xfer)
 	err = USBD_NORMAL_COMPLETION;
 
 fail:
-	mtx_enter(&sc->sc_lock);
+	s = splusb();
 	xfer->status = err;
 	usb_transfer_complete(xfer);
-	mtx_leave(&sc->sc_lock);
+	splx(s);
 
 	return USBD_IN_PROGRESS;
 }
@@ -913,15 +896,12 @@ dwc2_root_ctrl_done(struct usbd_xfer *xfer)
 STATIC usbd_status
 dwc2_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
 	usbd_status err;
 
 	DPRINTF("\n");
 
 	/* Insert last in queue. */
-	mtx_enter(&sc->sc_lock);
 	err = usb_insert_transfer(xfer);
-	mtx_leave(&sc->sc_lock);
 	if (err)
 		return err;
 
@@ -939,10 +919,8 @@ dwc2_root_intr_start(struct usbd_xfer *xfer)
 	if (sc->sc_dying)
 		return USBD_IOERROR;
 
-	mtx_enter(&sc->sc_lock);
 	KASSERT(sc->sc_intrxfer == NULL);
 	sc->sc_intrxfer = xfer;
-	mtx_leave(&sc->sc_lock);
 
 	return USBD_IN_PROGRESS;
 }
@@ -955,7 +933,6 @@ dwc2_root_intr_abort(struct usbd_xfer *xfer)
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	KASSERT(mtx_owned(&sc->sc_lock));
 	KASSERT(xfer->pipe->intrxfer == xfer);
 
 	sc->sc_intrxfer = NULL;
@@ -971,8 +948,6 @@ dwc2_root_intr_close(struct usbd_pipe *pipe)
 
 	DPRINTF("\n");
 
-	KASSERT(mtx_owned(&sc->sc_lock));
-
 	sc->sc_intrxfer = NULL;
 }
 
@@ -988,15 +963,12 @@ dwc2_root_intr_done(struct usbd_xfer *xfer)
 STATIC usbd_status
 dwc2_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
 	usbd_status err;
 
 	DPRINTF("\n");
 
 	/* Insert last in queue. */
-	mtx_enter(&sc->sc_lock);
 	err = usb_insert_transfer(xfer);
-	mtx_leave(&sc->sc_lock);
 	if (err)
 		return err;
 
@@ -1012,10 +984,8 @@ dwc2_device_ctrl_start(struct usbd_xfer *xfer)
 
 	DPRINTF("\n");
 
-	mtx_enter(&sc->sc_lock);
 	xfer->status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
-	mtx_leave(&sc->sc_lock);
 
 	if (sc->sc_bus.use_polling)
 		dwc2_waitintr(sc, xfer);
@@ -1026,11 +996,6 @@ dwc2_device_ctrl_start(struct usbd_xfer *xfer)
 STATIC void
 dwc2_device_ctrl_abort(struct usbd_xfer *xfer)
 {
-#ifdef DIAGNOSTIC
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-#endif
-	KASSERT(mtx_owned(&sc->sc_lock));
-
 	DPRINTF("xfer=%p\n", xfer);
 	dwc2_abort_xfer(xfer, USBD_CANCELLED);
 }
@@ -1055,15 +1020,12 @@ dwc2_device_ctrl_done(struct usbd_xfer *xfer)
 STATIC usbd_status
 dwc2_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
 	usbd_status err;
 
 	DPRINTF("xfer=%p\n", xfer);
 
 	/* Insert last in queue. */
-	mtx_enter(&sc->sc_lock);
 	err = usb_insert_transfer(xfer);
-	mtx_leave(&sc->sc_lock);
 	if (err)
 		return err;
 
@@ -1078,10 +1040,9 @@ dwc2_device_bulk_start(struct usbd_xfer *xfer)
 	usbd_status err;
 
 	DPRINTF("xfer=%p\n", xfer);
-	mtx_enter(&sc->sc_lock);
+
 	xfer->status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
-	mtx_leave(&sc->sc_lock);
 
 	if (sc->sc_bus.use_polling)
 		dwc2_waitintr(sc, xfer);
@@ -1092,12 +1053,8 @@ dwc2_device_bulk_start(struct usbd_xfer *xfer)
 STATIC void
 dwc2_device_bulk_abort(struct usbd_xfer *xfer)
 {
-#ifdef DIAGNOSTIC
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-#endif
-	KASSERT(mtx_owned(&sc->sc_lock));
-
 	DPRINTF("xfer=%p\n", xfer);
+
 	dwc2_abort_xfer(xfer, USBD_CANCELLED);
 }
 
@@ -1122,15 +1079,12 @@ dwc2_device_bulk_done(struct usbd_xfer *xfer)
 STATIC usbd_status
 dwc2_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
 	usbd_status err;
 
 	DPRINTF("xfer=%p\n", xfer);
 
 	/* Insert last in queue. */
-	mtx_enter(&sc->sc_lock);
 	err = usb_insert_transfer(xfer);
-	mtx_leave(&sc->sc_lock);
 	if (err)
 		return err;
 
@@ -1145,10 +1099,8 @@ dwc2_device_intr_start(struct usbd_xfer *xfer)
 	struct dwc2_softc *sc = DWC2_DPIPE2SC(dpipe);
 	usbd_status err;
 
-	mtx_enter(&sc->sc_lock);
 	xfer->status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
-	mtx_leave(&sc->sc_lock);
 
 	if (sc->sc_bus.use_polling)
 		dwc2_waitintr(sc, xfer);
@@ -1160,11 +1112,6 @@ dwc2_device_intr_start(struct usbd_xfer *xfer)
 STATIC void
 dwc2_device_intr_abort(struct usbd_xfer *xfer)
 {
-#ifdef DIAGNOSTIC
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-#endif
-
-	KASSERT(mtx_owned(&sc->sc_lock));
 	KASSERT(xfer->pipe->intrxfer == xfer);
 
 	DPRINTF("xfer=%p\n", xfer);
@@ -1198,15 +1145,12 @@ dwc2_device_intr_done(struct usbd_xfer *xfer)
 usbd_status
 dwc2_device_isoc_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
 	usbd_status err;
 
 	DPRINTF("xfer=%p\n", xfer);
 
 	/* Insert last in queue. */
-	mtx_enter(&sc->sc_lock);
 	err = usb_insert_transfer(xfer);
-	mtx_leave(&sc->sc_lock);
 	if (err)
 		return err;
 
@@ -1225,10 +1169,8 @@ dwc2_device_isoc_start(struct usbd_xfer *xfer)
 	if (sc->sc_bus.use_polling)
 		return (USBD_INVAL);
 
-	mtx_enter(&sc->sc_lock);
 	xfer->status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
-	mtx_leave(&sc->sc_lock);
 
 	return err;
 }
@@ -1236,12 +1178,8 @@ dwc2_device_isoc_start(struct usbd_xfer *xfer)
 void
 dwc2_device_isoc_abort(struct usbd_xfer *xfer)
 {
-#ifdef DIAGNOSTIC
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-#endif
-	KASSERT(mtx_owned(&sc->sc_lock));
-
 	DPRINTF("xfer=%p\n", xfer);
+
 	dwc2_abort_xfer(xfer, USBD_CANCELLED);
 }
 
@@ -1400,9 +1338,6 @@ dwc2_device_start(struct usbd_xfer *xfer)
 		dwc2_urb->interval = ival;
 	}
 
-	/* XXXNH bring down from callers?? */
-// 	mtx_enter(&sc->sc_lock);
-
 	xfer->actlen = 0;
 
 	KASSERT(xfertype != UE_ISOCHRONOUS ||
@@ -1441,8 +1376,6 @@ dwc2_device_start(struct usbd_xfer *xfer)
 fail:
 	mtx_leave(&hsotg->lock);
 
-// 	mtx_leave(&sc->sc_lock);
-
 	switch (retval) {
 	case 0:
 		break;
@@ -1475,7 +1408,6 @@ dwc2_worker(struct task *wk, void *priv)
 	dwc_free(NULL, dpipe->urb);
 #endif
 
-	mtx_enter(&sc->sc_lock);
 	if (wk == &hsotg->wf_otg) {
 		dwc2_conn_id_status_change(wk);
 	} else if (wk == &hsotg->start_work.work) {
@@ -1494,7 +1426,6 @@ dwc2_worker(struct task *wk, void *priv)
 		wakeup(&dxfer->flags);
 #endif
 	}
-	mtx_leave(&sc->sc_lock);
 }
 
 int dwc2_intr(void *p)
@@ -1615,8 +1546,6 @@ dwc2_init(struct dwc2_softc *sc)
 	sc->sc_bus.pipe_size = sizeof(struct dwc2_pipe);
 	sc->sc_hcdenabled = false;
 
-	mtx_init(&sc->sc_lock, IPL_SOFTUSB);
-
 	TAILQ_INIT(&sc->sc_complete);
 
 	sc->sc_rhc_si = softintr_establish(IPL_SOFTUSB, dwc2_rhc, sc);
diff --git a/sys/dev/usb/dwc2/dwc2var.h b/sys/dev/usb/dwc2/dwc2var.h
index 261d9651eca..cee00aa4608 100644
--- a/sys/dev/usb/dwc2/dwc2var.h
+++ b/sys/dev/usb/dwc2/dwc2var.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dwc2var.h,v 1.14 2015/02/12 11:38:42 uebayasi Exp $	*/
+/*	$OpenBSD: dwc2var.h,v 1.15 2015/09/03 12:55:54 visa Exp $	*/
 /*	$NetBSD: dwc2var.h,v 1.3 2013/10/22 12:57:40 skrll Exp $	*/
 
 /*-
@@ -91,8 +91,6 @@ typedef struct dwc2_softc {
 
 	struct dwc2_hsotg *sc_hsotg;
 
-	struct mutex sc_lock;
-
 	bool sc_hcdenabled;
 	void *sc_rhc_si;