diff --git a/lib/pbio/drv/bluetooth/bluetooth_nxt.c b/lib/pbio/drv/bluetooth/bluetooth_nxt.c index 76605778d..ef2a79715 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_nxt.c +++ b/lib/pbio/drv/bluetooth/bluetooth_nxt.c @@ -80,6 +80,13 @@ /** Time to wait for the BC4 to boot and announce itself. */ #define BT_RESET_TIMEOUT_MS (5000) +/** + * Time that sending stream data may stall before the link is taken as lost. + * The BC4 can stop taking data without dropping the stream (e.g. when the host + * closes the connection during a transfer), so it does not always say. + */ +#define BT_TX_TIMEOUT_MS (1000) + /** * PIN code sent when a host asks for one. The BC4 predates Secure Simple * Pairing, so the host always prompts for this. @@ -150,8 +157,10 @@ static volatile struct { int8_t handle; /** Whether the UART carries a raw byte stream instead of commands. */ bool streaming; - /** Set on soft-poweroff, to drop the link before the system goes down. */ - bool closing; + /** Set when the host connection should be dropped, e.g. on soft-poweroff. */ + bool disconnect_requested; + /** Set when the BC4 stopped taking stream data. */ + bool tx_stalled; /** Set when the BC4 sends a break condition. */ bool break_received; /** Number of messages dropped because of a bad checksum. */ @@ -176,6 +185,18 @@ static void bt_uart_write(const uint8_t *data, uint32_t size) { *AT91C_US1_IER = AT91C_US_TXBUFE; } +/** + * Discards the remainder of a transfer that the BC4 is not accepting. + */ +static void bt_uart_abort_write(void) { + *AT91C_US1_PTCR = AT91C_PDC_TXTDIS; + *AT91C_US1_TCR = 0; + *AT91C_US1_TNCR = 0; + *AT91C_US1_CR = AT91C_US_RSTTX; + *AT91C_US1_CR = AT91C_US_TXEN; + *AT91C_US1_PTCR = AT91C_PDC_TXTEN; +} + /** * Validates the trailing 16-bit checksum. The BC4 counts the length byte in * the messages it sends, but not in the ones it accepts. @@ -468,7 +489,7 @@ static pbio_error_t bt_process_thread(pbio_os_state_t *state, void *context) { DEBUG_PRINT("pins ok\n"); - while (!bt.closing) { + for (;;) { // Pulse reset so that the BC4 comes up in a known state, in command // mode. The UART is only enabled once it is out of reset, since a @@ -523,71 +544,94 @@ static pbio_error_t bt_process_thread(pbio_os_state_t *state, void *context) { continue; } - DEBUG_PRINT("listening\n"); - - while (!bt.streaming && !bt.closing) { - - if (bt.pin_requested) { - DEBUG_PRINT("pin req\n"); - memcpy(args, (const void *)bt.request_addr, BT_ADDR_SIZE); - memset(&args[BT_ADDR_SIZE], 0, BT_PIN_SIZE); - strncpy((char *)&args[BT_ADDR_SIZE], BT_PIN_CODE, BT_PIN_SIZE); - bt.pin_requested = false; - PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_PIN_CODE, args, BT_ADDR_SIZE + BT_PIN_SIZE, BT_MSG_PIN_CODE_ACK)); - continue; - } - - if (bt.connection_requested) { - DEBUG_PRINT("conn req\n"); - args[0] = true; - bt.connection_requested = false; - PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_ACCEPT_CONNECTION, args, 1, BT_MSG_NONE)); - continue; - } - - if (bt.handle >= 0) { - DEBUG_PRINT("stream h%d\n", bt.handle); - args[0] = bt.handle; - PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_OPEN_STREAM, args, 1, BT_MSG_NONE)); - - // The BC4 only switches once it has the whole command. - pbio_os_timer_set(&timer, BT_REPLY_TIMEOUT_MS); - PBIO_OS_AWAIT_UNTIL(state, !bt_uart_is_writing() || pbio_os_timer_is_expired(&timer)); - bt_set_streaming(true); - continue; + // Serve one connection after another, until the BC4 needs a reset. + for (;;) { + DEBUG_PRINT("listening\n"); + + while (!bt.streaming) { + + if (bt.pin_requested) { + DEBUG_PRINT("pin req\n"); + memcpy(args, (const void *)bt.request_addr, BT_ADDR_SIZE); + memset(&args[BT_ADDR_SIZE], 0, BT_PIN_SIZE); + strncpy((char *)&args[BT_ADDR_SIZE], BT_PIN_CODE, BT_PIN_SIZE); + bt.pin_requested = false; + PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_PIN_CODE, args, BT_ADDR_SIZE + BT_PIN_SIZE, BT_MSG_PIN_CODE_ACK)); + continue; + } + + if (bt.connection_requested) { + DEBUG_PRINT("conn req\n"); + args[0] = true; + bt.connection_requested = false; + PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_ACCEPT_CONNECTION, args, 1, BT_MSG_NONE)); + continue; + } + + if (bt.handle >= 0) { + DEBUG_PRINT("stream h%d\n", bt.handle); + args[0] = bt.handle; + PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_OPEN_STREAM, args, 1, BT_MSG_NONE)); + + // The BC4 only switches once it has the whole command. + pbio_os_timer_set(&timer, BT_REPLY_TIMEOUT_MS); + PBIO_OS_AWAIT_UNTIL(state, !bt_uart_is_writing() || pbio_os_timer_is_expired(&timer)); + bt_set_streaming(true); + continue; + } + + PBIO_OS_AWAIT_MS(state, &timer, 50); } - PBIO_OS_AWAIT_MS(state, &timer, 50); - } - - if (bt.streaming) { // The mode line follows the switch with some delay, so wait for it // to come up before taking it dropping as a disconnect. pbio_os_timer_set(&timer, BT_REPLY_TIMEOUT_MS); PBIO_OS_AWAIT_UNTIL(state, bt_bc4_is_streaming() || pbio_os_timer_is_expired(&timer)); DEBUG_PRINT("mode %u\n", bt_bc4_is_streaming()); - PBIO_OS_AWAIT_UNTIL(state, !bt_bc4_is_streaming() || bt.closing); + PBIO_OS_AWAIT_UNTIL(state, !bt_bc4_is_streaming() || bt.disconnect_requested); DEBUG_PRINT("disconn brk%u\n", bt.break_received); bt_set_streaming(false); - // The BC4 reports why it dropped the stream, and does not take - // commands until it has. - PBIO_OS_AWAIT_MS(state, &timer, 500); - } - } + // A BC4 that stopped taking data does not take commands either, + // so leave it to the reset that follows. + if (!bt.tx_stalled) { + // The BC4 reports why it dropped the stream, and does not take + // commands until it has. + PBIO_OS_AWAIT_MS(state, &timer, 500); + + // Drop the link explicitly, so that the host sees a disconnect + // instead of waiting for a timeout. + if (bt.disconnect_requested && bt.handle >= 0) { + args[0] = bt.handle; + PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_CLOSE_CONNECTION, args, 1, BT_MSG_CLOSE_CONNECTION_RESULT)); + } + + // Once the BC4 reports the connection closed, it can take the + // next one. + pbio_os_timer_set(&timer, BT_REPLY_TIMEOUT_MS); + PBIO_OS_AWAIT_UNTIL(state, bt.handle < 0 || pbio_os_timer_is_expired(&timer)); + } + + if (bt.disconnect_requested) { + DEBUG_PRINT("dropped\n"); + bt.disconnect_requested = false; + pbio_busy_count_down(); + } - // Drop the link explicitly, so that the host sees a disconnect instead of - // waiting for a timeout. - if (bt.handle >= 0) { - args[0] = bt.handle; - PBIO_OS_AWAIT(state, &sub, bt_command(&sub, BT_MSG_CLOSE_CONNECTION, args, 1, BT_MSG_CLOSE_CONNECTION_RESULT)); + // A reset drops the radio link without telling the host, which + // can keep it from connecting again for a while. So only reset + // the BC4 if it stopped responding. + if (bt.tx_stalled || bt.handle >= 0) { + bt.tx_stalled = false; + break; + } + } } - DEBUG_PRINT("closed\n"); - pbio_busy_count_down(); - PBIO_OS_ASYNC_END(PBIO_SUCCESS); + // Unreachable. + PBIO_OS_ASYNC_END(PBIO_ERROR_FAILED); } void pbdrv_bluetooth_init(void) { @@ -615,12 +659,12 @@ const char *pbdrv_bluetooth_classic_host_get_connected_name(void) { } void pbdrv_bluetooth_classic_host_disconnect(void) { - if (bt.closing) { + if (!bt.streaming || bt.disconnect_requested) { return; } - // Closing takes a few exchanges with the BC4, so hold off the shutdown - // that called this until the process is done. - bt.closing = true; + // Dropping the link takes a few exchanges with the BC4, so hold off a + // shutdown that called this until the process is done. + bt.disconnect_requested = true; pbio_busy_count_up(); pbio_os_request_poll(); } @@ -631,6 +675,8 @@ uint32_t pbdrv_bluetooth_classic_host_rx_read(uint8_t *data, uint32_t size) { pbio_error_t pbdrv_bluetooth_classic_host_tx_message(pbio_os_state_t *state, const uint8_t *data, uint32_t size) { + static pbio_os_timer_t timer; + PBIO_OS_ASYNC_BEGIN(state); if (!bt.streaming) { @@ -639,7 +685,17 @@ pbio_error_t pbdrv_bluetooth_classic_host_tx_message(pbio_os_state_t *state, con PBIO_OS_AWAIT_UNTIL(state, !bt_uart_is_writing()); bt_uart_write(data, size); - PBIO_OS_AWAIT_UNTIL(state, !bt.streaming || !bt_uart_is_writing()); + pbio_os_timer_set(&timer, BT_TX_TIMEOUT_MS); + PBIO_OS_AWAIT_UNTIL(state, !bt.streaming || !bt_uart_is_writing() || pbio_os_timer_is_expired(&timer)); + + // The rest of the message is of no use to anyone now, and would block + // commands to the BC4. + if (bt_uart_is_writing()) { + bt_uart_abort_write(); + bt.tx_stalled = bt.streaming; + pbdrv_bluetooth_classic_host_disconnect(); + return PBIO_ERROR_TIMEDOUT; + } if (!bt.streaming) { return PBIO_ERROR_INVALID_OP; diff --git a/lib/pbio/drv/usb/usb_nxt.c b/lib/pbio/drv/usb/usb_nxt.c index 71d309b01..5a64dcda1 100644 --- a/lib/pbio/drv/usb/usb_nxt.c +++ b/lib/pbio/drv/usb/usb_nxt.c @@ -278,32 +278,29 @@ static bool pbdrv_usb_nxt_tx_zlp[PBDRV_USB_NXT_N_ENDPOINTS]; #define CSR_NO_EFFECT_1_ALL (AT91C_UDP_TXCOMP | AT91C_UDP_RX_DATA_BK0 | \ AT91C_UDP_RXSETUP | AT91C_UDP_ISOERROR | AT91C_UDP_RX_DATA_BK1) -/* The flags in the UDP_CSR register are a little strange: writing to - * them does not instantly change their value. Their value will change - * to reflect the write when the USB controller has taken the change - * into account. The driver must wait until the controller - * acknowledges changes to CSR. - * - * These helpers set/clear CSR flags, and then loop waiting for the - * controller to synchronize +/* A CSR write takes effect only once it has crossed into the USB clock + * domain. The datasheet (doc6175, UDP_CSR) says to wait 3 UDPCK and 3 MCK + * cycles (125 ns) instead of polling for the new value. The controller may + * already have changed it back (e.g. TXPKTRDY once the packet is sent). */ +#define PBDRV_USB_NXT_CSR_SYNC_NS 150 + +static void pbdrv_usb_nxt_csr_write(uint8_t endpoint, uint32_t csr) { + AT91C_UDP_CSR[endpoint] = csr; + nx_systick_wait_ns(PBDRV_USB_NXT_CSR_SYNC_NS); +} + static void pbdrv_usb_nxt_csr_clear_flag(uint8_t endpoint, uint32_t flags) { uint32_t csr = AT91C_UDP_CSR[endpoint]; csr |= CSR_NO_EFFECT_1_ALL; csr &= ~(flags); - AT91C_UDP_CSR[endpoint] = csr; - while (AT91C_UDP_CSR[endpoint] & (flags)) { - ; - } + pbdrv_usb_nxt_csr_write(endpoint, csr); } static void pbdrv_usb_nxt_csr_set_flag(uint8_t endpoint, uint32_t flags) { uint32_t csr = AT91C_UDP_CSR[endpoint]; csr |= CSR_NO_EFFECT_1_ALL | (flags); - AT91C_UDP_CSR[endpoint] = csr; - while ((AT91C_UDP_CSR[endpoint] & (flags)) != (flags)) { - ; - } + pbdrv_usb_nxt_csr_write(endpoint, csr); } /* Starts sending data to the host. If the data cannot fit into a @@ -552,20 +549,9 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_setup_packet_t *packet) { /* Enable the CDC data and notification endpoints. */ pbdrv_usb_rx_bank = AT91C_UDP_RX_DATA_BK0; - AT91C_UDP_CSR[EP_BULK_OUT] = AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_BULK_OUT; - while (AT91C_UDP_CSR[EP_BULK_OUT] != (AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_BULK_OUT)) { - ; - } - - AT91C_UDP_CSR[EP_BULK_IN] = AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_BULK_IN; - while (AT91C_UDP_CSR[EP_BULK_IN] != (AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_BULK_IN)) { - ; - } - - AT91C_UDP_CSR[EP_NOTIF] = AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_INT_IN; - while (AT91C_UDP_CSR[EP_NOTIF] != (AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_INT_IN)) { - ; - } + pbdrv_usb_nxt_csr_write(EP_BULK_OUT, AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_BULK_OUT); + pbdrv_usb_nxt_csr_write(EP_BULK_IN, AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_BULK_IN); + pbdrv_usb_nxt_csr_write(EP_NOTIF, AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_INT_IN); pbdrv_usb_nxt_configured = packet->wValue > 0; break; @@ -685,11 +671,7 @@ static void pbdrv_usb_nxt_isr(void) { pbdrv_usb_rx_len = 0; /* Reset EP0 to a basic control endpoint. */ - /* TODO: The while is ugly. Fix it. */ - AT91C_UDP_CSR[0] = AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_CTRL; - while (AT91C_UDP_CSR[0] != (AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_CTRL)) { - ; - } + pbdrv_usb_nxt_csr_write(EP_CONTROL, AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_CTRL); /* Enable interrupt handling for all three endpoints, as well as * suspend/resume. @@ -727,8 +709,14 @@ static void pbdrv_usb_nxt_isr(void) { isr &= ~AT91C_UDP_RXRSM; } + /* Skip endpoints whose interrupt is masked (e.g. the bulk OUT endpoint + * while a received packet waits for the process), which UDP_ISR still + * reports. These can repeatedly sidetrack handling of a real interrupt, + * causing a hang. + */ + uint32_t pending = isr & *AT91C_UDP_IMR; for (endpoint = 0; endpoint < PBDRV_USB_NXT_N_ENDPOINTS; endpoint++) { - if (isr & (1 << endpoint)) { + if (pending & (1 << endpoint)) { break; } }