Re: [PATCH 1/2] serial: wk2xxx: Add WK2xxx SPI UART driver
From: Jiri Slaby
Date: Fri Sep 04 2026 - 04:22:47 EST
Hi,
On 04. 09. 26, 9:20, zjzhao wrote:
Add a driver for the WK2xxx SPI to UART bridge ICs (WK2124, WK2132,
WK2168, WK2202 and WK2204) from WKmic (Chengdu Weikai Microelectronics).
Each IC exposes two or four full-duplex UART channels with 256-byte
RX/TX FIFOs through a single SPI slave interface and one interrupt line.
The driver is a rework of the vendor driver (https://github.com/britus/
wk2xxx) modeled after the NXP sc16is7xx driver. It registers ttyWK0..N
lines (IDA-allocated), uses devm_request_threaded_irq with a kthread
worker for register access, falls back to polling when the interrupt
line is not described, and supports hardware flow control and RS485
where the IC provides them.
Have you checked that there is no similar driver which could be only extended?
Also allocate PORT_WK2XXX (124) and add the SERIAL_WK2XXX Kconfig...
option.
Tested on Raspberry Pi 5 boards (EDATEC IPC1200 with WK2132 on SPI0 and
SBC2300 with WK2204 on SPI1); loopback TX/RX tests pass.
Signed-off-by: zjzhao <zjzhao@xxxxxxxxx>
--- /dev/null...
+++ b/drivers/tty/serial/wk2xxx.c
@@ -0,0 +1,1245 @@
+static int wk2xxx_port_reg_write(struct wk2xxx_port *s, unsigned int portno,
+ u8 reg, u8 val)
+{
+ guard(mutex)(&s->reg_lock);
+ return wk2xxx_raw_port_write(s, portno, reg, val);
+}
+
+static void wk2xxx_port_reg_update(struct wk2xxx_port *s, unsigned int portno,
+ u8 reg, u8 mask, u8 val)
+{
+ u8 r = 0;
+
+ scoped_guard(mutex, &s->reg_lock) {
why is this one scoped?
+ if (wk2xxx_raw_port_read(s, portno, reg, &r))
+ return;
+ wk2xxx_raw_port_write(s, portno, reg, (r & ~mask) | val);
+ }
+}
...
+static void wk2xxx_throttle(struct uart_port *port)
+{
+ unsigned long flags;
+
+ /* Stop draining the RX FIFO to apply back-pressure. */
+ uart_port_lock_irqsave(port, &flags);
This can be a guard too, right?
+ wk2xxx_ier_clear(port, WK2XXX_SIER_RFTRIG_IEN_BIT);
+ uart_port_unlock_irqrestore(port, flags);
+}
+
+static void wk2xxx_unthrottle(struct uart_port *port)
+{
+ unsigned long flags;
+
+ uart_port_lock_irqsave(port, &flags);
Same here.
+ wk2xxx_ier_set(port, WK2XXX_SIER_RFTRIG_IEN_BIT);
+ uart_port_unlock_irqrestore(port, flags);
+}
+
+static void wk2xxx_handle_tx(struct uart_port *port)
+{
+ struct wk2xxx_one *one = to_wk2xxx_one(port, port);
+ struct wk2xxx_port *s = dev_get_drvdata(port->dev);
+ struct tty_port *tport = &port->state->port;
+ unsigned long flags;
+ unsigned int portno = port->iobase;
+ unsigned int txlen, to_send, sent;
+ const unsigned char *tail;
+ u8 fsr, tfcnt;
What's the reason not to use any of the uart_port_tx* helpers?
+ mutex_lock(&one->tx_lock);
Why not guard?
+
+ if (unlikely(port->x_char)) {
+ wk2xxx_port_reg_write(s, portno, WK2XXX_FDAT_REG, port->x_char);
+ port->icount.tx++;
+ port->x_char = 0;
+ goto out;
And kill the goto then.
+ }
+
+ if (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port)) {
+ uart_port_lock_irqsave(port, &flags);
+ wk2xxx_stop_tx(port);
+ uart_port_unlock_irqrestore(port, flags);
+ goto out;
+ }
+
+ /* Limit to the free space available in the TX FIFO. */
+ wk2xxx_port_reg_read(s, portno, WK2XXX_TFCNT_REG, &tfcnt);
+ if (tfcnt == 0) {
+ wk2xxx_port_reg_read(s, portno, WK2XXX_FSR_REG, &fsr);
+ txlen = (fsr & WK2XXX_FSR_TFULL_BIT) ? 0 : WK2XXX_FIFO_SIZE;
+ } else {
+ txlen = WK2XXX_FIFO_SIZE - tfcnt;
+ }
+ if (txlen > WK2XXX_MAX_TX_CHARS)
+ txlen = WK2XXX_MAX_TX_CHARS;
+
+ to_send = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail, txlen);
+ sent = to_send;
+ while (to_send) {
+ unsigned int chunk = min_t(unsigned int, to_send,
+ WK2XXX_MAX_SPI_LEN);
+
+ wk2xxx_fifo_write(s, portno, tail, chunk);
+ tail += chunk;
+ to_send -= chunk;
+ }
+ uart_xmit_advance(port, sent);
+
+ uart_port_lock_irqsave(port, &flags);
+ if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ if (kfifo_is_empty(&tport->xmit_fifo))
+ wk2xxx_stop_tx(port);
+ else
+ wk2xxx_ier_set(port, WK2XXX_SIER_TFTRIG_IEN_BIT);
+ uart_port_unlock_irqrestore(port, flags);
+
+out:
+ mutex_unlock(&one->tx_lock);
+}
...
+static irqreturn_t wk2xxx_irq(int irq, void *dev_id)
+{
+ struct wk2xxx_port *s = dev_id;
+ bool keep_polling;
+
+ do {
+ u8 gifr;
+ int i;
+
+ keep_polling = false;
+
+ if (wk2xxx_reg_read(s, WK2XXX_GIFR_REG, &gifr))
+ return IRQ_HANDLED; /* Bus error; give up this pass. */
+
+ for (i = 0; i < s->devtype->nr_uart; ++i)
+ if (gifr & BIT(i))
+ keep_polling |= wk2xxx_port_irq(s, i);
+ } while (keep_polling);
Should you perhaps cap the loop count as well?
+
+ return IRQ_HANDLED;
Pointed out by sashiko, this is indeed bad for shared irqs.
+}
...
+static void wk2xxx_conf_port(struct uart_port *port, u8 lcr, u8 fwcr,
+ u8 baud0, u8 baud1, u8 pres)
+{
+ struct wk2xxx_port *s = dev_get_drvdata(port->dev);
+ unsigned int portno = port->iobase;
+ u8 sier, scr, fsr;
+ int count = 200;
+
+ scoped_guard(mutex, &s->reg_lock) {
Why is this scoped again?
+ /* Disable all sub-UART interrupts. */
+ wk2xxx_raw_port_read(s, portno, WK2XXX_SIER_REG, &sier);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SIER_REG, 0);
+
+ /* Wait for the transmitter to become idle. */
+ do {
+ wk2xxx_raw_port_read(s, portno, WK2XXX_FSR_REG, &fsr);
+ } while ((fsr & WK2XXX_FSR_TBUSY_BIT) && count--);
+
+ /* Disable the transmitter and receiver. */
+ wk2xxx_raw_port_read(s, portno, WK2XXX_SCR_REG, &scr);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SCR_REG,
+ scr & ~(WK2XXX_SCR_TXEN_BIT |
+ WK2XXX_SCR_RXEN_BIT));
+
+ /* Program the line control register. */
+ wk2xxx_raw_port_write(s, portno, WK2XXX_LCR_REG, lcr);
+
+ /* Configure hardware flow control levels. */
+ if (fwcr) {
+ wk2xxx_raw_port_write(s, portno, WK2XXX_FWCR_REG, fwcr);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SPAGE_REG, 1);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_FWTH_REG, 0xf0);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_FWTL_REG, 0x80);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SPAGE_REG, 0);
+ }
+
+ /* Program the baud rate generator (page 1 registers). */
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SPAGE_REG, 1);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_BAUD0_REG, baud0);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_BAUD1_REG, baud1);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_PRES_REG, pres);
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SPAGE_REG, 0);
+
+ /* Re-enable the transmitter and receiver. */
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SCR_REG,
+ scr | (WK2XXX_SCR_TXEN_BIT |
+ WK2XXX_SCR_RXEN_BIT));
+
+ /* Restore the interrupt enable register. */
+ wk2xxx_raw_port_write(s, portno, WK2XXX_SIER_REG, sier);
+ }
+}
+
+static void wk2xxx_calc_divisor(unsigned long clk, unsigned int baud,
+ u8 *baud0, u8 *baud1, u8 *pres)
+{
+ unsigned int div;
+
+ div = clk / (baud * 16);
+ if (div == 0)
+ div = 1;
+ div--;
+ *baud0 = div & 0xff;
+ *baud1 = (div >> 8) & 0xff;
+ *pres = ((unsigned long long)(clk % (baud * 16)) * 100 / baud + 50) / 100;
Did you mean to use explicit u64?
+}
+
+static void wk2xxx_set_termios(struct uart_port *port, struct ktermios *termios,
+ const struct ktermios *old)
+{
+ unsigned int baud;
+ unsigned long flags;
+ u8 lcr = 0, fwcr = 0;
+ u8 baud0, baud1, pres;
+
+ /* The WK2xxx supports 8 data bits only. */
+ termios->c_cflag &= ~CSIZE;
+ termios->c_cflag |= CS8;
+
+ /* Parity. */
+ if (termios->c_cflag & PARENB) {
+ lcr |= WK2XXX_LCR_PAEN_BIT;
+ switch (termios->c_cflag & (PARODD | CMSPAR)) {
+ case 0:
+ lcr |= WK2XXX_LCR_PAM1_BIT; /* even */
+ break;
+ case PARODD:
+ lcr |= WK2XXX_LCR_PAM0_BIT; /* odd */
+ break;
+ case CMSPAR:
+ break; /* space */
+ case PARODD | CMSPAR:
+ lcr |= WK2XXX_LCR_PAM1_BIT |
+ WK2XXX_LCR_PAM0_BIT; /* mark */
+ break;
+ }
+ }
+
+ /* Stop bits. */
+ if (termios->c_cflag & CSTOPB)
+ lcr |= WK2XXX_LCR_STPL_BIT;
+
+ /* Set read status mask. */
+ port->read_status_mask = WK2XXX_LSR_OE_BIT;
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= WK2XXX_LSR_PE_BIT |
+ WK2XXX_LSR_FE_BIT;
+ if (termios->c_iflag & (BRKINT | PARMRK))
+ port->read_status_mask |= WK2XXX_LSR_BI_BIT;
+
+ /* Set status ignore mask. */
+ port->ignore_status_mask = 0;
+ if (termios->c_iflag & IGNBRK)
+ port->ignore_status_mask |= WK2XXX_LSR_BI_BIT;
+ if (!(termios->c_cflag & CREAD))
+ port->ignore_status_mask |= WK2XXX_LSR_BRK_ERROR_MASK;
+
+ /* Configure flow control. */
+ port->status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS);
+ if (termios->c_cflag & CRTSCTS) {
+ fwcr = WK2XXX_FWCR_FWM_RTS_CTS;
+ port->status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
+ }
+
+ /* Get the baud rate generator configuration. */
+ baud = uart_get_baud_rate(port, termios, old,
+ port->uartclk / 16 / 0xffff,
+ port->uartclk / 16);
+
+ wk2xxx_calc_divisor(port->uartclk, baud, &baud0, &baud1, &pres);
+ wk2xxx_conf_port(port, lcr, fwcr, baud0, baud1, pres);
+
+ uart_port_lock_irqsave(port, &flags);
guard
+ uart_update_timeout(port, termios->c_cflag, baud);
+ uart_port_unlock_irqrestore(port, flags);
+}
...
+static const struct uart_ops wk2xxx_ops = {
+ .tx_empty = wk2xxx_tx_empty,
+ .set_mctrl = wk2xxx_set_mctrl,
+ .get_mctrl = wk2xxx_get_mctrl,
+ .stop_tx = wk2xxx_stop_tx,
+ .start_tx = wk2xxx_start_tx,
+ .throttle = wk2xxx_throttle,
+ .unthrottle = wk2xxx_unthrottle,
+ .stop_rx = wk2xxx_stop_rx,
+ .enable_ms = wk2xxx_enable_ms,
+ .break_ctl = wk2xxx_break_ctl,
+ .startup = wk2xxx_startup,
+ .shutdown = wk2xxx_shutdown,
+ .set_termios = wk2xxx_set_termios,
+ .type = wk2xxx_type,
+ .request_port = wk2xxx_request_port,
+ .release_port = wk2xxx_null_void,
req + rel are optional. Drop them.
+ .config_port = wk2xxx_config_port,...
+ .verify_port = wk2xxx_verify_port,
+};
+static int wk2xxx_probe(struct spi_device *spi)
+{
+ const struct wk2xxx_devtype *devtype;
+ struct device *dev = &spi->dev;
+ struct wk2xxx_port *s;
+ unsigned long uartclk;
+ u32 clock_freq = 0;
+ bool port_registered[WK2XXX_MAX_PORTS];
+ u8 val;
+ int i, ret;
+
+ /* Setup SPI bus. The SPI mode follows the device tree (spi-cpha,
+ * spi-cpol); it defaults to SPI mode 0 when unspecified.
+ */
+ spi->bits_per_word = 8;
+ spi->max_speed_hz = spi->max_speed_hz ? : 10 * HZ_PER_MHZ;
+ ret = spi_setup(spi);
+ if (ret)
+ return ret;
+
+ devtype = spi_get_device_match_data(spi);
+ if (!devtype)
+ return dev_err_probe(dev, -ENODEV, "Failed to match device\n");
+
+ /* Allocate port structure. */
+ s = devm_kzalloc(dev, struct_size(s, p, devtype->nr_uart), GFP_KERNEL);
+ if (!s)
+ return dev_err_probe(dev, -ENOMEM,
+ "Error allocating port structure\n");
+
+ s->devtype = devtype;
+ s->spi = spi;
+ mutex_init(&s->reg_lock);
+ dev_set_drvdata(dev, s);
+
+ /*
+ * The WK2xxx has no identification register, so the best we can do
+ * is to check that communication is at all possible.
+ */
+ ret = wk2xxx_reg_read(s, WK2XXX_GENA_REG, &val);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read GENA register\n");
+
+ /* Crystal clock; allow an optional DT override. */
+ uartclk = devtype->crystal_freq;
+ if (device_property_read_u32(dev, "clock-frequency", &clock_freq) == 0)
+ uartclk = clock_freq;
+
+ /* Mark each port line and status as uninitialized. */
+ for (i = 0; i < devtype->nr_uart; ++i) {
+ s->p[i].port.line = WK2XXX_MAX_DEVS;
+ port_registered[i] = false;
+ }
+
+ kthread_init_worker(&s->kworker);
+ s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker,
+ "wk2xxx");
+ if (IS_ERR(s->kworker_task)) {
+ ret = PTR_ERR(s->kworker_task);
+ goto out_ports;
+ }
+ sched_set_fifo(s->kworker_task);
+
+ for (i = 0; i < devtype->nr_uart; ++i) {
+ ret = ida_alloc_max(&wk2xxx_lines, WK2XXX_MAX_DEVS - 1,
+ GFP_KERNEL);
+ if (ret < 0)
+ goto out_ports;
+
+ s->p[i].port.line = ret;
+
+ /* Initialize port data. */
+ s->p[i].port.dev = dev;
+ s->p[i].port.irq = spi->irq;
+ s->p[i].port.type = PORT_WK2XXX;
+ s->p[i].port.fifosize = WK2XXX_FIFO_SIZE;
+ s->p[i].port.flags = UPF_FIXED_TYPE | UPF_LOW_LATENCY;
+ s->p[i].port.iobase = i;
+ /*
+ * Use all ones as membase so that uart_configure_port() in
+ * serial_core.c does not abort for SPI devices.
+ */
+ s->p[i].port.membase = (void __iomem *)~0;
0 is int, does the compiler not complain about cast to pointer? Or sparse? Maybe not, I am not sure anymore...
+ s->p[i].port.iotype = UPIO_PORT;thanks,
+ s->p[i].port.uartclk = uartclk;
+ s->p[i].port.rs485_config = wk2xxx_config_rs485;
+ s->p[i].port.rs485_supported = wk2xxx_rs485_supported;
+ s->p[i].port.ops = &wk2xxx_ops;
--
js
suse labs