On 20. 09. 23, 4:26, Max Filippov wrote:
Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:
...
+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); + + 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()?
+ 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); ?
+ } + spin_unlock_irqrestore(&port->lock, flags); + + tty_flip_buffer_push(tty_port); +}
...
+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?
+ 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); +} + +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?
+ 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; + 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.
+ 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; + + 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() ?
+ (1 << port_variant(port)->txfifo_empty_thrhd_shift));
And here?
+ 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?
+ 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?
+ 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; + + if (div <= port_variant(port)->clkdiv_mask) { + esp32_uart_write(port, UART_CLKDIV_REG, + div | FIELD_PREP(UART_CLKDIV_FRAG, frag)); + return true; + }
\n
+ return false; +}
...
+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
+ return esp32xx_uart_early_console_setup(device, options); +}
...
+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; + 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.
+ + esp32_uart_ports[port->line] = sport; + + platform_set_drvdata(pdev, port); + + return uart_add_one_port(&esp32_uart_reg, port); +}
regards, -- js suse labs