* [PATCH 1/3] lib/circbuf: Make circbuf selectable symbol @ 2021-08-19 11:13 Loic Poulain 2021-08-19 11:13 ` [PATCH 2/3] usb: gadget: Add CDC ACM function Loic Poulain 2021-08-19 11:13 ` [PATCH 3/3] cmd: add acmconsole command Loic Poulain 0 siblings, 2 replies; 10+ messages in thread From: Loic Poulain @ 2021-08-19 11:13 UTC (permalink / raw) To: marex; +Cc: u-boot, lukma, Loic Poulain It is currenly only used from usbtty driver but make it properly selectable via Kconfig symbol, for future usage. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- lib/Kconfig | 3 +++ lib/Makefile | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/Kconfig b/lib/Kconfig index c535147..1bd54d8 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -290,6 +290,9 @@ config TRACE_EARLY_ADDR the size is too small then the message which says the amount of early data being coped will the the same as the +config CIRCBUF + bool + source lib/dhry/Kconfig menu "Security support" diff --git a/lib/Makefile b/lib/Makefile index 8ba745f..4daeee2 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -29,7 +29,13 @@ ifneq ($(CONFIG_CHARSET),) obj-y += charset.o endif endif -obj-$(CONFIG_USB_TTY) += circbuf.o + +ifdef CONFIG_USB_TTY +obj-y += circbuf.o +else +obj-$(CONFIG_CIRCBUF) += circbuf.o +endif + obj-y += crc8.o obj-y += crc16.o obj-$(CONFIG_ERRNO_STR) += errno_str.o -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] usb: gadget: Add CDC ACM function 2021-08-19 11:13 [PATCH 1/3] lib/circbuf: Make circbuf selectable symbol Loic Poulain @ 2021-08-19 11:13 ` Loic Poulain 2021-09-07 10:21 ` Loic Poulain 2021-09-27 19:55 ` Pali Rohár 2021-08-19 11:13 ` [PATCH 3/3] cmd: add acmconsole command Loic Poulain 1 sibling, 2 replies; 10+ messages in thread From: Loic Poulain @ 2021-08-19 11:13 UTC (permalink / raw) To: marex; +Cc: u-boot, lukma, Loic Poulain Add support for CDC ACM using the new UDC and gadget API. This protocol can be used for serial over USB data transfer and is widely supported by various OS (GNU/Linux, MS-Windows, OSX...). The usual purpose of such link is to access device debug console and can be useful for products not exposing regular UART to the user. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- drivers/usb/gadget/Kconfig | 9 + drivers/usb/gadget/Makefile | 1 + drivers/usb/gadget/f_acm.c | 663 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 673 insertions(+) create mode 100644 drivers/usb/gadget/f_acm.c diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 327ea86..d81a9c5 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -182,6 +182,15 @@ config USB_FUNCTION_THOR Enable Tizen's THOR download protocol support in U-Boot. It allows downloading images into memory and flash them to target device. +config USB_FUNCTION_ACM + bool "Enable CDC ACM gadget" + select SYS_STDIO_DEREGISTER + select CIRCBUF + help + ACM serial link. This function can be used to create a stdio device to + interoperate with MS-Windows hosts or with the Linux-USB "cdc-acm" + driver. + endif # USB_GADGET_DOWNLOAD config USB_ETHER diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index f560068..d5d891b 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += f_rockusb.o +obj-$(CONFIG_USB_FUNCTION_ACM) += f_acm.o endif endif ifdef CONFIG_USB_ETHER diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c new file mode 100644 index 0000000..5115f1d --- /dev/null +++ b/drivers/usb/gadget/f_acm.c @@ -0,0 +1,663 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * USB CDC serial (ACM) function driver + * + * Copyright (C) 2003 Al Borchers (alborchers@steinerpoint.com) + * Copyright (C) 2008 by David Brownell + * Copyright (C) 2008 by Nokia Corporation + * Copyright (C) 2009 by Samsung Electronics + * Copyright (c) 2021, Linaro Ltd <loic.poulain@linaro.org> + */ + +#include <circbuf.h> +#include <common.h> +#include <console.h> +#include <errno.h> +#include <g_dnl.h> +#include <malloc.h> +#include <memalign.h> +#include <stdio_dev.h> +#include <version.h> +#include <watchdog.h> + +#include <linux/usb/ch9.h> +#include <linux/usb/gadget.h> +#include <linux/usb/composite.h> +#include <linux/usb/cdc.h> + +#define REQ_SIZE_MAX 512 + +struct f_acm { + int ctrl_id; + int data_id; + + struct usb_ep *ep_in; + struct usb_ep *ep_out; + struct usb_ep *ep_notify; + + struct usb_request *req_in; + struct usb_request *req_out; + + bool connected; + bool tx_on; + + circbuf_t rx_buf; + circbuf_t tx_buf; + + struct stdio_dev stdio; + struct usb_function usb_function; + + struct usb_cdc_line_coding line_coding; + u16 handshake_bits; +#define ACM_CTRL_RTS BIT(1) /* unused with full duplex */ +#define ACM_CTRL_DTR BIT(0) /* host is ready for data r/w */ + + int controller_index; +}; + +static inline struct f_acm *func_to_acm(struct usb_function *f) +{ + return container_of(f, struct f_acm, usb_function); +} + +static inline struct f_acm *stdio_to_acm(struct stdio_dev *s) +{ + /* stdio dev is cloned on registration, do not use container_of */ + return s->priv; +} + +static struct usb_interface_assoc_descriptor +acm_iad_descriptor = { + .bLength = sizeof(acm_iad_descriptor), + .bDescriptorType = USB_DT_INTERFACE_ASSOCIATION, + .bFirstInterface = 0, + .bInterfaceCount = 2, // control + data + .bFunctionClass = USB_CLASS_COMM, + .bFunctionSubClass = USB_CDC_SUBCLASS_ACM, + .bFunctionProtocol = USB_CDC_ACM_PROTO_AT_V25TER, +}; + +static struct usb_interface_descriptor acm_control_intf_desc = { + .bLength = USB_DT_INTERFACE_SIZE, + .bDescriptorType = USB_DT_INTERFACE, + .bNumEndpoints = 1, + .bInterfaceClass = USB_CLASS_COMM, + .bInterfaceSubClass = USB_CDC_SUBCLASS_ACM, + .bInterfaceProtocol = USB_CDC_ACM_PROTO_AT_V25TER, +}; + +static struct usb_interface_descriptor acm_data_intf_desc = { + .bLength = sizeof(acm_data_intf_desc), + .bDescriptorType = USB_DT_INTERFACE, + .bNumEndpoints = 2, + .bInterfaceClass = USB_CLASS_CDC_DATA, +}; + +static struct usb_cdc_header_desc acm_header_desc = { + .bLength = sizeof(acm_header_desc), + .bDescriptorType = USB_DT_CS_INTERFACE, + .bDescriptorSubType = USB_CDC_HEADER_TYPE, + .bcdCDC = cpu_to_le16(0x0110), +}; + +static struct usb_cdc_call_mgmt_descriptor acm_call_mgmt_desc = { + .bLength = sizeof(acm_call_mgmt_desc), + .bDescriptorType = USB_DT_CS_INTERFACE, + .bDescriptorSubType = USB_CDC_CALL_MANAGEMENT_TYPE, + .bmCapabilities = 0, + .bDataInterface = 0x01, +}; + +static struct usb_cdc_acm_descriptor acm_desc = { + .bLength = sizeof(acm_desc), + .bDescriptorType = USB_DT_CS_INTERFACE, + .bDescriptorSubType = USB_CDC_ACM_TYPE, + .bmCapabilities = USB_CDC_CAP_LINE, +}; + +static struct usb_cdc_union_desc acm_union_desc = { + .bLength = sizeof(acm_union_desc), + .bDescriptorType = USB_DT_CS_INTERFACE, + .bDescriptorSubType = USB_CDC_UNION_TYPE, + .bMasterInterface0 = 0x00, + .bSlaveInterface0 = 0x01, +}; + +static struct usb_endpoint_descriptor acm_fs_notify_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + .bEndpointAddress = 3 | USB_DIR_IN, + .bmAttributes = USB_ENDPOINT_XFER_INT, + .wMaxPacketSize = cpu_to_le16(64), + .bInterval = 32, +}; + +static struct usb_endpoint_descriptor acm_fs_in_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + .bEndpointAddress = USB_DIR_IN, + .bmAttributes = USB_ENDPOINT_XFER_BULK, +}; + +static struct usb_endpoint_descriptor acm_fs_out_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + .bEndpointAddress = USB_DIR_OUT, + .bmAttributes = USB_ENDPOINT_XFER_BULK, +}; + +static struct usb_descriptor_header *acm_fs_function[] = { + (struct usb_descriptor_header *)&acm_iad_descriptor, + (struct usb_descriptor_header *)&acm_control_intf_desc, + (struct usb_descriptor_header *)&acm_header_desc, + (struct usb_descriptor_header *)&acm_call_mgmt_desc, + (struct usb_descriptor_header *)&acm_desc, + (struct usb_descriptor_header *)&acm_union_desc, + (struct usb_descriptor_header *)&acm_fs_notify_desc, + (struct usb_descriptor_header *)&acm_data_intf_desc, + (struct usb_descriptor_header *)&acm_fs_in_desc, + (struct usb_descriptor_header *)&acm_fs_out_desc, + NULL, +}; + +static struct usb_endpoint_descriptor acm_hs_notify_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + .bmAttributes = USB_ENDPOINT_XFER_INT, + .wMaxPacketSize = cpu_to_le16(64), + .bInterval = 11, +}; + +static struct usb_endpoint_descriptor acm_hs_in_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + .bmAttributes = USB_ENDPOINT_XFER_BULK, + .wMaxPacketSize = cpu_to_le16(512), +}; + +static struct usb_endpoint_descriptor acm_hs_out_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + .bmAttributes = USB_ENDPOINT_XFER_BULK, + .wMaxPacketSize = cpu_to_le16(512), +}; + +static struct usb_descriptor_header *acm_hs_function[] = { + (struct usb_descriptor_header *)&acm_iad_descriptor, + (struct usb_descriptor_header *)&acm_control_intf_desc, + (struct usb_descriptor_header *)&acm_header_desc, + (struct usb_descriptor_header *)&acm_call_mgmt_desc, + (struct usb_descriptor_header *)&acm_desc, + (struct usb_descriptor_header *)&acm_union_desc, + (struct usb_descriptor_header *)&acm_hs_notify_desc, + (struct usb_descriptor_header *)&acm_data_intf_desc, + (struct usb_descriptor_header *)&acm_hs_in_desc, + (struct usb_descriptor_header *)&acm_hs_out_desc, + NULL, +}; + +static inline struct usb_endpoint_descriptor * +ep_desc(struct usb_gadget *g, struct usb_endpoint_descriptor *hs, + struct usb_endpoint_descriptor *fs) +{ + if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH) + return hs; + return fs; +} + +static int acm_bind(struct usb_configuration *c, struct usb_function *f) +{ + struct usb_gadget *gadget = c->cdev->gadget; + struct f_acm *f_acm = func_to_acm(f); + struct usb_ep *ep; + int id; + + id = usb_interface_id(c, f); + if (id < 0) + return id; + + acm_iad_descriptor.bFirstInterface = id; + acm_control_intf_desc.bInterfaceNumber = id; + acm_union_desc.bMasterInterface0 = id; + + f_acm->ctrl_id = id; + + id = usb_interface_id(c, f); + if (id < 0) + return id; + + acm_data_intf_desc.bInterfaceNumber = id; + acm_union_desc.bSlaveInterface0 = id; + acm_call_mgmt_desc.bDataInterface = id; + + f_acm->data_id = id; + + /* allocate instance-specific endpoints */ + ep = usb_ep_autoconfig(gadget, &acm_fs_in_desc); + if (!ep) + return -ENODEV; + + f_acm->ep_in = ep; + + ep = usb_ep_autoconfig(gadget, &acm_fs_out_desc); + if (!ep) + return -ENODEV; + + f_acm->ep_out = ep; + + ep = usb_ep_autoconfig(gadget, &acm_fs_notify_desc); + if (!ep) + return -ENODEV; + + f_acm->ep_notify = ep; + + if (gadget_is_dualspeed(gadget)) { + /* Assume endpoint addresses are the same for both speeds */ + acm_hs_in_desc.bEndpointAddress = acm_fs_in_desc.bEndpointAddress; + acm_hs_out_desc.bEndpointAddress = acm_fs_out_desc.bEndpointAddress; + acm_hs_notify_desc.bEndpointAddress = acm_fs_notify_desc.bEndpointAddress; + } + + return 0; +} + +static void acm_unbind(struct usb_configuration *c, struct usb_function *f) +{ + struct f_acm *f_acm = func_to_acm(f); + struct stdio_dev *stdio; + + stdio = stdio_get_by_name(f_acm->stdio.name); + stdio_deregister_dev(stdio, 1); + + buf_free(&f_acm->rx_buf); + buf_free(&f_acm->tx_buf); + + free(f_acm); +} + +static void acm_notify_complete(struct usb_ep *ep, struct usb_request *req) +{ + if (req->status) + printf("CDC ACM notify failed (%d)\n", req->status); + + /* nothing to do */ +} + +static void acm_tx_complete(struct usb_ep *ep, struct usb_request *req) +{ + static int once; /* prevent infinite recursion */ + + if (req->status && !once++) + printf("CDC ACM TX failed (%d)\n", req->status); + + struct f_acm *f_acm = req->context; + + f_acm->tx_on = true; +} + +static void acm_rx_complete(struct usb_ep *ep, struct usb_request *req) +{ + struct f_acm *f_acm = req->context; + + if (req->status != 0) + printf("CDC ACM RX failed (%d)\n", req->status); + + buf_push(&f_acm->rx_buf, req->buf, req->actual); + + /* Queue RX req again */ + req->actual = 0; + usb_ep_queue(ep, req, 0); +} + +static struct usb_request *acm_start_ep(struct usb_ep *ep, void *complete_cb, + void *context) +{ + struct usb_request *req; + + req = usb_ep_alloc_request(ep, 0); + if (!req) + return NULL; + + req->length = REQ_SIZE_MAX; + req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE, REQ_SIZE_MAX); + if (!req->buf) { + usb_ep_free_request(ep, req); + return NULL; + } + + memset(req->buf, 0, req->length); + req->complete = complete_cb; + req->context = context; + + return req; +} + +static int acm_start_data(struct f_acm *f_acm, struct usb_gadget *gadget) +{ + const struct usb_endpoint_descriptor *d; + int ret; + + /* EP IN */ + d = ep_desc(gadget, &acm_hs_in_desc, &acm_fs_in_desc); + ret = usb_ep_enable(f_acm->ep_in, d); + if (ret) + return ret; + + f_acm->req_in = acm_start_ep(f_acm->ep_in, acm_tx_complete, f_acm); + + /* EP OUT */ + d = ep_desc(gadget, &acm_hs_out_desc, &acm_fs_out_desc); + ret = usb_ep_enable(f_acm->ep_out, d); + if (ret) + return ret; + + f_acm->req_out = acm_start_ep(f_acm->ep_out, acm_rx_complete, f_acm); + + /* Start OUT transfer (EP OUT) */ + ret = usb_ep_queue(f_acm->ep_out, f_acm->req_out, 0); + if (ret) + return ret; + + return 0; +} + +static int acm_start_ctrl(struct f_acm *f_acm, struct usb_gadget *gadget) +{ + const struct usb_endpoint_descriptor *d; + + usb_ep_disable(f_acm->ep_notify); + + d = ep_desc(gadget, &acm_hs_notify_desc, &acm_fs_notify_desc); + usb_ep_enable(f_acm->ep_notify, d); + + acm_start_ep(f_acm->ep_notify, acm_notify_complete, f_acm); + + return 0; +} + +static int acm_set_alt(struct usb_function *f, unsigned int intf, unsigned int alt) +{ + struct usb_gadget *gadget = f->config->cdev->gadget; + struct f_acm *f_acm = func_to_acm(f); + + if (intf == f_acm->ctrl_id) { + return acm_start_ctrl(f_acm, gadget); + } else if (intf == f_acm->data_id) { + acm_start_data(f_acm, gadget); + f_acm->connected = true; + f_acm->tx_on = true; + return 0; + } + + return -EINVAL; +} + +static int acm_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) +{ + struct usb_gadget *gadget = f->config->cdev->gadget; + struct usb_request *req = f->config->cdev->req; + u16 w_index = le16_to_cpu(ctrl->wIndex); + u16 w_value = le16_to_cpu(ctrl->wValue); + u16 w_length = le16_to_cpu(ctrl->wLength); + struct f_acm *f_acm = func_to_acm(f); + int value = -1; + + switch ((ctrl->bRequestType << 8) | ctrl->bRequest) { + case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8) + | USB_CDC_REQ_SET_LINE_CODING: + { + /* SET_LINE_CODING */ + + if (w_length != sizeof(f_acm->line_coding) || w_index != f_acm->ctrl_id) + goto invalid; + + value = w_length; + + memcpy(&f_acm->line_coding, req->buf, value); + + break; + } + case ((USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8) + | USB_CDC_REQ_GET_LINE_CODING: + { + /* GET_LINE_CODING */ + + if (w_length != sizeof(f_acm->line_coding) || w_index != f_acm->ctrl_id) + goto invalid; + + value = w_length; + + memcpy(req->buf, &f_acm->line_coding, value); + + break; + } + case ((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8) + | USB_CDC_REQ_SET_CONTROL_LINE_STATE: + { + /* SET_CONTROL_LINE_STATE */ + + if (w_index != f_acm->ctrl_id) + goto invalid; + + value = 0; + + f_acm->handshake_bits = w_value; + + break; + } + default: +invalid: + printf("invalid control req%02x.%02x v%04x i%04x l%d\n", + ctrl->bRequestType, ctrl->bRequest, w_value, w_index, + w_length); + } + + /* respond with data transfer or status phase? */ + if (value >= 0) { + req->zero = 0; + req->length = value; + usb_ep_queue(gadget->ep0, req, GFP_ATOMIC); + } + + return 0; +} + +static void acm_disable(struct usb_function *f) +{ + struct f_acm *f_acm = func_to_acm(f); + + usb_ep_disable(f_acm->ep_out); + usb_ep_disable(f_acm->ep_in); + usb_ep_disable(f_acm->ep_notify); + + if (f_acm->req_out) { + free(f_acm->req_out->buf); + usb_ep_free_request(f_acm->ep_out, f_acm->req_out); + f_acm->req_out = NULL; + } + + if (f_acm->req_in) { + free(f_acm->req_in->buf); + usb_ep_free_request(f_acm->ep_in, f_acm->req_in); + f_acm->req_in = NULL; + } +} + +/* static strings, in UTF-8 */ +static struct usb_string acm_string_defs[] = { + [0].s = "CDC Abstract Control Model (ACM)", + [1].s = "CDC ACM Data", + [2].s = "CDC Serial", + { } /* end of list */ +}; + +static struct usb_gadget_strings acm_string_table = { + .language = 0x0409, /* en-us */ + .strings = acm_string_defs, +}; + +static struct usb_gadget_strings *acm_strings[] = { + &acm_string_table, + NULL, +}; + +void __acm_tx(struct f_acm *f_acm) +{ + int len, ret; + + do { + usb_gadget_handle_interrupts(f_acm->controller_index); + + if (!(f_acm->handshake_bits & ACM_CTRL_DTR)) + break; + + if (!f_acm->tx_on) + continue; + + len = buf_pop(&f_acm->tx_buf, f_acm->req_in->buf, REQ_SIZE_MAX); + if (!len) + break; + + f_acm->req_in->length = len; + + ret = usb_ep_queue(f_acm->ep_in, f_acm->req_in, 0); + if (ret) + break; + + f_acm->tx_on = false; + + /* Do not reset the watchdog, if TX is stuck there is probably + * a real issue. + */ + } while (1); +} + +static bool acm_connected(struct stdio_dev *dev) +{ + struct f_acm *f_acm = stdio_to_acm(dev); + + /* give a chance to process udc irq */ + usb_gadget_handle_interrupts(f_acm->controller_index); + + return f_acm->connected; +} + +/* STDIO */ +static int acm_stdio_tstc(struct stdio_dev *dev) +{ + struct f_acm *f_acm = stdio_to_acm(dev); + + usb_gadget_handle_interrupts(f_acm->controller_index); + + return (f_acm->rx_buf.size > 0); +} + +static int acm_stdio_getc(struct stdio_dev *dev) +{ + struct f_acm *f_acm = stdio_to_acm(dev); + char c; + + /* Wait for a character to arrive. */ + while (!acm_stdio_tstc(dev)) + WATCHDOG_RESET(); + + buf_pop(&f_acm->rx_buf, &c, 1); + + return c; +} + +static void acm_stdio_putc(struct stdio_dev *dev, const char c) +{ + struct f_acm *f_acm = stdio_to_acm(dev); + + buf_push(&f_acm->tx_buf, &c, 1); + + if (!f_acm->connected) + return; + + __acm_tx(f_acm); +} + +static void acm_stdio_puts(struct stdio_dev *dev, const char *str) +{ + struct f_acm *f_acm = stdio_to_acm(dev); + + buf_push(&f_acm->tx_buf, str, strlen(str)); + + if (!f_acm->connected) + return; + + __acm_tx(f_acm); +} + +static int acm_stdio_start(struct stdio_dev *dev) +{ + while (!acm_connected(dev)) { + if (ctrlc()) + return -ECANCELED; + + WATCHDOG_RESET(); + } + + return 0; +} + +static int acm_stdio_stop(struct stdio_dev *dev) +{ + /* nothing to do */ + return 0; +} + +int acm_add(struct usb_configuration *c) +{ + struct stdio_dev *stdio; + struct f_acm *f_acm; + int status; + + f_acm = calloc(1, sizeof(*f_acm)); + if (!f_acm) + return -ENOMEM; + + f_acm->usb_function.name = "f_acm"; + f_acm->usb_function.bind = acm_bind; + f_acm->usb_function.unbind = acm_unbind; + f_acm->usb_function.set_alt = acm_set_alt; + f_acm->usb_function.disable = acm_disable; + f_acm->usb_function.strings = acm_strings; + f_acm->usb_function.descriptors = acm_fs_function; + f_acm->usb_function.hs_descriptors = acm_hs_function; + f_acm->usb_function.setup = acm_setup; + f_acm->controller_index = 0; + + status = usb_add_function(c, &f_acm->usb_function); + if (status) { + free(f_acm); + return status; + } + + stdio = &f_acm->stdio; + + buf_init(&f_acm->rx_buf, 2048); + buf_init(&f_acm->tx_buf, 2048); + + strcpy(stdio->name, "stdio_acm"); + stdio->flags = DEV_FLAGS_INPUT | DEV_FLAGS_OUTPUT; + stdio->tstc = acm_stdio_tstc; + stdio->getc = acm_stdio_getc; + stdio->putc = acm_stdio_putc; + stdio->puts = acm_stdio_puts; + stdio->start = acm_stdio_start; + stdio->stop = acm_stdio_stop; + stdio->priv = f_acm; + stdio->ext = 0; + + status = stdio_register(&f_acm->stdio); + if (status) { + buf_free(&f_acm->rx_buf); + buf_free(&f_acm->tx_buf); + } + + return status; +} + +DECLARE_GADGET_BIND_CALLBACK(usb_serial_acm, acm_add); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: gadget: Add CDC ACM function 2021-08-19 11:13 ` [PATCH 2/3] usb: gadget: Add CDC ACM function Loic Poulain @ 2021-09-07 10:21 ` Loic Poulain 2021-09-27 19:55 ` Pali Rohár 1 sibling, 0 replies; 10+ messages in thread From: Loic Poulain @ 2021-09-07 10:21 UTC (permalink / raw) To: Marek Vasut; +Cc: u-boot, Lukasz Majewski Hi folks, Any comments on this? On Thu, 19 Aug 2021 at 13:02, Loic Poulain <loic.poulain@linaro.org> wrote: > > Add support for CDC ACM using the new UDC and gadget API. This protocol > can be used for serial over USB data transfer and is widely supported > by various OS (GNU/Linux, MS-Windows, OSX...). The usual purpose of > such link is to access device debug console and can be useful for > products not exposing regular UART to the user. > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > drivers/usb/gadget/Kconfig | 9 + > drivers/usb/gadget/Makefile | 1 + > drivers/usb/gadget/f_acm.c | 663 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 673 insertions(+) > create mode 100644 drivers/usb/gadget/f_acm. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: gadget: Add CDC ACM function 2021-08-19 11:13 ` [PATCH 2/3] usb: gadget: Add CDC ACM function Loic Poulain 2021-09-07 10:21 ` Loic Poulain @ 2021-09-27 19:55 ` Pali Rohár 1 sibling, 0 replies; 10+ messages in thread From: Pali Rohár @ 2021-09-27 19:55 UTC (permalink / raw) To: Loic Poulain; +Cc: marex, u-boot, lukma On Thursday 19 August 2021 13:13:05 Loic Poulain wrote: > +static void acm_notify_complete(struct usb_ep *ep, struct usb_request *req) > +{ > + if (req->status) > + printf("CDC ACM notify failed (%d)\n", req->status); > + > + /* nothing to do */ > +} > + > +static void acm_tx_complete(struct usb_ep *ep, struct usb_request *req) > +{ > + static int once; /* prevent infinite recursion */ > + > + if (req->status && !once++) > + printf("CDC ACM TX failed (%d)\n", req->status); > + > + struct f_acm *f_acm = req->context; > + > + f_acm->tx_on = true; > +} > + > +static void acm_rx_complete(struct usb_ep *ep, struct usb_request *req) > +{ > + struct f_acm *f_acm = req->context; > + > + if (req->status != 0) > + printf("CDC ACM RX failed (%d)\n", req->status); I guess all these prints can cause issues like infinite loop. If stdout is set to this usb acm gadget then printf will call these functions again. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] cmd: add acmconsole command 2021-08-19 11:13 [PATCH 1/3] lib/circbuf: Make circbuf selectable symbol Loic Poulain 2021-08-19 11:13 ` [PATCH 2/3] usb: gadget: Add CDC ACM function Loic Poulain @ 2021-08-19 11:13 ` Loic Poulain 2021-09-27 20:04 ` Pali Rohár 1 sibling, 1 reply; 10+ messages in thread From: Loic Poulain @ 2021-08-19 11:13 UTC (permalink / raw) To: marex; +Cc: u-boot, lukma, Loic Poulain This command allows to start CDC ACM function and redirect console (stdin, stdout, stderr) to USB (acmconsole start). The console can then be accessed through the USB host for debugging purpose. The host can stop the session (acmconsole stop) to revert back console to serial and unregister CDC ACM USB function. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- cmd/Kconfig | 8 ++++++++ cmd/Makefile | 2 ++ cmd/acmconsole.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 cmd/acmconsole.c diff --git a/cmd/Kconfig b/cmd/Kconfig index ffef3cc..7cc9b1c 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1382,6 +1382,14 @@ config CMD_AXI Interface) busses, a on-chip interconnect specification for managing functional blocks in SoC designs, which is also often used in designs involving FPGAs (e.g. communication with IP cores in Xilinx FPGAs). + +config CMD_USB_ACM_CONSOLE + bool "ACM USB console" + select USB_FUNCTION_ACM + help + Enable the command "acmconsole" for accessing u-boot console from a + USB host through CDC ACM protocol. + endmenu diff --git a/cmd/Makefile b/cmd/Makefile index ed36694..c116091 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o obj-$(CONFIG_CMD_BOOTZ) += bootz.o obj-$(CONFIG_CMD_BOOTI) += booti.o +ob-y += acmconsole.o obj-$(CONFIG_CMD_BTRFS) += btrfs.o obj-$(CONFIG_CMD_BUTTON) += button.o obj-$(CONFIG_CMD_CACHE) += cache.o @@ -174,6 +175,7 @@ obj-$(CONFIG_CMD_FS_UUID) += fs_uuid.o obj-$(CONFIG_CMD_USB_MASS_STORAGE) += usb_mass_storage.o obj-$(CONFIG_CMD_USB_SDP) += usb_gadget_sdp.o obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o +obj-$(CONFIG_CMD_USB_ACM_CONSOLE) += acmconsole.o obj-$(CONFIG_CMD_XIMG) += ximg.o obj-$(CONFIG_CMD_YAFFS2) += yaffs2.o obj-$(CONFIG_CMD_SPL) += spl.o diff --git a/cmd/acmconsole.c b/cmd/acmconsole.c new file mode 100644 index 0000000..dac8136 --- /dev/null +++ b/cmd/acmconsole.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * acmconsole.c -- Console over USB CDC serial (ACM) function gadget function + * + * Copyright (c) 2021, Linaro Ltd <loic.poulain@linaro.org> + */ + +#include <common.h> +#include <console.h> +#include <dfu.h> +#include <g_dnl.h> +#include <stdio_dev.h> +#include <usb.h> +#include <watchdog.h> + +int do_usb_acmconsole(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + int ret; + + if (argc != 2) + return CMD_RET_USAGE; + + if (!strcmp(argv[1], "start")) { + ret = g_dnl_register("usb_serial_acm"); + if (ret) + return ret; + + /* ACM function creates a stdio device name 'stdio_acm' */ + console_assign(stdin, "stdio_acm"); + console_assign(stdout, "stdio_acm"); + console_assign(stderr, "stdio_acm"); + } else if (!strcmp(argv[1], "stop")) { + /* default to serial (TODO: restore initial devices) */ + console_assign(stdin, "serial"); + console_assign(stdout, "serial"); + console_assign(stderr, "serial"); + + g_dnl_unregister(); + g_dnl_clear_detach(); + } else { + return CMD_RET_USAGE; + } + + return 0; +} + +U_BOOT_CMD(acmconsole, CONFIG_SYS_MAXARGS, 1, do_usb_acmconsole, + "Console over USB CDC ACM", + "start - start console over USB CDC ACM gadget function\n" \ + "acmconsole stop - stop USB console"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] cmd: add acmconsole command 2021-08-19 11:13 ` [PATCH 3/3] cmd: add acmconsole command Loic Poulain @ 2021-09-27 20:04 ` Pali Rohár 2021-11-16 19:05 ` Loic Poulain 0 siblings, 1 reply; 10+ messages in thread From: Pali Rohár @ 2021-09-27 20:04 UTC (permalink / raw) To: Loic Poulain; +Cc: marex, u-boot, lukma On Thursday 19 August 2021 13:13:06 Loic Poulain wrote: > This command allows to start CDC ACM function and redirect console > (stdin, stdout, stderr) to USB (acmconsole start). The console can > then be accessed through the USB host for debugging purpose. The > host can stop the session (acmconsole stop) to revert back console > to serial and unregister CDC ACM USB function. Hello! Could you rather implement this CDC ACM gadget in a way that calling 'setenv stdout cdc_acm' (or 'setenv stdout usbtty') would do this console redirect? Like it is supported for current usbtty or vga output code. Then this acmconsole command would not be needed at all. Nokia N900 uses usbtty and has in its default env defined: "stdin=usbtty,serial,vga\0" "stdout=usbtty,serial,vga\0" "stderr=usbtty,serial,vga\0" With this configuration, U-Boot console is automatically activated on usbtty, serial console and also on vga display. And no additional command is needed. U-Boot console works on all "outputs" at the same time. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] cmd: add acmconsole command 2021-09-27 20:04 ` Pali Rohár @ 2021-11-16 19:05 ` Loic Poulain 2021-11-16 19:36 ` Pali Rohár 0 siblings, 1 reply; 10+ messages in thread From: Loic Poulain @ 2021-11-16 19:05 UTC (permalink / raw) To: Pali Rohár, marex; +Cc: u-boot, lukma Hi Pali, Sorry for the late reply, On Mon, 27 Sept 2021 at 22:04, Pali Rohár <pali@kernel.org> wrote: > > On Thursday 19 August 2021 13:13:06 Loic Poulain wrote: > > This command allows to start CDC ACM function and redirect console > > (stdin, stdout, stderr) to USB (acmconsole start). The console can > > then be accessed through the USB host for debugging purpose. The > > host can stop the session (acmconsole stop) to revert back console > > to serial and unregister CDC ACM USB function. > > Hello! > > Could you rather implement this CDC ACM gadget in a way that calling > 'setenv stdout cdc_acm' (or 'setenv stdout usbtty') would do this > console redirect? Like it is supported for current usbtty or vga output > code. > > Then this acmconsole command would not be needed at all. Yes, that would be good, but AFAIR I restricted this cdc_acm usage to this command because we can't have fine grained control like e.g. using cdc_acm as stdout only, when used, it should necessarily be STDIN (at least). The reason is because of the single-tasking nature of u-boot, and the fact that we need to poll the USB controller for events via the 'usb_gadget_handle_interrupts()' function. In our case we simply do that in the getc() function, which is only called for input consoles in the u-boot mainloop. An alternative would be to have a u-boot API to register generic callbacks to be executed in the mainloop, but this is probably a different thread. Also we could possibly live with this 'bug' and simply accept that stdout-only configuration will be broken. Regards, Loic ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] cmd: add acmconsole command 2021-11-16 19:05 ` Loic Poulain @ 2021-11-16 19:36 ` Pali Rohár 2021-11-22 10:33 ` Loic Poulain 0 siblings, 1 reply; 10+ messages in thread From: Pali Rohár @ 2021-11-16 19:36 UTC (permalink / raw) To: Loic Poulain; +Cc: marex, u-boot, lukma Hello! On Tuesday 16 November 2021 20:05:07 Loic Poulain wrote: > Hi Pali, > > Sorry for the late reply, > > On Mon, 27 Sept 2021 at 22:04, Pali Rohár <pali@kernel.org> wrote: > > > > On Thursday 19 August 2021 13:13:06 Loic Poulain wrote: > > > This command allows to start CDC ACM function and redirect console > > > (stdin, stdout, stderr) to USB (acmconsole start). The console can > > > then be accessed through the USB host for debugging purpose. The > > > host can stop the session (acmconsole stop) to revert back console > > > to serial and unregister CDC ACM USB function. > > > > Hello! > > > > Could you rather implement this CDC ACM gadget in a way that calling > > 'setenv stdout cdc_acm' (or 'setenv stdout usbtty') would do this > > console redirect? Like it is supported for current usbtty or vga output > > code. > > > > Then this acmconsole command would not be needed at all. > > Yes, that would be good, but AFAIR I restricted this cdc_acm usage to > this command because we can't have fine grained control like e.g. > using cdc_acm as stdout only, when used, it should necessarily be > STDIN (at least). The reason is because of the single-tasking nature > of u-boot, and the fact that we need to poll the USB controller for > events via the 'usb_gadget_handle_interrupts()' function. There was already discussion that custom commands for activating drivers are against driver model design. See: https://lore.kernel.org/u-boot/20211026151742.42b0fcfa@thinkpad/ So I think that there should not be any driver specific command which just activates device (in this case console). > In our case > we simply do that in the getc() function, which is only called for > input consoles in the u-boot mainloop. This looks like a hack in the current implementation which seems that also misuse driver model design. At least this should be documented as this is limitation of functionality. > An alternative would be to have a u-boot API to register generic > callbacks to be executed in the mainloop, but this is probably a > different thread. I guess that this should be discussed, so some optimal solution for issue is chosen. Otherwise we will have there second implementation of "usbtty", which even would not provide all functionality as original "usbtty" code. > Also we could possibly live with this 'bug' and simply accept that > stdout-only configuration will be broken. > > Regards, > Loic ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] cmd: add acmconsole command 2021-11-16 19:36 ` Pali Rohár @ 2021-11-22 10:33 ` Loic Poulain 2021-11-22 11:54 ` Pali Rohár 0 siblings, 1 reply; 10+ messages in thread From: Loic Poulain @ 2021-11-22 10:33 UTC (permalink / raw) To: Pali Rohár; +Cc: marex, u-boot, lukma Hi Pali, On Tue, 16 Nov 2021 at 20:36, Pali Rohár <pali@kernel.org> wrote: > > Hello! > > On Tuesday 16 November 2021 20:05:07 Loic Poulain wrote: > > Hi Pali, > > > > Sorry for the late reply, > > > > On Mon, 27 Sept 2021 at 22:04, Pali Rohár <pali@kernel.org> wrote: > > > > > > On Thursday 19 August 2021 13:13:06 Loic Poulain wrote: > > > > This command allows to start CDC ACM function and redirect console > > > > (stdin, stdout, stderr) to USB (acmconsole start). The console can > > > > then be accessed through the USB host for debugging purpose. The > > > > host can stop the session (acmconsole stop) to revert back console > > > > to serial and unregister CDC ACM USB function. > > > > > > Hello! > > > > > > Could you rather implement this CDC ACM gadget in a way that calling > > > 'setenv stdout cdc_acm' (or 'setenv stdout usbtty') would do this > > > console redirect? Like it is supported for current usbtty or vga output > > > code. > > > > > > Then this acmconsole command would not be needed at all. > > > > Yes, that would be good, but AFAIR I restricted this cdc_acm usage to > > this command because we can't have fine grained control like e.g. > > using cdc_acm as stdout only, when used, it should necessarily be > > STDIN (at least). The reason is because of the single-tasking nature > > of u-boot, and the fact that we need to poll the USB controller for > > events via the 'usb_gadget_handle_interrupts()' function. > > There was already discussion that custom commands for activating drivers > are against driver model design. See: > https://lore.kernel.org/u-boot/20211026151742.42b0fcfa@thinkpad/ > > So I think that there should not be any driver specific command which > just activates device (in this case console). That's a bit different here since it's not a standard device/peripheral driver but a USB function, which is something the 'user' can decide to enable or not, all the other USB functions have dedicated commands AFAIK. It's not clear otherwise at which place we should register the acm_console? should it be unconditionally registered at init? Regards, Loic ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] cmd: add acmconsole command 2021-11-22 10:33 ` Loic Poulain @ 2021-11-22 11:54 ` Pali Rohár 0 siblings, 0 replies; 10+ messages in thread From: Pali Rohár @ 2021-11-22 11:54 UTC (permalink / raw) To: Loic Poulain; +Cc: marex, u-boot, lukma On Monday 22 November 2021 11:33:15 Loic Poulain wrote: > Hi Pali, > > On Tue, 16 Nov 2021 at 20:36, Pali Rohár <pali@kernel.org> wrote: > > > > Hello! > > > > On Tuesday 16 November 2021 20:05:07 Loic Poulain wrote: > > > Hi Pali, > > > > > > Sorry for the late reply, > > > > > > On Mon, 27 Sept 2021 at 22:04, Pali Rohár <pali@kernel.org> wrote: > > > > > > > > On Thursday 19 August 2021 13:13:06 Loic Poulain wrote: > > > > > This command allows to start CDC ACM function and redirect console > > > > > (stdin, stdout, stderr) to USB (acmconsole start). The console can > > > > > then be accessed through the USB host for debugging purpose. The > > > > > host can stop the session (acmconsole stop) to revert back console > > > > > to serial and unregister CDC ACM USB function. > > > > > > > > Hello! > > > > > > > > Could you rather implement this CDC ACM gadget in a way that calling > > > > 'setenv stdout cdc_acm' (or 'setenv stdout usbtty') would do this > > > > console redirect? Like it is supported for current usbtty or vga output > > > > code. > > > > > > > > Then this acmconsole command would not be needed at all. > > > > > > Yes, that would be good, but AFAIR I restricted this cdc_acm usage to > > > this command because we can't have fine grained control like e.g. > > > using cdc_acm as stdout only, when used, it should necessarily be > > > STDIN (at least). The reason is because of the single-tasking nature > > > of u-boot, and the fact that we need to poll the USB controller for > > > events via the 'usb_gadget_handle_interrupts()' function. > > > > There was already discussion that custom commands for activating drivers > > are against driver model design. See: > > https://lore.kernel.org/u-boot/20211026151742.42b0fcfa@thinkpad/ > > > > So I think that there should not be any driver specific command which > > just activates device (in this case console). > > That's a bit different here since it's not a standard > device/peripheral driver but a USB function, which is something the > 'user' can decide to enable or not, all the other USB functions have > dedicated commands AFAIK. It's not clear otherwise at which place we > should register the acm_console? should it be unconditionally > registered at init? I really do not know. But for me it could make sense that activation happens when calling standard "setenv std* cdc_acm" commands... ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-22 11:54 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-19 11:13 [PATCH 1/3] lib/circbuf: Make circbuf selectable symbol Loic Poulain 2021-08-19 11:13 ` [PATCH 2/3] usb: gadget: Add CDC ACM function Loic Poulain 2021-09-07 10:21 ` Loic Poulain 2021-09-27 19:55 ` Pali Rohár 2021-08-19 11:13 ` [PATCH 3/3] cmd: add acmconsole command Loic Poulain 2021-09-27 20:04 ` Pali Rohár 2021-11-16 19:05 ` Loic Poulain 2021-11-16 19:36 ` Pali Rohár 2021-11-22 10:33 ` Loic Poulain 2021-11-22 11:54 ` Pali Rohár
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.