2011-04-19 20:16:27

by John Linn

[permalink] [raw]
Subject: [PATCH] tty/serial: add support for Xilinx PS UART

The Xilinx PS Uart is used on the new ARM based SoC. This
UART is not compatible with others such that a seperate
driver is required.

Signed-off-by: John Linn <[email protected]>
---
Documentation/devices.txt | 4 +
drivers/tty/serial/Kconfig | 13 +
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/xilinx_uartps.c | 1109 ++++++++++++++++++++++++++++++++++++
include/linux/serial_core.h | 3 +
5 files changed, 1130 insertions(+), 0 deletions(-)
create mode 100644 drivers/tty/serial/xilinx_uartps.c

diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index eccffe7..644db8c 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -2812,6 +2812,10 @@ Your cooperation is appreciated.
210 = /dev/ttyMAX1 MAX3100 serial port 1
211 = /dev/ttyMAX2 MAX3100 serial port 2
212 = /dev/ttyMAX3 MAX3100 serial port 3
+ 213 = /dev/ttyPS0 Xilinx PS serial port 0
+ 214 = /dev/ttyPS1 Xilinx PS serial port 1
+ 215 = /dev/ttyPS2 Xilinx PS serial port 2
+ 216 = /dev/ttyPS3 Xilinx PS serial port 3

205 char Low-density serial ports (alternate device)
0 = /dev/culu0 Callout device for ttyLU0
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 80484af..84876ec 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1612,4 +1612,17 @@ config SERIAL_MXS_AUART_CONSOLE
help
Enable a MXS AUART port to be the system console.

+config SERIAL_XILINX_PS_UART
+ tristate "Xilinx PS UART support"
+ select SERIAL_CORE
+ help
+ This driver supports the Xilinx PS UART port.
+
+config SERIAL_XILINX_PS_UART_CONSOLE
+ bool "Xilinx PS UART console support"
+ depends on SERIAL_XILINX_PS_UART=y
+ select SERIAL_CORE_CONSOLE
+ help
+ Enable a Xilinx PS UART port to be the system console.
+
endmenu
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index fee0690..aafddf1 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -94,3 +94,4 @@ obj-$(CONFIG_SERIAL_IFX6X60) += ifx6x60.o
obj-$(CONFIG_SERIAL_PCH_UART) += pch_uart.o
obj-$(CONFIG_SERIAL_MSM_SMD) += msm_smd_tty.o
obj-$(CONFIG_SERIAL_MXS_AUART) += mxs-auart.o
+obj-$(CONFIG_SERIAL_XILINX_PS_UART) += xilinx_uartps.o
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
new file mode 100644
index 0000000..a95a473
--- /dev/null
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -0,0 +1,1109 @@
+/*
+ * Xilinx PS UART driver
+ *
+ * 2011 (c) Xilinx Inc.
+ *
+ * This program is free software; you can redistribute it
+ * and/or modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation;
+ * either version 2 of the License, or (at your option) any
+ * later version.
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/serial_core.h>
+#include <linux/console.h>
+#include <linux/serial.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/of.h>
+
+#define XUARTPS_TTY_NAME "ttyPS"
+#define XUARTPS_NAME "xuartps"
+#define XUARTPS_MAJOR 204
+#define XUARTPS_MINOR 213
+#define XUARTPS_NR_PORTS 2
+#define XUARTPS_FIFO_SIZE 16 /* FIFO size */
+#define XUARTPS_REGISTER_SPACE 0xFFF
+
+#define xuartps_readl(offset) ioread32(port->membase + offset)
+#define xuartps_writel(val, offset) iowrite32(val, port->membase + offset)
+
+/********************************Register Map********************************/
+/** UART
+ *
+ * Register offsets for the UART.
+ *
+ */
+#define XUARTPS_CR_OFFSET 0x00 /* Control Register [8:0] */
+#define XUARTPS_MR_OFFSET 0x04 /* Mode Register [10:0] */
+#define XUARTPS_IER_OFFSET 0x08 /* Interrupt Enable [10:0] */
+#define XUARTPS_IDR_OFFSET 0x0C /* Interrupt Disable [10:0] */
+#define XUARTPS_IMR_OFFSET 0x10 /* Interrupt Mask [10:0] */
+#define XUARTPS_ISR_OFFSET 0x14 /* Interrupt Status [10:0]*/
+#define XUARTPS_BAUDGEN_OFFSET 0x18 /* Baud Rate Generator [15:0] */
+#define XUARTPS_RXTOUT_OFFSET 0x1C /* RX Timeout [7:0] */
+#define XUARTPS_RXWM_OFFSET 0x20 /* RX FIFO Trigger Level [5:0] */
+#define XUARTPS_MODEMCR_OFFSET 0x24 /* Modem Control [5:0] */
+#define XUARTPS_MODEMSR_OFFSET 0x28 /* Modem Status [8:0] */
+#define XUARTPS_SR_OFFSET 0x2C /* Channel Status [11:0] */
+#define XUARTPS_FIFO_OFFSET 0x30 /* FIFO [15:0] or [7:0] */
+#define XUARTPS_BAUDDIV_OFFSET 0x34 /* Baud Rate Divider [7:0] */
+#define XUARTPS_FLOWDEL_OFFSET 0x38 /* Flow Delay [15:0] */
+#define XUARTPS_IRRX_PWIDTH_OFFSET 0x3C /* IR Minimum Received Pulse
+ Width [15:0] */
+#define XUARTPS_IRTX_PWIDTH_OFFSET 0x40 /* IR Transmitted pulse
+ Width [7:0] */
+#define XUARTPS_TXWM_OFFSET 0x44 /* TX FIFO Trigger Level [5:0] */
+
+/** Control Register
+ *
+ * The Control register (CR) controls the major functions of the device.
+ *
+ * Control Register Bit Definitions
+ */
+#define XUARTPS_CR_STOPBRK 0x00000100 /* Stop TX break */
+#define XUARTPS_CR_STARTBRK 0x00000080 /* Set TX break */
+#define XUARTPS_CR_TX_DIS 0x00000020 /* TX disabled. */
+#define XUARTPS_CR_TX_EN 0x00000010 /* TX enabled */
+#define XUARTPS_CR_RX_DIS 0x00000008 /* RX disabled. */
+#define XUARTPS_CR_RX_EN 0x00000004 /* RX enabled */
+#define XUARTPS_CR_TXRST 0x00000002 /* TX logic reset */
+#define XUARTPS_CR_RXRST 0x00000001 /* RX logic reset */
+#define XUARTPS_CR_RST_TO 0x00000040 /* Restart Timeout Counter */
+
+/** Mode Register
+ *
+ * The mode register (MR) defines the mode of transfer as well as the data
+ * format. If this register is modified during transmission or reception,
+ * data validity cannot be guaranteed.
+ *
+ * Mode Register Bit Definitions
+ *
+ */
+#define XUARTPS_MR_CLKSEL 0x00000001 /* Pre-scalar selection */
+#define XUARTPS_MR_CHMODE_L_LOOP 0x00000200 /* Local loop back mode */
+#define XUARTPS_MR_CHMODE_NORM 0x00000000 /* Normal mode */
+
+#define XUARTPS_MR_STOPMODE_2_BIT 0x00000080 /* 2 stop bits */
+#define XUARTPS_MR_STOPMODE_1_BIT 0x00000000 /* 1 stop bit */
+
+#define XUARTPS_MR_PARITY_NONE 0x00000020 /* No parity mode */
+#define XUARTPS_MR_PARITY_MARK 0x00000018 /* Mark parity mode */
+#define XUARTPS_MR_PARITY_SPACE 0x00000010 /* Space parity mode */
+#define XUARTPS_MR_PARITY_ODD 0x00000008 /* Odd parity mode */
+#define XUARTPS_MR_PARITY_EVEN 0x00000000 /* Even parity mode */
+
+#define XUARTPS_MR_CHARLEN_6_BIT 0x00000006 /* 6 bits data */
+#define XUARTPS_MR_CHARLEN_7_BIT 0x00000004 /* 7 bits data */
+#define XUARTPS_MR_CHARLEN_8_BIT 0x00000000 /* 8 bits data */
+
+/** Interrupt Registers
+ *
+ * Interrupt control logic uses the interrupt enable register (IER) and the
+ * interrupt disable register (IDR) to set the value of the bits in the
+ * interrupt mask register (IMR). The IMR determines whether to pass an
+ * interrupt to the interrupt status register (ISR).
+ * Writing a 1 to IER Enables an interrupt, writing a 1 to IDR disables an
+ * interrupt. IMR and ISR are read only, and IER and IDR are write only.
+ * Reading either IER or IDR returns 0x00.
+ *
+ * All four registers have the same bit definitions.
+ */
+#define XUARTPS_IXR_TOUT 0x00000100 /* RX Timeout error interrupt */
+#define XUARTPS_IXR_PARITY 0x00000080 /* Parity error interrupt */
+#define XUARTPS_IXR_FRAMING 0x00000040 /* Framing error interrupt */
+#define XUARTPS_IXR_OVERRUN 0x00000020 /* Overrun error interrupt */
+#define XUARTPS_IXR_TXFULL 0x00000010 /* TX FIFO Full interrupt */
+#define XUARTPS_IXR_TXEMPTY 0x00000008 /* TX FIFO empty interrupt */
+#define XUARTPS_ISR_RXEMPTY 0x00000002 /* RX FIFO empty interrupt */
+#define XUARTPS_IXR_RXTRIG 0x00000001 /* RX FIFO trigger interrupt */
+#define XUARTPS_IXR_RXFULL 0x00000004 /* RX FIFO full interrupt. */
+#define XUARTPS_IXR_RXEMPTY 0x00000002 /* RX FIFO empty interrupt. */
+#define XUARTPS_IXR_MASK 0x00001FFF /* Valid bit mask */
+
+
+/** Channel Status Register
+ *
+ * The channel status register (CSR) is provided to enable the control logic
+ * to monitor the status of bits in the channel interrupt status register,
+ * even if these are masked out by the interrupt mask register.
+ */
+#define XUARTPS_SR_RXEMPTY 0x00000002 /* RX FIFO empty */
+#define XUARTPS_SR_TXEMPTY 0x00000008 /* TX FIFO empty */
+#define XUARTPS_SR_TXFULL 0x00000010 /* TX FIFO full */
+#define XUARTPS_SR_RXTRIG 0x00000001 /* Rx Trigger */
+
+
+/**
+ * xuartps_isr - Interrupt handler
+ * @irq: Irq number
+ * @dev_id: Id of the port
+ *
+ * Returns IRQHANDLED
+ **/
+static irqreturn_t xuartps_isr(int irq, void *dev_id)
+{
+ struct uart_port *port = (struct uart_port *)dev_id;
+ struct tty_struct *tty = port->state->port.tty;
+ unsigned long flags;
+ unsigned int isrstatus, numbytes;
+ unsigned int data;
+ char status = TTY_NORMAL;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ /* Read the interrupt status register to determine which
+ * interrupt(s) is/are active.
+ */
+ isrstatus = xuartps_readl(XUARTPS_ISR_OFFSET);
+
+ /* drop byte with parity error if IGNPAR specified */
+ if (isrstatus & port->ignore_status_mask & XUARTPS_IXR_PARITY)
+ isrstatus &= ~(XUARTPS_IXR_RXTRIG | XUARTPS_IXR_TOUT);
+
+ isrstatus &= port->read_status_mask;
+ isrstatus &= ~port->ignore_status_mask;
+
+ if ((isrstatus & XUARTPS_IXR_TOUT) ||
+ (isrstatus & XUARTPS_IXR_RXTRIG)) {
+ /* Receive Timeout Interrupt */
+ while ((xuartps_readl(XUARTPS_SR_OFFSET) &
+ XUARTPS_SR_RXEMPTY) != XUARTPS_SR_RXEMPTY) {
+ data = xuartps_readl(XUARTPS_FIFO_OFFSET);
+ port->icount.rx++;
+
+ if (isrstatus & XUARTPS_IXR_PARITY) {
+ port->icount.parity++;
+ status = TTY_PARITY;
+ } else if (isrstatus & XUARTPS_IXR_FRAMING) {
+ port->icount.frame++;
+ status = TTY_FRAME;
+ } else if (isrstatus & XUARTPS_IXR_OVERRUN)
+ port->icount.overrun++;
+
+ uart_insert_char(port, isrstatus, XUARTPS_IXR_OVERRUN,
+ data, status);
+ }
+ spin_unlock(&port->lock);
+ tty_flip_buffer_push(tty);
+ spin_lock(&port->lock);
+ }
+
+ /* Dispatch an appropriate handler */
+ if ((isrstatus & XUARTPS_IXR_TXEMPTY) == XUARTPS_IXR_TXEMPTY) {
+ if (uart_circ_empty(&port->state->xmit)) {
+ xuartps_writel(XUARTPS_IXR_TXEMPTY,
+ XUARTPS_IDR_OFFSET);
+ } else {
+ numbytes = port->fifosize;
+ /* Break if no more data available in the UART buffer */
+ while (numbytes--) {
+ if (uart_circ_empty(&port->state->xmit))
+ break;
+ /* Get the data from the UART circular buffer
+ * and write it to the xuartps's TX_FIFO
+ * register.
+ */
+ xuartps_writel(
+ port->state->xmit.buf[port->state->xmit.
+ tail], XUARTPS_FIFO_OFFSET);
+
+ port->icount.tx++;
+
+ /* Adjust the tail of the UART buffer and wrap
+ * the buffer if it reaches limit.
+ */
+ port->state->xmit.tail =
+ (port->state->xmit.tail + 1) & \
+ (UART_XMIT_SIZE - 1);
+ }
+
+ if (uart_circ_chars_pending(
+ &port->state->xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+ }
+ }
+
+ xuartps_writel(isrstatus, XUARTPS_ISR_OFFSET);
+ spin_unlock_irqrestore(&port->lock, flags);
+ return IRQ_HANDLED;
+}
+
+/**
+ * xuartps_set_baud_rate - Calculate and set the baud rate
+ * @port: Handle to the uart port structure
+ * @baud: Baud rate to set
+ *
+ **/
+static void xuartps_set_baud_rate(struct uart_port *port, unsigned int baud)
+{
+ unsigned int sel_clk;
+ unsigned int calc_baud;
+ unsigned int brgr_val, brdiv_val;
+ unsigned int bauderror, percent_err = 100;
+
+ /* Formula to obtain baud rate is
+ * baud_tx/rx rate = sel_clk/CD * (BDIV + 1)
+ * input_clk = (Uart User Defined Clock or Apb Clock)
+ * depends on UCLKEN in MR Reg
+ * sel_clk = input_clk or input_clk/8;
+ * depends on CLKS in MR reg
+ * CD and BDIV depends on values in
+ * baud rate generate register
+ * baud rate clock divisor register
+ */
+ sel_clk = port->uartclk;
+ if (xuartps_readl(XUARTPS_MR_OFFSET) & XUARTPS_MR_CLKSEL)
+ sel_clk = sel_clk / 8;
+
+ /* Check for the values of baud clk divisor value ranging from 4 to 255
+ * where the percent_err for the given baud rate is acceptable.
+ */
+ for (brdiv_val = 4; brdiv_val < 255; brdiv_val++) {
+
+ brgr_val = sel_clk / (baud * (brdiv_val + 1));
+ if (brgr_val < 2 || brgr_val > 65535)
+ continue;
+
+ calc_baud = sel_clk / (brgr_val * (brdiv_val + 1));
+
+ if (baud > calc_baud)
+ bauderror = baud - calc_baud;
+ else
+ bauderror = calc_baud - baud;
+
+ percent_err = (bauderror * 100) / baud;
+ if (percent_err < 3)
+ break;
+ }
+
+ if (percent_err >= 3)
+ dev_err(port->dev, "Error too large, baud rate not set\n");
+ else {
+ /* Set the values for the new baud rate */
+ xuartps_writel(brgr_val, XUARTPS_BAUDGEN_OFFSET);
+ xuartps_writel(brdiv_val, XUARTPS_BAUDDIV_OFFSET);
+
+ }
+}
+
+
+/*----------------------Uart Operations---------------------------*/
+
+/**
+ * xuartps_start_tx - Start transmitting bytes
+ * @port: Handle to the uart port structure
+ *
+ **/
+static void xuartps_start_tx(struct uart_port *port)
+{
+ unsigned int status, numbytes = port->fifosize;
+
+ if (uart_circ_empty(&port->state->xmit) || uart_tx_stopped(port))
+ return;
+
+ status = xuartps_readl(XUARTPS_CR_OFFSET);
+ /* Set the TX enable bit and clear the TX disable bit to enable the
+ * transmitter.
+ */
+ xuartps_writel((status & ~XUARTPS_CR_TX_DIS) | XUARTPS_CR_TX_EN,
+ XUARTPS_CR_OFFSET);
+
+ while (numbytes-- && ((xuartps_readl(XUARTPS_SR_OFFSET)
+ & XUARTPS_SR_TXFULL)) != XUARTPS_SR_TXFULL) {
+
+ /* Break if no more data available in the UART buffer */
+ if (uart_circ_empty(&port->state->xmit))
+ break;
+
+ /* Get the data from the UART circular buffer and
+ * write it to the xuartps's TX_FIFO register.
+ */
+ xuartps_writel(
+ port->state->xmit.buf[port->state->xmit.tail],
+ XUARTPS_FIFO_OFFSET);
+ port->icount.tx++;
+
+ /* Adjust the tail of the UART buffer and wrap
+ * the buffer if it reaches limit.
+ */
+ port->state->xmit.tail = (port->state->xmit.tail + 1) &
+ (UART_XMIT_SIZE - 1);
+ }
+
+ /* Enable the TX Empty interrupt */
+ xuartps_writel(XUARTPS_IXR_TXEMPTY, XUARTPS_IER_OFFSET);
+
+ if (uart_circ_chars_pending(&port->state->xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+}
+
+/**
+ * xuartps_stop_tx - Stop TX
+ * @port: Handle to the uart port structure
+ *
+ **/
+static void xuartps_stop_tx(struct uart_port *port)
+{
+ unsigned int regval;
+
+ regval = xuartps_readl(XUARTPS_CR_OFFSET);
+ regval |= XUARTPS_CR_TX_DIS;
+ /* Disable the transmitter */
+ xuartps_writel(regval, XUARTPS_CR_OFFSET);
+}
+
+/**
+ * xuartps_stop_rx - Stop RX
+ * @port: Handle to the uart port structure
+ *
+ **/
+static void xuartps_stop_rx(struct uart_port *port)
+{
+ unsigned int regval;
+
+ regval = xuartps_readl(XUARTPS_CR_OFFSET);
+ regval |= XUARTPS_CR_RX_DIS;
+ /* Disable the receiver */
+ xuartps_writel(regval, XUARTPS_CR_OFFSET);
+}
+
+/**
+ * xuartps_tx_empty - Check whether TX is empty
+ * @port: Handle to the uart port structure
+ *
+ * Returns TIOCSER_TEMT on success, 0 otherwise
+ **/
+static unsigned int xuartps_tx_empty(struct uart_port *port)
+{
+ unsigned int status;
+
+ status = xuartps_readl(XUARTPS_ISR_OFFSET) & XUARTPS_IXR_TXEMPTY;
+ return status ? TIOCSER_TEMT : 0;
+
+}
+
+
+/**
+ * xuartps_break_ctl - Based on the input ctl we have to start or stop
+ * transmitting char breaks
+ * @port: Handle to the uart port structure
+ * @ctl: Value based on which start or stop decision is taken
+ *
+ **/
+static void xuartps_break_ctl(struct uart_port *port, int ctl)
+{
+ unsigned int status;
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ status = xuartps_readl(XUARTPS_CR_OFFSET);
+
+ if (ctl == -1)
+ xuartps_writel(XUARTPS_CR_STARTBRK | status,
+ XUARTPS_CR_OFFSET);
+ else {
+ if ((status & XUARTPS_CR_STOPBRK) == 0)
+ xuartps_writel(XUARTPS_CR_STOPBRK | status,
+ XUARTPS_CR_OFFSET);
+ }
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+/**
+ * xuartps_set_termios - termios operations, handling data length, parity,
+ * stop bits, flow control, baud rate
+ * @port: Handle to the uart port structure
+ * @termios: Handle to the input termios structure
+ * @old: Values of the previously saved termios structure
+ *
+ **/
+static void xuartps_set_termios(struct uart_port *port,
+ struct ktermios *termios, struct ktermios *old)
+{
+ unsigned int cval = 0;
+ unsigned int baud;
+ unsigned long flags;
+ unsigned int ctrl_reg, mode_reg;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ /* Empty the receive FIFO 1st before making changes */
+ while ((xuartps_readl(XUARTPS_SR_OFFSET) &
+ XUARTPS_SR_RXEMPTY) != XUARTPS_SR_RXEMPTY) {
+ xuartps_readl(XUARTPS_FIFO_OFFSET);
+ }
+
+ /* Disable the TX and RX to set baud rate */
+ xuartps_writel(xuartps_readl(XUARTPS_CR_OFFSET) |
+ (XUARTPS_CR_TX_DIS | XUARTPS_CR_RX_DIS),
+ XUARTPS_CR_OFFSET);
+
+ /* Min baud rate = 6bps and Max Baud Rate is 10Mbps for 100Mhz clk */
+ baud = uart_get_baud_rate(port, termios, old, 0, 460800);
+ xuartps_set_baud_rate(port, baud);
+
+ /*
+ * Update the per-port timeout.
+ */
+ uart_update_timeout(port, termios->c_cflag, baud);
+
+ /* Set TX/RX Reset */
+ xuartps_writel(xuartps_readl(XUARTPS_CR_OFFSET) |
+ (XUARTPS_CR_TXRST | XUARTPS_CR_RXRST),
+ XUARTPS_CR_OFFSET);
+
+ ctrl_reg = xuartps_readl(XUARTPS_CR_OFFSET);
+
+ /* Clear the RX disable and TX disable bits and then set the TX enable
+ * bit and RX enable bit to enable the transmitter and receiver.
+ */
+ xuartps_writel(
+ (ctrl_reg & ~(XUARTPS_CR_TX_DIS | XUARTPS_CR_RX_DIS))
+ | (XUARTPS_CR_TX_EN | XUARTPS_CR_RX_EN),
+ XUARTPS_CR_OFFSET);
+
+ xuartps_writel(10, XUARTPS_RXTOUT_OFFSET);
+
+ port->read_status_mask = XUARTPS_IXR_TXEMPTY | XUARTPS_IXR_RXTRIG |
+ XUARTPS_IXR_OVERRUN | XUARTPS_IXR_TOUT;
+ port->ignore_status_mask = 0;
+
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= XUARTPS_IXR_PARITY |
+ XUARTPS_IXR_FRAMING;
+
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= XUARTPS_IXR_PARITY |
+ XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
+
+ /* ignore all characters if CREAD is not set */
+ if ((termios->c_cflag & CREAD) == 0)
+ port->ignore_status_mask |= XUARTPS_IXR_RXTRIG |
+ XUARTPS_IXR_TOUT | XUARTPS_IXR_PARITY |
+ XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
+
+ mode_reg = xuartps_readl(XUARTPS_MR_OFFSET);
+
+ /* Handling Data Size */
+ switch (termios->c_cflag & CSIZE) {
+ case CS6:
+ cval |= XUARTPS_MR_CHARLEN_6_BIT;
+ break;
+ case CS7:
+ cval |= XUARTPS_MR_CHARLEN_7_BIT;
+ break;
+ default:
+ case CS8:
+ cval |= XUARTPS_MR_CHARLEN_8_BIT;
+ break;
+ }
+
+ /* Handling Parity and Stop Bits length */
+ if (termios->c_cflag & CSTOPB)
+ cval |= XUARTPS_MR_STOPMODE_2_BIT; /* 2 STOP bits */
+ else
+ cval |= XUARTPS_MR_STOPMODE_1_BIT; /* 1 STOP bit */
+
+ if (termios->c_cflag & PARENB) {
+ /* Mark or Space parity */
+ if (termios->c_cflag & CMSPAR) {
+ if (termios->c_cflag & PARODD)
+ cval |= XUARTPS_MR_PARITY_MARK;
+ else
+ cval |= XUARTPS_MR_PARITY_SPACE;
+ } else if (termios->c_cflag & PARODD)
+ cval |= XUARTPS_MR_PARITY_ODD;
+ else
+ cval |= XUARTPS_MR_PARITY_EVEN;
+ } else
+ cval |= XUARTPS_MR_PARITY_NONE;
+ xuartps_writel(cval , XUARTPS_MR_OFFSET);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+/**
+ * xuartps_startup - Called when an application opens a xuartps port
+ * @port: Handle to the uart port structure
+ *
+ * Returns 0 on success, negative error otherwise
+ **/
+static int xuartps_startup(struct uart_port *port)
+{
+ unsigned int retval = 0, status = 0;
+
+ retval = request_irq(port->irq, xuartps_isr, 0, XUARTPS_NAME,
+ (void *)port);
+ if (retval)
+ return retval;
+
+ /* Disable the TX and RX to set baud rate */
+ xuartps_writel(XUARTPS_CR_TX_DIS | XUARTPS_CR_RX_DIS,
+ XUARTPS_CR_OFFSET);
+
+ xuartps_set_baud_rate(port, 9600);
+
+ /* Set the Control Register with TX/RX Enable, TX/RX Reset,
+ * no break chars.
+ */
+ xuartps_writel(XUARTPS_CR_TXRST | XUARTPS_CR_RXRST,
+ XUARTPS_CR_OFFSET);
+
+ status = xuartps_readl(XUARTPS_CR_OFFSET);
+
+ /* Clear the RX disable and TX disable bits and then set the TX enable
+ * bit and RX enable bit to enable the transmitter and receiver.
+ */
+ xuartps_writel((status & ~(XUARTPS_CR_TX_DIS | XUARTPS_CR_RX_DIS))
+ | (XUARTPS_CR_TX_EN | XUARTPS_CR_RX_EN |
+ XUARTPS_CR_STOPBRK), XUARTPS_CR_OFFSET);
+
+ /* Set the Mode Register with normal mode,8 data bits,1 stop bit,
+ * no parity.
+ */
+ xuartps_writel(XUARTPS_MR_CHMODE_NORM | XUARTPS_MR_STOPMODE_1_BIT
+ | XUARTPS_MR_PARITY_NONE | XUARTPS_MR_CHARLEN_8_BIT,
+ XUARTPS_MR_OFFSET);
+
+ /* Set the RX FIFO Trigger level to 14 assuming FIFO size as 16 */
+ xuartps_writel(14, XUARTPS_RXWM_OFFSET);
+
+ /* Receive Timeout register is enabled with value of 10 */
+ xuartps_writel(10, XUARTPS_RXTOUT_OFFSET);
+
+
+ /* Set the Interrupt Registers with desired interrupts */
+ xuartps_writel(XUARTPS_IXR_TXEMPTY | XUARTPS_IXR_PARITY |
+ XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN |
+ XUARTPS_IXR_RXTRIG | XUARTPS_IXR_TOUT, XUARTPS_IER_OFFSET);
+ xuartps_writel(~(XUARTPS_IXR_TXEMPTY | XUARTPS_IXR_PARITY |
+ XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN |
+ XUARTPS_IXR_RXTRIG | XUARTPS_IXR_TOUT), XUARTPS_IDR_OFFSET);
+
+ return retval;
+}
+
+/**
+ * xuartps_shutdown - Called when an application closes a xuartps port
+ * @port: Handle to the uart port structure
+ *
+ **/
+static void xuartps_shutdown(struct uart_port *port)
+{
+ int status;
+
+ /* Disable interrupts */
+ status = xuartps_readl(XUARTPS_IMR_OFFSET);
+ xuartps_writel(status, XUARTPS_IDR_OFFSET);
+
+ /* Disable the TX and RX */
+ xuartps_writel(XUARTPS_CR_TX_DIS | XUARTPS_CR_RX_DIS,
+ XUARTPS_CR_OFFSET);
+ free_irq(port->irq, port);
+}
+
+/**
+ * xuartps_type - Set UART type to xuartps port
+ * @port: Handle to the uart port structure
+ *
+ * Returns string on success, NULL otherwise
+ **/
+static const char *xuartps_type(struct uart_port *port)
+{
+ return port->type == PORT_XUARTPS ? XUARTPS_NAME : NULL;
+}
+
+/**
+ * xuartps_verify_port - Verify the port params
+ * @port: Handle to the uart port structure
+ * @ser: Handle to the structure whose members are compared
+ *
+ * Returns 0 if success otherwise -EINVAL
+ **/
+static int xuartps_verify_port(struct uart_port *port,
+ struct serial_struct *ser)
+{
+ if (ser->type != PORT_UNKNOWN && ser->type != PORT_XUARTPS)
+ return -EINVAL;
+ if (port->irq != ser->irq)
+ return -EINVAL;
+ if (ser->io_type != UPIO_MEM)
+ return -EINVAL;
+ if (port->iobase != ser->port)
+ return -EINVAL;
+ if (ser->hub6 != 0)
+ return -EINVAL;
+ return 0;
+}
+
+/**
+ * xuartps_request_port - Claim the memory region attached to xuartps port,
+ * called when the driver adds a xuartps port via
+ * uart_add_one_port()
+ * @port: Handle to the uart port structure
+ *
+ * Returns 0, -ENOMEM if request fails
+ **/
+static int xuartps_request_port(struct uart_port *port)
+{
+ if (!request_mem_region(port->mapbase, XUARTPS_REGISTER_SPACE,
+ XUARTPS_NAME)) {
+ return -ENOMEM;
+ }
+
+ port->membase = ioremap(port->mapbase, XUARTPS_REGISTER_SPACE);
+ if (!port->membase) {
+ dev_err(port->dev, "Unable to map registers\n");
+ release_mem_region(port->mapbase, XUARTPS_REGISTER_SPACE);
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+/**
+ * xuartps_release_port - Release the memory region attached to a xuartps
+ * port, called when the driver removes a xuartps
+ * port via uart_remove_one_port().
+ * @port: Handle to the uart port structure
+ *
+ **/
+static void xuartps_release_port(struct uart_port *port)
+{
+ release_mem_region(port->mapbase, XUARTPS_REGISTER_SPACE);
+ iounmap(port->membase);
+ port->membase = NULL;
+}
+
+/**
+ * xuartps_config_port - Configure xuartps, called when the driver adds a
+ * xuartps port
+ * @port: Handle to the uart port structure
+ * @flags: If any
+ *
+ **/
+static void xuartps_config_port(struct uart_port *port, int flags)
+{
+ if (flags & UART_CONFIG_TYPE && xuartps_request_port(port) == 0)
+ port->type = PORT_XUARTPS;
+
+}
+
+/**
+ * xuartps_get_mctrl - Get the modem control state
+ *
+ * @port: Handle to the uart port structure
+ *
+ * Returns the modem control state
+ *
+ **/
+static unsigned int xuartps_get_mctrl(struct uart_port *port)
+{
+ return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+}
+
+static void xuartps_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+ /* N/A */
+}
+
+static void xuartps_enable_ms(struct uart_port *port)
+{
+ /* N/A */
+}
+
+/** The UART operations structure
+ */
+static struct uart_ops xuartps_ops = {
+ .set_mctrl = xuartps_set_mctrl,
+ .get_mctrl = xuartps_get_mctrl,
+ .enable_ms = xuartps_enable_ms,
+
+ .start_tx = xuartps_start_tx, /* Start transmitting */
+ .stop_tx = xuartps_stop_tx, /* Stop transmission */
+ .stop_rx = xuartps_stop_rx, /* Stop reception */
+ .tx_empty = xuartps_tx_empty, /* Transmitter busy? */
+ .break_ctl = xuartps_break_ctl, /* Start/stop
+ * transmitting break
+ */
+ .set_termios = xuartps_set_termios, /* Set termios */
+ .startup = xuartps_startup, /* App opens xuartps */
+ .shutdown = xuartps_shutdown, /* App closes xuartps */
+ .type = xuartps_type, /* Set UART type */
+ .verify_port = xuartps_verify_port, /* Verification of port
+ * params
+ */
+ .request_port = xuartps_request_port, /* Claim resources
+ * associated with a
+ * xuartps port
+ */
+ .release_port = xuartps_release_port, /* Release resources
+ * associated with a
+ * xuartps port
+ */
+ .config_port = xuartps_config_port, /* Configure when driver
+ * adds a xuartps port
+ */
+};
+
+static struct uart_port xuartps_port[2];
+
+/**
+ * xuartps_get_port - Configure the port from the platform device resource
+ * info
+ *
+ * Returns a pointer to a uart_port or NULL for failure
+ **/
+static struct uart_port *xuartps_get_port(void)
+{
+ struct uart_port *port;
+ int id;
+
+ /* Find the next unused port */
+ for (id = 0; id < XUARTPS_NR_PORTS; id++)
+ if (xuartps_port[id].mapbase == 0)
+ break;
+
+ if (id >= XUARTPS_NR_PORTS)
+ return NULL;
+
+ port = &xuartps_port[id];
+
+ /* At this point, we've got an empty uart_port struct, initialize it */
+ spin_lock_init(&port->lock);
+ port->membase = NULL;
+ port->iobase = 1; /* mark port in use */
+ port->irq = NO_IRQ;
+ port->type = PORT_UNKNOWN;
+ port->iotype = UPIO_MEM32;
+ port->flags = UPF_BOOT_AUTOCONF;
+ port->ops = &xuartps_ops;
+ port->fifosize = XUARTPS_FIFO_SIZE;
+ port->line = id;
+ port->dev = NULL;
+ return port;
+}
+
+/*-----------------------Console driver operations--------------------------*/
+
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+/**
+ * xuartps_console_wait_tx - Wait for the TX to be full
+ * @port: Handle to the uart port structure
+ *
+ **/
+static void xuartps_console_wait_tx(struct uart_port *port)
+{
+ while ((xuartps_readl(XUARTPS_SR_OFFSET) & XUARTPS_SR_TXEMPTY)
+ != XUARTPS_SR_TXEMPTY)
+ barrier();
+}
+
+/**
+ * xuartps_console_putchar - write the character to the FIFO buffer
+ * @port: Handle to the uart port structure
+ * @ch: Character to be written
+ *
+ **/
+static void xuartps_console_putchar(struct uart_port *port, int ch)
+{
+ xuartps_console_wait_tx(port);
+ xuartps_writel(ch, XUARTPS_FIFO_OFFSET);
+}
+
+/**
+ * xuartps_console_write - perform write operation
+ * @port: Handle to the uart port structure
+ * @s: Pointer to character array
+ * @count: No of characters
+ **/
+static void xuartps_console_write(struct console *co, const char *s,
+ unsigned int count)
+{
+ struct uart_port *port = &xuartps_port[co->index];
+ unsigned long flags;
+ unsigned int imr;
+ int locked = 1;
+
+ if (oops_in_progress)
+ locked = spin_trylock_irqsave(&port->lock, flags);
+ else
+ spin_lock_irqsave(&port->lock, flags);
+
+ /* save and disable interrupt */
+ imr = xuartps_readl(XUARTPS_IMR_OFFSET);
+ xuartps_writel(imr, XUARTPS_IDR_OFFSET);
+
+ uart_console_write(port, s, count, xuartps_console_putchar);
+ xuartps_console_wait_tx(port);
+
+ /* restore interrupt state, it seems like there may be a h/w bug
+ * in that the interrupt enable register should not need to be
+ * written based on the data sheet
+ */
+ xuartps_writel(~imr, XUARTPS_IDR_OFFSET);
+ xuartps_writel(imr, XUARTPS_IER_OFFSET);
+
+ if (locked)
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+/**
+ * xuartps_console_setup - Initialize the uart to default config
+ * @co: Console handle
+ * @options: Initial settings of uart
+ *
+ * Returns 0, -ENODEV if no device
+ **/
+static int __init xuartps_console_setup(struct console *co, char *options)
+{
+ struct uart_port *port = &xuartps_port[co->index];
+ int baud = 9600;
+ int bits = 8;
+ int parity = 'n';
+ int flow = 'n';
+
+ if (co->index < 0 || co->index >= XUARTPS_NR_PORTS)
+ return -EINVAL;
+
+ if (!port->mapbase) {
+ pr_debug("console on ttyPS%i not present\n", co->index);
+ return -ENODEV;
+ }
+
+ if (options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+ return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver xuartps_uart_driver;
+
+static struct console xuartps_console = {
+ .name = XUARTPS_TTY_NAME,
+ .write = xuartps_console_write,
+ .device = uart_console_device,
+ .setup = xuartps_console_setup,
+ .flags = CON_PRINTBUFFER,
+ .index = -1, /* Specified on the cmdline (e.g. console=ttyPS ) */
+ .data = &xuartps_uart_driver,
+};
+
+/**
+ * xuartps_console_init - Initialization call
+ *
+ * Returns 0 on success, negative error otherwise
+ **/
+static int __init xuartps_console_init(void)
+{
+ register_console(&xuartps_console);
+ return 0;
+}
+
+console_initcall(xuartps_console_init);
+
+#endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */
+
+/** Structure Definitions
+ */
+static struct uart_driver xuartps_uart_driver = {
+ .owner = THIS_MODULE, /* Owner */
+ .driver_name = XUARTPS_NAME, /* Driver name */
+ .dev_name = XUARTPS_TTY_NAME, /* Node name */
+ .major = XUARTPS_MAJOR, /* Major number */
+ .minor = XUARTPS_MINOR, /* Minor number */
+ .nr = XUARTPS_NR_PORTS, /* Number of UART ports */
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+ .cons = &xuartps_console, /* Console */
+#endif
+};
+
+/* ---------------------------------------------------------------------
+ * Platform bus binding
+ */
+/**
+ * xuartps_probe - Platform driver probe
+ * @pdev: Pointer to the platform device structure
+ *
+ * Returns 0 on success, negative error otherwise
+ **/
+static int __devinit xuartps_probe(struct platform_device *pdev)
+{
+ int rc;
+ struct uart_port *port;
+ struct resource *res, *res2;
+ int clk = 0;
+
+#ifdef CONFIG_OF
+ const unsigned int *prop;
+
+ prop = of_get_property(pdev->dev.of_node, "clock", NULL);
+ if (prop)
+ clk = be32_to_cpup(prop);
+#else
+ clk = *((unsigned int *)(pdev->dev.platform_data));
+#endif
+ if (!clk) {
+ dev_err(&pdev->dev, "no clock specified\n");
+ return -ENODEV;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (!res2)
+ return -ENODEV;
+
+ /* Initialize the port structure */
+ port = xuartps_get_port();
+
+ if (!port) {
+ dev_err(&pdev->dev, "Cannot get uart_port structure\n");
+ return -ENODEV;
+ } else {
+ /* Register the port.
+ * This function also registers this device with the tty layer
+ * and triggers invocation of the config_port() entry point.
+ */
+ port->mapbase = res->start;
+ port->irq = res2->start;
+ port->dev = &pdev->dev;
+ port->uartclk = clk;
+ dev_set_drvdata(&pdev->dev, port);
+ rc = uart_add_one_port(&xuartps_uart_driver, port);
+ if (rc) {
+ dev_err(&pdev->dev, "uart_add_one_port() failed; \
+ err=%i\n", rc);
+ dev_set_drvdata(&pdev->dev, NULL);
+ return rc;
+ }
+ return 0;
+ }
+}
+
+/**
+ * xuartps_remove - called when the platform driver is unregistered
+ * @pdev: Pointer to the platform device structure
+ *
+ * Returns 0 on success, negative error otherwise
+ **/
+static int __devexit xuartps_remove(struct platform_device *pdev)
+{
+ struct uart_port *port = dev_get_drvdata(&pdev->dev);
+ int rc = 0;
+
+ /* Remove the xuartps port from the serial core */
+ if (port) {
+ rc = uart_remove_one_port(&xuartps_uart_driver, port);
+ dev_set_drvdata(&pdev->dev, NULL);
+ port->mapbase = 0;
+ }
+ return rc;
+}
+
+/**
+ * xuartps_suspend - suspend event
+ * @pdev: Pointer to the platform device structure
+ * @state: State of the device
+ *
+ * Returns 0
+ **/
+static int xuartps_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ /* Call the API provided in serial_core.c file which handles
+ * the suspend.
+ */
+ uart_suspend_port(&xuartps_uart_driver, &xuartps_port[pdev->id]);
+ return 0;
+}
+
+/**
+ * xuartps_resume - Resume after a previous suspend
+ * @pdev: Pointer to the platform device structure
+ *
+ * Returns 0
+ **/
+static int xuartps_resume(struct platform_device *pdev)
+{
+ uart_resume_port(&xuartps_uart_driver, &xuartps_port[pdev->id]);
+ return 0;
+}
+
+/* Match table for of_platform binding */
+
+#ifdef CONFIG_OF
+static struct of_device_id xuartps_of_match[] __devinitdata = {
+ { .compatible = "xlnx,xuartps", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, xuartps_of_match);
+#endif
+
+static struct platform_driver xuartps_platform_driver = {
+ .probe = xuartps_probe, /* Probe method */
+ .remove = __exit_p(xuartps_remove), /* Detach method */
+ .suspend = xuartps_suspend, /* Suspend */
+ .resume = xuartps_resume, /* Resume after a suspend */
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = XUARTPS_NAME, /* Driver name */
+#ifdef CONFIG_OF
+ .of_match_table = xuartps_of_match,
+#endif
+ },
+};
+
+
+/* ---------------------------------------------------------------------
+ * Module Init and Exit
+ */
+/**
+ * xuartps_init - Initial driver registration call
+ *
+ * Returns whether the registration was successful or not
+ **/
+static int __init xuartps_init(void)
+{
+ int retval = 0;
+
+ /* Register the xuartps driver with the serial core */
+ retval = uart_register_driver(&xuartps_uart_driver);
+ if (retval)
+ return retval;
+
+ /* Register the platform driver */
+ retval = platform_driver_register(&xuartps_platform_driver);
+ if (retval)
+ uart_unregister_driver(&xuartps_uart_driver);
+
+ return retval;
+}
+
+/**
+ * xuartps_exit - Driver unregistration call
+ **/
+static void __exit xuartps_exit(void)
+{
+ /* The order of unregistration is important. Unregister the
+ * UART driver before the platform driver crashes the system.
+ */
+
+ /* Unregister the platform driver */
+ platform_driver_unregister(&xuartps_platform_driver);
+
+ /* Unregister the xuartps driver */
+ uart_unregister_driver(&xuartps_uart_driver);
+}
+
+module_init(xuartps_init);
+module_exit(xuartps_exit);
+
+MODULE_DESCRIPTION("Driver for PS UART");
+MODULE_AUTHOR("Xilinx Inc.");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 758c5b0..95d479b 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -202,6 +202,9 @@
/* VIA VT8500 SoC */
#define PORT_VT8500 97

+/* Xilinx PSS UART */
+#define PORT_XUARTPS 98
+
#ifdef __KERNEL__

#include <linux/compiler.h>
--
1.5.4.7



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.


2011-04-19 21:14:32

by Alan

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART

On Tue, 19 Apr 2011 14:14:52 -0600
John Linn <[email protected]> wrote:

> The Xilinx PS Uart is used on the new ARM based SoC. This
> UART is not compatible with others such that a seperate
> driver is required.

Joyous. I wish people would standardise.

> + 213 = /dev/ttyPS0 Xilinx PS serial port 0
> + 214 = /dev/ttyPS1 Xilinx PS serial port 1
> + 215 = /dev/ttyPS2 Xilinx PS serial port 2
> + 216 = /dev/ttyPS3 Xilinx PS serial port 3

Is there a specific reason you need fixed minor numbers ? If not please
use a dynamic range and keep Linus happy.

> +/**
> + * xuartps_isr - Interrupt handler
> + * @irq: Irq number
> + * @dev_id: Id of the port
> + *
> + * Returns IRQHANDLED
> + **/
> +static irqreturn_t xuartps_isr(int irq, void *dev_id)
> +{
> + struct uart_port *port = (struct uart_port *)dev_id;
> + struct tty_struct *tty = port->state->port.tty;
> + unsigned long flags;
> + unsigned int isrstatus, numbytes;
> + unsigned int data;
> + char status = TTY_NORMAL;
> +
> + spin_lock_irqsave(&port->lock, flags);

The ttys are refcounting now and your locking subtly wrong if you want to
avoid it (you lookup port-> stuff before you lock it)

The best way to do this is

tty = tty_port_tty_get(&port->state->port);

/* Returns a tty with reference or NULL */

Do stuff

tty_kref_put(tty);



> +static void xuartps_set_termios(struct uart_port *port,
> + struct ktermios *termios, struct ktermios *old)
> +{

> + /* Min baud rate = 6bps and Max Baud Rate is 10Mbps for 100Mhz clk */
> + baud = uart_get_baud_rate(port, termios, old, 0, 460800);
> + xuartps_set_baud_rate(port, baud);

So why pass 460800 ?

> + if (termios->c_iflag & INPCK)
> + port->read_status_mask |= XUARTPS_IXR_PARITY |
> + XUARTPS_IXR_FRAMING;
> +
> + if (termios->c_iflag & IGNPAR)
> + port->ignore_status_mask |= XUARTPS_IXR_PARITY |
> + XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
> +
> + /* ignore all characters if CREAD is not set */
> + if ((termios->c_cflag & CREAD) == 0)
> + port->ignore_status_mask |= XUARTPS_IXR_RXTRIG |
> + XUARTPS_IXR_TOUT | XUARTPS_IXR_PARITY |
> + XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
> +
> + mode_reg = xuartps_readl(XUARTPS_MR_OFFSET);
> +
> + /* Handling Data Size */
> + switch (termios->c_cflag & CSIZE) {
> + case CS6:
> + cval |= XUARTPS_MR_CHARLEN_6_BIT;
> + break;
> + case CS7:
> + cval |= XUARTPS_MR_CHARLEN_7_BIT;
> + break;
> + default:
> + case CS8:
> + cval |= XUARTPS_MR_CHARLEN_8_BIT;
> + break;

You need to clear/set appropriately any flags for hardware requests you
cannot meet. So your default should be

termios->c_cflag &= ~CSIZE;
termios->c_cflag |= CS8;

to rewrite CS5

And set the baud rate (see 8250.c for an example). Note that the helper
functions know about mapping slight errors so if you are asked for 9600
and the hardware does 9575 it will report B9600 as you'd expect not do
something crazy.

> + xuartps_set_baud_rate(port, 9600);

This seems a bit odd in the startup or is there a hardware need to do
this and then fix the rate ? Ditto some of the others


Otherwise looks fine.

Alan

2011-04-19 21:51:05

by John Linn

[permalink] [raw]
Subject: RE: [PATCH] tty/serial: add support for Xilinx PS UART

> -----Original Message-----
> From: Alan Cox [mailto:[email protected]]
> Sent: Tuesday, April 19, 2011 3:16 PM
> To: John Linn
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
>
> On Tue, 19 Apr 2011 14:14:52 -0600
> John Linn <[email protected]> wrote:
>
> > The Xilinx PS Uart is used on the new ARM based SoC. This
> > UART is not compatible with others such that a seperate
> > driver is required.
>
> Joyous. I wish people would standardise.

Yes I agree, I don't always have control of this situation. I'd
also rather not re-invent the wheel.

>
> > + 213 = /dev/ttyPS0 Xilinx PS serial port 0
> > + 214 = /dev/ttyPS1 Xilinx PS serial port 1
> > + 215 = /dev/ttyPS2 Xilinx PS serial port 2
> > + 216 = /dev/ttyPS3 Xilinx PS serial port 3
>
> Is there a specific reason you need fixed minor numbers ? If not
please
> use a dynamic range and keep Linus happy.

I don't know honestly. I'll need to dig into this feature and understand
it
better.

>
> > +/**
> > + * xuartps_isr - Interrupt handler
> > + * @irq: Irq number
> > + * @dev_id: Id of the port
> > + *
> > + * Returns IRQHANDLED
> > + **/
> > +static irqreturn_t xuartps_isr(int irq, void *dev_id)
> > +{
> > + struct uart_port *port = (struct uart_port *)dev_id;
> > + struct tty_struct *tty = port->state->port.tty;
> > + unsigned long flags;
> > + unsigned int isrstatus, numbytes;
> > + unsigned int data;
> > + char status = TTY_NORMAL;
> > +
> > + spin_lock_irqsave(&port->lock, flags);
>
> The ttys are refcounting now and your locking subtly wrong if you want
> to
> avoid it (you lookup port-> stuff before you lock it)
>
> The best way to do this is
>
> tty = tty_port_tty_get(&port->state->port);
>
> /* Returns a tty with reference or NULL */
>
> Do stuff
>
> tty_kref_put(tty);

Thanks, I'll update to this new way.

>
>
>
> > +static void xuartps_set_termios(struct uart_port *port,
> > + struct ktermios *termios, struct
ktermios *old)
> > +{
>
> > + /* Min baud rate = 6bps and Max Baud Rate is 10Mbps for 100Mhz
> clk */
> > + baud = uart_get_baud_rate(port, termios, old, 0, 460800);
> > + xuartps_set_baud_rate(port, baud);
>
> So why pass 460800 ?

Seems like 115200 is better number.

>
> > + if (termios->c_iflag & INPCK)
> > + port->read_status_mask |= XUARTPS_IXR_PARITY |
> > + XUARTPS_IXR_FRAMING;
> > +
> > + if (termios->c_iflag & IGNPAR)
> > + port->ignore_status_mask |= XUARTPS_IXR_PARITY |
> > + XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
> > +
> > + /* ignore all characters if CREAD is not set */
> > + if ((termios->c_cflag & CREAD) == 0)
> > + port->ignore_status_mask |= XUARTPS_IXR_RXTRIG |
> > + XUARTPS_IXR_TOUT | XUARTPS_IXR_PARITY |
> > + XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
> > +
> > + mode_reg = xuartps_readl(XUARTPS_MR_OFFSET);
> > +
> > + /* Handling Data Size */
> > + switch (termios->c_cflag & CSIZE) {
> > + case CS6:
> > + cval |= XUARTPS_MR_CHARLEN_6_BIT;
> > + break;
> > + case CS7:
> > + cval |= XUARTPS_MR_CHARLEN_7_BIT;
> > + break;
> > + default:
> > + case CS8:
> > + cval |= XUARTPS_MR_CHARLEN_8_BIT;
> > + break;
>
> You need to clear/set appropriately any flags for hardware requests
you
> cannot meet. So your default should be
>
> termios->c_cflag &= ~CSIZE;
> termios->c_cflag |= CS8;
>
> to rewrite CS5
>

Makes sense, easy to fix.

> And set the baud rate (see 8250.c for an example). Note that the
helper
> functions know about mapping slight errors so if you are asked for
9600
> and the hardware does 9575 it will report B9600 as you'd expect not do
> something crazy.
>

Sorry I didn't follow what you meant above. The h/w is a bit different
with it's
baud rate settings due to 2 different dividers.

> > + xuartps_set_baud_rate(port, 9600);
>
> This seems a bit odd in the startup or is there a hardware need to do
> this and then fix the rate ? Ditto some of the others

Could be left over from quite some time ago when the h/w was early and
buggy.
I'll check it out better and see if it can go away.

Thanks, appreciate the review and input.
John

>
>
> Otherwise looks fine.
>
> Alan


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2011-04-19 22:23:52

by John Linn

[permalink] [raw]
Subject: RE: [PATCH] tty/serial: add support for Xilinx PS UART

> -----Original Message-----
> From: Alan Cox [mailto:[email protected]]
> Sent: Tuesday, April 19, 2011 3:16 PM
> To: John Linn
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
>
> On Tue, 19 Apr 2011 14:14:52 -0600
> John Linn <[email protected]> wrote:
>
> > The Xilinx PS Uart is used on the new ARM based SoC. This
> > UART is not compatible with others such that a seperate
> > driver is required.
>
> Joyous. I wish people would standardise.
>
> > + 213 = /dev/ttyPS0 Xilinx PS serial port 0
> > + 214 = /dev/ttyPS1 Xilinx PS serial port 1
> > + 215 = /dev/ttyPS2 Xilinx PS serial port 2
> > + 216 = /dev/ttyPS3 Xilinx PS serial port 3
>
> Is there a specific reason you need fixed minor numbers ? If not
please
> use a dynamic range and keep Linus happy.

Hi Alan,

I hope you don't mind me asking a bit more to better understand. Here's
my
concerns (maybe not valid).

It seems like since this is a console it can get hard to debug with
dynamic
nodes for this driver.

This driver is for an embedded device where we don't want to require
udev
or mdev to assign nodes.

It seems like the static device nodes are typical for serial consoles in
the
drivers. I'm still digging in on the subject.

Thanks,
John

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2011-04-19 23:22:15

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART

On Tue, Apr 19, 2011 at 04:22:00PM -0600, John Linn wrote:
> > -----Original Message-----
> > From: Alan Cox [mailto:[email protected]]
> > Sent: Tuesday, April 19, 2011 3:16 PM
> > To: John Linn
> > Cc: [email protected]; [email protected]
> > Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
> >
> > On Tue, 19 Apr 2011 14:14:52 -0600
> > John Linn <[email protected]> wrote:
> >
> > > The Xilinx PS Uart is used on the new ARM based SoC. This
> > > UART is not compatible with others such that a seperate
> > > driver is required.
> >
> > Joyous. I wish people would standardise.
> >
> > > + 213 = /dev/ttyPS0 Xilinx PS serial port 0
> > > + 214 = /dev/ttyPS1 Xilinx PS serial port 1
> > > + 215 = /dev/ttyPS2 Xilinx PS serial port 2
> > > + 216 = /dev/ttyPS3 Xilinx PS serial port 3
> >
> > Is there a specific reason you need fixed minor numbers ? If not
> please
> > use a dynamic range and keep Linus happy.
>
> Hi Alan,
>
> I hope you don't mind me asking a bit more to better understand. Here's
> my
> concerns (maybe not valid).
>
> It seems like since this is a console it can get hard to debug with
> dynamic
> nodes for this driver.
>
> This driver is for an embedded device where we don't want to require
> udev
> or mdev to assign nodes.

Why not use devtmpfs? There's no need to use udev or mdev at all.

thanks,

greg k-h

2011-04-19 23:39:46

by John Linn

[permalink] [raw]
Subject: RE: [PATCH] tty/serial: add support for Xilinx PS UART

> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Tuesday, April 19, 2011 5:22 PM
> To: John Linn
> Cc: Alan Cox; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
>
> On Tue, Apr 19, 2011 at 04:22:00PM -0600, John Linn wrote:
> > > -----Original Message-----
> > > From: Alan Cox [mailto:[email protected]]
> > > Sent: Tuesday, April 19, 2011 3:16 PM
> > > To: John Linn
> > > Cc: [email protected]; [email protected]
> > > Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
> > >
> > > On Tue, 19 Apr 2011 14:14:52 -0600
> > > John Linn <[email protected]> wrote:
> > >
> > > > The Xilinx PS Uart is used on the new ARM based SoC. This
> > > > UART is not compatible with others such that a seperate
> > > > driver is required.
> > >
> > > Joyous. I wish people would standardise.
> > >
> > > > + 213 = /dev/ttyPS0 Xilinx PS serial
port 0
> > > > + 214 = /dev/ttyPS1 Xilinx PS serial
port 1
> > > > + 215 = /dev/ttyPS2 Xilinx PS serial
port 2
> > > > + 216 = /dev/ttyPS3 Xilinx PS serial
port 3
> > >
> > > Is there a specific reason you need fixed minor numbers ? If not
> > please
> > > use a dynamic range and keep Linus happy.
> >
> > Hi Alan,
> >
> > I hope you don't mind me asking a bit more to better understand.
> Here's
> > my
> > concerns (maybe not valid).
> >
> > It seems like since this is a console it can get hard to debug with
> > dynamic
> > nodes for this driver.
> >
> > This driver is for an embedded device where we don't want to require
> > udev
> > or mdev to assign nodes.
>
> Why not use devtmpfs? There's no need to use udev or mdev at all.
>

Hi Greg,

Reading up on it a bit as I'm not an expert on it (like you).
It's not clear to me what the device driver uses for it's major/minor in
this case.

Maybe it doesn't matter, the driver can use ttyS0 and then it gets
deleted
later if you want to change it.

The only reason we didn't just use ttyS0 was that it's not a 8250 really
and
since we have are an FPGA people can add real 8250s in soft logic and
then
the system would get confusing.

Thanks,
John

> thanks,
>
> greg k-h


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2011-04-19 23:42:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART

On Tue, Apr 19, 2011 at 05:39:30PM -0600, John Linn wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > Sent: Tuesday, April 19, 2011 5:22 PM
> > To: John Linn
> > Cc: Alan Cox; [email protected]; linux-
> > [email protected]
> > Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
> >
> > On Tue, Apr 19, 2011 at 04:22:00PM -0600, John Linn wrote:
> > > > -----Original Message-----
> > > > From: Alan Cox [mailto:[email protected]]
> > > > Sent: Tuesday, April 19, 2011 3:16 PM
> > > > To: John Linn
> > > > Cc: [email protected]; [email protected]
> > > > Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
> > > >
> > > > On Tue, 19 Apr 2011 14:14:52 -0600
> > > > John Linn <[email protected]> wrote:
> > > >
> > > > > The Xilinx PS Uart is used on the new ARM based SoC. This
> > > > > UART is not compatible with others such that a seperate
> > > > > driver is required.
> > > >
> > > > Joyous. I wish people would standardise.
> > > >
> > > > > + 213 = /dev/ttyPS0 Xilinx PS serial
> port 0
> > > > > + 214 = /dev/ttyPS1 Xilinx PS serial
> port 1
> > > > > + 215 = /dev/ttyPS2 Xilinx PS serial
> port 2
> > > > > + 216 = /dev/ttyPS3 Xilinx PS serial
> port 3
> > > >
> > > > Is there a specific reason you need fixed minor numbers ? If not
> > > please
> > > > use a dynamic range and keep Linus happy.
> > >
> > > Hi Alan,
> > >
> > > I hope you don't mind me asking a bit more to better understand.
> > Here's
> > > my
> > > concerns (maybe not valid).
> > >
> > > It seems like since this is a console it can get hard to debug with
> > > dynamic
> > > nodes for this driver.
> > >
> > > This driver is for an embedded device where we don't want to require
> > > udev
> > > or mdev to assign nodes.
> >
> > Why not use devtmpfs? There's no need to use udev or mdev at all.
> >
>
> Hi Greg,
>
> Reading up on it a bit as I'm not an expert on it (like you).
> It's not clear to me what the device driver uses for it's major/minor in
> this case.
>
> Maybe it doesn't matter, the driver can use ttyS0 and then it gets
> deleted later if you want to change it.
>
> The only reason we didn't just use ttyS0 was that it's not a 8250 really
> and since we have are an FPGA people can add real 8250s in soft logic
> and then the system would get confusing.

No, the point is that you don't care about the major/minor number, as
userspace can open the proper named device node, with no need for udev
at all. And you can dynamically allocate your major/minor number for
your device and it all "just works".

So please follow what Alan said to do, there's no excuse that you have
to use udev to take advantage of dynamice device numbers on embedded
systems anymore.

thanks,

greg k-h

2011-04-19 23:47:23

by John Linn

[permalink] [raw]
Subject: RE: [PATCH] tty/serial: add support for Xilinx PS UART

> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Tuesday, April 19, 2011 5:43 PM
> To: John Linn
> Cc: Alan Cox; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
>
> On Tue, Apr 19, 2011 at 05:39:30PM -0600, John Linn wrote:
> > > -----Original Message-----
> > > From: Greg KH [mailto:[email protected]]
> > > Sent: Tuesday, April 19, 2011 5:22 PM
> > > To: John Linn
> > > Cc: Alan Cox; [email protected]; linux-
> > > [email protected]
> > > Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
> > >
> > > On Tue, Apr 19, 2011 at 04:22:00PM -0600, John Linn wrote:
> > > > > -----Original Message-----
> > > > > From: Alan Cox [mailto:[email protected]]
> > > > > Sent: Tuesday, April 19, 2011 3:16 PM
> > > > > To: John Linn
> > > > > Cc: [email protected]; [email protected]
> > > > > Subject: Re: [PATCH] tty/serial: add support for Xilinx PS
UART
> > > > >
> > > > > On Tue, 19 Apr 2011 14:14:52 -0600
> > > > > John Linn <[email protected]> wrote:
> > > > >
> > > > > > The Xilinx PS Uart is used on the new ARM based SoC. This
> > > > > > UART is not compatible with others such that a seperate
> > > > > > driver is required.
> > > > >
> > > > > Joyous. I wish people would standardise.
> > > > >
> > > > > > + 213 = /dev/ttyPS0 Xilinx PS serial
> > port 0
> > > > > > + 214 = /dev/ttyPS1 Xilinx PS serial
> > port 1
> > > > > > + 215 = /dev/ttyPS2 Xilinx PS serial
> > port 2
> > > > > > + 216 = /dev/ttyPS3 Xilinx PS serial
> > port 3
> > > > >
> > > > > Is there a specific reason you need fixed minor numbers ? If
> not
> > > > please
> > > > > use a dynamic range and keep Linus happy.
> > > >
> > > > Hi Alan,
> > > >
> > > > I hope you don't mind me asking a bit more to better understand.
> > > Here's
> > > > my
> > > > concerns (maybe not valid).
> > > >
> > > > It seems like since this is a console it can get hard to debug
> with
> > > > dynamic
> > > > nodes for this driver.
> > > >
> > > > This driver is for an embedded device where we don't want to
> require
> > > > udev
> > > > or mdev to assign nodes.
> > >
> > > Why not use devtmpfs? There's no need to use udev or mdev at all.
> > >
> >
> > Hi Greg,
> >
> > Reading up on it a bit as I'm not an expert on it (like you).
> > It's not clear to me what the device driver uses for it's
major/minor
> in
> > this case.
> >
> > Maybe it doesn't matter, the driver can use ttyS0 and then it gets
> > deleted later if you want to change it.
> >
> > The only reason we didn't just use ttyS0 was that it's not a 8250
> really
> > and since we have are an FPGA people can add real 8250s in soft
logic
> > and then the system would get confusing.
>
> No, the point is that you don't care about the major/minor number, as
> userspace can open the proper named device node, with no need for udev
> at all. And you can dynamically allocate your major/minor number for
> your device and it all "just works".
>
> So please follow what Alan said to do, there's no excuse that you have
> to use udev to take advantage of dynamice device numbers on embedded
> systems anymore.
>

Great, I will dig into it more and give it a test. Learn something new
everyday in Linux.

Thanks for clarifying.
John

> thanks,
>
> greg k-h


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2011-04-20 09:24:02

by Alan

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART

> > > + /* Min baud rate = 6bps and Max Baud Rate is 10Mbps for 100Mhz
> > clk */
> > > + baud = uart_get_baud_rate(port, termios, old, 0, 460800);
> > > + xuartps_set_baud_rate(port, baud);
> >
> > So why pass 460800 ?
>
> Seems like 115200 is better number.

Well if it can do 10Mbit why not pass 10Mbit as the upper limit ?

> > And set the baud rate (see 8250.c for an example). Note that the
> helper
> > functions know about mapping slight errors so if you are asked for
> 9600
> > and the hardware does 9575 it will report B9600 as you'd expect not do
> > something crazy.
> >
>
> Sorry I didn't follow what you meant above. The h/w is a bit different
> with it's
> baud rate settings due to 2 different dividers.

After you've worked out what baud rate you actually set do

/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);


which will ensure that the termios reflects the actual rate.

Alan

2011-04-20 09:38:33

by Alan

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART

> Reading up on it a bit as I'm not an expert on it (like you).
> It's not clear to me what the device driver uses for it's major/minor in
> this case.

If you don't specify then the kernel picks one. udev can then create the
right /dev/ttyPS1/2/3 nodes itself. If you have devtmpfs enabled then
devtmpfs is a virtual file system which will just have the nodes in at as
specified by the drivers. It eliminates all the hard work maintaining
device nodes/ranges in user space.

> The only reason we didn't just use ttyS0 was that it's not a 8250 really

If you'd attempted to use ttyS0 you would indeed have been told to change
it
> and
> since we have are an FPGA people can add real 8250s in soft logic and
> then
> the system would get confusing.

and that's exactly why !

Alan

2011-04-20 13:43:49

by John Linn

[permalink] [raw]
Subject: RE: [PATCH] tty/serial: add support for Xilinx PS UART

> -----Original Message-----
> From: Alan Cox [mailto:[email protected]]
> Sent: Wednesday, April 20, 2011 3:25 AM
> To: John Linn
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
>
> > > > + /* Min baud rate = 6bps and Max Baud Rate is 10Mbps for
> 100Mhz
> > > clk */
> > > > + baud = uart_get_baud_rate(port, termios, old, 0,
460800);
> > > > + xuartps_set_baud_rate(port, baud);
> > >
> > > So why pass 460800 ?
> >
> > Seems like 115200 is better number.
>
> Well if it can do 10Mbit why not pass 10Mbit as the upper limit ?

The only reason would be because we don't test it, but yes that could be
done.

>
> > > And set the baud rate (see 8250.c for an example). Note that the
> > helper
> > > functions know about mapping slight errors so if you are asked for
> > 9600
> > > and the hardware does 9575 it will report B9600 as you'd expect
not
> do
> > > something crazy.
> > >
> >
> > Sorry I didn't follow what you meant above. The h/w is a bit
> different
> > with it's
> > baud rate settings due to 2 different dividers.
>
> After you've worked out what baud rate you actually set do
>
> /* Don't rewrite B0 */
> if (tty_termios_baud_rate(termios))
> tty_termios_encode_baud_rate(termios, baud, baud);
>
>
> which will ensure that the termios reflects the actual rate.

Thanks for clarifying, I'll look at that.

-- John

>
> Alan


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2011-04-20 15:19:40

by John Linn

[permalink] [raw]
Subject: RE: [PATCH] tty/serial: add support for Xilinx PS UART

> -----Original Message-----
> From: Alan Cox [mailto:[email protected]]
> Sent: Tuesday, April 19, 2011 3:16 PM
> To: John Linn
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART
>
> On Tue, 19 Apr 2011 14:14:52 -0600
> John Linn <[email protected]> wrote:
>
> > The Xilinx PS Uart is used on the new ARM based SoC. This
> > UART is not compatible with others such that a seperate
> > driver is required.
>
> Joyous. I wish people would standardise.
>
> > + 213 = /dev/ttyPS0 Xilinx PS serial port 0
> > + 214 = /dev/ttyPS1 Xilinx PS serial port 1
> > + 215 = /dev/ttyPS2 Xilinx PS serial port 2
> > + 216 = /dev/ttyPS3 Xilinx PS serial port 3
>
> Is there a specific reason you need fixed minor numbers ? If not
please
> use a dynamic range and keep Linus happy.
>
> > +/**
> > + * xuartps_isr - Interrupt handler
> > + * @irq: Irq number
> > + * @dev_id: Id of the port
> > + *
> > + * Returns IRQHANDLED
> > + **/
> > +static irqreturn_t xuartps_isr(int irq, void *dev_id)
> > +{
> > + struct uart_port *port = (struct uart_port *)dev_id;
> > + struct tty_struct *tty = port->state->port.tty;
> > + unsigned long flags;
> > + unsigned int isrstatus, numbytes;
> > + unsigned int data;
> > + char status = TTY_NORMAL;
> > +
> > + spin_lock_irqsave(&port->lock, flags);
>
> The ttys are refcounting now and your locking subtly wrong if you want
> to
> avoid it (you lookup port-> stuff before you lock it)
>
> The best way to do this is
>
> tty = tty_port_tty_get(&port->state->port);
>
> /* Returns a tty with reference or NULL */
>
> Do stuff
>
> tty_kref_put(tty);

I see the subtle locking issue you mention.

This looks like a good way to help it, but it's not clear to me what to
do in an isr
if the function returns a NULL for the tty. In other places you can say
the tty
was busy, but in an ISR that doesn't make much sense (to me anyway).

It seems easier in this case to just move the tty =
port->state->port.tty to after
the lock is acquired.

I'm sure there's something obvious I'm missing here.

Thanks,
John

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2011-04-20 15:34:36

by Alan

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: add support for Xilinx PS UART

> I see the subtle locking issue you mention.
>
> This looks like a good way to help it, but it's not clear to me what to
> do in an isr
> if the function returns a NULL for the tty. In other places you can say
> the tty

It can happen, you must deal with it.

> It seems easier in this case to just move the tty =
> port->state->port.tty to after
> the lock is acquired.

It can still be NULL.

> I'm sure there's something obvious I'm missing here.

It varies what drivers do but essentially what has occurred is that the
user side of the tty has gone away and you have an IRQ still. Your port
is still around as the lifetime of the port is the lifetime of your
serial driver creating/deleting it, but your tty has gone.

In those cases you need to do whatever needs doing to handle the IRQ and
processing that doesn't require the tty. That varies by device, in many
cases for example you read the data and chuck it rather than stuffing it
into the flip buffer.

Alan