serial: add drivers for the ESP32xx serial devices

uart_get_baud_rate has input parameters 'min' and 'max' limiting the range of acceptable baud rates from the caller's perspective. If neither current or old termios structures have acceptable baud rate setting and 9600 is not in the min/max range either the function returns 0 and issues a warning.
However for a UART that does not support speed of 9600 baud this is expected behavior.
Clarify that 0 can be (and always could be) returned from the uart_get_baud_rate. Don't issue a warning in that case.
Move the warinng to the uart_get_divisor instead, which is often called with the uart_get_baud_rate return value.

Signed-off-by Max Filippov <[email protected]>

---
 drivers/tty/serial/serial_core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/tty/serial/serial_core.c +1 -1

index 7bdc21d5e13b..a8e2915832e8 100644
@@ -431,7 +431,7 @@ EXPORT_SYMBOL(uart_update_timeout);
  * baud.
  *
  * If the new baud rate is invalid, try the @old termios setting. If it's still
- * invalid, we try 9600 baud.
+ * invalid, we try 9600 baud. If that is also invalid 0 is returned.
  *
  * The @termios structure is updated to reflect the baud rate we're actually
  * going to be using. Don't do this for the case where B0 is requested ("hang

drivers/tty/serial/serial_core.c uart_get_divisor(struct uart_port *port, unsigned int baud) · line 540

Quoted 5 lines · Max Filippov

drivers/tty/serial/serial_core.c +1 -0

@@ -539,6 +537,7 @@ uart_get_divisor(struct uart_port *port, unsigned int baud)
 {
      unsigned int quot;
 
+     WARN_ON(baud == 0);

Why is this needed? If this isn't happening today, then there's no need to check for this here.

Add documentation for the ESP32xx UART controllers.

Signed-off-by Max Filippov <[email protected]>

Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +10 -0

+properties:
+  compatible:
+    enum:
+      - esp,esp32-uart
+      - esp,esp32s3-uart
+
+  reg:
+    maxItems: 1
+
+  interrupts:

Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:

drivers/tty/serial/esp32_uart.c new file +7 -0

+static void esp32_uart_rxint(struct uart_port *port)
+{
+	struct tty_port *tty_port = &port->state->port;
+	u32 rx_fifo_cnt = esp32_uart_rx_fifo_cnt(port);
+	unsigned long flags;
+	u32 i;
+

drivers/tty/serial/esp32_uart.c new file +5 -0

+	}
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	tty_flip_buffer_push(tty_port);
+}

drivers/tty/serial/esp32_uart.c new file +5 -0

+	while (esp32_uart_tx_fifo_cnt(port) >= ESP32_UART_TX_FIFO_SIZE) {
+		if (time_after(jiffies, timeout)) {
+			dev_warn(port->dev, "timeout waiting for TX FIFO\n");
+			return;
+		}

drivers/tty/serial/esp32_uart.c new file +19 -0

+		unsigned int pending;
+		u8 ch;
+
+		pending = uart_port_tx_limited(port, ch,
+					       ESP32_UART_TX_FIFO_SIZE - tx_fifo_used,
+					       true, esp32_uart_put_char(port, ch),
+					       ({}));
+		if (pending) {
+			u32 int_ena;
+
+			int_ena = esp32_uart_read(port, UART_INT_ENA_REG);
+			int_ena |= UART_TXFIFO_EMPTY_INT;
+			esp32_uart_write(port, UART_INT_ENA_REG, int_ena);
+		}
+	}
+}
 
 
+static irqreturn_t esp32_uart_int(int irq, void *dev_id)
+{
+	struct uart_port *port = dev_id;

drivers/tty/serial/esp32_uart.c new file +12 -0

+	return IRQ_RETVAL(status);
+}
 
+static int esp32_uart_startup(struct uart_port *port)
+{
+	int ret = 0;
+	unsigned long flags;
+	struct esp32_port *sport = container_of(port, struct esp32_port, port);
+
+	ret = clk_prepare_enable(sport->clk);
+	if (ret)
+		return ret;
+

(quoted diff) +2 -0

+	return false;
+}

(quoted diff) +2 -0

+	return esp32xx_uart_early_console_setup(device, options);
+}

drivers/tty/serial/esp32_uart.c new file +44 -0

+static int esp32_uart_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	static const struct of_device_id *match;
+	struct uart_port *port;
+	struct esp32_port *sport;
+	struct resource *res;
+	int ret;
+
+	match = of_match_device(esp32_uart_dt_ids, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
+	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
+	if (!sport)
+		return -ENOMEM;
+
+	port = &sport->port;
+
+	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);
+
+	sport->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(sport->clk))
+		return PTR_ERR(sport->clk);
+

Quoted 2 lines · Max Filippov

Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:

...

Quoted 11 lines

drivers/tty/serial/esp32_uart.c new file +11 -0

+static void esp32_uart_rxint(struct uart_port *port)
+{
+     struct tty_port *tty_port = &port->state->port;
+     u32 rx_fifo_cnt = esp32_uart_rx_fifo_cnt(port);
+     unsigned long flags;
+     u32 i;
+
+     if (!rx_fifo_cnt)
+             return;
+
+     spin_lock_irqsave(&port->lock, flags);

drivers/tty/serial/esp32_uart.c line 207

Quoted 6 lines

drivers/tty/serial/esp32_uart.c new file +6 -0

+
+     for (i = 0; i < rx_fifo_cnt; ++i) {
+             u32 rx = esp32_uart_read(port, UART_FIFO_REG);
+             u32 brk = 0;
+
+             ++port->icount.rx;

Should yuou increment this only in case of insert_flip_char()?

drivers/tty/serial/esp32_uart.c line 221

Quoted 13 lines

drivers/tty/serial/esp32_uart.c new file +13 -0

+             if (!rx) {
+                     brk = esp32_uart_read(port, UART_INT_ST_REG) &
+                             UART_BRK_DET_INT;
+             }
+             if (brk) {
+                     esp32_uart_write(port, UART_INT_CLR_REG, brk);
+                     ++port->icount.brk;
+                     uart_handle_break(port);
+             } else {
+                     if (uart_handle_sysrq_char(port, (unsigned char)rx))
+                             continue;
+                     tty_insert_flip_char(tty_port, rx, TTY_NORMAL);
+             }

This is heavy. Is it equivalent to:
if (!rx && esp32_uart_read(port, UART_INT_ST_REG) &

     UART_BRK_DET_INT) {
   esp32_uart_write(port, UART_INT_CLR_REG, brk);
   ++port->icount.brk;
   uart_handle_break(port);
   continue;
}

if (uart_handle_sysrq_char(port, (unsigned char)rx))
continue;

tty_insert_flip_char(tty_port, rx, TTY_NORMAL);

?

Quoted 5 lines

drivers/tty/serial/esp32_uart.c new file +5 -0

+     }
+     spin_unlock_irqrestore(&port->lock, flags);
+
+     tty_flip_buffer_push(tty_port);
+}

...

drivers/tty/serial/esp32_uart.c line 237

Quoted 5 lines

drivers/tty/serial/esp32_uart.c new file +5 -0

+static void esp32_uart_put_char_sync(struct uart_port *port, u8 c)
+{
+     unsigned long timeout;
+
+     timeout = jiffies + msecs_to_jiffies(1000);

I.e. plus HZ?

Quoted 9 lines

drivers/tty/serial/esp32_uart.c new file +9 -0

+     while (esp32_uart_tx_fifo_cnt(port) >= ESP32_UART_TX_FIFO_SIZE) {
+             if (time_after(jiffies, timeout)) {
+                     dev_warn(port->dev, "timeout waiting for TX FIFO\n");
+                     return;
+             }
+             cpu_relax();
+     }
+     esp32_uart_put_char(port, c);
+}

drivers/tty/serial/esp32_uart.c line 252

Quoted 6 lines

drivers/tty/serial/esp32_uart.c new file +6 -0

+
+static void esp32_uart_transmit_buffer(struct uart_port *port)
+{
+     u32 tx_fifo_used = esp32_uart_tx_fifo_cnt(port);
+
+     if (tx_fifo_used < ESP32_UART_TX_FIFO_SIZE) {

Invert the condition and return here?

Quoted 16 lines

drivers/tty/serial/esp32_uart.c new file +16 -0

+             unsigned int pending;
+             u8 ch;
+
+             pending = uart_port_tx_limited(port, ch,
+                                            ESP32_UART_TX_FIFO_SIZE - tx_fifo_used,
+                                            true, esp32_uart_put_char(port, ch),
+                                            ({}));
+             if (pending) {
+                     u32 int_ena;
+
+                     int_ena = esp32_uart_read(port, UART_INT_ENA_REG);
+                     int_ena |= UART_TXFIFO_EMPTY_INT;
+                     esp32_uart_write(port, UART_INT_ENA_REG, int_ena);
+             }
+     }
+}

drivers/tty/serial/esp32_uart.c line 287

Quoted 13 lines

drivers/tty/serial/esp32_uart.c new file +13 -0

+static irqreturn_t esp32_uart_int(int irq, void *dev_id)
+{
+     struct uart_port *port = dev_id;
+     u32 status;
+
+     status = esp32_uart_read(port, UART_INT_ST_REG);
+
+     if (status & (UART_RXFIFO_FULL_INT | UART_BRK_DET_INT))
+             esp32_uart_rxint(port);
+     if (status & UART_TXFIFO_EMPTY_INT)
+             esp32_uart_txint(port);
+
+     esp32_uart_write(port, UART_INT_CLR_REG, status);

\n here please.

Quoted 13 lines

drivers/tty/serial/esp32_uart.c new file +12 -0

+     return IRQ_RETVAL(status);
+}
 
+static int esp32_uart_startup(struct uart_port *port)
+{
+     int ret = 0;
+     unsigned long flags;
+     struct esp32_port *sport = container_of(port, struct esp32_port, port);
+
+     ret = clk_prepare_enable(sport->clk);
+     if (ret)
+             return ret;
+

drivers/tty/serial/esp32_uart.c line 324

Quoted 10 lines

drivers/tty/serial/esp32_uart.c new file +10 -0

+     ret = devm_request_irq(port->dev, port->irq, esp32_uart_int, 0,
+                            DRIVER_NAME, port);
+     if (ret) {
+             clk_disable_unprepare(sport->clk);
+             return ret;
+     }
+
+     spin_lock_irqsave(&port->lock, flags);
+     esp32_uart_write(port, UART_CONF1_REG,
+                      (1 << UART_RXFIFO_FULL_THRHD_SHIFT) |

BIT() ?

Quoted 1 line

+ (1 << port_variant(port)->txfifo_empty_thrhd_shift));

And here?

drivers/tty/serial/esp32_uart.c line 334

Quoted 9 lines

drivers/tty/serial/esp32_uart.c new file +9 -0

+     esp32_uart_write(port, UART_INT_CLR_REG, UART_RXFIFO_FULL_INT | UART_BRK_DET_INT);
+     esp32_uart_write(port, UART_INT_ENA_REG, UART_RXFIFO_FULL_INT | UART_BRK_DET_INT);
+     spin_unlock_irqrestore(&port->lock, flags);
+
+     pr_debug("%s, conf1 = %08x, int_st = %08x, status = %08x\n",
+              __func__,
+              esp32_uart_read(port, UART_CONF1_REG),
+              esp32_uart_read(port, UART_INT_ST_REG),
+              esp32_uart_read(port, UART_STATUS_REG));

\n here. Is this debug printout somehow useful?

drivers/tty/serial/esp32_uart.c line 343

Quoted 9 lines

drivers/tty/serial/esp32_uart.c new file +9 -0

+     return ret;
+}
+
+static void esp32_uart_shutdown(struct uart_port *port)
+{
+     struct esp32_port *sport = container_of(port, struct esp32_port, port);
+
+     esp32_uart_write(port, UART_INT_ENA_REG, 0);
+     devm_free_irq(port->dev, port->irq, port);

I wonder why to use devm_ after all?

Quoted 7 lines

drivers/tty/serial/esp32_uart.c new file +7 -0

+     clk_disable_unprepare(sport->clk);
+}
+
+static bool esp32_uart_set_baud(struct uart_port *port, u32 baud)
+{
+     u32 div = port->uartclk / baud;
+     u32 frag = (port->uartclk * 16) / baud - div * 16;

drivers/tty/serial/esp32_uart.c line 356

Quoted 6 lines

drivers/tty/serial/esp32_uart.c new file +6 -0

+
+     if (div <= port_variant(port)->clkdiv_mask) {
+             esp32_uart_write(port, UART_CLKDIV_REG,
+                              div | FIELD_PREP(UART_CLKDIV_FRAG, frag));
+             return true;
+     }

\n

Quoted 2 lines

(quoted diff) +2 -0

+     return false;
+}

...

drivers/tty/serial/esp32_uart.c line 628

Quoted 4 lines

drivers/tty/serial/esp32_uart.c new file +4 -0

+static int __init esp32s3_uart_early_console_setup(struct earlycon_device *device,
+                                                const char *options)
+{
+     device->port.private_data = (void *)&esp32s3_variant;

No need to cast, right?

\n

Quoted 2 lines

(quoted diff) +2 -0

+     return esp32xx_uart_early_console_setup(device, options);
+}

...

Quoted 48 lines

drivers/tty/serial/esp32_uart.c new file +48 -0

+static int esp32_uart_probe(struct platform_device *pdev)
+{
+     struct device_node *np = pdev->dev.of_node;
+     static const struct of_device_id *match;
+     struct uart_port *port;
+     struct esp32_port *sport;
+     struct resource *res;
+     int ret;
+
+     match = of_match_device(esp32_uart_dt_ids, &pdev->dev);
+     if (!match)
+             return -ENODEV;
+
+     sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
+     if (!sport)
+             return -ENOMEM;
+
+     port = &sport->port;
+
+     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);
+
+     sport->clk = devm_clk_get(&pdev->dev, NULL);
+     if (IS_ERR(sport->clk))
+             return PTR_ERR(sport->clk);
+
+     port->uartclk = clk_get_rate(sport->clk);
+     port->dev = &pdev->dev;
+     port->type = PORT_ESP32UART;
+     port->iotype = UPIO_MEM;

drivers/tty/serial/esp32_uart.c line 696

Quoted 6 lines

drivers/tty/serial/esp32_uart.c new file +6 -0

+     port->irq = platform_get_irq(pdev, 0);
+     port->ops = &esp32_uart_pops;
+     port->flags = UPF_BOOT_AUTOCONF;
+     port->has_sysrq = 1;
+     port->fifosize = ESP32_UART_TX_FIFO_SIZE;
+     port->private_data = (void *)match->data;

No need to cast.

drivers/tty/serial/esp32_uart.c line 703

Quoted 7 lines

drivers/tty/serial/esp32_uart.c new file +7 -0

+
+     esp32_uart_ports[port->line] = sport;
+
+     platform_set_drvdata(pdev, port);
+
+     return uart_add_one_port(&esp32_uart_reg, port);
+}

regards,
--
js
suse labs