serial: add drivers for the ESP32xx serial devices
Subsystems (2)
cover serial: add drivers for the ESP32xx serial devices
Hello,
this series adds drivers for the UART and ACM controllers found in the Espressif ESP32 and ESP32S3 SoCs.
Max Filippov (4):
dt-bindings: serial: document esp32-uart bindings drivers/tty/serial: add driver for the ESP32 UART
dt-bindings: serial: document esp32s3-acm bindings drivers/tty/serial: add ESP32S3 ACM device driver
.../bindings/serial/esp,esp32-acm.yaml | 40 + .../bindings/serial/esp,esp32-uart.yaml | 48 ++ drivers/tty/serial/Kconfig | 27 + drivers/tty/serial/Makefile | 2 + drivers/tty/serial/esp32_acm.c | 473 +++++++++++ drivers/tty/serial/esp32_uart.c | 766 ++++++++++++++++++ include/uapi/linux/serial_core.h | 6 + 7 files changed, 1362 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml create mode 100644 drivers/tty/serial/esp32_acm.c create mode 100644 drivers/tty/serial/esp32_uart.c
Signature · 1 line
2.30.2
PATCH 1/4 dt-bindings: serial: document esp32-uart bindings
Add documentation for the ESP32xx UART controllers.
--- .../bindings/serial/esp,esp32-uart.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +48 -0
@@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/esp,esp32-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ESP32 UART controller + +maintainers: + - Max Filippov <[email protected]> + +description: | + ESP32 UART controller is a part of ESP32 SoC series. + +properties: + compatible: + oneOf: + - description: UART controller for the ESP32 SoC + const: esp,esp32-uart + - description: UART controller for the ESP32S3 SoC + const: esp,esp32s3-uart + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + - clocks + +additionalProperties: false + +examples: + - | + serial0: serial@60000000 { + compatible = "esp,esp32s3-uart"; + reg = <0x60000000 0x80>; + interrupts = <27 1 0>; + clocks = <&serial_clk>; + };
Signature · 1 line
2.30.2
Re: [PATCH 1/4] dt-bindings: serial: document esp32-uart bindings
Quoted 17 lines · Max Filippov
Add documentation for the ESP32xx UART controllers.
Signed-off-by Max Filippov <[email protected]>--- .../bindings/serial/esp,esp32-uart.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-uart.yamlDocumentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +3
-0@@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +10
-0+--- +$id: http://devicetree.org/schemas/serial/esp,esp32-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ESP32 UART controller + +maintainers: + - Max Filippov <[email protected]> + +description: |
Do not need '|' unless you need to preserve formatting.
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 18
Quoted 5 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +5
-0+ ESP32 UART controller is a part of ESP32 SoC series. + +properties: + compatible: + oneOf:
That's just enum. Your descriptions are useless - tell nothing - so drop them.
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 20
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +2
-0+ - description: UART controller for the ESP32 SoC + const: esp,esp32-uart
Looks quite generic, so just to be sure? This is not a family name, right? Neither family names nor wildcards are allowed.
Quoted 13 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +13
-0+ - description: UART controller for the ESP32S3 SoC + const: esp,esp32s3-uart + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + +required:
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 43
Quoted 10 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +10
-0+ - compatible + - reg + - interrupts + - clocks + +additionalProperties: false + +examples: + - | + serial0: serial@60000000 {
Drop unused label.
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 44
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +1
-0+ compatible = "esp,esp32s3-uart";
Use 4 spaces for example indentation.
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 46
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +2
-0+ reg = <0x60000000 0x80>; + interrupts = <27 1 0>;
Use proper define for IRQ flags.
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 48
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +2
-0+ clocks = <&serial_clk>; + };
Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: serial: document esp32-uart bindings
Quoted 23 lines · Krzysztof Kozlowski
Quoted 21 lines · Max Filippov
Add documentation for the ESP32xx UART controllers.
Signed-off-by Max Filippov <[email protected]>--- .../bindings/serial/esp,esp32-uart.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-uart.yamlDocumentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +7
-0@@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/esp,esp32-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# +
Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 13
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +6
-0+title: ESP32 UART controller + +maintainers: + - Max Filippov <[email protected]> + +description: |Do not need '|' unless you need to preserve formatting.
Ok.
Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 18
Quoted 5 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +5
-0+ ESP32 UART controller is a part of ESP32 SoC series. + +properties: + compatible: + oneOf:That's just enum. Your descriptions are useless - tell nothing - so drop them.
Ok.
Quoted 5 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 20
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +2
-0+ - description: UART controller for the ESP32 SoC + const: esp,esp32-uartLooks quite generic, so just to be sure? This is not a family name, right? Neither family names nor wildcards are allowed.
ESP32 is the official name of a specific SoC. More recent SoC models of that family are named ESP32-S2 and ESP32-S3, sometimes they are collectively referred to as the 'ESP32 series'. In this binding 'esp32' is used for the ESP32 SoC and 'esp32s3' is used for the ESP32-S3.
Quoted 17 lines
Quoted 17 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +17
-0+ - description: UART controller for the ESP32S3 SoC + const: esp,esp32s3-uart + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + - clocks
Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 43
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +6
-0+ +additionalProperties: false + +examples: + - | + serial0: serial@60000000 {Drop unused label.
Ok.
Quoted 3 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 44
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +1
-0+ compatible = "esp,esp32s3-uart";Use 4 spaces for example indentation.
Ok.
Quoted 4 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 46
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +2
-0+ reg = <0x60000000 0x80>; + interrupts = <27 1 0>;Use proper define for IRQ flags.
These are not IRQ flags. 1 and 0 are the routing parts of the IRQ specification.
Signature · 2 lines
Thanks. -- Max
Re: [PATCH 1/4] dt-bindings: serial: document esp32-uart bindings
In reply to Max FilippovQuoted 18 lines · Max Filippov
Add documentation for the ESP32xx UART controllers.
Signed-off-by Max Filippov <[email protected]>--- .../bindings/serial/esp,esp32-uart.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-uart.yamlDocumentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +4
-0@@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2 +---
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 14
Quoted 10 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +10
-0+$id: http://devicetree.org/schemas/serial/esp,esp32-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ESP32 UART controller + +maintainers: + - Max Filippov <[email protected]> + +description: | + ESP32 UART controller is a part of ESP32 SoC series.
1. Company name?
2. ESP32 SoC series suggests esp32 is a series.
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 20
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +6
-0+ +properties: + compatible: + oneOf: + - description: UART controller for the ESP32 SoC + const: esp,esp32-uart
Also, the vendor prefix looks incorrect, so again - what is the company name?
Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: serial: document esp32-uart bindings
Quoted 33 lines · Krzysztof Kozlowski
Quoted 18 lines · Max Filippov
Add documentation for the ESP32xx UART controllers.
Signed-off-by Max Filippov <[email protected]>--- .../bindings/serial/esp,esp32-uart.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-uart.yamlDocumentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +4
-0@@ -0,0 +1,48 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2 +---Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 14
Quoted 10 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +10
-0+$id: http://devicetree.org/schemas/serial/esp,esp32-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ESP32 UART controller + +maintainers: + - Max Filippov <[email protected]> + +description: | + ESP32 UART controller is a part of ESP32 SoC series.1. Company name?
2. ESP32 SoC series suggests esp32 is a series.
Quoted 9 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 20
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +6
-0+ +properties: + compatible: + oneOf: + - description: UART controller for the ESP32 SoC + const: esp,esp32-uartAlso, the vendor prefix looks incorrect, so again - what is the company name?
esp32 is made by expressif, which would match with "esp" as a vendor prefix.
Re: [PATCH 1/4] dt-bindings: serial: document esp32-uart bindings
Quoted 18 lines · Conor Dooley
Quoted 7 lines · Krzysztof Kozlowski
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 14
Quoted 2 lines · Max Filippov
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +2
-0+description: | + ESP32 UART controller is a part of ESP32 SoC series.1. Company name?
2. ESP32 SoC series suggests esp32 is a series.Quoted 9 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml line 20
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-uart.yaml new file +6
-0+ +properties: + compatible: + oneOf: + - description: UART controller for the ESP32 SoC + const: esp,esp32-uartAlso, the vendor prefix looks incorrect, so again - what is the company name?
Quoted 2 lines
esp32 is made by expressif, which would match with "esp" as a vendor prefix.
It's 'Espressif', but otherwise yes, this is a registered vendor prefix. See
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/vendor-prefixes.yaml?h=v6.6-rc1#n443
Signature · 2 lines
Thanks. -- Max
PATCH 2/4 drivers/tty/serial: add driver for the ESP32 UART
In reply to Max FilippovAdd driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:
https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf
(Chapter 13 UART Controller)
https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
(Chapter 26 UART Controller)
--- drivers/tty/serial/Kconfig | 13 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/esp32_uart.c | 766 +++++++++++++++++++++++++++++++ include/uapi/linux/serial_core.h | 3 + 4 files changed, 783 insertions(+) create mode 100644 drivers/tty/serial/esp32_uart.c
drivers/tty/serial/Kconfig +13 -0
@@ -1578,6 +1578,19 @@ config SERIAL_NUVOTON_MA35D1_CONSOLE but you can alter that using a kernel command line option such as "console=ttyNVTx". +config SERIAL_ESP32 + tristate "Espressif ESP32 UART support" + depends on XTENSA_PLATFORM_ESP32 || (COMPILE_TEST && OF) + select SERIAL_CORE + select SERIAL_CORE_CONSOLE + select SERIAL_EARLYCON + help + Driver for the UART controllers of the Espressif ESP32xx SoCs. + When earlycon option is enabled the following kernel command line + snippets may be used: + earlycon=esp32s3uart,mmio32,0x60000000,115200n8,40000000 + earlycon=esp32uart,mmio32,0x3ff40000,115200n8 + endmenu config SERIAL_MCTRL_GPIO
drivers/tty/serial/Makefile +1 -0
@@ -88,6 +88,7 @@ obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o 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 # GPIOLIB helpers for modem control lines obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
drivers/tty/serial/esp32_uart.c new file +766 -0
766 lines not on this page · raw message
include/uapi/linux/serial_core.h +3 -0
@@ -245,4 +245,7 @@/* Sunplus UART */ #define PORT_SUNPLUS 123 +/* Espressif ESP32 UART */ +#define PORT_ESP32UART 124 +#endif /* _UAPILINUX_SERIAL_CORE_H */
Signature · 1 line
2.30.2
Jiri Slaby
maintainerRe: [PATCH 2/4] drivers/tty/serial: add driver for the ESP32 UART
Quoted 10 lines · Max Filippov
Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:
https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf
(Chapter 13 UART Controller)
https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
(Chapter 26 UART Controller)Signed-off-by Max Filippov <[email protected]>---
...
Quoted 7 lines
drivers/tty/serial/esp32_uart.c new file +7
-0+#define UART_FIFO_REG 0x00 +#define UART_INT_RAW_REG 0x04 +#define UART_INT_ST_REG 0x08 +#define UART_INT_ENA_REG 0x0c +#define UART_INT_CLR_REG 0x10 +#define UART_RXFIFO_FULL_INT_MASK 0x00000001 +#define UART_TXFIFO_EMPTY_INT_MASK 0x00000002
drivers/tty/serial/esp32_uart.c line 39
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+#define UART_BRK_DET_INT_MASK 0x00000080 +#define UART_CLKDIV_REG 0x14 +#define ESP32_UART_CLKDIV_MASK 0x000fffff +#define ESP32S3_UART_CLKDIV_MASK 0x00000fff +#define UART_CLKDIV_SHIFT 0 +#define UART_CLKDIV_FRAG_MASK 0x00f00000 +#define UART_CLKDIV_FRAG_SHIFT 20 +#define UART_STATUS_REG 0x1c +#define ESP32_UART_RXFIFO_CNT_MASK 0x000000ff +#define ESP32S3_UART_RXFIFO_CNT_MASK 0x000003ff
Can you use GENMASK() for all these?
Quoted 38 lines
drivers/tty/serial/esp32_uart.c new file +38
-0+#define UART_RXFIFO_CNT_SHIFT 0 +#define UART_DSRN_MASK 0x00002000 +#define UART_CTSN_MASK 0x00004000 +#define ESP32_UART_TXFIFO_CNT_MASK 0x00ff0000 +#define ESP32S3_UART_TXFIFO_CNT_MASK 0x03ff0000 +#define UART_TXFIFO_CNT_SHIFT 16 +#define UART_ST_UTX_OUT_MASK 0x0f000000 +#define UART_ST_UTX_OUT_IDLE 0x00000000 +#define UART_ST_UTX_OUT_SHIFT 24 +#define UART_CONF0_REG 0x20 +#define UART_PARITY_MASK 0x00000001 +#define UART_PARITY_EN_MASK 0x00000002 +#define UART_BIT_NUM_MASK 0x0000000c +#define UART_BIT_NUM_5 0x00000000 +#define UART_BIT_NUM_6 0x00000004 +#define UART_BIT_NUM_7 0x00000008 +#define UART_BIT_NUM_8 0x0000000c +#define UART_STOP_BIT_NUM_MASK 0x00000030 +#define UART_STOP_BIT_NUM_1 0x00000010 +#define UART_STOP_BIT_NUM_2 0x00000030 +#define UART_SW_RTS_MASK 0x00000040 +#define UART_SW_DTR_MASK 0x00000080 +#define UART_LOOPBACK_MASK 0x00004000 +#define UART_TX_FLOW_EN_MASK 0x00008000 +#define UART_RTS_INV_MASK 0x00800000 +#define UART_DTR_INV_MASK 0x01000000 +#define ESP32_UART_TICK_REF_ALWAYS_ON_MASK 0x08000000 +#define ESP32S3_UART_TICK_REF_ALWAYS_ON_MASK 0x00000000 +#define UART_CONF1_REG 0x24 +#define ESP32_UART_RXFIFO_FULL_THRHD_MASK 0x0000007f +#define ESP32S3_UART_RXFIFO_FULL_THRHD_MASK 0x000003ff +#define UART_RXFIFO_FULL_THRHD_SHIFT 0 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_MASK 0x00007f00 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_MASK 0x000ffc00 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_SHIFT 8 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_SHIFT 10 +#define ESP32_UART_RX_FLOW_EN_MASK 0x00800000 +#define ESP32S3_UART_RX_FLOW_EN_MASK 0x00400000
...
Quoted 1 line
+static void
esp32_uart_put_char_sync(struct uart_port *port, unsigned char c)
u8 for characters everywhere, please.
drivers/tty/serial/esp32_uart.c line 251
Quoted 3 lines
drivers/tty/serial/esp32_uart.c new file +3
-0+{ +while (esp32_uart_tx_fifo_cnt (port) >=ESP32_UART_TX_FIFO_SIZE ) +cpu_relax ();
No timeout? There should be one.
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+esp32_uart_put_char (port, c); +} + +static void esp32_uart_transmit_buffer (struct uart_port *port)
drivers/tty/serial/esp32_uart.c line 265
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+{ +struct circ_buf *xmit = &port->state->xmit; +u32 tx_fifo_used =esp32_uart_tx_fifo_cnt (port); + +while (!uart_circ_empty (xmit) && tx_fifo_used <ESP32_UART_TX_FIFO_SIZE ) { +esp32_uart_put_char (port, xmit->buf[xmit->tail]); + xmit->tail = (xmit->tail +1 ) & (UART_XMIT_SIZE -1 ); + port->icount.tx++; + ++tx_fifo_used; + }
Why not using uart_port_tx_limited()?
Quoted 23 lines
drivers/tty/serial/esp32_uart.c new file +23
-0+if (uart_circ_chars_pending (xmit) <WAKEUP_CHARS ) +uart_write_wakeup (port); + +if (uart_circ_empty (xmit)) { +esp32_uart_stop_tx (port); + }else { +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena |UART_TXFIFO_EMPTY_INT_MASK ); + } +} + +static void esp32_uart_txint (struct uart_port *port) +{ +esp32_uart_transmit_buffer (port); +} + +static irqreturn_t esp32_uart_int (int irq,void *dev_id) +{ +struct uart_port *port = dev_id; +u32 status;
drivers/tty/serial/esp32_uart.c line 299
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+ + status =esp32_uart_read (port,UART_INT_ST_REG ); + +if (status & (UART_RXFIFO_FULL_INT_MASK |UART_BRK_DET_INT_MASK )) +esp32_uart_rxint (port); +if (status &UART_TXFIFO_EMPTY_INT_MASK ) +esp32_uart_txint (port); + +esp32_uart_write (port,UART_INT_CLR_REG , status); +return IRQ_HANDLED ;
This should be IRQ_RETVAL(status) or similar. To let the system disable the irq in case the HW gets crazy.
Quoted 25 lines
drivers/tty/serial/esp32_uart.c new file +25
-0+static void esp32_uart_start_tx (struct uart_port *port) +{ +esp32_uart_transmit_buffer (port); +} + +static void esp32_uart_stop_rx (struct uart_port *port) +{ +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena & ~UART_RXFIFO_FULL_INT_MASK ); +} + +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; + +spin_lock_irqsave (&port->lock, flags);
drivers/tty/serial/esp32_uart.c line 336
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+esp32_uart_write (port,UART_CONF1_REG , + (1 <<UART_RXFIFO_FULL_THRHD_SHIFT ) | + (1 <<port_variant (port)->txfifo_empty_thrhd_shift)); +esp32_uart_write (port,UART_INT_CLR_REG , +UART_RXFIFO_FULL_INT_MASK | +UART_BRK_DET_INT_MASK ); +esp32_uart_write (port,UART_INT_ENA_REG , +UART_RXFIFO_FULL_INT_MASK | +UART_BRK_DET_INT_MASK ); +spin_unlock_irqrestore (&port->lock, flags);
So interrupts can be coming now, but you don't handle them yet?
drivers/tty/serial/esp32_uart.c line 339
Quoted 2 lines
drivers/tty/serial/esp32_uart.c new file +2
-0+ ret =devm_request_irq (port->dev, port->irq, esp32_uart_int,0 , +DRIVER_NAME , port);
You don't disable clk and interrupts in case of failure?
Quoted 7 lines
drivers/tty/serial/esp32_uart.c new file +7
-0+pr_debug ("%s, request_irq: %d, conf1 = %08x, int_st = %08x, status = %08x\n" , + __func__, ret, +esp32_uart_read (port,UART_CONF1_REG ), +esp32_uart_read (port,UART_INT_ST_REG ), +esp32_uart_read (port,UART_STATUS_REG )); +return ret; +}
...
drivers/tty/serial/esp32_uart.c line 389
Quoted 13 lines
drivers/tty/serial/esp32_uart.c new file +13
-0+static void esp32_uart_set_termios (struct uart_port *port, +struct ktermios *termios, +const struct ktermios *old) +{ +unsigned long flags; +u32 conf0, conf1; +u32 baud; +const u32 rx_flow_en =port_variant (port)->rx_flow_en; + +spin_lock_irqsave (&port->lock, flags); + conf0 =esp32_uart_read (port,UART_CONF0_REG ) & + ~(UART_PARITY_EN_MASK |UART_PARITY_MASK | +UART_BIT_NUM_MASK |UART_STOP_BIT_NUM_MASK );
Perhaps it would be more readable as:
conf0 = esp32_uart_read(port, UART_CONF0_REG);
conf0 &= ~(UART_PARITY_EN_MASK | ...);
?
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+ conf1 =esp32_uart_read (port,UART_CONF1_REG ) & + ~rx_flow_en; + +if (termios->c_cflag &PARENB ) { + conf0 |=UART_PARITY_EN_MASK ; +if (termios->c_cflag &PARODD ) + conf0 |=UART_PARITY_MASK ; + }
drivers/tty/serial/esp32_uart.c line 448
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+static void esp32_uart_release_port (struct uart_port *port) +{ +} + +static int esp32_uart_request_port (struct uart_port *port) +{ +return 0 ; +}
Drop these two.
Quoted 52 lines
drivers/tty/serial/esp32_uart.c new file +52
-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 ; + port->irq =platform_get_irq (pdev,0 ); + port->ops = &esp32_uart_pops; + port->flags =UPF_BOOT_AUTOCONF ;
drivers/tty/serial/esp32_uart.c line 718
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+ port->has_sysrq =1 ; + port->fifosize =ESP32_UART_TX_FIFO_SIZE ; + port->private_data = (void *)match->data; + + esp32_uart_ports[port->line] = sport; + +platform_set_drvdata (pdev, port); + + ret =uart_add_one_port (&esp32_uart_reg, port); +return ret;
You can skip ret here and return directly.
thanks,
Signature · 2 lines
js suse labs
Re: [PATCH 2/4] drivers/tty/serial: add driver for the ESP32 UART
Quoted 24 lines · Jiri Slaby
Quoted 10 lines · Max Filippov
Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:
https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf
(Chapter 13 UART Controller)
https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
(Chapter 26 UART Controller)Signed-off-by Max Filippov <[email protected]>---
...
Quoted 11 lines
drivers/tty/serial/esp32_uart.c new file +11
-0+#define UART_FIFO_REG 0x00 +#define UART_INT_RAW_REG 0x04 +#define UART_INT_ST_REG 0x08 +#define UART_INT_ENA_REG 0x0c +#define UART_INT_CLR_REG 0x10 +#define UART_RXFIFO_FULL_INT_MASK 0x00000001 +#define UART_TXFIFO_EMPTY_INT_MASK 0x00000002 +#define UART_BRK_DET_INT_MASK 0x00000080 +#define UART_CLKDIV_REG 0x14 +#define ESP32_UART_CLKDIV_MASK 0x000fffff +#define ESP32S3_UART_CLKDIV_MASK 0x00000fff
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 39
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+#define UART_CLKDIV_SHIFT 0 +#define UART_CLKDIV_FRAG_MASK 0x00f00000 +#define UART_CLKDIV_FRAG_SHIFT 20 +#define UART_STATUS_REG 0x1c +#define ESP32_UART_RXFIFO_CNT_MASK 0x000000ff +#define ESP32S3_UART_RXFIFO_CNT_MASK 0x000003ffCan you use
GENMASK()for all these?
Ok.
Quoted 40 lines
Quoted 38 lines
drivers/tty/serial/esp32_uart.c new file +38
-0+#define UART_RXFIFO_CNT_SHIFT 0 +#define UART_DSRN_MASK 0x00002000 +#define UART_CTSN_MASK 0x00004000 +#define ESP32_UART_TXFIFO_CNT_MASK 0x00ff0000 +#define ESP32S3_UART_TXFIFO_CNT_MASK 0x03ff0000 +#define UART_TXFIFO_CNT_SHIFT 16 +#define UART_ST_UTX_OUT_MASK 0x0f000000 +#define UART_ST_UTX_OUT_IDLE 0x00000000 +#define UART_ST_UTX_OUT_SHIFT 24 +#define UART_CONF0_REG 0x20 +#define UART_PARITY_MASK 0x00000001 +#define UART_PARITY_EN_MASK 0x00000002 +#define UART_BIT_NUM_MASK 0x0000000c +#define UART_BIT_NUM_5 0x00000000 +#define UART_BIT_NUM_6 0x00000004 +#define UART_BIT_NUM_7 0x00000008 +#define UART_BIT_NUM_8 0x0000000c +#define UART_STOP_BIT_NUM_MASK 0x00000030 +#define UART_STOP_BIT_NUM_1 0x00000010 +#define UART_STOP_BIT_NUM_2 0x00000030 +#define UART_SW_RTS_MASK 0x00000040 +#define UART_SW_DTR_MASK 0x00000080 +#define UART_LOOPBACK_MASK 0x00004000 +#define UART_TX_FLOW_EN_MASK 0x00008000 +#define UART_RTS_INV_MASK 0x00800000 +#define UART_DTR_INV_MASK 0x01000000 +#define ESP32_UART_TICK_REF_ALWAYS_ON_MASK 0x08000000 +#define ESP32S3_UART_TICK_REF_ALWAYS_ON_MASK 0x00000000 +#define UART_CONF1_REG 0x24 +#define ESP32_UART_RXFIFO_FULL_THRHD_MASK 0x0000007f +#define ESP32S3_UART_RXFIFO_FULL_THRHD_MASK 0x000003ff +#define UART_RXFIFO_FULL_THRHD_SHIFT 0 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_MASK 0x00007f00 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_MASK 0x000ffc00 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_SHIFT 8 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_SHIFT 10 +#define ESP32_UART_RX_FLOW_EN_MASK 0x00800000 +#define ESP32S3_UART_RX_FLOW_EN_MASK 0x00400000...
Quoted 3 lines
Quoted 1 line
+static void
esp32_uart_put_char_sync(struct uart_port *port, unsigned char c)u8 for characters everywhere, please.
Ok, but I guess not everywhere? E.g. uart_ops::poll_put_char has 'unsigned char' in the definition, it seems better to keep implementation in sync.
Quoted 5 lines
drivers/tty/serial/esp32_uart.c line 251
Quoted 3 lines
drivers/tty/serial/esp32_uart.c new file +3
-0+{ +while (esp32_uart_tx_fifo_cnt (port) >=ESP32_UART_TX_FIFO_SIZE ) +cpu_relax ();No timeout? There should be one.
Ok.
Quoted 8 lines
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+esp32_uart_put_char (port, c); +} + +static void esp32_uart_transmit_buffer (struct uart_port *port) +{ +struct circ_buf *xmit = &port->state->xmit; +u32 tx_fifo_used =esp32_uart_tx_fifo_cnt (port); +
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 265
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+while (!uart_circ_empty (xmit) && tx_fifo_used <ESP32_UART_TX_FIFO_SIZE ) { +esp32_uart_put_char (port, xmit->buf[xmit->tail]); + xmit->tail = (xmit->tail +1 ) & (UART_XMIT_SIZE -1 ); + port->icount.tx++; + ++tx_fifo_used; + }Why not using
uart_port_tx_limited()?
Ok.
Quoted 28 lines
Quoted 28 lines
drivers/tty/serial/esp32_uart.c new file +28
-0+if (uart_circ_chars_pending (xmit) <WAKEUP_CHARS ) +uart_write_wakeup (port); + +if (uart_circ_empty (xmit)) { +esp32_uart_stop_tx (port); + }else { +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena |UART_TXFIFO_EMPTY_INT_MASK ); + } +} + +static void esp32_uart_txint (struct uart_port *port) +{ +esp32_uart_transmit_buffer (port); +} + +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_MASK |UART_BRK_DET_INT_MASK )) +esp32_uart_rxint (port);
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 299
Quoted 5 lines
drivers/tty/serial/esp32_uart.c new file +5
-0+if (status &UART_TXFIFO_EMPTY_INT_MASK ) +esp32_uart_txint (port); + +esp32_uart_write (port,UART_INT_CLR_REG , status); +return IRQ_HANDLED ;This should be
IRQ_RETVAL(status)or similar. To let the system disable the irq in case the HW gets crazy.
Ok.
Quoted 29 lines
Quoted 29 lines
drivers/tty/serial/esp32_uart.c new file +29
-0+static void esp32_uart_start_tx (struct uart_port *port) +{ +esp32_uart_transmit_buffer (port); +} + +static void esp32_uart_stop_rx (struct uart_port *port) +{ +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena & ~UART_RXFIFO_FULL_INT_MASK ); +} + +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; + +spin_lock_irqsave (&port->lock, flags); +esp32_uart_write (port,UART_CONF1_REG , + (1 <<UART_RXFIFO_FULL_THRHD_SHIFT ) | + (1 <<port_variant (port)->txfifo_empty_thrhd_shift)); + esp32_uart_write(port, UART_INT_CLR_REG,
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 336
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+UART_RXFIFO_FULL_INT_MASK | + UART_BRK_DET_INT_MASK); +esp32_uart_write (port,UART_INT_ENA_REG , +UART_RXFIFO_FULL_INT_MASK | +UART_BRK_DET_INT_MASK ); +spin_unlock_irqrestore (&port->lock, flags);So interrupts can be coming now, but you don't handle them yet?
Reordered it with the irq handler installation.
Quoted 4 lines
drivers/tty/serial/esp32_uart.c line 339
Quoted 2 lines
drivers/tty/serial/esp32_uart.c new file +2
-0+ ret =devm_request_irq (port->dev, port->irq, esp32_uart_int,0 , +DRIVER_NAME , port);You don't disable clk and interrupts in case of failure?
Fixed.
Quoted 18 lines
Quoted 7 lines
drivers/tty/serial/esp32_uart.c new file +7
-0+pr_debug ("%s, request_irq: %d, conf1 = %08x, int_st = %08x, status = %08x\n" , + __func__, ret, +esp32_uart_read (port,UART_CONF1_REG ), +esp32_uart_read (port,UART_INT_ST_REG ), +esp32_uart_read (port,UART_STATUS_REG )); +return ret; +}...
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+static void esp32_uart_set_termios (struct uart_port *port, +struct ktermios *termios, +const struct ktermios *old) +{ +unsigned long flags; +u32 conf0, conf1; +u32 baud; +const u32 rx_flow_en =port_variant (port)->rx_flow_en; + +spin_lock_irqsave (&port->lock, flags);
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 389
Quoted 3 lines
drivers/tty/serial/esp32_uart.c new file +3
-0+ conf0 =esp32_uart_read (port,UART_CONF0_REG ) & + ~(UART_PARITY_EN_MASK |UART_PARITY_MASK | +UART_BIT_NUM_MASK |UART_STOP_BIT_NUM_MASK );Perhaps it would be more readable as:
conf0 =esp32_uart_read(port, UART_CONF0_REG);
conf0 &= ~(UART_PARITY_EN_MASK | ...);
?
Ok.
Quoted 10 lines
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+ conf1 =esp32_uart_read (port,UART_CONF1_REG ) & + ~rx_flow_en; + +if (termios->c_cflag &PARENB ) { + conf0 |=UART_PARITY_EN_MASK ; +if (termios->c_cflag &PARODD ) + conf0 |=UART_PARITY_MASK ; + }
Quoted 10 lines
drivers/tty/serial/esp32_uart.c line 448
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+static void esp32_uart_release_port (struct uart_port *port) +{ +} + +static int esp32_uart_request_port (struct uart_port *port) +{ +return 0 ; +}Drop these two.
Ok
Quoted 56 lines
Quoted 56 lines
drivers/tty/serial/esp32_uart.c new file +56
-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 ; + 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; +
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 718
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+ esp32_uart_ports[port->line] = sport; + +platform_set_drvdata (pdev, port); + + ret =uart_add_one_port (&esp32_uart_reg, port); +return ret;You can skip ret here and return directly.
Ok
Signature · 2 lines
Thanks. -- Max
Re: [PATCH 2/4] drivers/tty/serial: add driver for the ESP32 UART
In reply to Max FilippovQuoted 79 lines · Max Filippov
Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:
https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf
(Chapter 13 UART Controller)
https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
(Chapter 26 UART Controller)Signed-off-by Max Filippov <[email protected]>--- drivers/tty/serial/Kconfig | 13 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/esp32_uart.c | 766 +++++++++++++++++++++++++++++++ include/uapi/linux/serial_core.h | 3 + 4 files changed, 783 insertions(+) create mode 100644 drivers/tty/serial/esp32_uart.cdrivers/tty/serial/Kconfig +13
-0@@ -1578,6 +1578,19 @@ config SERIAL_NUVOTON_MA35D1_CONSOLE but you can alter that using a kernel command line option such as "console=ttyNVTx". +config SERIAL_ESP32 + tristate "Espressif ESP32 UART support" + depends on XTENSA_PLATFORM_ESP32 || (COMPILE_TEST && OF) + select SERIAL_CORE + select SERIAL_CORE_CONSOLE + select SERIAL_EARLYCON + help + Driver for the UART controllers of the Espressif ESP32xx SoCs. + When earlycon option is enabled the following kernel command line + snippets may be used: + earlycon=esp32s3uart,mmio32,0x60000000,115200n8,40000000 + earlycon=esp32uart,mmio32,0x3ff40000,115200n8 + endmenu config SERIAL_MCTRL_GPIOdrivers/tty/serial/Makefile +1
-0@@ -88,6 +88,7 @@ obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o 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 # GPIOLIB helpers for modem control lines obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.odrivers/tty/serial/esp32_uart.c new file +20
-0@@ -0,0 +1,766 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <linux/clk.h> +#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 "esp32-uart" +#define DEV_NAME "ttyS" +#define UART_NR 3 + +#define ESP32_UART_TX_FIFO_SIZE 127
drivers/tty/serial/esp32_uart.c line 30
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+#define ESP32_UART_RX_FIFO_SIZE 127 + +#define UART_FIFO_REG 0x00 +#define UART_INT_RAW_REG 0x04 +#define UART_INT_ST_REG 0x08 +#define UART_INT_ENA_REG 0x0c +#define UART_INT_CLR_REG 0x10 +#define UART_RXFIFO_FULL_INT_MASK 0x00000001 +#define UART_TXFIFO_EMPTY_INT_MASK 0x00000002 +#define UART_BRK_DET_INT_MASK 0x00000080
Use BIT() for these and do not call them MASK if they're just for a single flag bit.
drivers/tty/serial/esp32_uart.c line 34
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+#define UART_CLKDIV_REG 0x14 +#define ESP32_UART_CLKDIV_MASK 0x000fffff +#define ESP32S3_UART_CLKDIV_MASK 0x00000fff +#define UART_CLKDIV_SHIFT 0
Use GENMASK() and drop the related *_SHIFT defines as FIELD_GET()/FIELD_PREP() will not need it.
Usually _MASK postfix is just waste of screen space and provides no useful information so consider dropping that as well from the names.
Quoted 110 lines
drivers/tty/serial/esp32_uart.c new file +110
-0+#define UART_CLKDIV_FRAG_MASK 0x00f00000 +#define UART_CLKDIV_FRAG_SHIFT 20 +#define UART_STATUS_REG 0x1c +#define ESP32_UART_RXFIFO_CNT_MASK 0x000000ff +#define ESP32S3_UART_RXFIFO_CNT_MASK 0x000003ff +#define UART_RXFIFO_CNT_SHIFT 0 +#define UART_DSRN_MASK 0x00002000 +#define UART_CTSN_MASK 0x00004000 +#define ESP32_UART_TXFIFO_CNT_MASK 0x00ff0000 +#define ESP32S3_UART_TXFIFO_CNT_MASK 0x03ff0000 +#define UART_TXFIFO_CNT_SHIFT 16 +#define UART_ST_UTX_OUT_MASK 0x0f000000 +#define UART_ST_UTX_OUT_IDLE 0x00000000 +#define UART_ST_UTX_OUT_SHIFT 24 +#define UART_CONF0_REG 0x20 +#define UART_PARITY_MASK 0x00000001 +#define UART_PARITY_EN_MASK 0x00000002 +#define UART_BIT_NUM_MASK 0x0000000c +#define UART_BIT_NUM_5 0x00000000 +#define UART_BIT_NUM_6 0x00000004 +#define UART_BIT_NUM_7 0x00000008 +#define UART_BIT_NUM_8 0x0000000c +#define UART_STOP_BIT_NUM_MASK 0x00000030 +#define UART_STOP_BIT_NUM_1 0x00000010 +#define UART_STOP_BIT_NUM_2 0x00000030 +#define UART_SW_RTS_MASK 0x00000040 +#define UART_SW_DTR_MASK 0x00000080 +#define UART_LOOPBACK_MASK 0x00004000 +#define UART_TX_FLOW_EN_MASK 0x00008000 +#define UART_RTS_INV_MASK 0x00800000 +#define UART_DTR_INV_MASK 0x01000000 +#define ESP32_UART_TICK_REF_ALWAYS_ON_MASK 0x08000000 +#define ESP32S3_UART_TICK_REF_ALWAYS_ON_MASK 0x00000000 +#define UART_CONF1_REG 0x24 +#define ESP32_UART_RXFIFO_FULL_THRHD_MASK 0x0000007f +#define ESP32S3_UART_RXFIFO_FULL_THRHD_MASK 0x000003ff +#define UART_RXFIFO_FULL_THRHD_SHIFT 0 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_MASK 0x00007f00 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_MASK 0x000ffc00 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_SHIFT 8 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_SHIFT 10 +#define ESP32_UART_RX_FLOW_EN_MASK 0x00800000 +#define ESP32S3_UART_RX_FLOW_EN_MASK 0x00400000 + +struct esp32_port { +struct uart_port port; +struct clk *clk; +}; + +struct esp32_uart_variant { +u32 clkdiv_mask; +u32 rxfifo_cnt_mask; +u32 txfifo_cnt_mask; +u32 rxfifo_full_thrhd_mask; +u32 txfifo_empty_thrhd_mask; +u32 txfifo_empty_thrhd_shift; +u32 rx_flow_en; +const char *type; +}; + +static const struct esp32_uart_variant esp32_variant = { + .clkdiv_mask =ESP32_UART_CLKDIV_MASK , + .rxfifo_cnt_mask =ESP32_UART_RXFIFO_CNT_MASK , + .txfifo_cnt_mask =ESP32_UART_TXFIFO_CNT_MASK , + .rxfifo_full_thrhd_mask =ESP32_UART_RXFIFO_FULL_THRHD_MASK , + .txfifo_empty_thrhd_mask =ESP32_UART_TXFIFO_EMPTY_THRHD_MASK , + .txfifo_empty_thrhd_shift =ESP32_UART_TXFIFO_EMPTY_THRHD_SHIFT , + .rx_flow_en =ESP32_UART_RX_FLOW_EN_MASK , + .type ="ESP32 UART" , +}; + +static const struct esp32_uart_variant esp32s3_variant = { + .clkdiv_mask =ESP32S3_UART_CLKDIV_MASK , + .rxfifo_cnt_mask =ESP32S3_UART_RXFIFO_CNT_MASK , + .txfifo_cnt_mask =ESP32S3_UART_TXFIFO_CNT_MASK , + .rxfifo_full_thrhd_mask =ESP32S3_UART_RXFIFO_FULL_THRHD_MASK , + .txfifo_empty_thrhd_mask =ESP32S3_UART_TXFIFO_EMPTY_THRHD_MASK , + .txfifo_empty_thrhd_shift =ESP32S3_UART_TXFIFO_EMPTY_THRHD_SHIFT , + .rx_flow_en =ESP32S3_UART_RX_FLOW_EN_MASK , + .type ="ESP32S3 UART" , +}; + +static const struct of_device_id esp32_uart_dt_ids[] = { + { + .compatible ="esp,esp32-uart" , + .data = &esp32_variant, + }, { + .compatible ="esp,esp32s3-uart" , + .data = &esp32s3_variant, + }, {/* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, esp32_uart_dt_ids); + +static struct esp32_port *esp32_uart_ports[UART_NR ]; + +static const struct esp32_uart_variant *port_variant (struct uart_port *port) +{ +return port->private_data; +} + +static void esp32_uart_write (struct uart_port *port,unsigned long reg,u32 v) +{ +writel (v, port->membase + reg); +} + +static u32 esp32_uart_read (struct uart_port *port,unsigned long reg) +{ +return readl (port->membase + reg); +} +
drivers/tty/serial/esp32_uart.c line 154
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+static u32 esp32_uart_tx_fifo_cnt (struct uart_port *port) +{ +return (esp32_uart_read (port,UART_STATUS_REG ) & +port_variant (port)->txfifo_cnt_mask) >>UART_TXFIFO_CNT_SHIFT ; +} + +static u32 esp32_uart_rx_fifo_cnt (struct uart_port *port) +{ +return (esp32_uart_read (port,UART_STATUS_REG ) & +port_variant (port)->rxfifo_cnt_mask) >>UART_RXFIFO_CNT_SHIFT ;
FIELD_GET().
Use more lines (and a local variable) instead of splitting one line. It will be more readable that way.
drivers/tty/serial/esp32_uart.c line 161
Quoted 7 lines
drivers/tty/serial/esp32_uart.c new file +7
-0+} + +/* return TIOCSER_TEMT when transmitter is not busy */ +static unsigned int esp32_uart_tx_empty (struct uart_port *port) +{ +u32 status =esp32_uart_read (port,UART_STATUS_REG ) & + (port_variant (port)->txfifo_cnt_mask |UART_ST_UTX_OUT_MASK );
Ditto.
drivers/tty/serial/esp32_uart.c line 172
Quoted 11 lines
drivers/tty/serial/esp32_uart.c new file +11
-0+ +pr_debug ("%s: %08x\n" , __func__, status); +return status ==UART_ST_UTX_OUT_IDLE ?TIOCSER_TEMT :0 ; +} + +static void esp32_uart_set_mctrl (struct uart_port *port,unsigned int mctrl) +{ +u32 conf0 =esp32_uart_read (port,UART_CONF0_REG ) & + ~(UART_LOOPBACK_MASK | +UART_SW_RTS_MASK |UART_RTS_INV_MASK | +UART_SW_DTR_MASK |UART_DTR_INV_MASK );
Ditto.
Quoted 18 lines
drivers/tty/serial/esp32_uart.c new file +18
-0+if (mctrl &TIOCM_RTS ) + conf0 |=UART_SW_RTS_MASK ; +if (mctrl &TIOCM_DTR ) + conf0 |=UART_SW_DTR_MASK ; +if (mctrl &TIOCM_LOOP ) + conf0 |=UART_LOOPBACK_MASK ; + +esp32_uart_write (port,UART_CONF0_REG , conf0); +} + +static unsigned int esp32_uart_get_mctrl (struct uart_port *port) +{ +u32 status =esp32_uart_read (port,UART_STATUS_REG ); +unsigned int ret =TIOCM_CAR ; + +if (status &UART_DSRN_MASK ) + ret |=TIOCM_DSR ; +if (status &UART_CTSN_MASK )
drivers/tty/serial/esp32_uart.c line 201
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+ ret |=TIOCM_CTS ; + +return ret; +} + +static void esp32_uart_stop_tx (struct uart_port *port) +{ +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG );
int_ena &= ~UART_TXFIFO_EMPTY_INT_MASK;
drivers/tty/serial/esp32_uart.c line 203
Quoted 2 lines
drivers/tty/serial/esp32_uart.c new file +2
-0+esp32_uart_write (port,UART_INT_ENA_REG , + int_ena & ~UART_TXFIFO_EMPTY_INT_MASK );
Just int_ena is enough after adding the logic op on its own line.
Quoted 55 lines
drivers/tty/serial/esp32_uart.c new file +55
-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); + +for (i =0 ; i < rx_fifo_cnt; ++i) { +u32 rx =esp32_uart_read (port,UART_FIFO_REG ); +u32 brk =0 ; + + ++port->icount.rx; + +if (!rx) { + brk =esp32_uart_read (port,UART_INT_ST_REG ) & +UART_BRK_DET_INT_MASK ; + } +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 ); + } + } +spin_unlock_irqrestore (&port->lock, flags); + +tty_flip_buffer_push (tty_port); +} + +static void esp32_uart_put_char (struct uart_port *port,unsigned char c) +{ +esp32_uart_write (port,UART_FIFO_REG , c); +} + +static void esp32_uart_put_char_sync (struct uart_port *port,unsigned char c) +{ +while (esp32_uart_tx_fifo_cnt (port) >=ESP32_UART_TX_FIFO_SIZE ) +cpu_relax (); +esp32_uart_put_char (port, c); +} + +static void esp32_uart_transmit_buffer (struct uart_port *port) +{ +struct circ_buf *xmit = &port->state->xmit; +u32 tx_fifo_used =esp32_uart_tx_fifo_cnt (port);
drivers/tty/serial/esp32_uart.c line 268
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+ +while (!uart_circ_empty (xmit) && tx_fifo_used <ESP32_UART_TX_FIFO_SIZE ) { +esp32_uart_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);
Too much open-coding here. Please use the helpers (likely the ones which replace the whole tx loop + its common surrounding code).
drivers/tty/serial/esp32_uart.c line 277
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+if (uart_circ_empty (xmit)) { +esp32_uart_stop_tx (port); + }else { +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena |UART_TXFIFO_EMPTY_INT_MASK );
Own line for logic op.
Quoted 26 lines
drivers/tty/serial/esp32_uart.c new file +26
-0+ } +} + +static void esp32_uart_txint (struct uart_port *port) +{ +esp32_uart_transmit_buffer (port); +} + +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_MASK |UART_BRK_DET_INT_MASK )) +esp32_uart_rxint (port); +if (status &UART_TXFIFO_EMPTY_INT_MASK ) +esp32_uart_txint (port); + +esp32_uart_write (port,UART_INT_CLR_REG , status); +return IRQ_HANDLED ; +} + +static void esp32_uart_start_tx (struct uart_port *port) +{
drivers/tty/serial/esp32_uart.c line 313
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+esp32_uart_transmit_buffer (port); +} + +static void esp32_uart_stop_rx (struct uart_port *port) +{ +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena & ~UART_RXFIFO_FULL_INT_MASK );
Ditto.
Quoted 5 lines
drivers/tty/serial/esp32_uart.c new file +5
-0+} + +static int esp32_uart_startup (struct uart_port *port) +{ +int ret =0 ;
drivers/tty/serial/esp32_uart.c line 328
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-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; + +spin_lock_irqsave (&port->lock, flags); + esp32_uart_write(port, UART_CONF1_REG, + (1 << UART_RXFIFO_FULL_THRHD_SHIFT) |
Use FIELD_PREP()
drivers/tty/serial/esp32_uart.c line 332
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+ (1 << port_variant(port)->txfifo_empty_thrhd_shift)); +esp32_uart_write (port,UART_INT_CLR_REG , +UART_RXFIFO_FULL_INT_MASK | +UART_BRK_DET_INT_MASK );
This fits to two lines (well one too, especially if you remove those _MASK postfixes).
drivers/tty/serial/esp32_uart.c line 332
Quoted 3 lines
drivers/tty/serial/esp32_uart.c new file +3
-0+esp32_uart_write (port,UART_INT_ENA_REG , +UART_RXFIFO_FULL_INT_MASK | +UART_BRK_DET_INT_MASK );
Ditto.
Quoted 20 lines
drivers/tty/serial/esp32_uart.c new file +20
-0+spin_unlock_irqrestore (&port->lock, flags); + + ret =devm_request_irq (port->dev, port->irq, esp32_uart_int,0 , +DRIVER_NAME , port); + +pr_debug ("%s, request_irq: %d, conf1 = %08x, int_st = %08x, status = %08x\n" , + __func__, ret, +esp32_uart_read (port,UART_CONF1_REG ), +esp32_uart_read (port,UART_INT_ST_REG ), +esp32_uart_read (port,UART_STATUS_REG )); +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); +clk_disable_unprepare (sport->clk);
drivers/tty/serial/esp32_uart.c line 365
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+} + +static void 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 | (frag <<UART_CLKDIV_FRAG_SHIFT ));
FIELD_PREP(). Also div be encapsulated into FIELD_PREP here even if it's shift is 0.
drivers/tty/serial/esp32_uart.c line 369
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+ }else { + dev_warn(port->dev, + "%s: %d baud is out of reach, setting %d\n", + __func__, baud,
Don't print __func__.
drivers/tty/serial/esp32_uart.c line 373
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+ port->uartclk / port_variant(port)->clkdiv_mask); +esp32_uart_write (port,UART_CLKDIV_REG , +port_variant (port)->clkdiv_mask | +UART_CLKDIV_FRAG_MASK );
I think you want to make the meaning more obvious here by using FIELD_MAX(UART_CLKDIV_FRAG_MASK);
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+ } +} + +static void esp32_uart_set_termios (struct uart_port *port, +struct ktermios *termios, +const struct ktermios *old) +{ +unsigned long flags;
drivers/tty/serial/esp32_uart.c line 391
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+u32 conf0, conf1; +u32 baud; +const u32 rx_flow_en =port_variant (port)->rx_flow_en; + +spin_lock_irqsave (&port->lock, flags); + conf0 =esp32_uart_read (port,UART_CONF0_REG ) & + ~(UART_PARITY_EN_MASK |UART_PARITY_MASK | +UART_BIT_NUM_MASK |UART_STOP_BIT_NUM_MASK ); + conf1 =esp32_uart_read (port,UART_CONF1_REG ) & + ~rx_flow_en;
Split logic to oen lines please.
Quoted 19 lines
drivers/tty/serial/esp32_uart.c new file +19
-0+ +if (termios->c_cflag &PARENB ) { + conf0 |=UART_PARITY_EN_MASK ; +if (termios->c_cflag &PARODD ) + conf0 |=UART_PARITY_MASK ; + } + +switch (termios->c_cflag &CSIZE ) { +case CS5 : + conf0 |=UART_BIT_NUM_5 ; +break ; +case CS6 : + conf0 |=UART_BIT_NUM_6 ; +break ; +case CS7 : + conf0 |=UART_BIT_NUM_7 ; +break ; +case CS8 : + conf0 |=UART_BIT_NUM_8 ;
drivers/tty/serial/esp32_uart.c line 420
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+break ; + } + +if (termios->c_cflag &CSTOPB ) + conf0 |=UART_STOP_BIT_NUM_2 ; +else + conf0 |=UART_STOP_BIT_NUM_1 ; + +if (termios->c_cflag &CRTSCTS ) + conf1 |= rx_flow_en;
If you don't support some bits, you should clear them (CMSPAR).
drivers/tty/serial/esp32_uart.c line 431
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+esp32_uart_write (port,UART_CONF0_REG , conf0); +esp32_uart_write (port,UART_CONF1_REG , conf1); +spin_unlock_irqrestore (&port->lock, flags); + +/* +* esp32s3 may not support 9600, passing its minimal baud rate +* as the min argument would trigger a WARN inside uart_get_baud_rate +*/ + baud =uart_get_baud_rate (port, termios, old, +0 , port->uartclk /16 );
This looks questionable solution to the problem mentioned in the comment. What happens when user asks for baudrates below the minimum supported one?
You might need to do something touching the core code to handle the case where 9600 is not possible fallback.
drivers/tty/serial/esp32_uart.c line 434
Quoted 3 lines
drivers/tty/serial/esp32_uart.c new file +3
-0+if (baud) +esp32_uart_set_baud (port, baud); +}
set_termios() function lacks call to uart_update_timeout().
Quoted 96 lines
drivers/tty/serial/esp32_uart.c new file +96
-0+ +static const char *esp32_uart_type (struct uart_port *port) +{ +return port_variant (port)->type; +} + +static void esp32_uart_release_port (struct uart_port *port) +{ +} + +static int esp32_uart_request_port (struct uart_port *port) +{ +return 0 ; +} + +/* configure/auto-configure the port */ +static void esp32_uart_config_port (struct uart_port *port,int flags) +{ +if (flags &UART_CONFIG_TYPE ) + port->type =PORT_ESP32UART ; +} + +#ifdef CONFIG_CONSOLE_POLL +static int esp32_uart_poll_init (struct uart_port *port) +{ + struct esp32_port *sport = container_of(port, struct esp32_port, port); + +return clk_prepare_enable (sport->clk); +} + +static void esp32_uart_poll_put_char (struct uart_port *port,unsigned char c) +{ +esp32_uart_put_char_sync (port, c); +} + +static int esp32_uart_poll_get_char (struct uart_port *port) +{ +if (esp32_uart_rx_fifo_cnt (port)) +return esp32_uart_read (port,UART_FIFO_REG ); +else +return NO_POLL_CHAR ; + +} +#endif + +static const struct uart_ops esp32_uart_pops = { + .tx_empty = esp32_uart_tx_empty, + .set_mctrl = esp32_uart_set_mctrl, + .get_mctrl = esp32_uart_get_mctrl, + .stop_tx = esp32_uart_stop_tx, + .start_tx = esp32_uart_start_tx, + .stop_rx = esp32_uart_stop_rx, + .startup = esp32_uart_startup, + .shutdown = esp32_uart_shutdown, + .set_termios = esp32_uart_set_termios, + .type = esp32_uart_type, + .release_port = esp32_uart_release_port, + .request_port = esp32_uart_request_port, + .config_port = esp32_uart_config_port, +#ifdef CONFIG_CONSOLE_POLL + .poll_init = esp32_uart_poll_init, + .poll_put_char = esp32_uart_poll_put_char, + .poll_get_char = esp32_uart_poll_get_char, +#endif +}; + +static void esp32_uart_console_putchar (struct uart_port *port,unsigned char c) +{ +esp32_uart_put_char_sync (port, c); +} + +static void esp32_uart_string_write (struct uart_port *port,const char *s, +unsigned int count) +{ +uart_console_write (port, s, count, esp32_uart_console_putchar); +} + +static void +esp32_uart_console_write (struct console *co,const char *s,unsigned int count) +{ +struct esp32_port *sport = esp32_uart_ports[co->index]; +struct uart_port *port = &sport->port; +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); + +esp32_uart_string_write (port, s, count); + +if (locked) +spin_unlock_irqrestore (&port->lock, flags);
drivers/tty/serial/esp32_uart.c line 540
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+} + +static int __init esp32_uart_console_setup (struct console *co,char *options) +{ +struct esp32_port *sport; +int baud =115200 ; +int bits =8 ; +int parity ='n' ; +int flow ='n' ; +int ret;
This lacks newline after declarations.
Quoted 133 lines
drivers/tty/serial/esp32_uart.c new file +133
-0+/* +* check whether an invalid uart number has been specified, and +* if so, search for the first available port that does have +* console support. +*/ +if (co->index ==-1 || co->index >=ARRAY_SIZE (esp32_uart_ports)) + co->index =0 ; + + sport = esp32_uart_ports[co->index]; +if (!sport) +return -ENODEV ; + + ret =clk_prepare_enable (sport->clk); +if (ret) +return ret; + +if (options) +uart_parse_options (options, &baud, &parity, &bits, &flow); + +return uart_set_options (&sport->port, co, baud, parity, bits, flow); +} + +static int esp32_uart_console_exit (struct console *co) +{ +struct esp32_port *sport = esp32_uart_ports[co->index]; + +clk_disable_unprepare (sport->clk); +return 0 ; +} + +static struct uart_driver esp32_uart_reg; +static struct console esp32_uart_console = { + .name =DEV_NAME , + .write = esp32_uart_console_write, + .device = uart_console_device, + .setup = esp32_uart_console_setup, + .exit = esp32_uart_console_exit, + .flags =CON_PRINTBUFFER , + .index =-1 , + .data = &esp32_uart_reg, +}; + +static void esp32_uart_earlycon_putchar (struct uart_port *port,unsigned char c) +{ +esp32_uart_put_char_sync (port, c); +} + +static void esp32_uart_earlycon_write (struct console *con,const char *s, +unsigned int n) +{ +struct earlycon_device *dev = con->data; + +uart_console_write (&dev->port, s, n, esp32_uart_earlycon_putchar); +} + +#ifdef CONFIG_CONSOLE_POLL +static int esp32_uart_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 =esp32_uart_poll_get_char (&dev->port); + +if (c ==NO_POLL_CHAR ) +break ; + s[num_read++] = c; + } +return num_read; +} +#endif + +static int __init esp32xx_uart_early_console_setup (struct earlycon_device *device, +const char *options) +{ +if (!device->port.membase) +return -ENODEV ; + + device->con->write = esp32_uart_earlycon_write; +#ifdef CONFIG_CONSOLE_POLL + device->con->read = esp32_uart_earlycon_read; +#endif +if (device->port.uartclk !=BASE_BAUD *16 ) +esp32_uart_set_baud (&device->port, device->baud); + +return 0 ; +} + +static int __init esp32_uart_early_console_setup (struct earlycon_device *device, +const char *options) +{ + device->port.private_data = (void *)&esp32_variant; +return esp32xx_uart_early_console_setup (device, options); +} + +OF_EARLYCON_DECLARE (esp32uart,"esp,esp32-uart" , + esp32_uart_early_console_setup); + +static int __init esp32s3_uart_early_console_setup (struct earlycon_device *device, +const char *options) +{ + device->port.private_data = (void *)&esp32s3_variant; +return esp32xx_uart_early_console_setup (device, options); +} + +OF_EARLYCON_DECLARE (esp32s3uart,"esp,esp32s3-uart" , + esp32s3_uart_early_console_setup); + +static struct uart_driver esp32_uart_reg = { + .owner =THIS_MODULE , + .driver_name =DRIVER_NAME , + .dev_name =DEV_NAME , + .nr =ARRAY_SIZE (esp32_uart_ports), + .cons = &esp32_uart_console, +}; + +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 ; +
drivers/tty/serial/esp32_uart.c line 683
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+ 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 );
One line.
Quoted 71 lines
drivers/tty/serial/esp32_uart.c new file +71
-0+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; + + esp32_uart_ports[port->line] = sport; + +platform_set_drvdata (pdev, port); + + ret =uart_add_one_port (&esp32_uart_reg, port); +return ret; +} + +static int esp32_uart_remove (struct platform_device *pdev) +{ +struct uart_port *port =platform_get_drvdata (pdev); + +uart_remove_one_port (&esp32_uart_reg, port); + +return 0 ; +} + + +static struct platform_driver esp32_uart_driver = { + .probe = esp32_uart_probe, + .remove = esp32_uart_remove, + .driver = { + .name =DRIVER_NAME , + .of_match_table = esp32_uart_dt_ids, + }, +}; + +static int __init esp32_uart_init (void ) +{ +int ret; + + ret =uart_register_driver (&esp32_uart_reg); +if (ret) +return ret; + + ret =platform_driver_register (&esp32_uart_driver); +if (ret) +uart_unregister_driver (&esp32_uart_reg); + +return ret; +} +
drivers/tty/serial/esp32_uart.c line 764
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+static void __exit esp32_uart_exit(void) +{ +platform_driver_unregister (&esp32_uart_driver); +uart_unregister_driver (&esp32_uart_reg); +} + +module_init (esp32_uart_init); +module_exit (esp32_uart_exit); + +MODULE_DESCRIPTION ("Espressif ESP32 UART driver." );
Drop .
Quoted 15 lines
drivers/tty/serial/esp32_uart.c new file +2
-0+MODULE_AUTHOR ("Max Filippov <[email protected]>" ); +MODULE_LICENSE ("GPL" );include/uapi/linux/serial_core.h +3
-0@@ -245,4 +245,7 @@/* Sunplus UART */ #define PORT_SUNPLUS 123 +/* Espressif ESP32 UART */ +#define PORT_ESP32UART 124 +#endif /* _UAPILINUX_SERIAL_CORE_H */
Signature · 1 line
i.
Re: [PATCH 2/4] drivers/tty/serial: add driver for the ESP32 UART
Quoted 87 lines · Ilpo Järvinen
Quoted 84 lines · Max Filippov
Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:
https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf
(Chapter 13 UART Controller)
https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
(Chapter 26 UART Controller)Signed-off-by Max Filippov <[email protected]>--- drivers/tty/serial/Kconfig | 13 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/esp32_uart.c | 766 +++++++++++++++++++++++++++++++ include/uapi/linux/serial_core.h | 3 + 4 files changed, 783 insertions(+) create mode 100644 drivers/tty/serial/esp32_uart.cdrivers/tty/serial/Kconfig +13
-0@@ -1578,6 +1578,19 @@ config SERIAL_NUVOTON_MA35D1_CONSOLE but you can alter that using a kernel command line option such as "console=ttyNVTx". +config SERIAL_ESP32 + tristate "Espressif ESP32 UART support" + depends on XTENSA_PLATFORM_ESP32 || (COMPILE_TEST && OF) + select SERIAL_CORE + select SERIAL_CORE_CONSOLE + select SERIAL_EARLYCON + help + Driver for the UART controllers of the Espressif ESP32xx SoCs. + When earlycon option is enabled the following kernel command line + snippets may be used: + earlycon=esp32s3uart,mmio32,0x60000000,115200n8,40000000 + earlycon=esp32uart,mmio32,0x3ff40000,115200n8 + endmenu config SERIAL_MCTRL_GPIOdrivers/tty/serial/Makefile +1
-0@@ -88,6 +88,7 @@ obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o 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 # GPIOLIB helpers for modem control lines obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.odrivers/tty/serial/esp32_uart.c new file +25
-0@@ -0,0 +1,766 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <linux/clk.h> +#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 "esp32-uart" +#define DEV_NAME "ttyS" +#define UART_NR 3 + +#define ESP32_UART_TX_FIFO_SIZE 127 +#define ESP32_UART_RX_FIFO_SIZE 127 + +#define UART_FIFO_REG 0x00 +#define UART_INT_RAW_REG 0x04 +#define UART_INT_ST_REG 0x08
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 30
Quoted 5 lines
drivers/tty/serial/esp32_uart.c new file +5
-0+#define UART_INT_ENA_REG 0x0c +#define UART_INT_CLR_REG 0x10 +#define UART_RXFIFO_FULL_INT_MASK 0x00000001 +#define UART_TXFIFO_EMPTY_INT_MASK 0x00000002 +#define UART_BRK_DET_INT_MASK 0x00000080Use
BIT()for these and do not call them MASK if they're just for a single flag bit.
Ok.
Quoted 7 lines
drivers/tty/serial/esp32_uart.c line 34
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+#define UART_CLKDIV_REG 0x14 +#define ESP32_UART_CLKDIV_MASK 0x000fffff +#define ESP32S3_UART_CLKDIV_MASK 0x00000fff +#define UART_CLKDIV_SHIFT 0Use
GENMASK()and drop the related*_SHIFTdefines as FIELD_GET()/FIELD_PREP() will not need it.
Ok.
Quoted 117 lines
Usually
_MASKpostfix is just waste of screen space and provides no useful information so consider dropping that as well from the names.Quoted 114 lines
drivers/tty/serial/esp32_uart.c new file +114
-0+#define UART_CLKDIV_FRAG_MASK 0x00f00000 +#define UART_CLKDIV_FRAG_SHIFT 20 +#define UART_STATUS_REG 0x1c +#define ESP32_UART_RXFIFO_CNT_MASK 0x000000ff +#define ESP32S3_UART_RXFIFO_CNT_MASK 0x000003ff +#define UART_RXFIFO_CNT_SHIFT 0 +#define UART_DSRN_MASK 0x00002000 +#define UART_CTSN_MASK 0x00004000 +#define ESP32_UART_TXFIFO_CNT_MASK 0x00ff0000 +#define ESP32S3_UART_TXFIFO_CNT_MASK 0x03ff0000 +#define UART_TXFIFO_CNT_SHIFT 16 +#define UART_ST_UTX_OUT_MASK 0x0f000000 +#define UART_ST_UTX_OUT_IDLE 0x00000000 +#define UART_ST_UTX_OUT_SHIFT 24 +#define UART_CONF0_REG 0x20 +#define UART_PARITY_MASK 0x00000001 +#define UART_PARITY_EN_MASK 0x00000002 +#define UART_BIT_NUM_MASK 0x0000000c +#define UART_BIT_NUM_5 0x00000000 +#define UART_BIT_NUM_6 0x00000004 +#define UART_BIT_NUM_7 0x00000008 +#define UART_BIT_NUM_8 0x0000000c +#define UART_STOP_BIT_NUM_MASK 0x00000030 +#define UART_STOP_BIT_NUM_1 0x00000010 +#define UART_STOP_BIT_NUM_2 0x00000030 +#define UART_SW_RTS_MASK 0x00000040 +#define UART_SW_DTR_MASK 0x00000080 +#define UART_LOOPBACK_MASK 0x00004000 +#define UART_TX_FLOW_EN_MASK 0x00008000 +#define UART_RTS_INV_MASK 0x00800000 +#define UART_DTR_INV_MASK 0x01000000 +#define ESP32_UART_TICK_REF_ALWAYS_ON_MASK 0x08000000 +#define ESP32S3_UART_TICK_REF_ALWAYS_ON_MASK 0x00000000 +#define UART_CONF1_REG 0x24 +#define ESP32_UART_RXFIFO_FULL_THRHD_MASK 0x0000007f +#define ESP32S3_UART_RXFIFO_FULL_THRHD_MASK 0x000003ff +#define UART_RXFIFO_FULL_THRHD_SHIFT 0 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_MASK 0x00007f00 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_MASK 0x000ffc00 +#define ESP32_UART_TXFIFO_EMPTY_THRHD_SHIFT 8 +#define ESP32S3_UART_TXFIFO_EMPTY_THRHD_SHIFT 10 +#define ESP32_UART_RX_FLOW_EN_MASK 0x00800000 +#define ESP32S3_UART_RX_FLOW_EN_MASK 0x00400000 + +struct esp32_port { +struct uart_port port; +struct clk *clk; +}; + +struct esp32_uart_variant { +u32 clkdiv_mask; +u32 rxfifo_cnt_mask; +u32 txfifo_cnt_mask; +u32 rxfifo_full_thrhd_mask; +u32 txfifo_empty_thrhd_mask; +u32 txfifo_empty_thrhd_shift; +u32 rx_flow_en; +const char *type; +}; + +static const struct esp32_uart_variant esp32_variant = { + .clkdiv_mask =ESP32_UART_CLKDIV_MASK , + .rxfifo_cnt_mask =ESP32_UART_RXFIFO_CNT_MASK , + .txfifo_cnt_mask =ESP32_UART_TXFIFO_CNT_MASK , + .rxfifo_full_thrhd_mask =ESP32_UART_RXFIFO_FULL_THRHD_MASK , + .txfifo_empty_thrhd_mask =ESP32_UART_TXFIFO_EMPTY_THRHD_MASK , + .txfifo_empty_thrhd_shift =ESP32_UART_TXFIFO_EMPTY_THRHD_SHIFT , + .rx_flow_en =ESP32_UART_RX_FLOW_EN_MASK , + .type ="ESP32 UART" , +}; + +static const struct esp32_uart_variant esp32s3_variant = { + .clkdiv_mask =ESP32S3_UART_CLKDIV_MASK , + .rxfifo_cnt_mask =ESP32S3_UART_RXFIFO_CNT_MASK , + .txfifo_cnt_mask =ESP32S3_UART_TXFIFO_CNT_MASK , + .rxfifo_full_thrhd_mask =ESP32S3_UART_RXFIFO_FULL_THRHD_MASK , + .txfifo_empty_thrhd_mask =ESP32S3_UART_TXFIFO_EMPTY_THRHD_MASK , + .txfifo_empty_thrhd_shift =ESP32S3_UART_TXFIFO_EMPTY_THRHD_SHIFT , + .rx_flow_en =ESP32S3_UART_RX_FLOW_EN_MASK , + .type ="ESP32S3 UART" , +}; + +static const struct of_device_id esp32_uart_dt_ids[] = { + { + .compatible ="esp,esp32-uart" , + .data = &esp32_variant, + }, { + .compatible ="esp,esp32s3-uart" , + .data = &esp32s3_variant, + }, {/* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, esp32_uart_dt_ids); + +static struct esp32_port *esp32_uart_ports[UART_NR ]; + +static const struct esp32_uart_variant *port_variant (struct uart_port *port) +{ +return port->private_data; +} + +static void esp32_uart_write (struct uart_port *port,unsigned long reg,u32 v) +{ +writel (v, port->membase + reg); +} + +static u32 esp32_uart_read (struct uart_port *port,unsigned long reg) +{ +return readl (port->membase + reg); +} + +static u32 esp32_uart_tx_fifo_cnt (struct uart_port *port) +{ +return (esp32_uart_read (port,UART_STATUS_REG ) & +port_variant (port)->txfifo_cnt_mask) >>UART_TXFIFO_CNT_SHIFT ;
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 154
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+} + +static u32 esp32_uart_rx_fifo_cnt (struct uart_port *port) +{ +return (esp32_uart_read (port,UART_STATUS_REG ) & +port_variant (port)->rxfifo_cnt_mask) >>UART_RXFIFO_CNT_SHIFT ;
FIELD_GET().
I see how FIELD_GET can be used in other places, but here port_variant(port)->rxfifo_cnt_mask is not a runtime constant and FIELD_GET does not work with non-constant field definitions. Any suggestions?
Quoted 2 lines
Use more lines (and a local variable) instead of splitting one line. It will be more readable that way.
Ok.
Quoted 10 lines
drivers/tty/serial/esp32_uart.c line 161
Quoted 7 lines
drivers/tty/serial/esp32_uart.c new file +7
-0+} + +/* return TIOCSER_TEMT when transmitter is not busy */ +static unsigned int esp32_uart_tx_empty (struct uart_port *port) +{ +u32 status =esp32_uart_read (port,UART_STATUS_REG ) & + (port_variant (port)->txfifo_cnt_mask |UART_ST_UTX_OUT_MASK );Ditto.
Quoted 13 lines
drivers/tty/serial/esp32_uart.c line 172
Quoted 11 lines
drivers/tty/serial/esp32_uart.c new file +11
-0+ +pr_debug ("%s: %08x\n" , __func__, status); +return status ==UART_ST_UTX_OUT_IDLE ?TIOCSER_TEMT :0 ; +} + +static void esp32_uart_set_mctrl (struct uart_port *port,unsigned int mctrl) +{ +u32 conf0 =esp32_uart_read (port,UART_CONF0_REG ) & + ~(UART_LOOPBACK_MASK | +UART_SW_RTS_MASK |UART_RTS_INV_MASK | +UART_SW_DTR_MASK |UART_DTR_INV_MASK );Ditto.
Ok.
Quoted 31 lines
Quoted 18 lines
drivers/tty/serial/esp32_uart.c new file +18
-0+if (mctrl &TIOCM_RTS ) + conf0 |=UART_SW_RTS_MASK ; +if (mctrl &TIOCM_DTR ) + conf0 |=UART_SW_DTR_MASK ; +if (mctrl &TIOCM_LOOP ) + conf0 |=UART_LOOPBACK_MASK ; + +esp32_uart_write (port,UART_CONF0_REG , conf0); +} + +static unsigned int esp32_uart_get_mctrl (struct uart_port *port) +{ +u32 status =esp32_uart_read (port,UART_STATUS_REG ); +unsigned int ret =TIOCM_CAR ; + +if (status &UART_DSRN_MASK ) + ret |=TIOCM_DSR ; +if (status &UART_CTSN_MASK )drivers/tty/serial/esp32_uart.c line 201
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+ ret |=TIOCM_CTS ; + +return ret; +} + +static void esp32_uart_stop_tx (struct uart_port *port) +{ +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG );int_ena &= ~UART_TXFIFO_EMPTY_INT_MASK;
Quoted 4 lines
drivers/tty/serial/esp32_uart.c line 203
Quoted 2 lines
drivers/tty/serial/esp32_uart.c new file +2
-0+esp32_uart_write (port,UART_INT_ENA_REG , + int_ena & ~UART_TXFIFO_EMPTY_INT_MASK );Just
int_enais enough after adding the logic op on its own line.
Ok.
Quoted 60 lines
Quoted 60 lines
drivers/tty/serial/esp32_uart.c new file +60
-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); + +for (i =0 ; i < rx_fifo_cnt; ++i) { +u32 rx =esp32_uart_read (port,UART_FIFO_REG ); +u32 brk =0 ; + + ++port->icount.rx; + +if (!rx) { + brk =esp32_uart_read (port,UART_INT_ST_REG ) & +UART_BRK_DET_INT_MASK ; + } +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 ); + } + } +spin_unlock_irqrestore (&port->lock, flags); + +tty_flip_buffer_push (tty_port); +} + +static void esp32_uart_put_char (struct uart_port *port,unsigned char c) +{ +esp32_uart_write (port,UART_FIFO_REG , c); +} + +static void esp32_uart_put_char_sync (struct uart_port *port,unsigned char c) +{ +while (esp32_uart_tx_fifo_cnt (port) >=ESP32_UART_TX_FIFO_SIZE ) +cpu_relax (); +esp32_uart_put_char (port, c); +} + +static void esp32_uart_transmit_buffer (struct uart_port *port) +{ +struct circ_buf *xmit = &port->state->xmit; +u32 tx_fifo_used =esp32_uart_tx_fifo_cnt (port); + +while (!uart_circ_empty (xmit) && tx_fifo_used <ESP32_UART_TX_FIFO_SIZE ) { +esp32_uart_put_char (port, xmit->buf[xmit->tail]); + xmit->tail = (xmit->tail +1 ) & (UART_XMIT_SIZE -1 ); + port->icount.tx++;
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 268
Quoted 5 lines
drivers/tty/serial/esp32_uart.c new file +5
-0+ ++tx_fifo_used; + } + +if (uart_circ_chars_pending (xmit) <WAKEUP_CHARS ) +uart_write_wakeup (port);Too much open-coding here. Please use the helpers (likely the ones which replace the whole tx loop + its common surrounding code).
Ok.
Quoted 10 lines
drivers/tty/serial/esp32_uart.c line 277
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +8
-0+if (uart_circ_empty (xmit)) { +esp32_uart_stop_tx (port); + }else { +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena |UART_TXFIFO_EMPTY_INT_MASK );Own line for logic op.
Ok.
Quoted 30 lines
Quoted 30 lines
drivers/tty/serial/esp32_uart.c new file +30
-0+ } +} + +static void esp32_uart_txint (struct uart_port *port) +{ +esp32_uart_transmit_buffer (port); +} + +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_MASK |UART_BRK_DET_INT_MASK )) +esp32_uart_rxint (port); +if (status &UART_TXFIFO_EMPTY_INT_MASK ) +esp32_uart_txint (port); + +esp32_uart_write (port,UART_INT_CLR_REG , status); +return IRQ_HANDLED ; +} + +static void esp32_uart_start_tx (struct uart_port *port) +{ +esp32_uart_transmit_buffer (port); +} + +static void esp32_uart_stop_rx (struct uart_port *port)
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 313
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+{ +u32 int_ena; + + int_ena =esp32_uart_read (port,UART_INT_ENA_REG ); +esp32_uart_write (port,UART_INT_ENA_REG , + int_ena & ~UART_RXFIFO_FULL_INT_MASK );Ditto.
Ok.
Quoted 18 lines
Quoted 5 lines
drivers/tty/serial/esp32_uart.c new file +5
-0+} + +static int esp32_uart_startup (struct uart_port *port) +{ +int ret =0 ;drivers/tty/serial/esp32_uart.c line 328
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-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; + +spin_lock_irqsave (&port->lock, flags); + esp32_uart_write(port, UART_CONF1_REG, + (1 << UART_RXFIFO_FULL_THRHD_SHIFT) |Use
FIELD_PREP()
Quoted 7 lines
drivers/tty/serial/esp32_uart.c line 332
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+ (1 << port_variant(port)->txfifo_empty_thrhd_shift)); +esp32_uart_write (port,UART_INT_CLR_REG , +UART_RXFIFO_FULL_INT_MASK | +UART_BRK_DET_INT_MASK );This fits to two lines (well one too, especially if you remove those
_MASKpostfixes).
Ok.
Quoted 5 lines
drivers/tty/serial/esp32_uart.c line 332
Quoted 3 lines
drivers/tty/serial/esp32_uart.c new file +3
-0+esp32_uart_write (port,UART_INT_ENA_REG , +UART_RXFIFO_FULL_INT_MASK | +UART_BRK_DET_INT_MASK );Ditto.
Ok.
Quoted 24 lines
Quoted 24 lines
drivers/tty/serial/esp32_uart.c new file +24
-0+spin_unlock_irqrestore (&port->lock, flags); + + ret =devm_request_irq (port->dev, port->irq, esp32_uart_int,0 , +DRIVER_NAME , port); + +pr_debug ("%s, request_irq: %d, conf1 = %08x, int_st = %08x, status = %08x\n" , + __func__, ret, +esp32_uart_read (port,UART_CONF1_REG ), +esp32_uart_read (port,UART_INT_ST_REG ), +esp32_uart_read (port,UART_STATUS_REG )); +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); +clk_disable_unprepare (sport->clk); +} + +static void esp32_uart_set_baud (struct uart_port *port,u32 baud) +{
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 365
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+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 | (frag <<UART_CLKDIV_FRAG_SHIFT ));
FIELD_PREP().
Ok.
Quoted 2 lines
Also div be encapsulated into
FIELD_PREPhere even if it's shift is 0.
This field's mask, port_variant(port)->clkdiv_mask, is, again, not a runtime constant. Any suggestion for this?
Quoted 6 lines
drivers/tty/serial/esp32_uart.c line 369
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+ }else { + dev_warn(port->dev, + "%s: %d baud is out of reach, setting %d\n", + __func__, baud,Don't print
__func__.
Ok.
Quoted 7 lines
drivers/tty/serial/esp32_uart.c line 373
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+ port->uartclk / port_variant(port)->clkdiv_mask); +esp32_uart_write (port,UART_CLKDIV_REG , +port_variant (port)->clkdiv_mask | +UART_CLKDIV_FRAG_MASK );I think you want to make the meaning more obvious here by using
FIELD_MAX(UART_CLKDIV_FRAG_MASK);
No. I think FIELD_MAX makes it less obvious, because to work properly it must be not just
FIELD_MAX(UART_CLKDIV_FRAG),
but
FIELD_PREP(UART_CLKDIV_FRAG, FIELD_MAX(UART_CLKDIV_FRAG)).
Quoted 12 lines
Quoted 12 lines
drivers/tty/serial/esp32_uart.c new file +12
-0+ } +} + +static void esp32_uart_set_termios (struct uart_port *port, +struct ktermios *termios, +const struct ktermios *old) +{ +unsigned long flags; +u32 conf0, conf1; +u32 baud; +const u32 rx_flow_en =port_variant (port)->rx_flow_en; +
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 391
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+spin_lock_irqsave (&port->lock, flags); + conf0 =esp32_uart_read (port,UART_CONF0_REG ) & + ~(UART_PARITY_EN_MASK |UART_PARITY_MASK | +UART_BIT_NUM_MASK |UART_STOP_BIT_NUM_MASK ); + conf1 =esp32_uart_read (port,UART_CONF1_REG ) & + ~rx_flow_en;Split logic to oen lines please.
Ok.
Quoted 23 lines
Quoted 23 lines
drivers/tty/serial/esp32_uart.c new file +23
-0+ +if (termios->c_cflag &PARENB ) { + conf0 |=UART_PARITY_EN_MASK ; +if (termios->c_cflag &PARODD ) + conf0 |=UART_PARITY_MASK ; + } + +switch (termios->c_cflag &CSIZE ) { +case CS5 : + conf0 |=UART_BIT_NUM_5 ; +break ; +case CS6 : + conf0 |=UART_BIT_NUM_6 ; +break ; +case CS7 : + conf0 |=UART_BIT_NUM_7 ; +break ; +case CS8 : + conf0 |=UART_BIT_NUM_8 ; +break ; + } + +if (termios->c_cflag &CSTOPB )
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 420
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+ conf0 |=UART_STOP_BIT_NUM_2 ; + else + conf0 |= UART_STOP_BIT_NUM_1; + +if (termios->c_cflag &CRTSCTS ) + conf1 |= rx_flow_en;If you don't support some bits, you should clear them (CMSPAR).
Ok.
Quoted 11 lines
drivers/tty/serial/esp32_uart.c line 431
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+esp32_uart_write (port,UART_CONF0_REG , conf0); +esp32_uart_write (port,UART_CONF1_REG , conf1); +spin_unlock_irqrestore (&port->lock, flags); + +/* +* esp32s3 may not support 9600, passing its minimal baud rate +* as the min argument would trigger a WARN inside uart_get_baud_rate +*/ + baud =uart_get_baud_rate (port, termios, old, +0 , port->uartclk /16 );
Quoted 2 lines
This looks questionable solution to the problem mentioned in the comment. What happens when user asks for baudrates below the minimum supported one?
I'll change it to refuse to change to unsupported baud rate and set default to 115200.
Quoted 2 lines
You might need to do something touching the core code to handle the case where 9600 is not possible fallback.
I'd rather make a fallback to 115200, as it's a much more reasonable default.
Quoted 5 lines
drivers/tty/serial/esp32_uart.c line 434
Quoted 3 lines
drivers/tty/serial/esp32_uart.c new file +3
-0+if (baud) +esp32_uart_set_baud (port, baud); +}
set_termios()function lacks call touart_update_timeout().
Fixed.
Quoted 100 lines
Quoted 100 lines
drivers/tty/serial/esp32_uart.c new file +100
-0+ +static const char *esp32_uart_type (struct uart_port *port) +{ +return port_variant (port)->type; +} + +static void esp32_uart_release_port (struct uart_port *port) +{ +} + +static int esp32_uart_request_port (struct uart_port *port) +{ +return 0 ; +} + +/* configure/auto-configure the port */ +static void esp32_uart_config_port (struct uart_port *port,int flags) +{ +if (flags &UART_CONFIG_TYPE ) + port->type =PORT_ESP32UART ; +} + +#ifdef CONFIG_CONSOLE_POLL +static int esp32_uart_poll_init (struct uart_port *port) +{ + struct esp32_port *sport = container_of(port, struct esp32_port, port); + +return clk_prepare_enable (sport->clk); +} + +static void esp32_uart_poll_put_char (struct uart_port *port,unsigned char c) +{ +esp32_uart_put_char_sync (port, c); +} + +static int esp32_uart_poll_get_char (struct uart_port *port) +{ +if (esp32_uart_rx_fifo_cnt (port)) +return esp32_uart_read (port,UART_FIFO_REG ); +else +return NO_POLL_CHAR ; + +} +#endif + +static const struct uart_ops esp32_uart_pops = { + .tx_empty = esp32_uart_tx_empty, + .set_mctrl = esp32_uart_set_mctrl, + .get_mctrl = esp32_uart_get_mctrl, + .stop_tx = esp32_uart_stop_tx, + .start_tx = esp32_uart_start_tx, + .stop_rx = esp32_uart_stop_rx, + .startup = esp32_uart_startup, + .shutdown = esp32_uart_shutdown, + .set_termios = esp32_uart_set_termios, + .type = esp32_uart_type, + .release_port = esp32_uart_release_port, + .request_port = esp32_uart_request_port, + .config_port = esp32_uart_config_port, +#ifdef CONFIG_CONSOLE_POLL + .poll_init = esp32_uart_poll_init, + .poll_put_char = esp32_uart_poll_put_char, + .poll_get_char = esp32_uart_poll_get_char, +#endif +}; + +static void esp32_uart_console_putchar (struct uart_port *port,unsigned char c) +{ +esp32_uart_put_char_sync (port, c); +} + +static void esp32_uart_string_write (struct uart_port *port,const char *s, +unsigned int count) +{ +uart_console_write (port, s, count, esp32_uart_console_putchar); +} + +static void +esp32_uart_console_write (struct console *co,const char *s,unsigned int count) +{ +struct esp32_port *sport = esp32_uart_ports[co->index]; +struct uart_port *port = &sport->port; +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); + +esp32_uart_string_write (port, s, count); + +if (locked) +spin_unlock_irqrestore (&port->lock, flags); +} + +static int __init esp32_uart_console_setup (struct console *co,char *options) +{
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 540
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+struct esp32_port *sport; +int baud =115200 ; +int bits =8 ; +int parity ='n' ; +int flow ='n' ; +int ret;This lacks newline after declarations.
Ok.
Quoted 137 lines
Quoted 137 lines
drivers/tty/serial/esp32_uart.c new file +137
-0+/* +* check whether an invalid uart number has been specified, and +* if so, search for the first available port that does have +* console support. +*/ +if (co->index ==-1 || co->index >=ARRAY_SIZE (esp32_uart_ports)) + co->index =0 ; + + sport = esp32_uart_ports[co->index]; +if (!sport) +return -ENODEV ; + + ret =clk_prepare_enable (sport->clk); +if (ret) +return ret; + +if (options) +uart_parse_options (options, &baud, &parity, &bits, &flow); + +return uart_set_options (&sport->port, co, baud, parity, bits, flow); +} + +static int esp32_uart_console_exit (struct console *co) +{ +struct esp32_port *sport = esp32_uart_ports[co->index]; + +clk_disable_unprepare (sport->clk); +return 0 ; +} + +static struct uart_driver esp32_uart_reg; +static struct console esp32_uart_console = { + .name =DEV_NAME , + .write = esp32_uart_console_write, + .device = uart_console_device, + .setup = esp32_uart_console_setup, + .exit = esp32_uart_console_exit, + .flags =CON_PRINTBUFFER , + .index =-1 , + .data = &esp32_uart_reg, +}; + +static void esp32_uart_earlycon_putchar (struct uart_port *port,unsigned char c) +{ +esp32_uart_put_char_sync (port, c); +} + +static void esp32_uart_earlycon_write (struct console *con,const char *s, +unsigned int n) +{ +struct earlycon_device *dev = con->data; + +uart_console_write (&dev->port, s, n, esp32_uart_earlycon_putchar); +} + +#ifdef CONFIG_CONSOLE_POLL +static int esp32_uart_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 =esp32_uart_poll_get_char (&dev->port); + +if (c ==NO_POLL_CHAR ) +break ; + s[num_read++] = c; + } +return num_read; +} +#endif + +static int __init esp32xx_uart_early_console_setup (struct earlycon_device *device, +const char *options) +{ +if (!device->port.membase) +return -ENODEV ; + + device->con->write = esp32_uart_earlycon_write; +#ifdef CONFIG_CONSOLE_POLL + device->con->read = esp32_uart_earlycon_read; +#endif +if (device->port.uartclk !=BASE_BAUD *16 ) +esp32_uart_set_baud (&device->port, device->baud); + +return 0 ; +} + +static int __init esp32_uart_early_console_setup (struct earlycon_device *device, +const char *options) +{ + device->port.private_data = (void *)&esp32_variant; +return esp32xx_uart_early_console_setup (device, options); +} + +OF_EARLYCON_DECLARE (esp32uart,"esp,esp32-uart" , + esp32_uart_early_console_setup); + +static int __init esp32s3_uart_early_console_setup (struct earlycon_device *device, +const char *options) +{ + device->port.private_data = (void *)&esp32s3_variant; +return esp32xx_uart_early_console_setup (device, options); +} + +OF_EARLYCON_DECLARE (esp32s3uart,"esp,esp32s3-uart" , + esp32s3_uart_early_console_setup); + +static struct uart_driver esp32_uart_reg = { + .owner =THIS_MODULE , + .driver_name =DRIVER_NAME , + .dev_name =DEV_NAME , + .nr =ARRAY_SIZE (esp32_uart_ports), + .cons = &esp32_uart_console, +}; + +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 ) {
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 683
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-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 );One line.
Ok.
Quoted 75 lines
Quoted 75 lines
drivers/tty/serial/esp32_uart.c new file +75
-0+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; + + esp32_uart_ports[port->line] = sport; + +platform_set_drvdata (pdev, port); + + ret =uart_add_one_port (&esp32_uart_reg, port); +return ret; +} + +static int esp32_uart_remove (struct platform_device *pdev) +{ +struct uart_port *port =platform_get_drvdata (pdev); + +uart_remove_one_port (&esp32_uart_reg, port); + +return 0 ; +} + + +static struct platform_driver esp32_uart_driver = { + .probe = esp32_uart_probe, + .remove = esp32_uart_remove, + .driver = { + .name =DRIVER_NAME , + .of_match_table = esp32_uart_dt_ids, + }, +}; + +static int __init esp32_uart_init (void ) +{ +int ret; + + ret =uart_register_driver (&esp32_uart_reg); +if (ret) +return ret; + + ret =platform_driver_register (&esp32_uart_driver); +if (ret) +uart_unregister_driver (&esp32_uart_reg); + +return ret; +} + +static void __exit esp32_uart_exit(void) +{ +platform_driver_unregister (&esp32_uart_driver); +uart_unregister_driver (&esp32_uart_reg);
Quoted 8 lines
drivers/tty/serial/esp32_uart.c line 764
Quoted 6 lines
drivers/tty/serial/esp32_uart.c new file +6
-0+} + +module_init (esp32_uart_init); +module_exit (esp32_uart_exit); + +MODULE_DESCRIPTION ("Espressif ESP32 UART driver." );Drop .
Ok.
Quoted 19 lines
Quoted 8 lines
drivers/tty/serial/esp32_uart.c new file +2
-0+MODULE_AUTHOR ("Max Filippov <[email protected]>" ); +MODULE_LICENSE ("GPL" );include/uapi/linux/serial_core.h +0
-0include/uapi/linux/serial_core.h line 251
Quoted 7 lines
include/uapi/linux/serial_core.h +3
-0@@ -245,4 +245,7 @@/* Sunplus UART */ #define PORT_SUNPLUS 123 +/* Espressif ESP32 UART */ +#define PORT_ESP32UART 124 +#endif /* _UAPILINUX_SERIAL_CORE_H */ --
i.
Signature · 2 lines
Thanks. -- Max
Re: [PATCH 2/4] drivers/tty/serial: add driver for the ESP32 UART
Quoted 77 lines · Max Filippov
Quoted 62 lines · Ilpo Järvinen
Quoted 58 lines · Max Filippov
Add driver for the UART controllers of the Espressif ESP32 and ESP32S3 SoCs. Hardware specification is available at the following URLs:
https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf
(Chapter 13 UART Controller)
https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf
(Chapter 26 UART Controller)Signed-off-by Max Filippov <[email protected]>--- drivers/tty/serial/Kconfig | 13 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/esp32_uart.c | 766 +++++++++++++++++++++++++++++++ include/uapi/linux/serial_core.h | 3 + 4 files changed, 783 insertions(+) create mode 100644 drivers/tty/serial/esp32_uart.cdrivers/tty/serial/Kconfig +13
-0@@ -1578,6 +1578,19 @@ config SERIAL_NUVOTON_MA35D1_CONSOLE but you can alter that using a kernel command line option such as "console=ttyNVTx". +config SERIAL_ESP32 + tristate "Espressif ESP32 UART support" + depends on XTENSA_PLATFORM_ESP32 || (COMPILE_TEST && OF) + select SERIAL_CORE + select SERIAL_CORE_CONSOLE + select SERIAL_EARLYCON + help + Driver for the UART controllers of the Espressif ESP32xx SoCs. + When earlycon option is enabled the following kernel command line + snippets may be used: + earlycon=esp32s3uart,mmio32,0x60000000,115200n8,40000000 + earlycon=esp32uart,mmio32,0x3ff40000,115200n8 + endmenu config SERIAL_MCTRL_GPIOdrivers/tty/serial/Makefile +1
-0@@ -88,6 +88,7 @@ obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o 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 # GPIOLIB helpers for modem control lines obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.odrivers/tty/serial/esp32_uart.c new file +0
-0Quoted 12 lines
drivers/tty/serial/esp32_uart.c line 154
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+static u32 esp32_uart_tx_fifo_cnt (struct uart_port *port) +{ +return (esp32_uart_read (port,UART_STATUS_REG ) & +port_variant (port)->txfifo_cnt_mask) >>UART_TXFIFO_CNT_SHIFT ; +} + +static u32 esp32_uart_rx_fifo_cnt (struct uart_port *port) +{ +return (esp32_uart_read (port,UART_STATUS_REG ) & +port_variant (port)->rxfifo_cnt_mask) >>UART_RXFIFO_CNT_SHIFT ;
FIELD_GET().
Quoted 4 lines
I see how
FIELD_GETcan be used in other places, but here port_variant(port)->rxfifo_cnt_mask is not a runtime constant andFIELD_GETdoes not work with non-constant field definitions. Any suggestions?
Ah, I sorry. I probably moved around while composing the reply and it seems I returned to add the comment to a wrong line.
I don't have a good suggestion for this one besides breking it into multiple lines for better readability.
Quoted 13 lines
Quoted 2 lines
Use more lines (and a local variable) instead of splitting one line. It will be more readable that way.
Ok.
Quoted 7 lines
drivers/tty/serial/esp32_uart.c line 365
Quoted 5 lines
drivers/tty/serial/esp32_uart.c new file +5
-0+u32 frag = (port->uartclk *16 ) / baud - div *16 ; + +if (div <=port_variant (port)->clkdiv_mask) { +esp32_uart_write (port,UART_CLKDIV_REG , + div | (frag <<UART_CLKDIV_FRAG_SHIFT ));
FIELD_PREP().
Quoted 7 lines
Ok.
Quoted 2 lines
Also div be encapsulated into
FIELD_PREPhere even if it's shift is 0.This field's mask, port_variant(port)->clkdiv_mask, is, again, not a runtime constant. Any suggestion for this?
So you're saying there are two different sized fields. I suppose it can be left as is then.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
drivers/tty/serial/esp32_uart.c new file +4
-0+ port->uartclk / port_variant(port)->clkdiv_mask); +esp32_uart_write (port,UART_CLKDIV_REG , +port_variant (port)->clkdiv_mask | +UART_CLKDIV_FRAG_MASK );
Quoted 8 lines
Quoted 2 lines
I think you want to make the meaning more obvious here by using
FIELD_MAX(UART_CLKDIV_FRAG_MASK);No. I think
FIELD_MAXmakes it less obvious, because to work properly it must be not just
FIELD_MAX(UART_CLKDIV_FRAG),
but
FIELD_PREP(UART_CLKDIV_FRAG, FIELD_MAX(UART_CLKDIV_FRAG)).
Maybe add FIELD_PREP_MAX() in a preparatory patch to bitfield.h as this looks something that could be of use beyond this driver.
Quoted 14 lines
Quoted 11 lines
drivers/tty/serial/esp32_uart.c line 431
Quoted 10 lines
drivers/tty/serial/esp32_uart.c new file +10
-0+esp32_uart_write (port,UART_CONF0_REG , conf0); +esp32_uart_write (port,UART_CONF1_REG , conf1); +spin_unlock_irqrestore (&port->lock, flags); + +/* +* esp32s3 may not support 9600, passing its minimal baud rate +* as the min argument would trigger a WARN inside uart_get_baud_rate +*/ + baud =uart_get_baud_rate (port, termios, old, +0 , port->uartclk /16 );Quoted 2 lines
This looks questionable solution to the problem mentioned in the comment. What happens when user asks for baudrates below the minimum supported one?
Quoted 7 lines
I'll change it to refuse to change to unsupported baud rate and set default to 115200.
Quoted 2 lines
You might need to do something touching the core code to handle the case where 9600 is not possible fallback.
I'd rather make a fallback to 115200, as it's a much more reasonable default.
I'll have to see the code before commenting on these.
Signature · 1 line
i.
PATCH 3/4 dt-bindings: serial: document esp32s3-acm bindings
In reply to Max FilippovAdd documentation for the ESP32S3 ACM controller.
--- .../bindings/serial/esp,esp32-acm.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +40 -0
@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/esp,esp32-acm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ESP32S3 ACM controller + +maintainers: + - Max Filippov <[email protected]> + +description: | + ESP32S3 ACM controller is a communication device found in the ESP32S3 + SoC that is connected to one of its USB controllers. + +properties: + compatible: + const: esp,esp32s3-acm + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + +additionalProperties: false + +examples: + - | + acm@60038000 { + compatible = "esp,esp32s3-acm"; + reg = <0x60038000 0x1000>; + interrupts = <96 3 0>; + };
Signature · 1 line
2.30.2
Re: [PATCH 3/4] dt-bindings: serial: document esp32s3-acm bindings
Quoted 1 line · Max Filippov
Add documentation for the ESP32S3 ACM controller.
A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings.
Quoted 16 lines
Signed-off-by Max Filippov <[email protected]>--- .../bindings/serial/esp,esp32-acm.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-acm.yamlDocumentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +3
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +10
-0+--- +$id: http://devicetree.org/schemas/serial/esp,esp32-acm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ESP32S3 ACM controller + +maintainers: + - Max Filippov <[email protected]> + +description: |
Do not need '|' unless you need to preserve formatting.
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 14
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ ESP32S3 ACM controller is a communication device found in the ESP32S3
What is "ACM"? Why is this in serial? Only serial controllers are in serial. The description is very vague, way too vague.
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 15
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ SoC that is connected to one of its USB controllers.
Same comments as previous patch.
Quoted 12 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +12
-0+ +properties: + compatible: + const: esp,esp32s3-acm + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required:
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 37
Quoted 10 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +10
-0+ - compatible + - reg + - interrupts + +additionalProperties: false + +examples: + - | + acm@60038000 { + compatible = "esp,esp32s3-acm";
Use 4 spaces for example indentation.
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 39
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +2
-0+ reg = <0x60038000 0x1000>; + interrupts = <96 3 0>;
Same comments as previous patch.
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 40
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ };
Best regards,
Krzysztof
Re: [PATCH 3/4] dt-bindings: serial: document esp32s3-acm bindings
Quoted 6 lines · Krzysztof Kozlowski
Quoted 1 line · Max Filippov
Add documentation for the ESP32S3 ACM controller.
A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings.
Ok.
Quoted 19 lines
Quoted 19 lines
Signed-off-by Max Filippov <[email protected]>--- .../bindings/serial/esp,esp32-acm.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-acm.yamlDocumentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +7
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/esp,esp32-acm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# +
Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 13
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +6
-0+title: ESP32S3 ACM controller + +maintainers: + - Max Filippov <[email protected]> + +description: |Do not need '|' unless you need to preserve formatting.
Ok.
Quoted 3 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 14
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ ESP32S3 ACM controller is a communication device found in the ESP32S3What is "ACM"?
It's an 'Abstract Control Model' as in USB CDC-ACM: 'Communication Device Class
- Abstract Control Model'.
Quoted 1 line
Why is this in serial? Only serial controllers are in serial.
Because it's a serial communication device. The SoC TRM calls this peripheral 'USB Serial', but the USB part is fixed and is not controllable on the SoC side. When you plug it into a host USB socket you get a serial port called ttyACM on the host.
Quoted 1 line
The description is very vague, way too vague.
Is the following better?
Fixed function USB CDC-ACM device controller of the Espressif ESP32S3 SoC.
Quoted 20 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 15
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ SoC that is connected to one of its USB controllers.Same comments as previous patch.
Quoted 16 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +16
-0+ +properties: + compatible: + const: esp,esp32s3-acm + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - interrupts +
Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 37
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +6
-0+additionalProperties: false + +examples: + - | + acm@60038000 { + compatible = "esp,esp32s3-acm";Use 4 spaces for example indentation.
Ok.
Quoted 4 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 39
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +2
-0+ reg = <0x60038000 0x1000>; + interrupts = <96 3 0>;Same comments as previous patch.
These are not IRQ flags. In any case the contents of the IRQ specification cells is not relevant here, right?
Signature · 2 lines
Thanks. -- Max
Re: [PATCH 3/4] dt-bindings: serial: document esp32s3-acm bindings
Quoted 55 lines · Max Filippov
Quoted 6 lines · Krzysztof Kozlowski
Quoted 1 line · Max Filippov
Add documentation for the ESP32S3 ACM controller.
A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings.
Ok.
Quoted 19 lines
Quoted 19 lines
Signed-off-by Max Filippov <[email protected]>--- .../bindings/serial/esp,esp32-acm.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-acm.yamlDocumentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +7
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/esp,esp32-acm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# +Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 13
Quoted 6 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +6
-0+title: ESP32S3 ACM controller + +maintainers: + - Max Filippov <[email protected]> + +description: |Do not need '|' unless you need to preserve formatting.
Ok.
Quoted 3 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 14
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ ESP32S3 ACM controller is a communication device found in the ESP32S3What is "ACM"?
It's an 'Abstract Control Model' as in USB CDC-ACM: 'Communication Device Class
- Abstract Control Model'.Quoted 1 line
Why is this in serial? Only serial controllers are in serial.
Because it's a serial communication device. The SoC TRM calls this peripheral 'USB Serial', but the USB part is fixed and is not controllable on the SoC side. When you plug it into a host USB socket you get a serial port called ttyACM on the host.
Quoted 5 lines
Quoted 1 line
The description is very vague, way too vague.
Is the following better?
Fixed function USB CDC-ACM device controller of the Espressif ESP32S3 SoC.
Yes.
Quoted 18 lines
Quoted 18 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 15
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ SoC that is connected to one of its USB controllers.Same comments as previous patch.
Quoted 13 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +13
-0+ +properties: + compatible: + const: esp,esp32s3-acm + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible
Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 36
Quoted 8 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +8
-0+ - reg + - interrupts + +additionalProperties: false + +examples: + - | + acm@60038000 {
So this must be named "serial" now. ACM describes how this is interfaces to the SoC, right? Otherwise it would not be in "serial" directory and you would not be able to put serial devices as children.
Quoted 6 lines
Quoted 3 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 37
Quoted 1 line
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +1
-0+ compatible = "esp,esp32s3-acm";Use 4 spaces for example indentation.
Ok.
Quoted 7 lines
Quoted 4 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml line 39
Quoted 2 lines
Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml new file +2
-0+ reg = <0x60038000 0x1000>; + interrupts = <96 3 0>;Same comments as previous patch.
These are not IRQ flags. In any case the contents of the IRQ specification cells is not relevant here, right?
Yes, if 0 is not an IRQ flag :)
Quoted 1 line
Best regards,
Krzysztof
PATCH 4/4 drivers/tty/serial: add ESP32S3 ACM device driver
In reply to Max FilippovAdd 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)
--- 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
drivers/tty/serial/Kconfig +14 -0
@@ -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
drivers/tty/serial/Makefile +1 -0
@@ -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
drivers/tty/serial/esp32_acm.c new file +473 -0
473 lines not on this page · raw message
include/uapi/linux/serial_core.h +3 -0
@@ -248,4 +248,7 @@/* Espressif ESP32 UART */ #define PORT_ESP32UART 124 +/* Espressif ESP32 ACM */ +#define PORT_ESP32ACM 125 +#endif /* _UAPILINUX_SERIAL_CORE_H */
Signature · 1 line
2.30.2
Jiri Slaby
maintainerRe: [PATCH 4/4] drivers/tty/serial: add ESP32S3 ACM device driver
Quoted 5 lines · Max Filippov
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)
...
drivers/tty/serial/esp32_acm.c line 143
Quoted 4 lines
drivers/tty/serial/esp32_acm.c new file +4
-0+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...
drivers/tty/serial/esp32_acm.c line 149
Quoted 6 lines
drivers/tty/serial/esp32_acm.c new file +6
-0+esp32s3_acm_put_char (port, c); +esp32s3_acm_push (port); +} + +static void esp32s3_acm_transmit_buffer (struct uart_port *port) +{
tx helper.
Quoted 35 lines
drivers/tty/serial/esp32_acm.c new file +33
-0+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;
drivers/tty/serial/esp32_acm.c line 198
Quoted 10 lines
drivers/tty/serial/esp32_acm.c new file +10
-0+ + 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()
drivers/tty/serial/esp32_acm.c line 223
Quoted 11 lines
drivers/tty/serial/esp32_acm.c new file +10
-0+} +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.
drivers/tty/serial/esp32_acm.c line 300
Quoted 6 lines
drivers/tty/serial/esp32_acm.c new file +6
-0+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.
Quoted 10 lines
drivers/tty/serial/esp32_acm.c new file +10
-0+ +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); +
drivers/tty/serial/esp32_acm.c line 342
Quoted 10 lines
drivers/tty/serial/esp32_acm.c new file +8
-0+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?
Quoted 51 lines
drivers/tty/serial/esp32_acm.c new file +49
-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 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;
drivers/tty/serial/esp32_acm.c line 426
Quoted 10 lines
drivers/tty/serial/esp32_acm.c new file +10
-0+ 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.
Quoted 1 line
+}
regards,
Signature · 2 lines
js suse labs
Re: [PATCH 4/4] drivers/tty/serial: add ESP32S3 ACM device driver
Quoted 7 lines · Jiri Slaby
Quoted 5 lines · Max Filippov
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)
Quoted 8 lines
...
drivers/tty/serial/esp32_acm.c line 143
Quoted 4 lines
drivers/tty/serial/esp32_acm.c new file +4
-0+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.
Quoted 8 lines
drivers/tty/serial/esp32_acm.c line 149
Quoted 6 lines
drivers/tty/serial/esp32_acm.c new file +6
-0+esp32s3_acm_put_char (port, c); +esp32s3_acm_push (port); +} + +static void esp32s3_acm_transmit_buffer (struct uart_port *port) +{tx helper.
Ok.
Quoted 31 lines
Quoted 29 lines
drivers/tty/serial/esp32_acm.c new file +29
-0+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 ); +}
Quoted 16 lines
Quoted 4 lines
drivers/tty/serial/esp32_acm.c new file +4
-0+static irqreturn_t esp32s3_acm_int (int irq,void *dev_id) +{ +struct uart_port *port = dev_id; +u32 status;drivers/tty/serial/esp32_acm.c line 198
Quoted 10 lines
drivers/tty/serial/esp32_acm.c new file +10
-0+ + 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.
Quoted 14 lines
drivers/tty/serial/esp32_acm.c line 223
Quoted 11 lines
drivers/tty/serial/esp32_acm.c new file +10
-0+} +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.
Quoted 8 lines
drivers/tty/serial/esp32_acm.c line 300
Quoted 6 lines
drivers/tty/serial/esp32_acm.c new file +6
-0+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.
Quoted 15 lines
Quoted 13 lines
drivers/tty/serial/esp32_acm.c new file +13
-0+ +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); +}
Quoted 7 lines
drivers/tty/serial/esp32_acm.c line 342
Quoted 5 lines
drivers/tty/serial/esp32_acm.c new file +5
-0+#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.
Quoted 55 lines
Quoted 55 lines
drivers/tty/serial/esp32_acm.c new file +53
-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 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 ; +
Quoted 8 lines
drivers/tty/serial/esp32_acm.c line 426
Quoted 6 lines
drivers/tty/serial/esp32_acm.c new file +6
-0+ 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.
Signature · 2 lines
Thanks. -- Max
Re: [PATCH 4/4] drivers/tty/serial: add ESP32S3 ACM device driver
In reply to Max FilippovQuoted 6 lines · Max Filippov
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)
Quoted 8 lines
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.
Signature · 1 line
i.
Quoted 530 lines
drivers/tty/serial/Kconfig +14
-0@@ -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_GPIOdrivers/tty/serial/Makefile +1
-0@@ -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.odrivers/tty/serial/esp32_acm.c new file +473
-0@@ -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" );include/uapi/linux/serial_core.h +3
-0@@ -248,4 +248,7 @@/* Espressif ESP32 UART */ #define PORT_ESP32UART 124 +/* Espressif ESP32 ACM */ +#define PORT_ESP32ACM 125 +#endif /* _UAPILINUX_SERIAL_CORE_H */