From dee22cee18dd98502b19e41e45503f8c20f447d6 Mon Sep 17 00:00:00 2001
From: Diego Ismirlian <dismirlian (at) google's mail.com>
Date: Sun, 16 Jul 2017 20:01:50 -0300
Subject: USBH: remove unnecessary reschedules and add necessary ones

---
 os/hal/include/hal_usbh.h                    | 20 +++++++++++++++++---
 os/hal/ports/STM32/LLD/USBHv1/hal_usbh_lld.c |  8 +-------
 os/hal/src/hal_usbh.c                        | 12 +++++++-----
 os/hal/src/usbh/TODO.txt                     |  2 +-
 os/hal/src/usbh/hal_usbh_aoa.c               |  7 ++-----
 os/hal/src/usbh/hal_usbh_ftdi.c              |  7 ++-----
 os/hal/src/usbh/hal_usbh_hid.c               |  4 +---
 os/hal/src/usbh/hal_usbh_hub.c               |  5 +----
 os/hal/src/usbh/hal_usbh_uvc.c               |  8 +++-----
 9 files changed, 35 insertions(+), 38 deletions(-)

(limited to 'os')

diff --git a/os/hal/include/hal_usbh.h b/os/hal/include/hal_usbh.h
index 634520c..3e69607 100644
--- a/os/hal/include/hal_usbh.h
+++ b/os/hal/include/hal_usbh.h
@@ -358,10 +358,8 @@ extern "C" {
 		osalDbgCheck(ep != 0);
 		osalDbgCheckClassS();
 		osalDbgAssert(ep->status != USBH_EPSTATUS_UNINITIALIZED, "invalid state");
-		if (ep->status == USBH_EPSTATUS_CLOSED) {
-			osalOsRescheduleS();
+		if (ep->status == USBH_EPSTATUS_CLOSED)
 			return;
-		}
 		usbh_lld_ep_close(ep);
 	}
 	static inline void usbhEPClose(usbh_ep_t *ep) {
@@ -392,6 +390,22 @@ extern "C" {
 	void usbhURBCancelAndWaitS(usbh_urb_t *urb);
 	msg_t usbhURBWaitTimeoutS(usbh_urb_t *urb, systime_t timeout);
 
+	static inline void usbhURBSubmit(usbh_urb_t *urb) {
+		osalSysLock();
+		usbhURBSubmitI(urb);
+		osalOsRescheduleS();
+		osalSysUnlock();
+	}
+
+	static inline bool usbhURBCancel(usbh_urb_t *urb) {
+		bool ret;
+		osalSysLock();
+		ret = usbhURBCancelI(urb);
+		osalOsRescheduleS();
+		osalSysUnlock();
+		return ret;
+	}
+
 	/* Main loop */
 	void usbhMainLoop(USBHDriver *usbh);
 
diff --git a/os/hal/ports/STM32/LLD/USBHv1/hal_usbh_lld.c b/os/hal/ports/STM32/LLD/USBHv1/hal_usbh_lld.c
index 850f9ed..3a581f5 100644
--- a/os/hal/ports/STM32/LLD/USBHv1/hal_usbh_lld.c
+++ b/os/hal/ports/STM32/LLD/USBHv1/hal_usbh_lld.c
@@ -602,7 +602,6 @@ void usbh_lld_ep_object_init(usbh_ep_t *ep) {
 void usbh_lld_ep_open(usbh_ep_t *ep) {
 	uinfof("\t%s: Open EP", ep->name);
 	ep->status = USBH_EPSTATUS_OPEN;
-	osalOsRescheduleS();
 }
 
 void usbh_lld_ep_close(usbh_ep_t *ep) {
@@ -614,7 +613,6 @@ void usbh_lld_ep_close(usbh_ep_t *ep) {
 	}
 	uinfof("\t%s: Closed", ep->name);
 	ep->status = USBH_EPSTATUS_CLOSED;
-	osalOsRescheduleS();
 }
 
 bool usbh_lld_ep_reset(usbh_ep_t *ep) {
@@ -643,6 +641,7 @@ void usbh_lld_urb_submit(usbh_urb_t *urb) {
 	}
 }
 
+/* usbh_lld_urb_abort may require a reschedule if called from a S-locked state */
 bool usbh_lld_urb_abort(usbh_urb_t *urb, usbh_urbstatus_t status) {
 	osalDbgCheck(usbhURBIsBusy(urb));
 
@@ -1594,7 +1593,6 @@ usbh_urbstatus_t usbh_lld_root_hub_request(USBHDriver *usbh, uint8_t bmRequestTy
 			osalDbgAssert(0, "invalid wvalue");
 			break;
 		}
-		osalOsRescheduleS();
 		osalSysUnlock();
 		break;
 
@@ -1623,7 +1621,6 @@ usbh_urbstatus_t usbh_lld_root_hub_request(USBHDriver *usbh, uint8_t bmRequestTy
 		osalDbgCheck(wlength >= 4);
 		osalSysLock();
 		*(uint32_t *)buf = usbh->rootport.lld_status | (usbh->rootport.lld_c_status << 16);
-		osalOsRescheduleS();
 		osalSysUnlock();
 		break;
 
@@ -1653,7 +1650,6 @@ usbh_urbstatus_t usbh_lld_root_hub_request(USBHDriver *usbh, uint8_t bmRequestTy
 			osalThreadSleepS(MS2ST(60));
 			otg->HPRT = hprt;
 			usbh->rootport.lld_c_status |= USBH_PORTSTATUS_C_RESET;
-			osalOsRescheduleS();
 			osalSysUnlock();
 		} 	break;
 
@@ -1678,11 +1674,9 @@ usbh_urbstatus_t usbh_lld_root_hub_request(USBHDriver *usbh, uint8_t bmRequestTy
 uint8_t usbh_lld_roothub_get_statuschange_bitmap(USBHDriver *usbh) {
 	osalSysLock();
 	if (usbh->rootport.lld_c_status) {
-		osalOsRescheduleS();
 		osalSysUnlock();
 		return 1 << 1;
 	}
-	osalOsRescheduleS();
 	osalSysUnlock();
 	return 0;
 }
diff --git a/os/hal/src/hal_usbh.c b/os/hal/src/hal_usbh.c
index d72ab8c..1aa35d0 100644
--- a/os/hal/src/hal_usbh.c
+++ b/os/hal/src/hal_usbh.c
@@ -111,7 +111,6 @@ void usbhStart(USBHDriver *usbh) {
 				"invalid state");
 	usbh_lld_start(usbh);
 	usbh->status = USBH_STATUS_STARTED;
-	osalOsRescheduleS();
 	osalSysUnlock();
 }
 
@@ -224,6 +223,7 @@ void usbhURBObjectResetI(usbh_urb_t *urb) {
 	usbh_lld_urb_object_reset(urb);
 }
 
+/* usbhURBSubmitI may require a reschedule if called from a S-locked state */
 void usbhURBSubmitI(usbh_urb_t *urb) {
 	osalDbgCheckClassI();
 	_check_urb(urb);
@@ -245,6 +245,7 @@ void usbhURBSubmitI(usbh_urb_t *urb) {
 	usbh_lld_urb_submit(urb);
 }
 
+/* _usbh_urb_abortI may require a reschedule if called from a S-locked state */
 bool _usbh_urb_abortI(usbh_urb_t *urb, usbh_urbstatus_t status) {
 	osalDbgCheckClassI();
 	_check_urb(urb);
@@ -280,15 +281,18 @@ void _usbh_urb_abort_and_waitS(usbh_urb_t *urb, usbh_urbstatus_t status) {
 	}
 #if !(USBH_DEBUG_ENABLE && USBH_DEBUG_ENABLE_WARNINGS)
 	else {
+		/* This call is necessary because _usbh_urb_abortI may require a reschedule */
 		osalOsRescheduleS();
 	}
 #else
 	uwarn("URB aborted");
 	osalOsRescheduleS();	/* debug printing functions call I-class functions inside
-	 	 	 	 	 	 	 which may cause a priority violation without this call */
+	 	 	 	 	 	 	 which may cause a priority violation without this call
+							Also, _usbh_urb_abortI may require a reschedule */
 #endif
 }
 
+/* usbhURBCancelI may require a reschedule if called from a S-locked state */
 bool usbhURBCancelI(usbh_urb_t *urb) {
 	return _usbh_urb_abortI(urb, USBH_URBSTATUS_CANCELLED);
 }
@@ -313,7 +317,6 @@ msg_t usbhURBWaitTimeoutS(usbh_urb_t *urb, systime_t timeout) {
 
 	case USBH_URBSTATUS_OK:
 		ret = MSG_OK;
-		osalOsRescheduleS();
 		break;
 
 /*	case USBH_URBSTATUS_UNINITIALIZED:
@@ -324,7 +327,6 @@ msg_t usbhURBWaitTimeoutS(usbh_urb_t *urb, systime_t timeout) {
  *	case USBH_URBSTATUS_DISCONNECTED: */
 	default:
 		ret = MSG_RESET;
-		osalOsRescheduleS();
 		break;
 	}
 	return ret;
@@ -350,6 +352,7 @@ static inline msg_t _wakeup_message(usbh_urbstatus_t status) {
 	return MSG_RESET;
 }
 
+/* _usbh_urb_completeI may require a reschedule if called from a S-locked state */
 void _usbh_urb_completeI(usbh_urb_t *urb, usbh_urbstatus_t status) {
 	osalDbgCheckClassI();
 	_check_urb(urb);
@@ -1128,7 +1131,6 @@ static uint32_t _hub_get_status_change_bitmap(USBHDriver *host, USBHHubDriver *h
 		osalSysLock();
 		uint32_t ret = hub->statuschange;
 		hub->statuschange = 0;
-		osalOsRescheduleS();
 		osalSysUnlock();
 		return ret;
 	}
diff --git a/os/hal/src/usbh/TODO.txt b/os/hal/src/usbh/TODO.txt
index c3dd4a8..532420b 100644
--- a/os/hal/src/usbh/TODO.txt
+++ b/os/hal/src/usbh/TODO.txt
@@ -17,5 +17,5 @@ Enhancements:
 - Linked list for drivers for dynamic registration
 - A way to automate matching (similar to linux)
 - Hooks to override driver loading and to inform the user of problems
-
+- for STM32 LLD: think of a way to prevent Bulk IN NAK interrupt flood.
 
diff --git a/os/hal/src/usbh/hal_usbh_aoa.c b/os/hal/src/usbh/hal_usbh_aoa.c
index 1526aa3..b8e37ae 100644
--- a/os/hal/src/usbh/hal_usbh_aoa.c
+++ b/os/hal/src/usbh/hal_usbh_aoa.c
@@ -295,7 +295,6 @@ static void _aoa_unload(usbh_baseclassdriver_t *drv) {
 	_stop_channelS(&aoap->channel);
 	aoap->channel.state = USBHAOA_CHANNEL_STATE_STOP;
 	aoap->state = USBHAOA_STATE_STOP;
-	osalOsRescheduleS();
 	osalSysUnlock();
 }
 
@@ -521,6 +520,7 @@ static void _stop_channelS(USBHAOAChannel *aoacp) {
 	chThdDequeueAllI(&aoacp->oq_waiting, Q_RESET);
 	chnAddFlagsI(aoacp, CHN_DISCONNECTED);
 	aoacp->state = USBHAOA_CHANNEL_STATE_ACTIVE;
+	osalOsRescheduleS();
 }
 
 static void _vt(void *p) {
@@ -562,9 +562,7 @@ void usbhaoaChannelStart(USBHAOADriver *aoap) {
 	aoacp->iq_counter = 0;
 	aoacp->iq_ptr = aoacp->iq_buff;
 	usbhEPOpen(&aoacp->epin);
-	osalSysLock();
-	usbhURBSubmitI(&aoacp->iq_urb);
-	osalSysUnlock();
+	usbhURBSubmit(&aoacp->iq_urb);
 
 	chVTObjectInit(&aoacp->vt);
 	chVTSet(&aoacp->vt, MS2ST(16), _vt, aoacp);
@@ -579,7 +577,6 @@ void usbhaoaChannelStop(USBHAOADriver *aoap) {
 			|| (aoap->channel.state == USBHAOA_CHANNEL_STATE_READY));
 	osalSysLock();
 	_stop_channelS(&aoap->channel);
-	osalOsRescheduleS();
 	osalSysUnlock();
 }
 
diff --git a/os/hal/src/usbh/hal_usbh_ftdi.c b/os/hal/src/usbh/hal_usbh_ftdi.c
index edcf022..2e9f506 100644
--- a/os/hal/src/usbh/hal_usbh_ftdi.c
+++ b/os/hal/src/usbh/hal_usbh_ftdi.c
@@ -219,7 +219,6 @@ static void _ftdi_unload(usbh_baseclassdriver_t *drv) {
 	while (ftdipp) {
 		osalSysLock();
 		_stopS(ftdipp);
-		osalOsRescheduleS();
 		osalSysUnlock();
 		ftdipp = ftdipp->next;
 	}
@@ -636,6 +635,7 @@ static void _stopS(USBHFTDIPortDriver *ftdipp) {
 	chThdDequeueAllI(&ftdipp->iq_waiting, Q_RESET);
 	chThdDequeueAllI(&ftdipp->oq_waiting, Q_RESET);
 	ftdipp->state = USBHFTDIP_STATE_ACTIVE;
+	osalOsRescheduleS();
 }
 
 void usbhftdipStop(USBHFTDIPortDriver *ftdipp) {
@@ -646,7 +646,6 @@ void usbhftdipStop(USBHFTDIPortDriver *ftdipp) {
 	chMtxLockS(&ftdipp->ftdip->mtx);
 	_stopS(ftdipp);
 	chMtxUnlockS(&ftdipp->ftdip->mtx);
-	osalOsRescheduleS();
 	osalSysUnlock();
 }
 
@@ -688,9 +687,7 @@ void usbhftdipStart(USBHFTDIPortDriver *ftdipp, const USBHFTDIPortConfig *config
 	ftdipp->iq_counter = 0;
 	ftdipp->iq_ptr = ftdipp->iq_buff;
 	usbhEPOpen(&ftdipp->epin);
-	osalSysLock();
-	usbhURBSubmitI(&ftdipp->iq_urb);
-	osalSysUnlock();
+	usbhURBSubmit(&ftdipp->iq_urb);
 
 	chVTObjectInit(&ftdipp->vt);
 	chVTSet(&ftdipp->vt, MS2ST(16), _vt, ftdipp);
diff --git a/os/hal/src/usbh/hal_usbh_hid.c b/os/hal/src/usbh/hal_usbh_hid.c
index 575b0a6..2b2c5ce 100644
--- a/os/hal/src/usbh/hal_usbh_hid.c
+++ b/os/hal/src/usbh/hal_usbh_hid.c
@@ -244,9 +244,7 @@ void usbhhidStart(USBHHIDDriver *hidp, const USBHHIDConfig *cfg) {
 
 	usbhhidSetProtocol(hidp, cfg->protocol);
 
-	osalSysLock();
-	usbhURBSubmitI(&hidp->in_urb);
-	osalSysUnlock();
+	usbhURBSubmit(&hidp->in_urb);
 
 	hidp->state = USBHHID_STATE_READY;
 	chSemSignal(&hidp->sem);
diff --git a/os/hal/src/usbh/hal_usbh_hub.c b/os/hal/src/usbh/hal_usbh_hub.c
index f9b6418..6a83c66 100644
--- a/os/hal/src/usbh/hal_usbh_hub.c
+++ b/os/hal/src/usbh/hal_usbh_hub.c
@@ -252,10 +252,7 @@ alloc_ok:
 			_urb_complete, hubdp, hubdp->scbuff,
 			(hubdesc->bNbrPorts + 8) / 8);
 
-	osalSysLock();
-	usbhURBSubmitI(&hubdp->urb);
-	osalOsRescheduleS();
-	osalSysUnlock();
+	usbhURBSubmit(&hubdp->urb);
 
 	hubdp->dev = NULL;
 	return (usbh_baseclassdriver_t *)hubdp;
diff --git a/os/hal/src/usbh/hal_usbh_uvc.c b/os/hal/src/usbh/hal_usbh_uvc.c
index 7777823..a795cd8 100644
--- a/os/hal/src/usbh/hal_usbh_uvc.c
+++ b/os/hal/src/usbh/hal_usbh_uvc.c
@@ -361,10 +361,8 @@ bool usbhuvcStreamStart(USBHUVCDriver *uvcdp, uint16_t min_ep_sz) {
 		osalDbgCheck(msg);
 		usbhURBObjectInit(&uvcdp->urb_iso, &uvcdp->ep_iso, _cb_iso, uvcdp, msg->data, uvcdp->ep_iso.wMaxPacketSize);
 	}
-	osalSysLock();
-	usbhURBSubmitI(&uvcdp->urb_iso);
-	osalOsRescheduleS();
-	osalSysUnlock();
+
+	usbhURBSubmit(&uvcdp->urb_iso);
 
 	ret = HAL_SUCCESS;
 	goto exit;
@@ -695,7 +693,7 @@ alloc_ok:
 	osalSysLock();
 	usbhURBSubmitI(&uvcdp->urb_int);
 	uvcdp->state = USBHUVC_STATE_ACTIVE;
-	osalOsRescheduleS();
+	osalOsRescheduleS();	/* because of usbhURBSubmitI */
 	osalSysUnlock();
 
 	dev->keepFullCfgDesc++;
-- 
cgit v1.2.3