2023-09-14 04:56:24

by Max Filippov

[permalink] [raw]
Subject: [PATCH 4/4] drivers/tty/serial: add ESP32S3 ACM device driver

Add driver for the ACM controller of the Espressif ESP32S3 Soc.
Hardware specification is available at the following URL:

https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
(Chapter 33 USB Serial/JTAG Controller)

Signed-off-by: Max Filippov <[email protected]>
---
drivers/tty/serial/Kconfig | 14 +
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/esp32_acm.c | 473 +++++++++++++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
4 files changed, 491 insertions(+)
create mode 100644 drivers/tty/serial/esp32_acm.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index d9ca6b268f01..85807db8f7ce 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1591,6 +1591,20 @@ config SERIAL_ESP32
earlycon=esp32s3uart,mmio32,0x60000000,115200n8,40000000
earlycon=esp32uart,mmio32,0x3ff40000,115200n8

+config SERIAL_ESP32_ACM
+ tristate "Espressif ESP32 USB ACM support"
+ depends on XTENSA_PLATFORM_ESP32 || (COMPILE_TEST && OF)
+ select SERIAL_CORE
+ select SERIAL_CORE_CONSOLE
+ select SERIAL_EARLYCON
+ help
+ Driver for the CDC ACM controllers of the Espressif ESP32S3 SoCs
+ that share separate USB controller with the JTAG adapter.
+ The device name used for this controller is ttyACM.
+ When earlycon option is enabled the following kernel command line
+ snippet may be used:
+ earlycon=esp32s3acm,mmio32,0x60038000
+
endmenu

config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 7b73137df7f3..970a292ca418 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_SIFIVE) += sifive.o
obj-$(CONFIG_SERIAL_LITEUART) += liteuart.o
obj-$(CONFIG_SERIAL_SUNPLUS) += sunplus-uart.o
obj-$(CONFIG_SERIAL_ESP32) += esp32_uart.o
+obj-$(CONFIG_SERIAL_ESP32_ACM) += esp32_acm.o

# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/esp32_acm.c b/drivers/tty/serial/esp32_acm.c
new file mode 100644
index 000000000000..f178e6af93f3
--- /dev/null
+++ b/drivers/tty/serial/esp32_acm.c
@@ -0,0 +1,473 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/console.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/serial_core.h>
+#include <linux/slab.h>
+#include <linux/tty_flip.h>
+#include <linux/delay.h>
+#include <asm/serial.h>
+
+#define DRIVER_NAME "esp32s3-acm"
+#define DEV_NAME "ttyACM"
+#define UART_NR 4
+
+#define ESP32S3_ACM_TX_FIFO_SIZE 64
+
+#define USB_SERIAL_JTAG_EP1_REG 0x00
+#define USB_SERIAL_JTAG_EP1_CONF_REG 0x04
+#define USB_SERIAL_JTAG_WR_DONE_MASK 0x00000001
+#define USB_SERIAL_JTAG_SERIAL_IN_EP_DATA_FREE_MASK 0x00000002
+#define USB_SERIAL_JTAG_SERIAL_OUT_EP_DATA_AVAIL_MASK 0x00000008
+#define USB_SERIAL_JTAG_INT_ST_REG 0x0c
+#define USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ST_MASK 0x00000004
+#define USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ST_MASK 0x00000008
+#define USB_SERIAL_JTAG_INT_ENA_REG 0x10
+#define USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK 0x00000004
+#define USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK 0x00000008
+#define USB_SERIAL_JTAG_INT_CLR_REG 0x14
+#define USB_SERIAL_JTAG_IN_EP1_ST_REG 0x2c
+#define USB_SERIAL_JTAG_IN_EP1_WR_ADDR_MASK 0x000001fc
+#define USB_SERIAL_JTAG_IN_EP1_WR_ADDR_SHIFT 2
+#define USB_SERIAL_JTAG_OUT_EP1_ST_REG 0x3c
+#define USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_MASK 0x007f0000
+#define USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_SHIFT 16
+
+static const struct of_device_id esp32s3_acm_dt_ids[] = {
+ {
+ .compatible = "esp,esp32s3-acm",
+ }, { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, esp32s3_acm_dt_ids);
+
+static struct uart_port *esp32s3_acm_ports[UART_NR];
+
+static void esp32s3_acm_write(struct uart_port *port, unsigned long reg, u32 v)
+{
+ writel(v, port->membase + reg);
+}
+
+static u32 esp32s3_acm_read(struct uart_port *port, unsigned long reg)
+{
+ return readl(port->membase + reg);
+}
+
+static u32 esp32s3_acm_tx_fifo_free(struct uart_port *port)
+{
+ return esp32s3_acm_read(port, USB_SERIAL_JTAG_EP1_CONF_REG) &
+ USB_SERIAL_JTAG_SERIAL_IN_EP_DATA_FREE_MASK;
+}
+
+static u32 esp32s3_acm_tx_fifo_cnt(struct uart_port *port)
+{
+ u32 status = esp32s3_acm_read(port, USB_SERIAL_JTAG_IN_EP1_ST_REG);
+
+ return (status & USB_SERIAL_JTAG_IN_EP1_WR_ADDR_MASK) >>
+ USB_SERIAL_JTAG_IN_EP1_WR_ADDR_SHIFT;
+}
+
+static u32 esp32s3_acm_rx_fifo_cnt(struct uart_port *port)
+{
+ u32 status = esp32s3_acm_read(port, USB_SERIAL_JTAG_OUT_EP1_ST_REG);
+
+ return (status & USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_MASK) >>
+ USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_SHIFT;
+}
+
+/* return TIOCSER_TEMT when transmitter is not busy */
+static unsigned int esp32s3_acm_tx_empty(struct uart_port *port)
+{
+ return esp32s3_acm_tx_fifo_cnt(port) == 0 ? TIOCSER_TEMT : 0;
+}
+
+static void esp32s3_acm_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+}
+
+static unsigned int esp32s3_acm_get_mctrl(struct uart_port *port)
+{
+ return TIOCM_CAR;
+}
+
+static void esp32s3_acm_stop_tx(struct uart_port *port)
+{
+ u32 int_ena;
+
+ int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
+ int_ena & ~USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK);
+}
+
+static void esp32s3_acm_rxint(struct uart_port *port)
+{
+ struct tty_port *tty_port = &port->state->port;
+ u32 rx_fifo_cnt = esp32s3_acm_rx_fifo_cnt(port);
+ unsigned long flags;
+ u32 i;
+
+ if (!rx_fifo_cnt)
+ return;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ for (i = 0; i < rx_fifo_cnt; ++i) {
+ u32 rx = esp32s3_acm_read(port, USB_SERIAL_JTAG_EP1_REG);
+
+ ++port->icount.rx;
+ tty_insert_flip_char(tty_port, rx, TTY_NORMAL);
+ }
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ tty_flip_buffer_push(tty_port);
+}
+
+static void esp32s3_acm_push(struct uart_port *port)
+{
+ if (esp32s3_acm_tx_fifo_free(port))
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_CONF_REG,
+ USB_SERIAL_JTAG_WR_DONE_MASK);
+}
+
+static void esp32s3_acm_put_char(struct uart_port *port, unsigned char c)
+{
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_REG, c);
+}
+
+static void esp32s3_acm_put_char_sync(struct uart_port *port, unsigned char c)
+{
+ while (!esp32s3_acm_tx_fifo_free(port))
+ cpu_relax();
+ esp32s3_acm_put_char(port, c);
+ esp32s3_acm_push(port);
+}
+
+static void esp32s3_acm_transmit_buffer(struct uart_port *port)
+{
+ struct circ_buf *xmit = &port->state->xmit;
+ u32 tx_fifo_used = esp32s3_acm_tx_fifo_cnt(port);
+
+ if (esp32s3_acm_tx_fifo_free(port)) {
+ while (!uart_circ_empty(xmit) && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE) {
+ esp32s3_acm_put_char(port, xmit->buf[xmit->tail]);
+ xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+ port->icount.tx++;
+ ++tx_fifo_used;
+ }
+ }
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ if (uart_circ_empty(xmit)) {
+ esp32s3_acm_stop_tx(port);
+ } else {
+ u32 int_ena;
+
+ int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
+ int_ena | USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK);
+ }
+
+ if (tx_fifo_used > 0 && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE)
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_CONF_REG,
+ USB_SERIAL_JTAG_WR_DONE_MASK);
+}
+
+static void esp32s3_acm_txint(struct uart_port *port)
+{
+ esp32s3_acm_transmit_buffer(port);
+}
+
+static irqreturn_t esp32s3_acm_int(int irq, void *dev_id)
+{
+ struct uart_port *port = dev_id;
+ u32 status;
+
+ status = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ST_REG);
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_CLR_REG, status);
+
+ if (status & USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ST_MASK)
+ esp32s3_acm_rxint(port);
+ if (status & USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ST_MASK)
+ esp32s3_acm_txint(port);
+
+ return IRQ_HANDLED;
+}
+
+static void esp32s3_acm_start_tx(struct uart_port *port)
+{
+ esp32s3_acm_transmit_buffer(port);
+}
+
+static void esp32s3_acm_stop_rx(struct uart_port *port)
+{
+ u32 int_ena;
+
+ int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
+ int_ena & ~USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK);
+}
+
+static int esp32s3_acm_startup(struct uart_port *port)
+{
+ int ret = 0;
+
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
+ USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK);
+ ret = devm_request_irq(port->dev, port->irq, esp32s3_acm_int, 0,
+ DRIVER_NAME, port);
+ return ret;
+}
+
+static void esp32s3_acm_shutdown(struct uart_port *port)
+{
+ esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG, 0);
+ devm_free_irq(port->dev, port->irq, port);
+}
+
+static void esp32s3_acm_set_termios(struct uart_port *port,
+ struct ktermios *termios,
+ const struct ktermios *old)
+{
+}
+
+static const char *esp32s3_acm_type(struct uart_port *port)
+{
+ return "ESP32S3 ACM";
+}
+
+/* configure/auto-configure the port */
+static void esp32s3_acm_config_port(struct uart_port *port, int flags)
+{
+ if (flags & UART_CONFIG_TYPE)
+ port->type = PORT_ESP32ACM;
+}
+
+#ifdef CONFIG_CONSOLE_POLL
+static void esp32s3_acm_poll_put_char(struct uart_port *port, unsigned char c)
+{
+ esp32s3_acm_put_char_sync(port, c);
+}
+
+static int esp32s3_acm_poll_get_char(struct uart_port *port)
+{
+ if (esp32s3_acm_rx_fifo_cnt(port))
+ return esp32s3_acm_read(port, USB_SERIAL_JTAG_EP1_REG);
+ else
+ return NO_POLL_CHAR;
+
+}
+#endif
+
+static const struct uart_ops esp32s3_acm_pops = {
+ .tx_empty = esp32s3_acm_tx_empty,
+ .set_mctrl = esp32s3_acm_set_mctrl,
+ .get_mctrl = esp32s3_acm_get_mctrl,
+ .stop_tx = esp32s3_acm_stop_tx,
+ .start_tx = esp32s3_acm_start_tx,
+ .stop_rx = esp32s3_acm_stop_rx,
+ .startup = esp32s3_acm_startup,
+ .shutdown = esp32s3_acm_shutdown,
+ .set_termios = esp32s3_acm_set_termios,
+ .type = esp32s3_acm_type,
+ .config_port = esp32s3_acm_config_port,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_put_char = esp32s3_acm_poll_put_char,
+ .poll_get_char = esp32s3_acm_poll_get_char,
+#endif
+};
+
+static void esp32s3_acm_console_putchar(struct uart_port *port, unsigned char c)
+{
+ esp32s3_acm_put_char_sync(port, c);
+}
+
+static void esp32s3_acm_string_write(struct uart_port *port, const char *s,
+ unsigned int count)
+{
+ uart_console_write(port, s, count, esp32s3_acm_console_putchar);
+}
+
+static void
+esp32s3_acm_console_write(struct console *co, const char *s, unsigned int count)
+{
+ struct uart_port *port = esp32s3_acm_ports[co->index];
+ unsigned long flags;
+ int locked = 1;
+
+ if (port->sysrq)
+ locked = 0;
+ else if (oops_in_progress)
+ locked = spin_trylock_irqsave(&port->lock, flags);
+ else
+ spin_lock_irqsave(&port->lock, flags);
+
+ esp32s3_acm_string_write(port, s, count);
+
+ if (locked)
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static struct uart_driver esp32s3_acm_reg;
+static struct console esp32s3_acm_console = {
+ .name = DEV_NAME,
+ .write = esp32s3_acm_console_write,
+ .device = uart_console_device,
+ .flags = CON_PRINTBUFFER,
+ .index = -1,
+ .data = &esp32s3_acm_reg,
+};
+
+static void esp32s3_acm_earlycon_putchar(struct uart_port *port, unsigned char c)
+{
+ esp32s3_acm_put_char_sync(port, c);
+}
+
+static void esp32s3_acm_earlycon_write(struct console *con, const char *s,
+ unsigned int n)
+{
+ struct earlycon_device *dev = con->data;
+
+ uart_console_write(&dev->port, s, n, esp32s3_acm_earlycon_putchar);
+}
+
+#ifdef CONFIG_CONSOLE_POLL
+static int esp32s3_acm_earlycon_read(struct console *con, char *s, unsigned int n)
+{
+ struct earlycon_device *dev = con->data;
+ int num_read = 0;
+
+ while (num_read < n) {
+ int c = esp32s3_acm_poll_get_char(&dev->port);
+
+ if (c == NO_POLL_CHAR)
+ break;
+ s[num_read++] = c;
+ }
+ return num_read;
+}
+#endif
+
+static int __init esp32s3_acm_early_console_setup(struct earlycon_device *device,
+ const char *options)
+{
+ if (!device->port.membase)
+ return -ENODEV;
+
+ device->con->write = esp32s3_acm_earlycon_write;
+#ifdef CONFIG_CONSOLE_POLL
+ device->con->read = esp32s3_acm_earlycon_read;
+#endif
+ return 0;
+}
+
+OF_EARLYCON_DECLARE(esp32s3acm, "esp,esp32s3-acm",
+ esp32s3_acm_early_console_setup);
+
+static struct uart_driver esp32s3_acm_reg = {
+ .owner = THIS_MODULE,
+ .driver_name = DRIVER_NAME,
+ .dev_name = DEV_NAME,
+ .nr = ARRAY_SIZE(esp32s3_acm_ports),
+ .cons = &esp32s3_acm_console,
+};
+
+static int esp32s3_acm_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct uart_port *port;
+ struct resource *res;
+ int ret;
+
+ port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
+ return -ENOMEM;
+
+ ret = of_alias_get_id(np, "serial");
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
+ return ret;
+ }
+ if (ret >= UART_NR) {
+ dev_err(&pdev->dev, "driver limited to %d serial ports\n",
+ UART_NR);
+ return -ENOMEM;
+ }
+
+ port->line = ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ port->mapbase = res->start;
+ port->membase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(port->membase))
+ return PTR_ERR(port->membase);
+
+ port->dev = &pdev->dev;
+ port->type = PORT_ESP32ACM;
+ port->iotype = UPIO_MEM;
+ port->irq = platform_get_irq(pdev, 0);
+ port->ops = &esp32s3_acm_pops;
+ port->flags = UPF_BOOT_AUTOCONF;
+ port->has_sysrq = 1;
+ port->fifosize = ESP32S3_ACM_TX_FIFO_SIZE;
+
+ esp32s3_acm_ports[port->line] = port;
+
+ platform_set_drvdata(pdev, port);
+
+ ret = uart_add_one_port(&esp32s3_acm_reg, port);
+ return ret;
+}
+
+static int esp32s3_acm_remove(struct platform_device *pdev)
+{
+ struct uart_port *port = platform_get_drvdata(pdev);
+
+ uart_remove_one_port(&esp32s3_acm_reg, port);
+ return 0;
+}
+
+
+static struct platform_driver esp32s3_acm_driver = {
+ .probe = esp32s3_acm_probe,
+ .remove = esp32s3_acm_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .of_match_table = esp32s3_acm_dt_ids,
+ },
+};
+
+static int __init esp32s3_acm_init(void)
+{
+ int ret;
+
+ ret = uart_register_driver(&esp32s3_acm_reg);
+ if (ret)
+ return ret;
+
+ ret = platform_driver_register(&esp32s3_acm_driver);
+ if (ret)
+ uart_unregister_driver(&esp32s3_acm_reg);
+
+ return ret;
+}
+
+static void __exit esp32s3_acm_exit(void)
+{
+ platform_driver_unregister(&esp32s3_acm_driver);
+ uart_unregister_driver(&esp32s3_acm_reg);
+}
+
+module_init(esp32s3_acm_init);
+module_exit(esp32s3_acm_exit);
+
+MODULE_DESCRIPTION("Espressif ESP32S3 USB ACM driver.");
+MODULE_AUTHOR("Max Filippov <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index ff076d6be159..1045bf096837 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -248,4 +248,7 @@
/* Espressif ESP32 UART */
#define PORT_ESP32UART 124

+/* Espressif ESP32 ACM */
+#define PORT_ESP32ACM 125
+
#endif /* _UAPILINUX_SERIAL_CORE_H */
--
2.30.2


2023-09-14 07:25:10

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 4/4] drivers/tty/serial: add ESP32S3 ACM device driver

On 13. 09. 23, 23:14, Max Filippov wrote:
> Add driver for the ACM controller of the Espressif ESP32S3 Soc.
> Hardware specification is available at the following URL:
>
> https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
> (Chapter 33 USB Serial/JTAG Controller)
...

> +static void esp32s3_acm_put_char_sync(struct uart_port *port, unsigned char c)
> +{
> + while (!esp32s3_acm_tx_fifo_free(port))
> + cpu_relax();

No limits...

> + esp32s3_acm_put_char(port, c);
> + esp32s3_acm_push(port);
> +}
> +
> +static void esp32s3_acm_transmit_buffer(struct uart_port *port)
> +{

tx helper.

> + struct circ_buf *xmit = &port->state->xmit;
> + u32 tx_fifo_used = esp32s3_acm_tx_fifo_cnt(port);
> +
> + if (esp32s3_acm_tx_fifo_free(port)) {
> + while (!uart_circ_empty(xmit) && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE) {
> + esp32s3_acm_put_char(port, xmit->buf[xmit->tail]);
> + xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> + port->icount.tx++;
> + ++tx_fifo_used;
> + }
> + }
> +
> + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> + uart_write_wakeup(port);
> +
> + if (uart_circ_empty(xmit)) {
> + esp32s3_acm_stop_tx(port);
> + } else {
> + u32 int_ena;
> +
> + int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> + int_ena | USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK);
> + }
> +
> + if (tx_fifo_used > 0 && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE)
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_CONF_REG,
> + USB_SERIAL_JTAG_WR_DONE_MASK);
> +}


> +static irqreturn_t esp32s3_acm_int(int irq, void *dev_id)
> +{
> + struct uart_port *port = dev_id;
> + u32 status;
> +
> + status = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ST_REG);
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_CLR_REG, status);
> +
> + if (status & USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ST_MASK)
> + esp32s3_acm_rxint(port);
> + if (status & USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ST_MASK)
> + esp32s3_acm_txint(port);
> +
> + return IRQ_HANDLED;

IRQ_STATUS()

> +}

> +static int esp32s3_acm_startup(struct uart_port *port)
> +{
> + int ret = 0;
> +
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> + USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK);
> + ret = devm_request_irq(port->dev, port->irq, esp32s3_acm_int, 0,
> + DRIVER_NAME, port);
> + return ret;

No need for ret. Or not, you don't handle the failure properly again
(disable ints). And the order appears to be switched too.


> +static void
> +esp32s3_acm_console_write(struct console *co, const char *s, unsigned int count)
> +{
> + struct uart_port *port = esp32s3_acm_ports[co->index];
> + unsigned long flags;
> + int locked = 1;

bool? ANd in the otrher driver too.

> +
> + if (port->sysrq)
> + locked = 0;
> + else if (oops_in_progress)
> + locked = spin_trylock_irqsave(&port->lock, flags);
> + else
> + spin_lock_irqsave(&port->lock, flags);
> +
> + esp32s3_acm_string_write(port, s, count);
> +
> + if (locked)
> + spin_unlock_irqrestore(&port->lock, flags);
> +}


> +#ifdef CONFIG_CONSOLE_POLL
> +static int esp32s3_acm_earlycon_read(struct console *con, char *s, unsigned int n)
> +{
> + struct earlycon_device *dev = con->data;
> + int num_read = 0;

num looks like should be unsigned?

> +
> + while (num_read < n) {
> + int c = esp32s3_acm_poll_get_char(&dev->port);
> +
> + if (c == NO_POLL_CHAR)
> + break;
> + s[num_read++] = c;
> + }
> + return num_read;
> +}
> +#endif


> +static int esp32s3_acm_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct uart_port *port;
> + struct resource *res;
> + int ret;
> +
> + port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + ret = of_alias_get_id(np, "serial");
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> + return ret;
> + }
> + if (ret >= UART_NR) {
> + dev_err(&pdev->dev, "driver limited to %d serial ports\n",
> + UART_NR);
> + return -ENOMEM;
> + }
> +
> + port->line = ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + port->mapbase = res->start;
> + port->membase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(port->membase))
> + return PTR_ERR(port->membase);
> +
> + port->dev = &pdev->dev;
> + port->type = PORT_ESP32ACM;
> + port->iotype = UPIO_MEM;
> + port->irq = platform_get_irq(pdev, 0);
> + port->ops = &esp32s3_acm_pops;
> + port->flags = UPF_BOOT_AUTOCONF;
> + port->has_sysrq = 1;
> + port->fifosize = ESP32S3_ACM_TX_FIFO_SIZE;
> +
> + esp32s3_acm_ports[port->line] = port;
> +
> + platform_set_drvdata(pdev, port);
> +
> + ret = uart_add_one_port(&esp32s3_acm_reg, port);
> + return ret;

return imm.

> +}

regards,
--
js
suse labs

2023-09-14 21:45:47

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 4/4] drivers/tty/serial: add ESP32S3 ACM device driver

On Wed, 13 Sep 2023, Max Filippov wrote:

> Add driver for the ACM controller of the Espressif ESP32S3 Soc.
> Hardware specification is available at the following URL:
>
> https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
> (Chapter 33 USB Serial/JTAG Controller)
>
> Signed-off-by: Max Filippov <[email protected]>
> ---
> drivers/tty/serial/Kconfig | 14 +
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/esp32_acm.c | 473 +++++++++++++++++++++++++++++++
> include/uapi/linux/serial_core.h | 3 +
> 4 files changed, 491 insertions(+)
> create mode 100644 drivers/tty/serial/esp32_acm.c

Please go through this with the comments against the other patch in mind,
I don't want to spend my time on reviewing it yet just to mark all the
similar issues.

--
i.

>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index d9ca6b268f01..85807db8f7ce 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1591,6 +1591,20 @@ config SERIAL_ESP32
> earlycon=esp32s3uart,mmio32,0x60000000,115200n8,40000000
> earlycon=esp32uart,mmio32,0x3ff40000,115200n8
>
> +config SERIAL_ESP32_ACM
> + tristate "Espressif ESP32 USB ACM support"
> + depends on XTENSA_PLATFORM_ESP32 || (COMPILE_TEST && OF)
> + select SERIAL_CORE
> + select SERIAL_CORE_CONSOLE
> + select SERIAL_EARLYCON
> + help
> + Driver for the CDC ACM controllers of the Espressif ESP32S3 SoCs
> + that share separate USB controller with the JTAG adapter.
> + The device name used for this controller is ttyACM.
> + When earlycon option is enabled the following kernel command line
> + snippet may be used:
> + earlycon=esp32s3acm,mmio32,0x60038000
> +
> endmenu
>
> config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 7b73137df7f3..970a292ca418 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_SIFIVE) += sifive.o
> obj-$(CONFIG_SERIAL_LITEUART) += liteuart.o
> obj-$(CONFIG_SERIAL_SUNPLUS) += sunplus-uart.o
> obj-$(CONFIG_SERIAL_ESP32) += esp32_uart.o
> +obj-$(CONFIG_SERIAL_ESP32_ACM) += esp32_acm.o
>
> # GPIOLIB helpers for modem control lines
> obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/esp32_acm.c b/drivers/tty/serial/esp32_acm.c
> new file mode 100644
> index 000000000000..f178e6af93f3
> --- /dev/null
> +++ b/drivers/tty/serial/esp32_acm.c
> @@ -0,0 +1,473 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/console.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/serial_core.h>
> +#include <linux/slab.h>
> +#include <linux/tty_flip.h>
> +#include <linux/delay.h>
> +#include <asm/serial.h>
> +
> +#define DRIVER_NAME "esp32s3-acm"
> +#define DEV_NAME "ttyACM"
> +#define UART_NR 4
> +
> +#define ESP32S3_ACM_TX_FIFO_SIZE 64
> +
> +#define USB_SERIAL_JTAG_EP1_REG 0x00
> +#define USB_SERIAL_JTAG_EP1_CONF_REG 0x04
> +#define USB_SERIAL_JTAG_WR_DONE_MASK 0x00000001
> +#define USB_SERIAL_JTAG_SERIAL_IN_EP_DATA_FREE_MASK 0x00000002
> +#define USB_SERIAL_JTAG_SERIAL_OUT_EP_DATA_AVAIL_MASK 0x00000008
> +#define USB_SERIAL_JTAG_INT_ST_REG 0x0c
> +#define USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ST_MASK 0x00000004
> +#define USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ST_MASK 0x00000008
> +#define USB_SERIAL_JTAG_INT_ENA_REG 0x10
> +#define USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK 0x00000004
> +#define USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK 0x00000008
> +#define USB_SERIAL_JTAG_INT_CLR_REG 0x14
> +#define USB_SERIAL_JTAG_IN_EP1_ST_REG 0x2c
> +#define USB_SERIAL_JTAG_IN_EP1_WR_ADDR_MASK 0x000001fc
> +#define USB_SERIAL_JTAG_IN_EP1_WR_ADDR_SHIFT 2
> +#define USB_SERIAL_JTAG_OUT_EP1_ST_REG 0x3c
> +#define USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_MASK 0x007f0000
> +#define USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_SHIFT 16
> +
> +static const struct of_device_id esp32s3_acm_dt_ids[] = {
> + {
> + .compatible = "esp,esp32s3-acm",
> + }, { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, esp32s3_acm_dt_ids);
> +
> +static struct uart_port *esp32s3_acm_ports[UART_NR];
> +
> +static void esp32s3_acm_write(struct uart_port *port, unsigned long reg, u32 v)
> +{
> + writel(v, port->membase + reg);
> +}
> +
> +static u32 esp32s3_acm_read(struct uart_port *port, unsigned long reg)
> +{
> + return readl(port->membase + reg);
> +}
> +
> +static u32 esp32s3_acm_tx_fifo_free(struct uart_port *port)
> +{
> + return esp32s3_acm_read(port, USB_SERIAL_JTAG_EP1_CONF_REG) &
> + USB_SERIAL_JTAG_SERIAL_IN_EP_DATA_FREE_MASK;
> +}
> +
> +static u32 esp32s3_acm_tx_fifo_cnt(struct uart_port *port)
> +{
> + u32 status = esp32s3_acm_read(port, USB_SERIAL_JTAG_IN_EP1_ST_REG);
> +
> + return (status & USB_SERIAL_JTAG_IN_EP1_WR_ADDR_MASK) >>
> + USB_SERIAL_JTAG_IN_EP1_WR_ADDR_SHIFT;
> +}
> +
> +static u32 esp32s3_acm_rx_fifo_cnt(struct uart_port *port)
> +{
> + u32 status = esp32s3_acm_read(port, USB_SERIAL_JTAG_OUT_EP1_ST_REG);
> +
> + return (status & USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_MASK) >>
> + USB_SERIAL_JTAG_OUT_EP1_REC_DATA_CNT_SHIFT;
> +}
> +
> +/* return TIOCSER_TEMT when transmitter is not busy */
> +static unsigned int esp32s3_acm_tx_empty(struct uart_port *port)
> +{
> + return esp32s3_acm_tx_fifo_cnt(port) == 0 ? TIOCSER_TEMT : 0;
> +}
> +
> +static void esp32s3_acm_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +}
> +
> +static unsigned int esp32s3_acm_get_mctrl(struct uart_port *port)
> +{
> + return TIOCM_CAR;
> +}
> +
> +static void esp32s3_acm_stop_tx(struct uart_port *port)
> +{
> + u32 int_ena;
> +
> + int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> + int_ena & ~USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK);
> +}
> +
> +static void esp32s3_acm_rxint(struct uart_port *port)
> +{
> + struct tty_port *tty_port = &port->state->port;
> + u32 rx_fifo_cnt = esp32s3_acm_rx_fifo_cnt(port);
> + unsigned long flags;
> + u32 i;
> +
> + if (!rx_fifo_cnt)
> + return;
> +
> + spin_lock_irqsave(&port->lock, flags);
> +
> + for (i = 0; i < rx_fifo_cnt; ++i) {
> + u32 rx = esp32s3_acm_read(port, USB_SERIAL_JTAG_EP1_REG);
> +
> + ++port->icount.rx;
> + tty_insert_flip_char(tty_port, rx, TTY_NORMAL);
> + }
> + spin_unlock_irqrestore(&port->lock, flags);
> +
> + tty_flip_buffer_push(tty_port);
> +}
> +
> +static void esp32s3_acm_push(struct uart_port *port)
> +{
> + if (esp32s3_acm_tx_fifo_free(port))
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_CONF_REG,
> + USB_SERIAL_JTAG_WR_DONE_MASK);
> +}
> +
> +static void esp32s3_acm_put_char(struct uart_port *port, unsigned char c)
> +{
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_REG, c);
> +}
> +
> +static void esp32s3_acm_put_char_sync(struct uart_port *port, unsigned char c)
> +{
> + while (!esp32s3_acm_tx_fifo_free(port))
> + cpu_relax();
> + esp32s3_acm_put_char(port, c);
> + esp32s3_acm_push(port);
> +}
> +
> +static void esp32s3_acm_transmit_buffer(struct uart_port *port)
> +{
> + struct circ_buf *xmit = &port->state->xmit;
> + u32 tx_fifo_used = esp32s3_acm_tx_fifo_cnt(port);
> +
> + if (esp32s3_acm_tx_fifo_free(port)) {
> + while (!uart_circ_empty(xmit) && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE) {
> + esp32s3_acm_put_char(port, xmit->buf[xmit->tail]);
> + xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> + port->icount.tx++;
> + ++tx_fifo_used;
> + }
> + }
> +
> + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> + uart_write_wakeup(port);
> +
> + if (uart_circ_empty(xmit)) {
> + esp32s3_acm_stop_tx(port);
> + } else {
> + u32 int_ena;
> +
> + int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> + int_ena | USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK);
> + }
> +
> + if (tx_fifo_used > 0 && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE)
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_CONF_REG,
> + USB_SERIAL_JTAG_WR_DONE_MASK);
> +}
> +
> +static void esp32s3_acm_txint(struct uart_port *port)
> +{
> + esp32s3_acm_transmit_buffer(port);
> +}
> +
> +static irqreturn_t esp32s3_acm_int(int irq, void *dev_id)
> +{
> + struct uart_port *port = dev_id;
> + u32 status;
> +
> + status = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ST_REG);
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_CLR_REG, status);
> +
> + if (status & USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ST_MASK)
> + esp32s3_acm_rxint(port);
> + if (status & USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ST_MASK)
> + esp32s3_acm_txint(port);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void esp32s3_acm_start_tx(struct uart_port *port)
> +{
> + esp32s3_acm_transmit_buffer(port);
> +}
> +
> +static void esp32s3_acm_stop_rx(struct uart_port *port)
> +{
> + u32 int_ena;
> +
> + int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> + int_ena & ~USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK);
> +}
> +
> +static int esp32s3_acm_startup(struct uart_port *port)
> +{
> + int ret = 0;
> +
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> + USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK);
> + ret = devm_request_irq(port->dev, port->irq, esp32s3_acm_int, 0,
> + DRIVER_NAME, port);
> + return ret;
> +}
> +
> +static void esp32s3_acm_shutdown(struct uart_port *port)
> +{
> + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG, 0);
> + devm_free_irq(port->dev, port->irq, port);
> +}
> +
> +static void esp32s3_acm_set_termios(struct uart_port *port,
> + struct ktermios *termios,
> + const struct ktermios *old)
> +{
> +}
> +
> +static const char *esp32s3_acm_type(struct uart_port *port)
> +{
> + return "ESP32S3 ACM";
> +}
> +
> +/* configure/auto-configure the port */
> +static void esp32s3_acm_config_port(struct uart_port *port, int flags)
> +{
> + if (flags & UART_CONFIG_TYPE)
> + port->type = PORT_ESP32ACM;
> +}
> +
> +#ifdef CONFIG_CONSOLE_POLL
> +static void esp32s3_acm_poll_put_char(struct uart_port *port, unsigned char c)
> +{
> + esp32s3_acm_put_char_sync(port, c);
> +}
> +
> +static int esp32s3_acm_poll_get_char(struct uart_port *port)
> +{
> + if (esp32s3_acm_rx_fifo_cnt(port))
> + return esp32s3_acm_read(port, USB_SERIAL_JTAG_EP1_REG);
> + else
> + return NO_POLL_CHAR;
> +
> +}
> +#endif
> +
> +static const struct uart_ops esp32s3_acm_pops = {
> + .tx_empty = esp32s3_acm_tx_empty,
> + .set_mctrl = esp32s3_acm_set_mctrl,
> + .get_mctrl = esp32s3_acm_get_mctrl,
> + .stop_tx = esp32s3_acm_stop_tx,
> + .start_tx = esp32s3_acm_start_tx,
> + .stop_rx = esp32s3_acm_stop_rx,
> + .startup = esp32s3_acm_startup,
> + .shutdown = esp32s3_acm_shutdown,
> + .set_termios = esp32s3_acm_set_termios,
> + .type = esp32s3_acm_type,
> + .config_port = esp32s3_acm_config_port,
> +#ifdef CONFIG_CONSOLE_POLL
> + .poll_put_char = esp32s3_acm_poll_put_char,
> + .poll_get_char = esp32s3_acm_poll_get_char,
> +#endif
> +};
> +
> +static void esp32s3_acm_console_putchar(struct uart_port *port, unsigned char c)
> +{
> + esp32s3_acm_put_char_sync(port, c);
> +}
> +
> +static void esp32s3_acm_string_write(struct uart_port *port, const char *s,
> + unsigned int count)
> +{
> + uart_console_write(port, s, count, esp32s3_acm_console_putchar);
> +}
> +
> +static void
> +esp32s3_acm_console_write(struct console *co, const char *s, unsigned int count)
> +{
> + struct uart_port *port = esp32s3_acm_ports[co->index];
> + unsigned long flags;
> + int locked = 1;
> +
> + if (port->sysrq)
> + locked = 0;
> + else if (oops_in_progress)
> + locked = spin_trylock_irqsave(&port->lock, flags);
> + else
> + spin_lock_irqsave(&port->lock, flags);
> +
> + esp32s3_acm_string_write(port, s, count);
> +
> + if (locked)
> + spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static struct uart_driver esp32s3_acm_reg;
> +static struct console esp32s3_acm_console = {
> + .name = DEV_NAME,
> + .write = esp32s3_acm_console_write,
> + .device = uart_console_device,
> + .flags = CON_PRINTBUFFER,
> + .index = -1,
> + .data = &esp32s3_acm_reg,
> +};
> +
> +static void esp32s3_acm_earlycon_putchar(struct uart_port *port, unsigned char c)
> +{
> + esp32s3_acm_put_char_sync(port, c);
> +}
> +
> +static void esp32s3_acm_earlycon_write(struct console *con, const char *s,
> + unsigned int n)
> +{
> + struct earlycon_device *dev = con->data;
> +
> + uart_console_write(&dev->port, s, n, esp32s3_acm_earlycon_putchar);
> +}
> +
> +#ifdef CONFIG_CONSOLE_POLL
> +static int esp32s3_acm_earlycon_read(struct console *con, char *s, unsigned int n)
> +{
> + struct earlycon_device *dev = con->data;
> + int num_read = 0;
> +
> + while (num_read < n) {
> + int c = esp32s3_acm_poll_get_char(&dev->port);
> +
> + if (c == NO_POLL_CHAR)
> + break;
> + s[num_read++] = c;
> + }
> + return num_read;
> +}
> +#endif
> +
> +static int __init esp32s3_acm_early_console_setup(struct earlycon_device *device,
> + const char *options)
> +{
> + if (!device->port.membase)
> + return -ENODEV;
> +
> + device->con->write = esp32s3_acm_earlycon_write;
> +#ifdef CONFIG_CONSOLE_POLL
> + device->con->read = esp32s3_acm_earlycon_read;
> +#endif
> + return 0;
> +}
> +
> +OF_EARLYCON_DECLARE(esp32s3acm, "esp,esp32s3-acm",
> + esp32s3_acm_early_console_setup);
> +
> +static struct uart_driver esp32s3_acm_reg = {
> + .owner = THIS_MODULE,
> + .driver_name = DRIVER_NAME,
> + .dev_name = DEV_NAME,
> + .nr = ARRAY_SIZE(esp32s3_acm_ports),
> + .cons = &esp32s3_acm_console,
> +};
> +
> +static int esp32s3_acm_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct uart_port *port;
> + struct resource *res;
> + int ret;
> +
> + port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + ret = of_alias_get_id(np, "serial");
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> + return ret;
> + }
> + if (ret >= UART_NR) {
> + dev_err(&pdev->dev, "driver limited to %d serial ports\n",
> + UART_NR);
> + return -ENOMEM;
> + }
> +
> + port->line = ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + port->mapbase = res->start;
> + port->membase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(port->membase))
> + return PTR_ERR(port->membase);
> +
> + port->dev = &pdev->dev;
> + port->type = PORT_ESP32ACM;
> + port->iotype = UPIO_MEM;
> + port->irq = platform_get_irq(pdev, 0);
> + port->ops = &esp32s3_acm_pops;
> + port->flags = UPF_BOOT_AUTOCONF;
> + port->has_sysrq = 1;
> + port->fifosize = ESP32S3_ACM_TX_FIFO_SIZE;
> +
> + esp32s3_acm_ports[port->line] = port;
> +
> + platform_set_drvdata(pdev, port);
> +
> + ret = uart_add_one_port(&esp32s3_acm_reg, port);
> + return ret;
> +}
> +
> +static int esp32s3_acm_remove(struct platform_device *pdev)
> +{
> + struct uart_port *port = platform_get_drvdata(pdev);
> +
> + uart_remove_one_port(&esp32s3_acm_reg, port);
> + return 0;
> +}
> +
> +
> +static struct platform_driver esp32s3_acm_driver = {
> + .probe = esp32s3_acm_probe,
> + .remove = esp32s3_acm_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = esp32s3_acm_dt_ids,
> + },
> +};
> +
> +static int __init esp32s3_acm_init(void)
> +{
> + int ret;
> +
> + ret = uart_register_driver(&esp32s3_acm_reg);
> + if (ret)
> + return ret;
> +
> + ret = platform_driver_register(&esp32s3_acm_driver);
> + if (ret)
> + uart_unregister_driver(&esp32s3_acm_reg);
> +
> + return ret;
> +}
> +
> +static void __exit esp32s3_acm_exit(void)
> +{
> + platform_driver_unregister(&esp32s3_acm_driver);
> + uart_unregister_driver(&esp32s3_acm_reg);
> +}
> +
> +module_init(esp32s3_acm_init);
> +module_exit(esp32s3_acm_exit);
> +
> +MODULE_DESCRIPTION("Espressif ESP32S3 USB ACM driver.");
> +MODULE_AUTHOR("Max Filippov <[email protected]>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index ff076d6be159..1045bf096837 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -248,4 +248,7 @@
> /* Espressif ESP32 UART */
> #define PORT_ESP32UART 124
>
> +/* Espressif ESP32 ACM */
> +#define PORT_ESP32ACM 125
> +
> #endif /* _UAPILINUX_SERIAL_CORE_H */
>

2023-09-16 07:06:25

by Max Filippov

[permalink] [raw]
Subject: Re: [PATCH 4/4] drivers/tty/serial: add ESP32S3 ACM device driver

On Thu, Sep 14, 2023 at 12:16 AM Jiri Slaby <[email protected]> wrote:
>
> On 13. 09. 23, 23:14, Max Filippov wrote:
> > Add driver for the ACM controller of the Espressif ESP32S3 Soc.
> > Hardware specification is available at the following URL:
> >
> > https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
> > (Chapter 33 USB Serial/JTAG Controller)
> ...
>
> > +static void esp32s3_acm_put_char_sync(struct uart_port *port, unsigned char c)
> > +{
> > + while (!esp32s3_acm_tx_fifo_free(port))
> > + cpu_relax();
>
> No limits...

Fixed.

> > + esp32s3_acm_put_char(port, c);
> > + esp32s3_acm_push(port);
> > +}
> > +
> > +static void esp32s3_acm_transmit_buffer(struct uart_port *port)
> > +{
>
> tx helper.

Ok.

> > + struct circ_buf *xmit = &port->state->xmit;
> > + u32 tx_fifo_used = esp32s3_acm_tx_fifo_cnt(port);
> > +
> > + if (esp32s3_acm_tx_fifo_free(port)) {
> > + while (!uart_circ_empty(xmit) && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE) {
> > + esp32s3_acm_put_char(port, xmit->buf[xmit->tail]);
> > + xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> > + port->icount.tx++;
> > + ++tx_fifo_used;
> > + }
> > + }
> > +
> > + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> > + uart_write_wakeup(port);
> > +
> > + if (uart_circ_empty(xmit)) {
> > + esp32s3_acm_stop_tx(port);
> > + } else {
> > + u32 int_ena;
> > +
> > + int_ena = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ENA_REG);
> > + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> > + int_ena | USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ENA_MASK);
> > + }
> > +
> > + if (tx_fifo_used > 0 && tx_fifo_used < ESP32S3_ACM_TX_FIFO_SIZE)
> > + esp32s3_acm_write(port, USB_SERIAL_JTAG_EP1_CONF_REG,
> > + USB_SERIAL_JTAG_WR_DONE_MASK);
> > +}
>
>
> > +static irqreturn_t esp32s3_acm_int(int irq, void *dev_id)
> > +{
> > + struct uart_port *port = dev_id;
> > + u32 status;
> > +
> > + status = esp32s3_acm_read(port, USB_SERIAL_JTAG_INT_ST_REG);
> > + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_CLR_REG, status);
> > +
> > + if (status & USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ST_MASK)
> > + esp32s3_acm_rxint(port);
> > + if (status & USB_SERIAL_JTAG_SERIAL_IN_EMPTY_INT_ST_MASK)
> > + esp32s3_acm_txint(port);
> > +
> > + return IRQ_HANDLED;
>
> IRQ_STATUS()

Ok.

> > +}
>
> > +static int esp32s3_acm_startup(struct uart_port *port)
> > +{
> > + int ret = 0;
> > +
> > + esp32s3_acm_write(port, USB_SERIAL_JTAG_INT_ENA_REG,
> > + USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA_MASK);
> > + ret = devm_request_irq(port->dev, port->irq, esp32s3_acm_int, 0,
> > + DRIVER_NAME, port);
> > + return ret;
>
> No need for ret. Or not, you don't handle the failure properly again
> (disable ints). And the order appears to be switched too.

Fixed.

> > +static void
> > +esp32s3_acm_console_write(struct console *co, const char *s, unsigned int count)
> > +{
> > + struct uart_port *port = esp32s3_acm_ports[co->index];
> > + unsigned long flags;
> > + int locked = 1;
>
> bool? ANd in the otrher driver too.

Ok.

> > +
> > + if (port->sysrq)
> > + locked = 0;
> > + else if (oops_in_progress)
> > + locked = spin_trylock_irqsave(&port->lock, flags);
> > + else
> > + spin_lock_irqsave(&port->lock, flags);
> > +
> > + esp32s3_acm_string_write(port, s, count);
> > +
> > + if (locked)
> > + spin_unlock_irqrestore(&port->lock, flags);
> > +}
>
>
> > +#ifdef CONFIG_CONSOLE_POLL
> > +static int esp32s3_acm_earlycon_read(struct console *con, char *s, unsigned int n)
> > +{
> > + struct earlycon_device *dev = con->data;
> > + int num_read = 0;
>
> num looks like should be unsigned?

Ok.

> > +
> > + while (num_read < n) {
> > + int c = esp32s3_acm_poll_get_char(&dev->port);
> > +
> > + if (c == NO_POLL_CHAR)
> > + break;
> > + s[num_read++] = c;
> > + }
> > + return num_read;
> > +}
> > +#endif
>
>
> > +static int esp32s3_acm_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np = pdev->dev.of_node;
> > + struct uart_port *port;
> > + struct resource *res;
> > + int ret;
> > +
> > + port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
> > + if (!port)
> > + return -ENOMEM;
> > +
> > + ret = of_alias_get_id(np, "serial");
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> > + return ret;
> > + }
> > + if (ret >= UART_NR) {
> > + dev_err(&pdev->dev, "driver limited to %d serial ports\n",
> > + UART_NR);
> > + return -ENOMEM;
> > + }
> > +
> > + port->line = ret;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res)
> > + return -ENODEV;
> > +
> > + port->mapbase = res->start;
> > + port->membase = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(port->membase))
> > + return PTR_ERR(port->membase);
> > +
> > + port->dev = &pdev->dev;
> > + port->type = PORT_ESP32ACM;
> > + port->iotype = UPIO_MEM;
> > + port->irq = platform_get_irq(pdev, 0);
> > + port->ops = &esp32s3_acm_pops;
> > + port->flags = UPF_BOOT_AUTOCONF;
> > + port->has_sysrq = 1;
> > + port->fifosize = ESP32S3_ACM_TX_FIFO_SIZE;
> > +
> > + esp32s3_acm_ports[port->line] = port;
> > +
> > + platform_set_drvdata(pdev, port);
> > +
> > + ret = uart_add_one_port(&esp32s3_acm_reg, port);
> > + return ret;
>
> return imm.

Ok.

--
Thanks.
-- Max