Skip to content

Commit 340bd02

Browse files
c1728p9cmonr
authored andcommitted
Kinetis USB improvements and fixes
Make the following improvements and fixes: 1. Update the Kinetis USB driver so that endpointReadResult only reads the result of the last read and does not trigger a new read. Instead move the code to trigger new reads into endpointRead. 2. Fix the race condition in controlIn caused by a call to EP0read() followed immediately by EP0readStage(). This is done by setting up to read the next setup packet (ignoring the status stage) in endpointReadResult rather than in EP0readStage. This makes the function EP0readStage unnecissary. 3. Remove the Kinetis workaround in controlOut in USBDevice.cpp since point 2 fixes this bug. For more info on this see the PR which added this workaround - #414
1 parent 0d9ed27 commit 340bd02

File tree

2 files changed

+85
-42
lines changed

2 files changed

+85
-42
lines changed

features/unsupported/USBDevice/USBDevice/USBDevice.cpp

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -187,27 +187,8 @@ bool USBDevice::controlOut(void)
187187
/* Check we should be transferring data OUT */
188188
if (transfer.direction != HOST_TO_DEVICE)
189189
{
190-
#if defined(TARGET_KL25Z) | defined(TARGET_KL43Z) | defined(TARGET_KL46Z) | defined(TARGET_K20D5M) | defined(TARGET_K64F) | defined(TARGET_K22F) | defined(TARGET_TEENSY3_1)
191-
/*
192-
* We seem to have a pending device-to-host transfer. The host must have
193-
* sent a new control request without waiting for us to finish processing
194-
* the previous one. This appears to happen when we're connected to certain
195-
* USB 3.0 host chip set. Do a zeor-length send to tell the host we're not
196-
* ready for the new request - that'll make it resend - and then just
197-
* pretend we were successful here so that the pending transfer can finish.
198-
*/
199-
uint8_t buf[1] = { 0 };
200-
EP0write(buf, 0);
201-
202-
/* execute our pending ttransfer */
203-
controlIn();
204-
205-
/* indicate success */
206-
return true;
207-
#else
208190
/* for other platforms, count on the HAL to handle this case */
209191
return false;
210-
#endif
211192
}
212193

213194
/* Read from endpoint */

features/unsupported/USBDevice/targets/TARGET_Freescale/USBHAL_KL25Z.cpp

Lines changed: 85 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "fsl_common.h"
2323
#endif
2424
#include "USBHAL.h"
25+
#include "mbed_critical.h"
2526

2627
USBHAL * USBHAL::instance;
2728

@@ -64,6 +65,13 @@ typedef struct BDT {
6465
uint32_t address; // Addr
6566
} BDT;
6667

68+
typedef enum {
69+
CTRL_XFER_READY,
70+
CTRL_XFER_IN,
71+
CTRL_XFER_NONE,
72+
CTRL_XFER_OUT
73+
} ctrl_xfer_t;
74+
6775
// there are:
6876
// * 4 bidirectionnal endpt -> 8 physical endpt
6977
// * as there are ODD and EVEN buffer -> 8*2 bdt
@@ -73,6 +81,7 @@ uint8_t * endpoint_buffer[NUMBER_OF_PHYSICAL_ENDPOINTS * 2];
7381

7482
static uint8_t set_addr = 0;
7583
static uint8_t addr = 0;
84+
static ctrl_xfer_t ctrl_xfer = CTRL_XFER_READY;
7685

7786
static uint32_t Data1 = 0x55555555;
7887

@@ -223,11 +232,16 @@ bool USBHAL::realiseEndpoint(uint8_t endpoint, uint32_t maxPacket, uint32_t flag
223232
USB_ENDPT_EPRXEN_MASK; // en RX (OUT) tran.
224233
bdt[EP_BDT_IDX(log_endpoint, RX, ODD )].byte_count = maxPacket;
225234
bdt[EP_BDT_IDX(log_endpoint, RX, ODD )].address = (uint32_t) buf;
226-
bdt[EP_BDT_IDX(log_endpoint, RX, ODD )].info = BD_OWN_MASK | BD_DTS_MASK;
235+
bdt[EP_BDT_IDX(log_endpoint, RX, ODD )].info = BD_DTS_MASK;
227236
bdt[EP_BDT_IDX(log_endpoint, RX, EVEN)].info = 0;
237+
if (log_endpoint == 0) {
238+
// Prepare for setup packet
239+
bdt[EP_BDT_IDX(log_endpoint, RX, ODD )].info |= BD_OWN_MASK;
240+
}
228241
}
229242

230-
Data1 |= (1 << endpoint);
243+
// First transfer will be a DATA0 packet
244+
Data1 &= ~(1 << endpoint);
231245

232246
return true;
233247
}
@@ -239,13 +253,35 @@ void USBHAL::EP0setup(uint8_t *buffer) {
239253
}
240254

241255
void USBHAL::EP0readStage(void) {
242-
Data1 &= ~1UL; // set DATA0
243-
bdt[0].info = (BD_DTS_MASK | BD_OWN_MASK);
256+
// Not needed
244257
}
245258

246259
void USBHAL::EP0read(void) {
247-
uint32_t idx = EP_BDT_IDX(PHY_TO_LOG(EP0OUT), RX, 0);
248-
bdt[idx].byte_count = MAX_PACKET_SIZE_EP0;
260+
if (ctrl_xfer == CTRL_XFER_READY) {
261+
// Transfer is done so ignore call
262+
return;
263+
}
264+
if (ctrl_xfer == CTRL_XFER_IN) {
265+
ctrl_xfer = CTRL_XFER_READY;
266+
// Control transfer with a data IN stage.
267+
// The next packet received will be the status packet - an OUT packet using DATA1
268+
//
269+
// PROBLEM:
270+
// If a Setup packet is received after status packet of
271+
// a Control In transfer has been received in the RX buffer
272+
// but before the processor has had a chance the prepare
273+
// this buffer for the Setup packet, the Setup packet
274+
// will be dropped.
275+
//
276+
// WORKAROUND:
277+
// Set data toggle to DATA0 so if the status stage of a
278+
// Control In transfer arrives it will be ACKed by hardware
279+
// but will be discarded without filling the RX buffer.
280+
// This allows a subsequent SETUP packet to be stored
281+
// without any processor intervention.
282+
Data1 &= ~1UL; // set DATA0
283+
}
284+
endpointRead(EP0OUT, MAX_PACKET_SIZE_EP0);
249285
}
250286

251287
uint32_t USBHAL::EP0getReadResult(uint8_t *buffer) {
@@ -255,20 +291,50 @@ uint32_t USBHAL::EP0getReadResult(uint8_t *buffer) {
255291
}
256292

257293
void USBHAL::EP0write(uint8_t *buffer, uint32_t size) {
294+
if (ctrl_xfer == CTRL_XFER_READY) {
295+
// Transfer is done so ignore call
296+
return;
297+
}
298+
if ((ctrl_xfer == CTRL_XFER_NONE) || (ctrl_xfer == CTRL_XFER_OUT)) {
299+
// Prepare for next setup packet
300+
endpointRead(EP0OUT, MAX_PACKET_SIZE_EP0);
301+
ctrl_xfer = CTRL_XFER_READY;
302+
}
258303
endpointWrite(EP0IN, buffer, size);
259304
}
260305

261306
void USBHAL::EP0getWriteResult(void) {
262307
}
263308

264309
void USBHAL::EP0stall(void) {
310+
if (ctrl_xfer == CTRL_XFER_READY) {
311+
// Transfer is done so ignore call
312+
return;
313+
}
314+
ctrl_xfer = CTRL_XFER_READY;
315+
core_util_critical_section_enter();
265316
stallEndpoint(EP0OUT);
317+
// Prepare for next setup packet
318+
// Note - time between stalling and setting up the endpoint
319+
// must be kept to a minimum to prevent a dropped SETUP
320+
// packet.
321+
endpointRead(EP0OUT, MAX_PACKET_SIZE_EP0);
322+
core_util_critical_section_exit();
266323
}
267324

268325
EP_STATUS USBHAL::endpointRead(uint8_t endpoint, uint32_t maximumSize) {
269-
endpoint = PHY_TO_LOG(endpoint);
270-
uint32_t idx = EP_BDT_IDX(endpoint, RX, 0);
326+
uint8_t log_endpoint = PHY_TO_LOG(endpoint);
327+
328+
uint32_t idx = EP_BDT_IDX(log_endpoint, RX, 0);
271329
bdt[idx].byte_count = maximumSize;
330+
if ((Data1 >> endpoint) & 1) {
331+
bdt[idx].info = BD_OWN_MASK | BD_DTS_MASK | BD_DATA01_MASK;
332+
}
333+
else {
334+
bdt[idx].info = BD_OWN_MASK | BD_DTS_MASK;
335+
}
336+
337+
Data1 ^= (1 << endpoint);
272338
return EP_PENDING;
273339
}
274340

@@ -307,18 +373,14 @@ EP_STATUS USBHAL::endpointReadResult(uint8_t endpoint, uint8_t * buffer, uint32_
307373
buffer[n] = ep_buf[n];
308374
}
309375

310-
if (((Data1 >> endpoint) & 1) == ((bdt[idx].info >> 6) & 1)) {
311-
if (setup && (buffer[6] == 0)) // if no setup data stage,
312-
Data1 &= ~1UL; // set DATA0
313-
else
314-
Data1 ^= (1 << endpoint);
315-
}
316-
317-
if (((Data1 >> endpoint) & 1)) {
318-
bdt[idx].info = BD_DTS_MASK | BD_DATA01_MASK | BD_OWN_MASK;
319-
}
320-
else {
321-
bdt[idx].info = BD_DTS_MASK | BD_OWN_MASK;
376+
if (setup) {
377+
// Record the setup type
378+
if (buffer[6] == 0) {
379+
ctrl_xfer = CTRL_XFER_NONE;
380+
} else {
381+
uint8_t in_xfer = (buffer[0] >> 7) & 1;
382+
ctrl_xfer = in_xfer ? CTRL_XFER_IN : CTRL_XFER_OUT;
383+
}
322384
}
323385

324386
USB0->CTL &= ~USB_CTL_TXSUSPENDTOKENBUSY_MASK;
@@ -351,9 +413,9 @@ EP_STATUS USBHAL::endpointWrite(uint8_t endpoint, uint8_t *data, uint32_t size)
351413
}
352414

353415
if ((Data1 >> endpoint) & 1) {
354-
bdt[idx].info = BD_OWN_MASK | BD_DTS_MASK;
355-
} else {
356416
bdt[idx].info = BD_OWN_MASK | BD_DTS_MASK | BD_DATA01_MASK;
417+
} else {
418+
bdt[idx].info = BD_OWN_MASK | BD_DTS_MASK;
357419
}
358420

359421
Data1 ^= (1 << endpoint);
@@ -450,7 +512,7 @@ void USBHAL::usbisr(void) {
450512

451513
// setup packet
452514
if ((num == 0) && (TOK_PID((EP_BDT_IDX(num, dir, ev_odd))) == SETUP_TOKEN)) {
453-
Data1 &= ~0x02;
515+
Data1 |= 0x02 | 0x01; // set DATA1 for TX and RX
454516
bdt[EP_BDT_IDX(0, TX, EVEN)].info &= ~BD_OWN_MASK;
455517
bdt[EP_BDT_IDX(0, TX, ODD)].info &= ~BD_OWN_MASK;
456518

0 commit comments

Comments
 (0)