Skip to content

Commit f4e95cc

Browse files
tmon-nordiccarlescufi
tmon-nordic
authored and
carlescufi
committed
drivers: udc_dwc2: Properly revive STALLed endpoints
DWC2 documentation unfortunately assigns somewhat confusing semantics to endpoint "enable"/"disable" and "activate"/"deactivate". The Zephyr USB device stack endpoint enable/disable refers to DWC2 activate/deactivate. The DWC2 endpoint enable/disable actions can be loosely referred to Zephyr USB stack enqueue/dequeue. Rename the functions and rework internal working to match DWC2 Programming Guide. This makes endpoint halt work as expected by the stack and therefore fixes all classes that rely on correct STALL handling. Most notable STALL user is the Mass Storage class. Signed-off-by: Tomasz Moń <[email protected]>
1 parent 00e2b86 commit f4e95cc

File tree

1 file changed

+145
-9
lines changed

1 file changed

+145
-9
lines changed

‎drivers/usb/udc/udc_dwc2.c

Lines changed: 145 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ static int dwc2_tx_fifo_write(const struct device *dev,
259259
mem_addr_t dieptsiz_reg = (mem_addr_t)&base->in_ep[ep_idx].dieptsiz;
260260
/* TODO: use dwc2_get_dxepctl_reg() */
261261
mem_addr_t diepctl_reg = (mem_addr_t)&base->in_ep[ep_idx].diepctl;
262+
mem_addr_t diepint_reg = (mem_addr_t)&base->in_ep[ep_idx].diepint;
262263

263264
uint32_t max_xfersize, max_pktcnt, pktcnt, spcavail;
264265
const size_t d = sizeof(uint32_t);
@@ -321,6 +322,8 @@ static int dwc2_tx_fifo_write(const struct device *dev,
321322

322323
/* Clear NAK and set endpoint enable */
323324
sys_set_bits(diepctl_reg, USB_DWC2_DEPCTL_EPENA | USB_DWC2_DEPCTL_CNAK);
325+
/* Clear IN Endpoint NAK Effective interrupt in case it was set */
326+
sys_write32(USB_DWC2_DIEPINT_INEPNAKEFF, diepint_reg);
324327

325328
/* FIFO access is always in 32-bit words */
326329

@@ -749,6 +752,8 @@ static inline void dwc2_handle_rxflvl(const struct device *dev)
749752
break;
750753
case USB_DWC2_GRXSTSR_PKTSTS_SETUP_DONE:
751754
LOG_DBG("SETUP pktsts DONE");
755+
case USB_DWC2_GRXSTSR_PKTSTS_GLOBAL_OUT_NAK:
756+
LOG_DBG("Global OUT NAK");
752757
break;
753758
default:
754759
break;
@@ -960,6 +965,9 @@ static int udc_dwc2_ep_dequeue(const struct device *dev,
960965
}
961966

962967
irq_unlock(lock_key);
968+
969+
udc_ep_set_busy(dev, cfg->addr, false);
970+
963971
LOG_DBG("dequeue ep 0x%02x", cfg->addr);
964972

965973
return 0;
@@ -1108,8 +1116,8 @@ static int dwc2_ep_control_enable(const struct device *dev,
11081116
return 0;
11091117
}
11101118

1111-
static int udc_dwc2_ep_enable(const struct device *dev,
1112-
struct udc_ep_config *const cfg)
1119+
static int udc_dwc2_ep_activate(const struct device *dev,
1120+
struct udc_ep_config *const cfg)
11131121
{
11141122
struct usb_dwc2_reg *const base = dwc2_get_base(dev);
11151123
struct udc_dwc2_data *const priv = udc_get_private(dev);
@@ -1213,8 +1221,117 @@ static int dwc2_unset_dedicated_fifo(const struct device *dev,
12131221
return 0;
12141222
}
12151223

1216-
static int udc_dwc2_ep_disable(const struct device *dev,
1217-
struct udc_ep_config *const cfg)
1224+
static void dwc2_wait_for_bit(mem_addr_t addr, uint32_t bit)
1225+
{
1226+
k_timepoint_t timeout = sys_timepoint_calc(K_MSEC(100));
1227+
1228+
/* This could potentially be converted to use proper synchronization
1229+
* primitives instead of busy looping, but the number of interrupt bits
1230+
* this function can be waiting for is rather high.
1231+
*
1232+
* Busy looping is most likely fine unless profiling shows otherwise.
1233+
*/
1234+
while (!(sys_read32(addr) & bit)) {
1235+
if (sys_timepoint_expired(timeout)) {
1236+
LOG_ERR("Timeout waiting for bit 0x%08X at 0x%08X",
1237+
bit, (uint32_t)addr);
1238+
return;
1239+
}
1240+
}
1241+
}
1242+
1243+
/* Disabled IN endpoint means that device will send NAK (isochronous: ZLP) after
1244+
* receiving IN token from host even if there is packet available in TxFIFO.
1245+
* Disabled OUT endpoint means that device will NAK (isochronous: discard data)
1246+
* incoming OUT data (or HS PING) even if there is space available in RxFIFO.
1247+
*
1248+
* Set stall parameter to true if caller wants to send STALL instead of NAK.
1249+
*/
1250+
static void udc_dwc2_ep_disable(const struct device *dev,
1251+
struct udc_ep_config *const cfg, bool stall)
1252+
{
1253+
struct usb_dwc2_reg *const base = dwc2_get_base(dev);
1254+
uint8_t ep_idx = USB_EP_GET_IDX(cfg->addr);
1255+
mem_addr_t dxepctl_reg;
1256+
uint32_t dxepctl;
1257+
1258+
dxepctl_reg = dwc2_get_dxepctl_reg(dev, cfg->addr);
1259+
dxepctl = sys_read32(dxepctl_reg);
1260+
1261+
if (USB_EP_DIR_IS_OUT(cfg->addr)) {
1262+
mem_addr_t dctl_reg, gintsts_reg, doepint_reg;
1263+
uint32_t dctl;
1264+
1265+
dctl_reg = (mem_addr_t)&base->dctl;
1266+
gintsts_reg = (mem_addr_t)&base->gintsts;
1267+
doepint_reg = (mem_addr_t)&base->out_ep[ep_idx].doepint;
1268+
1269+
dctl = sys_read32(dctl_reg);
1270+
1271+
if (sys_read32(gintsts_reg) & USB_DWC2_GINTSTS_GOUTNAKEFF) {
1272+
LOG_ERR("GOUTNAKEFF already active");
1273+
} else {
1274+
dctl |= USB_DWC2_DCTL_SGOUTNAK;
1275+
sys_write32(dctl, dctl_reg);
1276+
dctl &= ~USB_DWC2_DCTL_SGOUTNAK;
1277+
}
1278+
1279+
dwc2_wait_for_bit(gintsts_reg, USB_DWC2_GINTSTS_GOUTNAKEFF);
1280+
1281+
dxepctl |= USB_DWC2_DEPCTL_EPENA | USB_DWC2_DEPCTL_EPDIS;
1282+
if (stall) {
1283+
/* For OUT endpoints STALL is set instead of SNAK */
1284+
dxepctl |= USB_DWC2_DEPCTL_STALL;
1285+
} else {
1286+
dxepctl |= USB_DWC2_DEPCTL_SNAK;
1287+
}
1288+
sys_write32(dxepctl, dxepctl_reg);
1289+
1290+
dwc2_wait_for_bit(doepint_reg, USB_DWC2_DOEPINT_EPDISBLD);
1291+
1292+
/* Clear Endpoint Disabled interrupt */
1293+
sys_write32(USB_DWC2_DIEPINT_EPDISBLD, doepint_reg);
1294+
1295+
dctl |= USB_DWC2_DCTL_CGOUTNAK;
1296+
sys_write32(dctl, dctl_reg);
1297+
} else {
1298+
mem_addr_t diepint_reg;
1299+
1300+
diepint_reg = (mem_addr_t)&base->in_ep[ep_idx].diepint;
1301+
1302+
dxepctl |= USB_DWC2_DEPCTL_SNAK;
1303+
if (stall) {
1304+
/* For IN endpoints STALL is set in addition to SNAK */
1305+
dxepctl |= USB_DWC2_DEPCTL_STALL;
1306+
}
1307+
sys_write32(dxepctl, dxepctl_reg);
1308+
1309+
dwc2_wait_for_bit(diepint_reg, USB_DWC2_DIEPINT_INEPNAKEFF);
1310+
1311+
dxepctl |= USB_DWC2_DEPCTL_EPENA | USB_DWC2_DEPCTL_EPDIS;
1312+
sys_write32(dxepctl, dxepctl_reg);
1313+
1314+
dwc2_wait_for_bit(diepint_reg, USB_DWC2_DIEPINT_EPDISBLD);
1315+
1316+
/* Clear Endpoint Disabled interrupt */
1317+
sys_write32(USB_DWC2_DIEPINT_EPDISBLD, diepint_reg);
1318+
1319+
/* TODO: Read DIEPTSIZn here? Programming Guide suggest it to
1320+
* let application know how many bytes of interrupted transfer
1321+
* were transferred to the host.
1322+
*/
1323+
1324+
dwc2_flush_tx_fifo(dev, ep_idx);
1325+
}
1326+
1327+
udc_ep_set_busy(dev, cfg->addr, false);
1328+
}
1329+
1330+
/* Deactivated endpoint means that there will be a bus timeout when the host
1331+
* tries to access the endpoint.
1332+
*/
1333+
static int udc_dwc2_ep_deactivate(const struct device *dev,
1334+
struct udc_ep_config *const cfg)
12181335
{
12191336
uint8_t ep_idx = USB_EP_GET_IDX(cfg->addr);
12201337
mem_addr_t dxepctl_reg;
@@ -1226,7 +1343,11 @@ static int udc_dwc2_ep_disable(const struct device *dev,
12261343
if (dxepctl & USB_DWC2_DEPCTL_USBACTEP) {
12271344
LOG_DBG("Disable ep 0x%02x DxEPCTL%u %x",
12281345
cfg->addr, ep_idx, dxepctl);
1229-
dxepctl |= USB_DWC2_DEPCTL_EPDIS | USB_DWC2_DEPCTL_SNAK;
1346+
1347+
udc_dwc2_ep_disable(dev, cfg, false);
1348+
1349+
dxepctl = sys_read32(dxepctl_reg);
1350+
dxepctl &= ~USB_DWC2_DEPCTL_USBACTEP;
12301351
} else {
12311352
LOG_WRN("ep 0x%02x is not active DxEPCTL%u %x",
12321353
cfg->addr, ep_idx, dxepctl);
@@ -1247,7 +1368,7 @@ static int udc_dwc2_ep_set_halt(const struct device *dev,
12471368
{
12481369
uint8_t ep_idx = USB_EP_GET_IDX(cfg->addr);
12491370

1250-
sys_set_bits(dwc2_get_dxepctl_reg(dev, cfg->addr), USB_DWC2_DEPCTL_STALL);
1371+
udc_dwc2_ep_disable(dev, cfg, true);
12511372

12521373
LOG_DBG("Set halt ep 0x%02x", cfg->addr);
12531374
if (ep_idx != 0) {
@@ -1260,11 +1381,26 @@ static int udc_dwc2_ep_set_halt(const struct device *dev,
12601381
static int udc_dwc2_ep_clear_halt(const struct device *dev,
12611382
struct udc_ep_config *const cfg)
12621383
{
1263-
sys_clear_bits(dwc2_get_dxepctl_reg(dev, cfg->addr), USB_DWC2_DEPCTL_STALL);
1384+
mem_addr_t dxepctl_reg = dwc2_get_dxepctl_reg(dev, cfg->addr);
1385+
uint32_t dxepctl;
1386+
struct dwc2_drv_event evt = {
1387+
.ep = cfg->addr,
1388+
.type = DWC2_DRV_EVT_XFER,
1389+
};
1390+
1391+
dxepctl = sys_read32(dxepctl_reg);
1392+
dxepctl &= ~USB_DWC2_DEPCTL_STALL;
1393+
dxepctl |= USB_DWC2_DEPCTL_SETD0PID;
1394+
sys_write32(dxepctl, dxepctl_reg);
12641395

12651396
LOG_DBG("Clear halt ep 0x%02x", cfg->addr);
12661397
cfg->stat.halted = false;
12671398

1399+
/* Resume queued transfers if any */
1400+
if (udc_buf_peek(dev, cfg->addr)) {
1401+
k_msgq_put(&drv_msgq, &evt, K_NO_WAIT);
1402+
}
1403+
12681404
return 0;
12691405
}
12701406

@@ -1753,8 +1889,8 @@ static const struct udc_api udc_dwc2_api = {
17531889
.set_address = udc_dwc2_set_address,
17541890
.test_mode = udc_dwc2_test_mode,
17551891
.host_wakeup = udc_dwc2_host_wakeup,
1756-
.ep_enable = udc_dwc2_ep_enable,
1757-
.ep_disable = udc_dwc2_ep_disable,
1892+
.ep_enable = udc_dwc2_ep_activate,
1893+
.ep_disable = udc_dwc2_ep_deactivate,
17581894
.ep_set_halt = udc_dwc2_ep_set_halt,
17591895
.ep_clear_halt = udc_dwc2_ep_clear_halt,
17601896
.ep_enqueue = udc_dwc2_ep_enqueue,

0 commit comments

Comments
 (0)