Skip to content

Commit cf2a1d8

Browse files
author
Alrik Vidstrom
committed
Improve source code comments
This commit improves the source code comments in several locations, adding explanations for things that might easily be misunderstood, or take a long time to understand.
1 parent a4ac6d1 commit cf2a1d8

File tree

2 files changed

+24
-6
lines changed

2 files changed

+24
-6
lines changed

src/USBHostMSD/USBHostMSD.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@ int USBHostMSD::checkResult(uint8_t res, USBEndpoint * ep)
193193
return -1;
194194
}
195195

196-
// if ep stalled: send clear feature
196+
// Notice that USB_TYPE_STALL_ERROR is never actually set anywhere in the library for the ST target, because
197+
// URB_STALL is never handled directly. Otherwise, this would be the code to unstall the EP.
197198
if (res == USB_TYPE_STALL_ERROR) {
198199
res = host->controlWrite( dev,
199200
USB_RECIPIENT_ENDPOINT | USB_HOST_TO_DEVICE | USB_REQUEST_TYPE_STANDARD,
@@ -285,7 +286,9 @@ int USBHostMSD::SCSITransfer(uint8_t * cmd, uint8_t cmd_len, int flags, uint8_t
285286
BO_MASS_STORAGE_RESET,
286287
0, msd_intf, NULL, 0);
287288

288-
// unstall [sic!] both endpoints
289+
// Unstall [sic!] both endpoints.
290+
// Notice that this is the unstall that is required by the specification as part of clearing a Phase Error,
291+
// _not_ the unstall for an ordinarily stalled EP!
289292
res = host->controlWrite( dev,
290293
USB_RECIPIENT_ENDPOINT | USB_HOST_TO_DEVICE | USB_REQUEST_TYPE_STANDARD,
291294
CLEAR_FEATURE,

src/targets/TARGET_STM/USBHALHost_STM.cpp

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,21 @@ uint32_t HAL_HCD_HC_GetType(HCD_HandleTypeDef *hhcd, uint8_t chnum)
7676
return hhcd->hc[chnum].ep_type;
7777
}
7878

79+
// The urb_state parameter can be one of the following according to the ST documentation, but the explanations
80+
// that follow are the result of an analysis of the ST HAL / LL source code, the Reference Manual for the
81+
// STM32H747 microcontroller, and the source code of this library:
82+
//
83+
// - URB_ERROR = various serious errors that may be hard or impossible to recover from without pulling
84+
// the thumb drive out and putting it back in again, but we will try
85+
// - URB_IDLE = set by a call to HAL_HCD_HC_SubmitRequest(), but never used by this library
86+
// - URB_NYET = never actually used by the ST HAL / LL at the time of writing, nor by this library
87+
// - URB_STALL = a stall response from the device - but it is never handled by the library and will end up
88+
// as a timeout at a higher layer, because of an ep_queue.get() timeout, which will activate
89+
// error recovery indirectly
90+
// - URB_DONE = the transfer completed normally without errrors
91+
// - URB_NOTREADY = a NAK, NYET, or not more than a couple of repeats of some of the errors that will
92+
// become URB_ERROR if they repeat several times in a row
93+
//
7994
void HAL_HCD_HC_NotifyURBChange_Callback(HCD_HandleTypeDef *hhcd, uint8_t chnum, HCD_URBStateTypeDef urb_state)
8095
{
8196
USBHALHost_Private_t *priv = (USBHALHost_Private_t *)(hhcd->pData);
@@ -107,14 +122,11 @@ void HAL_HCD_HC_NotifyURBChange_Callback(HCD_HandleTypeDef *hhcd, uint8_t chnum,
107122
}
108123
break;
109124
case URB_NOTREADY:
110-
/* try again */
111-
/* abritary limit , to avoid dead lock if other error than
112-
* slow response is */
113125
#if defined(MAX_NOTREADY_RETRY)
114126
if (td->retry < MAX_NOTREADY_RETRY) {
115-
/* increment retry counter */
116127
td->retry++;
117128
#endif
129+
// Submit the same request again, because the device wasn't ready to accept the last one
118130
length = td->size <= max_size ? td->size : max_size;
119131
HAL_HCD_HC_SubmitRequest(hhcd, chnum, dir, type, !td->setup, (uint8_t *) td->currBufPtr, length, 0);
120132
HAL_HCD_EnableInt(hhcd, chnum);
@@ -138,6 +150,9 @@ void HAL_HCD_HC_NotifyURBChange_Callback(HCD_HandleTypeDef *hhcd, uint8_t chnum,
138150
td->state = USB_TYPE_IDLE;
139151
}
140152
else if (urb_state == URB_ERROR) {
153+
// While USB_TYPE_ERROR in the endpoint state is used to activate error recovery, this value is actually never used.
154+
// Going here will lead to a timeout at a higher layer, because of ep_queue.get() timeout, which will activate error
155+
// recovery indirectly.
141156
td->state = USB_TYPE_ERROR;
142157
} else {
143158
td->state = USB_TYPE_PROCESSING;

0 commit comments

Comments
 (0)