From b0b2ba61c1e637533b8765b5f972338e9f78a82f Mon Sep 17 00:00:00 2001 From: Antonio Galea Date: Wed, 9 Oct 2019 11:20:52 +0200 Subject: [PATCH 1/4] USBUS CDC ACM STDIO: flush stdio buffer upon write --- sys/usb/usbus/cdc/acm/cdc_acm_stdio.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/sys/usb/usbus/cdc/acm/cdc_acm_stdio.c b/sys/usb/usbus/cdc/acm/cdc_acm_stdio.c index c61af0b223..35903ed033 100644 --- a/sys/usb/usbus/cdc/acm/cdc_acm_stdio.c +++ b/sys/usb/usbus/cdc/acm/cdc_acm_stdio.c @@ -52,10 +52,15 @@ ssize_t stdio_read(void* buffer, size_t len) ssize_t stdio_write(const void* buffer, size_t len) { - usbus_cdc_acm_submit(&cdcacm, buffer, len); - usbus_cdc_acm_flush(&cdcacm); - /* Use tsrb and flush */ - return len; + const char *start = buffer; + do { + size_t n = usbus_cdc_acm_submit(&cdcacm, buffer, len); + usbus_cdc_acm_flush(&cdcacm); + /* Use tsrb and flush */ + buffer = (char *)buffer + n; + len -= n; + } while (len); + return start - (char *)buffer; } static void _cdc_acm_rx_pipe(usbus_cdcacm_device_t *cdcacm, From 01c3043f6f2a06f260e0915c1950d41f3ec5186c Mon Sep 17 00:00:00 2001 From: Antonio Galea Date: Wed, 9 Oct 2019 18:27:06 +0200 Subject: [PATCH 2/4] USBUS CDC ACM: discard oldest data if unconnected --- sys/usb/usbus/cdc/acm/cdc_acm.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/sys/usb/usbus/cdc/acm/cdc_acm.c b/sys/usb/usbus/cdc/acm/cdc_acm.c index f67dfb7e43..a132fce5ba 100644 --- a/sys/usb/usbus/cdc/acm/cdc_acm.c +++ b/sys/usb/usbus/cdc/acm/cdc_acm.c @@ -126,7 +126,20 @@ static size_t _gen_full_acm_descriptor(usbus_t *usbus, void *arg) /* Submit (ACM interface in) */ size_t usbus_cdc_acm_submit(usbus_cdcacm_device_t *cdcacm, const uint8_t *buf, size_t len) { - return tsrb_add(&cdcacm->tsrb, buf, len); + if (cdcacm->state != USBUS_CDC_ACM_LINE_STATE_DISCONNECTED) { + return tsrb_add(&cdcacm->tsrb, buf, len); + } + /* stuff as much data as possible into tsrb, discarding the oldest */ + size_t n = tsrb_free(&cdcacm->tsrb); + if (len > n) { + n += tsrb_drop(&cdcacm->tsrb, len - n); + buf += len - n; + } else { + n = len; + } + tsrb_add(&cdcacm->tsrb, buf, n); + /* behave as if everything has been written correctly */ + return len; } void usbus_cdc_acm_set_coding_cb(usbus_cdcacm_device_t *cdcacm, @@ -161,7 +174,7 @@ void usbus_cdc_acm_init(usbus_t *usbus, usbus_cdcacm_device_t *cdcacm, static void _init(usbus_t *usbus, usbus_handler_t *handler) { - DEBUG("CDC_ACM: intialization\n"); + DEBUG("CDC_ACM: initialization\n"); usbus_cdcacm_device_t *cdcacm = (usbus_cdcacm_device_t*)handler; cdcacm->flush.handler = _handle_flush; From 6c04cb1a451ad0812b583398352882f7d1e3b41c Mon Sep 17 00:00:00 2001 From: Antonio Galea Date: Thu, 10 Oct 2019 08:26:45 +0200 Subject: [PATCH 3/4] USBUS CDC ACM: disable interrupts when using tsrb --- sys/usb/usbus/cdc/acm/cdc_acm.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/sys/usb/usbus/cdc/acm/cdc_acm.c b/sys/usb/usbus/cdc/acm/cdc_acm.c index a132fce5ba..a8e4bcccd9 100644 --- a/sys/usb/usbus/cdc/acm/cdc_acm.c +++ b/sys/usb/usbus/cdc/acm/cdc_acm.c @@ -126,11 +126,17 @@ static size_t _gen_full_acm_descriptor(usbus_t *usbus, void *arg) /* Submit (ACM interface in) */ size_t usbus_cdc_acm_submit(usbus_cdcacm_device_t *cdcacm, const uint8_t *buf, size_t len) { + size_t n; + unsigned old; if (cdcacm->state != USBUS_CDC_ACM_LINE_STATE_DISCONNECTED) { - return tsrb_add(&cdcacm->tsrb, buf, len); + old = irq_disable(); + n = tsrb_add(&cdcacm->tsrb, buf, len); + irq_restore(old); + return n; } /* stuff as much data as possible into tsrb, discarding the oldest */ - size_t n = tsrb_free(&cdcacm->tsrb); + old = irq_disable(); + n = tsrb_free(&cdcacm->tsrb); if (len > n) { n += tsrb_drop(&cdcacm->tsrb, len - n); buf += len - n; @@ -139,15 +145,16 @@ size_t usbus_cdc_acm_submit(usbus_cdcacm_device_t *cdcacm, const uint8_t *buf, s } tsrb_add(&cdcacm->tsrb, buf, n); /* behave as if everything has been written correctly */ + irq_restore(old); return len; } void usbus_cdc_acm_set_coding_cb(usbus_cdcacm_device_t *cdcacm, usbus_cdcacm_coding_cb_t coding_cb) { - irq_disable(); + unsigned old = irq_disable(); cdcacm->coding_cb = coding_cb; - irq_enable(); + irq_restore(old); } /* flush event */ @@ -286,6 +293,8 @@ static void _handle_in(usbus_cdcacm_device_t *cdcacm, (cdcacm->state != USBUS_CDC_ACM_LINE_STATE_DTE)) { return; } + /* copy at most USBUS_CDC_ACM_BULK_EP_SIZE chars from input into ep->buf */ + unsigned old = irq_disable(); while (!tsrb_empty(&cdcacm->tsrb)) { int c = tsrb_get_one(&cdcacm->tsrb); ep->buf[cdcacm->occupied++] = (uint8_t)c; @@ -293,6 +302,7 @@ static void _handle_in(usbus_cdcacm_device_t *cdcacm, break; } } + irq_restore(old); usbdev_ep_ready(ep, cdcacm->occupied); } From d1cc563ccfb5cd053d33d996ff6a7fbb7bb94011 Mon Sep 17 00:00:00 2001 From: Antonio Galea Date: Thu, 10 Oct 2019 14:56:58 +0200 Subject: [PATCH 4/4] USBUS CDC ACM STDIO: test for the buffering code fixes #12384 --- tests/usbus_cdc_acm_stdio/main.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/usbus_cdc_acm_stdio/main.c b/tests/usbus_cdc_acm_stdio/main.c index e342013afd..c41f763540 100644 --- a/tests/usbus_cdc_acm_stdio/main.c +++ b/tests/usbus_cdc_acm_stdio/main.c @@ -18,16 +18,46 @@ */ #include +#include #include "shell.h" #include "shell_commands.h" +static int cmd_text(int argc, char **argv) +{ + char *usage = "text [length]\n"; + if (argc != 2) { + puts(usage); + return -1; + } + int n = atoi(argv[1]); + if (n <= 0) { + puts(usage); + return -1; + } + for (int i=0; i < n; i++) { + if (i && (i % 80 == 0)) { + puts(""); + } + putc('0' + (i % 10), stdout); + } + puts(""); + return 0; +} + + +static const shell_command_t shell_commands[] = { + { "text", "Generates long text for testing stdio buffer", cmd_text }, + { NULL, NULL, NULL } +}; + + int main(void) { (void) puts("RIOT USB CDC ACM shell test"); char line_buf[SHELL_DEFAULT_BUFSIZE]; - shell_run(NULL, line_buf, SHELL_DEFAULT_BUFSIZE); + shell_run(shell_commands, line_buf, SHELL_DEFAULT_BUFSIZE); return 0; }