On 28. 03. 23, 4:19, Jacky Huang wrote:
+static void transmit_chars(struct uart_ma35d1_port *up) +{ + struct circ_buf *xmit = &up->port.state->xmit; + int count; + u8 ch; + + if (uart_tx_stopped(&up->port)) { + ma35d1serial_stop_tx(&up->port); + return; + } + if (uart_circ_empty(xmit)) { + __stop_tx(up); + return; + }
Why is this necessary?
+ count = UART_FIFO_DEPTH - ((serial_in(up, UART_REG_FSR) & FSR_TXPTR_MSK) >> 16); + + uart_port_tx_limited(&up->port, ch, count, + !(serial_in(up, UART_REG_FSR) & FSR_TX_FULL), + serial_out(up, UART_REG_THR, ch), + ({})); + + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) + uart_write_wakeup(&up->port); + + if (uart_circ_empty(xmit)) + __stop_tx(up);
uart_port_tx_limited() should take care about the above and this too, right?
+}
...
+static void receive_chars(struct uart_ma35d1_port *up) +{ + u8 flag; + u32 fsr; + unsigned int ch;
Shouldn't ch be u8 too?
+ int max_count = 256; + + fsr = serial_in(up, UART_REG_FSR); + do { + flag = TTY_NORMAL; + up->port.icount.rx++; + + if (unlikely(fsr & (FSR_BIF | FSR_FEF | FSR_PEF | FSR_RX_OVER_IF))) { + if (fsr & FSR_BIF) { + up->port.icount.brk++; + if (uart_handle_break(&up->port)) + continue; + } + if (fsr & FSR_FEF) + up->port.icount.frame++; + if (fsr & FSR_PEF) + up->port.icount.parity++; + if (fsr & FSR_RX_OVER_IF) + up->port.icount.overrun++; + + serial_out(up, UART_REG_FSR, fsr & + (FSR_BIF | FSR_FEF | FSR_PEF | FSR_RX_OVER_IF)); + + if (fsr & FSR_BIF) + flag = TTY_BREAK; + else if (fsr & FSR_PEF) + flag = TTY_PARITY; + else if (fsr & FSR_FEF) + flag = TTY_FRAME; + } + + ch = serial_in(up, UART_REG_RBR); + if (uart_handle_sysrq_char(&up->port, ch)) + continue; + + spin_lock(&up->port.lock); + uart_insert_char(&up->port, fsr, FSR_RX_OVER_IF, ch, flag); + spin_unlock(&up->port.lock); + + fsr = serial_in(up, UART_REG_FSR); + } while (!(fsr & FSR_RX_EMPTY) && (max_count-- > 0)); + + spin_lock(&up->port.lock); + tty_flip_buffer_push(&up->port.state->port); + spin_unlock(&up->port.lock); +}
...
+static int ma35d1serial_remove(struct platform_device *dev) +{ + struct uart_port *port = platform_get_drvdata(dev); + + if (port) {
Can this ever be NULL?
+ uart_remove_one_port(&ma35d1serial_reg, port); + free_irq(port->irq, port); + } + return 0; +} + +static int ma35d1serial_suspend(struct platform_device *dev, pm_message_t state) +{ + int i; + struct uart_ma35d1_port *up; + + if (dev->dev.of_node) + i = of_alias_get_id(dev->dev.of_node, "serial"); + if (i < 0) { + dev_err(&dev->dev, "failed to get alias/pdev id, errno %d\n", i); + return i; + } + up = &ma35d1serial_ports[i];
platform_get_drvdata(dev) ?
+ if (i == 0) { + up->console_baud_rate = serial_in(up, UART_REG_BAUD); + up->console_line = serial_in(up, UART_REG_LCR); + up->console_int = serial_in(up, UART_REG_IER); + } + return 0; +} + +static int ma35d1serial_resume(struct platform_device *dev) +{ + int i; + struct uart_ma35d1_port *up; + + if (dev->dev.of_node) + i = of_alias_get_id(dev->dev.of_node, "serial"); + if (i < 0) { + dev_err(&dev->dev, "failed to get alias/pdev id, errno %d\n", i); + return i; + } + up = &ma35d1serial_ports[i]; + if (i == 0) { + serial_out(up, UART_REG_BAUD, up->console_baud_rate); + serial_out(up, UART_REG_LCR, up->console_line); + serial_out(up, UART_REG_IER, up->console_int); + } + return 0; +}
No uart_suspend_port()/uart_resume_port()? You don't wait for transmitter to be empty in suspend. You don't stop tx etc.
+static struct platform_driver ma35d1serial_driver = { + .probe = ma35d1serial_probe, + .remove = ma35d1serial_remove, + .suspend = ma35d1serial_suspend, + .resume = ma35d1serial_resume, + .driver = { + .name = "ma35d1-uart", + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(ma35d1_serial_of_match), + }, +}; + +static int __init ma35d1serial_init(void) +{ + int ret; + + ret = uart_register_driver(&ma35d1serial_reg); + if (ret) + return ret; + ret = platform_driver_register(&ma35d1serial_driver); + if (ret) + uart_unregister_driver(&ma35d1serial_reg); + return ret; +} + +static void __exit ma35d1serial_exit(void) +{ + platform_driver_unregister(&ma35d1serial_driver); + uart_unregister_driver(&ma35d1serial_reg); +} + +module_init(ma35d1serial_init); +module_exit(ma35d1serial_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("MA35D1 serial driver");
+MODULE_ALIAS_CHARDEV_MAJOR(TTY_MAJOR);
Why is this needed? How are other platform drivers autoloaded? thanks, -- js suse labs