[PATCH v3] tty: serial: add Freescale lpuart driver support

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Jingchang Lu]

Add Freescale lpuart driver support. The lpuart device
can be founded on Vybrid MVF600 and Layerscape LS-1 SoCs.

Signed-off-by: Jingchang Lu <b35083@freescale.com>
---
v3:
  change the uart driver name from mvf to lpuart for further share between SoCs.
  add bind doc in Documentation/devicetree/bindings/tty/serial.
  remove unused #include header lines.
  clean up code.

 .../devicetree/bindings/tty/serial/fsl-lpuart.txt  |  14 +
 drivers/tty/serial/Kconfig                         |  14 +
 drivers/tty/serial/Makefile                        |   1 +
 drivers/tty/serial/fsl_lpuart.c                    | 916 +++++++++++++++++++++
 include/uapi/linux/serial_core.h                   |   3 +
 5 files changed, 948 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt
 create mode 100644 drivers/tty/serial/fsl_lpuart.c

diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt
new file mode 100644
index 0000000..46ffdb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt
@@ -0,0 +1,14 @@
+* Freescale Low Power Universal Asynchronous Receiver/Transmitter (LPUART)
+
+Required properties:
+- compatible : Should be "fsl,lpuart"
+- reg : Address and length of the register set for the device
+- interrupts : Should contain uart interrupt
+
+Example:
+
+uart0: serial at 40027000 {
+	       compatible = "fsl,lpuart";
+	       reg = <0x40027000 0x1000>;
+	       interrupts = <0 61 0x00>;
+       };
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 7e7006f..2cea1a0 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1484,6 +1484,20 @@ config SERIAL_RP2_NR_UARTS
 	  If multiple cards are present, the default limit of 32 ports may
 	  need to be increased.
 
+config SERIAL_FSL_LPUART
+	bool "Freescale Lpuart serial port support"
+	select SERIAL_CORE
+	help
+	  Support for the on-chip LPUARTs on some Freescal SOCs.
+
+config SERIAL_FSL_LPUART_CONSOLE
+	bool "Console on Vybrid serial port"
+	depends on SERIAL_FSL_LPUART
+	select SERIAL_CORE_CONSOLE
+	help
+	  If you have enabled the lpuart serial port on the Freescale SoCs,
+	  you can make it the console by answering Y to this option.
+
 endmenu
 
 endif # TTY
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index eedfec4..cf650f0 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -85,3 +85,4 @@ obj-$(CONFIG_SERIAL_AR933X)   += ar933x_uart.o
 obj-$(CONFIG_SERIAL_EFM32_UART) += efm32-uart.o
 obj-$(CONFIG_SERIAL_ARC)	+= arc_uart.o
 obj-$(CONFIG_SERIAL_RP2)	+= rp2.o
+obj-$(CONFIG_SERIAL_FSL_LPUART)	+= fsl_lpuart.o
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
new file mode 100644
index 0000000..8753bc2
--- /dev/null
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -0,0 +1,916 @@
+/*
+ *  Freescale lpuart ports driver
+ *
+ *  Copyright 2012-2013 Freescale Semiconductor, 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.
+ */
+
+#if defined(CONFIG_SERIAL_FSL_LPUART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/clk.h>
+#include <linux/of_device.h>
+#include <linux/console.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/serial_core.h>
+#include <linux/pinctrl/consumer.h>
+
+/* All registers are 8-bit width */
+#define UARTBDH			0x00
+#define UARTBDL			0x01
+#define UARTCR1			0x02
+#define UARTCR2			0x03
+#define UARTSR1			0x04
+#define UARTCR3			0x06
+#define UARTDR			0x07
+#define UARTCR4			0x0a
+#define UARTCR5			0x0b
+#define UARTMODEM		0x0d
+#define UARTPFIFO		0x10
+#define UARTCFIFO		0x11
+#define UARTSFIFO		0x12
+#define UARTTWFIFO		0x13
+#define UARTTCFIFO		0x14
+#define UARTRWFIFO		0x15
+
+#define UARTBDH_LBKDIE		0x80
+#define UARTBDH_RXEDGIE		0x40
+#define UARTBDH_SBR_MASK	0x1f
+
+#define UARTCR1_LOOPS		0x80
+#define UARTCR1_RSRC		0x20
+#define UARTCR1_M		0x10
+#define UARTCR1_WAKE		0x08
+#define UARTCR1_ILT		0x04
+#define UARTCR1_PE		0x02
+#define UARTCR1_PT		0x01
+
+#define UARTCR2_TIE		0x80
+#define UARTCR2_TCIE		0x40
+#define UARTCR2_RIE		0x20
+#define UARTCR2_ILIE		0x10
+#define UARTCR2_TE		0x08
+#define UARTCR2_RE		0x04
+#define UARTCR2_RWU		0x02
+#define UARTCR2_SBK		0x01
+
+#define UARTSR1_TDRE		0x80
+#define UARTSR1_TC		0x40
+#define UARTSR1_RDRF		0x20
+#define UARTSR1_IDLE		0x10
+#define UARTSR1_OR		0x08
+#define UARTSR1_NF		0x04
+#define UARTSR1_FE		0x02
+#define UARTSR1_PE		0x01
+
+#define UARTCR3_R8		0x80
+#define UARTCR3_T8		0x40
+#define UARTCR3_TXDIR		0x20
+#define UARTCR3_TXINV		0x10
+#define UARTCR3_ORIE		0x08
+#define UARTCR3_NEIE		0x04
+#define UARTCR3_FEIE		0x02
+#define UARTCR3_PEIE		0x01
+
+#define UARTCR4_MAEN1		0x80
+#define UARTCR4_MAEN2		0x40
+#define UARTCR4_M10		0x20
+#define UARTCR4_BRFA_MASK	0x1f
+#define UARTCR4_BRFA_OFF	0
+
+#define UARTCR5_TDMAS		0x80
+#define UARTCR5_RDMAS		0x20
+
+#define UARTMODEM_RXRTSE	0x08
+#define UARTMODEM_TXRTSPOL	0x04
+#define UARTMODEM_TXRTSE	0x02
+#define UARTMODEM_TXCTSE	0x01
+
+#define UARTPFIFO_TXFE		0x80
+#define UARTPFIFO_FIFOSIZE_MASK	0x7
+#define UARTPFIFO_TXSIZE_OFF	4
+#define UARTPFIFO_RXFE		0x08
+#define UARTPFIFO_RXSIZE_OFF	0
+
+#define UARTCFIFO_TXFLUSH	0x80
+#define UARTCFIFO_RXFLUSH	0x40
+#define UARTCFIFO_RXOFE		0x04
+#define UARTCFIFO_TXOFE		0x02
+#define UARTCFIFO_RXUFE		0x01
+
+#define UARTSFIFO_TXEMPT	0x80
+#define UARTSFIFO_RXEMPT	0x40
+#define UARTSFIFO_RXOF		0x04
+#define UARTSFIFO_TXOF		0x02
+#define UARTSFIFO_RXUF		0x01
+
+#define DRIVER_NAME	"LP-uart"
+#define DEV_NAME	"ttyLP"
+#define UART_NR		6
+
+struct lpuart_port {
+	struct uart_port	port;
+	struct clk		*clk;
+	unsigned int		tx_fifo_size;
+	unsigned int		rx_fifo_size;
+};
+
+static struct of_device_id lpuart_uart_dt_ids[] = {
+	{
+		.compatible = "fsl,lpuart",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, lpuart_uart_dt_ids);
+
+static void lpuart_stop_tx(struct uart_port *port)
+{
+	struct lpuart_port *sport = (struct lpuart_port *)port;
+	unsigned char temp;
+
+	temp = readb(sport->port.membase + UARTCR2);
+	temp &= ~(UARTCR2_TIE | UARTCR2_TCIE);
+	writeb(temp, sport->port.membase + UARTCR2);
+}
+
+static void lpuart_stop_rx(struct uart_port *port)
+{
+	struct lpuart_port *sport = (struct lpuart_port *)port;
+	unsigned char temp;
+
+	temp = readb(sport->port.membase + UARTCR2);
+	writeb(temp & ~UARTCR2_RE, sport->port.membase + UARTCR2);
+}
+
+static void lpuart_enable_ms(struct uart_port *port)
+{
+}
+
+static inline void lpuart_transmit_buffer(struct lpuart_port *sport)
+{
+	struct circ_buf *xmit = &sport->port.state->xmit;
+
+	while (!uart_circ_empty(xmit) &&
+	(readb(sport->port.membase + UARTTCFIFO) < sport->tx_fifo_size)) {
+		writeb(xmit->buf[xmit->tail], sport->port.membase + UARTDR);
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		sport->port.icount.tx++;
+	}
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&sport->port);
+
+	if (uart_circ_empty(xmit))
+		lpuart_stop_tx(&sport->port);
+}
+
+static void lpuart_start_tx(struct uart_port *port)
+{
+	struct lpuart_port *sport = (struct lpuart_port *)port;
+	unsigned char temp;
+
+	temp = readb(sport->port.membase + UARTCR2);
+	writeb(temp | UARTCR2_TIE,
+			sport->port.membase + UARTCR2);
+
+	if (readb(sport->port.membase + UARTSR1) & UARTSR1_TDRE)
+		lpuart_transmit_buffer(sport);
+}
+
+static irqreturn_t lpuart_txint(int irq, void *dev_id)
+{
+	struct lpuart_port *sport = dev_id;
+	struct circ_buf *xmit = &sport->port.state->xmit;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sport->port.lock, flags);
+	if (sport->port.x_char) {
+		writeb(sport->port.x_char, sport->port.membase + UARTDR);
+		goto out;
+	}
+
+	if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
+		lpuart_stop_tx(&sport->port);
+		goto out;
+	}
+
+	lpuart_transmit_buffer(sport);
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&sport->port);
+
+out:
+	spin_unlock_irqrestore(&sport->port.lock, flags);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t lpuart_rxint(int irq, void *dev_id)
+{
+	struct lpuart_port *sport = dev_id;
+	unsigned int flg, ignored = 0;
+	struct tty_port *port = &sport->port.state->port;
+	unsigned long flags;
+	unsigned char r8, rx, sr;
+
+	spin_lock_irqsave(&sport->port.lock, flags);
+
+	while (!(readb(sport->port.membase + UARTSFIFO) &
+				UARTSFIFO_RXEMPT)) {
+		flg = TTY_NORMAL;
+		sport->port.icount.rx++;
+
+		/*  To clear the FE, OR, NF, FE, PE flags when set,
+		 *  read SR1 then read DR
+		 */
+		sr = readb(sport->port.membase + UARTSR1);
+
+		r8 = readb(sport->port.membase + UARTCR3) & UARTCR3_R8;
+		rx = readb(sport->port.membase + UARTDR);
+
+		if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx))
+			continue;
+		if (sr & (UARTSR1_PE | UARTSR1_OR | UARTSR1_FE)) {
+			if (sr & UARTSR1_PE)
+				sport->port.icount.parity++;
+			else if (sr & UARTSR1_FE)
+				sport->port.icount.frame++;
+			if (sr & UARTSR1_OR)
+				sport->port.icount.overrun++;
+
+			if (sr & sport->port.ignore_status_mask) {
+				if (++ignored > 100)
+					goto out;
+				continue;
+			}
+
+			sr &= sport->port.read_status_mask;
+
+			if (sr & UARTSR1_PE)
+				flg = TTY_PARITY;
+			else if (sr & UARTSR1_FE)
+				flg = TTY_FRAME;
+			if (sr & UARTSR1_OR)
+				flg = TTY_OVERRUN;
+
+#ifdef SUPPORT_SYSRQ
+			sport->port.sysrq = 0;
+#endif
+		}
+
+		tty_insert_flip_char(port, rx, flg);
+	}
+
+out:
+	spin_unlock_irqrestore(&sport->port.lock, flags);
+
+	tty_flip_buffer_push(port);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t lpuart_int(int irq, void *dev_id)
+{
+	struct lpuart_port *sport = dev_id;
+	unsigned int sts;
+
+	sts = readb(sport->port.membase + UARTSR1);
+
+	if (sts & UARTSR1_RDRF)
+		lpuart_rxint(irq, dev_id);
+
+	if (sts & UARTSR1_TDRE &&
+		!(readb(sport->port.membase + UARTCR5) & UARTCR5_TDMAS))
+		lpuart_txint(irq, dev_id);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Return TIOCSER_TEMT when transmitter is not busy.
+ */
+static unsigned int lpuart_tx_empty(struct uart_port *port)
+{
+
+	return (readb(port->membase + UARTSR1) & UARTSR1_TC) ?
+		TIOCSER_TEMT : 0;
+}
+
+static unsigned int lpuart_get_mctrl(struct uart_port *port)
+{
+	unsigned int tmp = 0;
+	unsigned char reg;
+
+	reg = readb(port->membase + UARTMODEM);
+	if (reg & UARTMODEM_TXCTSE)
+		tmp |= TIOCM_CTS;
+
+	if (reg & UARTMODEM_RXRTSE)
+		tmp |= TIOCM_RTS;
+
+	return tmp;
+}
+
+static void lpuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	unsigned long temp;
+
+	temp = readb(port->membase + UARTMODEM) &
+			~(UARTMODEM_RXRTSE | UARTMODEM_TXCTSE);
+
+	if (mctrl & TIOCM_RTS)
+		temp |= UARTMODEM_RXRTSE;
+	if (mctrl & TIOCM_CTS)
+		temp |= UARTMODEM_TXCTSE;
+
+	writeb(temp, port->membase + UARTMODEM);
+}
+
+static void lpuart_break_ctl(struct uart_port *port, int break_state)
+{
+	unsigned char temp;
+
+	temp = readb(port->membase + UARTCR2) & ~UARTCR2_SBK;
+
+	if (break_state != 0)
+		temp |= UARTCR2_SBK;
+
+	writeb(temp, port->membase + UARTCR2);
+
+}
+
+static void lpuart_setup_watermark(struct lpuart_port *sport)
+{
+	unsigned char val, old_cr2, cr2;
+
+	/* set receiver/transmitter trigger level. */
+	old_cr2 = cr2 = readb(sport->port.membase + UARTCR2);
+	cr2 &= ~(UARTCR2_TIE | UARTCR2_TCIE | UARTCR2_TE |
+			UARTCR2_RIE | UARTCR2_RE);
+	writeb(cr2, sport->port.membase + UARTCR2);
+
+	writeb(2, sport->port.membase + UARTTWFIFO);
+	writeb(1, sport->port.membase + UARTRWFIFO);
+
+	/* determine FIFO size and enable */
+	val = readb(sport->port.membase + UARTPFIFO);
+
+	sport->tx_fifo_size = 0x1 << (((val >> UARTPFIFO_TXSIZE_OFF) &
+		UARTPFIFO_FIFOSIZE_MASK) + 1);
+
+	sport->rx_fifo_size = 0x1 << (((val >> UARTPFIFO_RXSIZE_OFF) &
+		UARTPFIFO_FIFOSIZE_MASK) + 1);
+
+	writeb(val | UARTPFIFO_TXFE | UARTPFIFO_RXFE,
+			sport->port.membase + UARTPFIFO);
+
+	/* Flush the Tx and Rx FIFO to a known state */
+	writeb(UARTCFIFO_TXFLUSH | UARTCFIFO_RXFLUSH,
+			sport->port.membase + UARTCFIFO);
+
+	/* restore CR2 */
+	writeb(old_cr2, sport->port.membase + UARTCR2);
+
+}
+
+static int lpuart_startup(struct uart_port *port)
+{
+	struct lpuart_port *sport = (struct lpuart_port *)port;
+	int retval;
+	unsigned long flags, temp;
+
+	spin_lock_irqsave(&sport->port.lock, flags);
+
+	lpuart_setup_watermark(sport);
+
+	temp = readb(sport->port.membase + UARTCR2);
+	writeb(temp & ~UARTCR2_RIE, sport->port.membase + UARTCR2);
+
+	/* clear and enable interrupts */
+	temp = readb(sport->port.membase + UARTCR2);
+	temp |= UARTCR2_RIE | UARTCR2_TIE;
+	writeb(temp, sport->port.membase + UARTCR2);
+
+	spin_unlock_irqrestore(&sport->port.lock, flags);
+	/*
+	 * request the IRQ
+	 */
+	retval = devm_request_irq(port->dev, port->irq, lpuart_int, 0,
+				DRIVER_NAME, sport);
+	if (retval)
+		return retval;
+
+	return 0;
+}
+
+static void lpuart_shutdown(struct uart_port *port)
+{
+	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
+	unsigned char temp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	temp = readb(port->membase + UARTCR2);
+	temp &= ~(UARTCR2_TE | UARTCR2_RE);
+	writeb(temp, port->membase + UARTCR2);
+	/*
+	 * Disable all interrupts
+	 */
+	temp = readb(port->membase + UARTCR2);
+	temp &= ~(UARTCR2_TIE | UARTCR2_TCIE | UARTCR2_RIE);
+	writeb(temp, port->membase + UARTCR2);
+	spin_unlock_irqrestore(&port->lock, flags);
+	/*
+	 * Free the irq
+	 */
+	devm_free_irq(port->dev, port->irq, sport);
+}
+
+static void
+lpuart_set_termios(struct uart_port *port, struct ktermios *termios,
+		   struct ktermios *old)
+{
+	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
+	unsigned long flags;
+	unsigned char cr1, old_cr1, old_cr2, cr4, bdh, modem;
+	unsigned int  baud;
+	unsigned int old_csize = old ? old->c_cflag & CSIZE : CS8;
+	unsigned int sbr, brfa;
+
+	cr1 = old_cr1 = readb(sport->port.membase + UARTCR1);
+	old_cr2 = readb(sport->port.membase + UARTCR2);
+	cr4 = readb(sport->port.membase + UARTCR4);
+	bdh = readb(sport->port.membase + UARTBDH);
+	modem = readb(sport->port.membase + UARTMODEM);
+
+	/*
+	 * We only support CS8 and CS7, and for CS7 must enable PE.
+	 * supported mode:
+	 *  - (7,e/o,1)
+	 *  - (8,n,1)
+	 *  - (8,m/s,1)
+	 *  - (8,e/o,1)
+	 */
+	while ((termios->c_cflag & CSIZE) != CS8 &&
+		(termios->c_cflag & CSIZE) != CS7) {
+		termios->c_cflag &= ~CSIZE;
+		termios->c_cflag |= old_csize;
+		old_csize = CS8;
+	}
+
+	if ((termios->c_cflag & CSIZE) == CS8 ||
+		(termios->c_cflag & CSIZE) == CS7)
+		cr1 = old_cr1 & ~UARTCR1_M;
+
+	if (termios->c_cflag & CMSPAR) {
+		if ((termios->c_cflag & CSIZE) != CS8) {
+			termios->c_cflag &= ~CSIZE;
+			termios->c_cflag |= CS8;
+		}
+		cr1 |= UARTCR1_M;
+	}
+
+	if (termios->c_cflag & CRTSCTS) {
+		modem |= (UARTMODEM_RXRTSE | UARTMODEM_TXCTSE);
+
+	} else {
+		termios->c_cflag &= ~CRTSCTS;
+		modem &= ~(UARTMODEM_RXRTSE | UARTMODEM_TXCTSE);
+	}
+
+	if (termios->c_cflag & CSTOPB)
+		termios->c_cflag &= ~CSTOPB;
+
+	/* parity must enable when CS7 to match 8-bits format */
+	if ((termios->c_cflag & CSIZE) == CS7)
+		termios->c_cflag |= PARENB;
+
+	if ((termios->c_cflag & PARENB)) {
+		if (termios->c_cflag & CMSPAR) {
+			cr1 &= ~UARTCR1_PE;
+			cr1 |= UARTCR1_M;
+		} else {
+			cr1 |= UARTCR1_PE;
+			if ((termios->c_cflag & CSIZE) == CS8)
+				cr1 |= UARTCR1_M;
+			if (termios->c_cflag & PARODD)
+				cr1 |= UARTCR1_PT;
+			else
+				cr1 &= ~UARTCR1_PT;
+		}
+	}
+
+	/*
+	 * Ask the core to calculate the divisor for us.
+	 */
+	baud = uart_get_baud_rate(port, termios, old, 50, port->uartclk / 16);
+
+	spin_lock_irqsave(&sport->port.lock, flags);
+
+	sport->port.read_status_mask = 0;
+	if (termios->c_iflag & INPCK)
+		sport->port.read_status_mask |=
+					(UARTSR1_FE | UARTSR1_PE);
+	if (termios->c_iflag & (BRKINT | PARMRK))
+		sport->port.read_status_mask |= UARTSR1_FE;
+
+	/*
+	 * Characters to ignore
+	 */
+	sport->port.ignore_status_mask = 0;
+	if (termios->c_iflag & IGNPAR)
+		sport->port.ignore_status_mask |= UARTSR1_PE;
+	if (termios->c_iflag & IGNBRK) {
+		sport->port.ignore_status_mask |= UARTSR1_FE;
+		/*
+		 * If we're ignoring parity and break indicators,
+		 * ignore overruns too (for real raw support).
+		 */
+		if (termios->c_iflag & IGNPAR)
+			sport->port.ignore_status_mask |= UARTSR1_OR;
+	}
+
+	/*
+	 * Update the per-port timeout.
+	 */
+	uart_update_timeout(port, termios->c_cflag, baud);
+
+	/* wait transmit engin complete */
+	while (!(readb(sport->port.membase + UARTSR1) & UARTSR1_TC))
+		barrier();
+
+	writeb(old_cr2 & ~(UARTCR2_TE | UARTCR2_RE),
+			sport->port.membase + UARTCR2);
+
+	/* disable transmit and receive */
+	sbr = sport->port.uartclk / (16 * baud);
+	brfa = ((sport->port.uartclk - (16 * sbr * baud)) * 2) / baud;
+
+	bdh &= ~UARTBDH_SBR_MASK;
+	bdh |= (sbr >> 8) & 0x1F;
+
+	cr4 &= ~UARTCR4_BRFA_MASK;
+	brfa &= UARTCR4_BRFA_MASK;
+	writeb(cr4 | brfa, sport->port.membase + UARTCR4);
+	writeb(bdh, sport->port.membase + UARTBDH);
+	writeb(sbr & 0xFF, sport->port.membase + UARTBDL);
+	writeb(cr1, sport->port.membase + UARTCR1);
+	writeb(modem, sport->port.membase + UARTMODEM);
+
+	/* restore control register */
+	writeb(old_cr2, sport->port.membase + UARTCR2);
+
+	spin_unlock_irqrestore(&sport->port.lock, flags);
+}
+
+static const char *lpuart_type(struct uart_port *port)
+{
+	return "FSL_LPUART";
+}
+
+static void lpuart_release_port(struct uart_port *port)
+{
+	/* Nothing to do */
+}
+
+static int lpuart_request_port(struct uart_port *port)
+{
+	return  0;
+}
+/*
+ * Configure/autoconfigure the port.
+ */
+static void lpuart_config_port(struct uart_port *port, int flags)
+{
+	if (flags & UART_CONFIG_TYPE)
+		port->type = PORT_LPUART;
+}
+
+static int lpuart_verify_port(struct uart_port *port, struct serial_struct *ser)
+{
+	int ret = 0;
+
+	if (ser->type != PORT_UNKNOWN && ser->type != PORT_LPUART)
+		ret = -EINVAL;
+	if (port->irq != ser->irq)
+		ret = -EINVAL;
+	if (ser->io_type != UPIO_MEM)
+		ret = -EINVAL;
+	if (port->uartclk / 16 != ser->baud_base)
+		ret = -EINVAL;
+	if (port->iobase != ser->port)
+		ret = -EINVAL;
+	if (ser->hub6 != 0)
+		ret = -EINVAL;
+	return ret;
+}
+
+static struct uart_ops lpuart_pops = {
+	.tx_empty	= lpuart_tx_empty,
+	.set_mctrl	= lpuart_set_mctrl,
+	.get_mctrl	= lpuart_get_mctrl,
+	.stop_tx	= lpuart_stop_tx,
+	.start_tx	= lpuart_start_tx,
+	.stop_rx	= lpuart_stop_rx,
+	.enable_ms	= lpuart_enable_ms,
+	.break_ctl	= lpuart_break_ctl,
+	.startup	= lpuart_startup,
+	.shutdown	= lpuart_shutdown,
+	.set_termios	= lpuart_set_termios,
+	.type		= lpuart_type,
+	.request_port	= lpuart_request_port,
+	.release_port	= lpuart_release_port,
+	.config_port	= lpuart_config_port,
+	.verify_port	= lpuart_verify_port,
+};
+
+static struct lpuart_port *lpuart_ports[UART_NR];
+
+#ifdef CONFIG_SERIAL_FSL_LPUART_CONSOLE
+static void lpuart_console_putchar(struct uart_port *port, int ch)
+{
+	while (!(readb(port->membase + UARTSR1) & UARTSR1_TDRE))
+		barrier();
+
+	writeb(ch, port->membase + UARTDR);
+}
+
+static void
+lpuart_console_write(struct console *co, const char *s, unsigned int count)
+{
+	struct lpuart_port *sport = lpuart_ports[co->index];
+	unsigned int  old_cr2, cr2;
+
+	/*
+	 * First, save CR2 and then disable interrupts
+	 */
+	cr2 = old_cr2 = readb(sport->port.membase + UARTCR2);
+
+	cr2 |= (UARTCR2_TE |  UARTCR2_RE);
+	cr2 &= ~(UARTCR2_TIE | UARTCR2_TCIE | UARTCR2_RIE);
+
+	writeb(cr2, sport->port.membase + UARTCR2);
+
+	uart_console_write(&sport->port, s, count, lpuart_console_putchar);
+
+	/*
+	 * wait for transmitter finish complete and restore CR2
+	 */
+	while (!(readb(sport->port.membase + UARTSR1) & UARTSR1_TC))
+		barrier();
+
+	writeb(old_cr2, sport->port.membase + UARTCR2);
+}
+
+/*
+ * If the port was already initialised (eg, by a boot loader),
+ * try to determine the current setup.
+ */
+static void __init
+lpuart_console_get_options(struct lpuart_port *sport, int *baud,
+			   int *parity, int *bits)
+{
+	unsigned char cr, bdh, bdl, brfa;
+	unsigned int sbr, uartclk;
+	unsigned int baud_raw;
+
+	cr = readb(sport->port.membase + UARTCR2);
+	cr &= UARTCR2_TE | UARTCR2_RE;
+	if (!cr)
+		return;
+
+	/* ok, the port was enabled */
+
+	cr = readb(sport->port.membase + UARTCR1);
+
+	*parity = 'n';
+	if (cr & UARTCR1_PE) {
+		if (cr & UARTCR1_PT)
+			*parity = 'o';
+		else
+			*parity = 'e';
+	}
+
+	if (cr & UARTCR1_M)
+		*bits = 9;
+	else
+		*bits = 8;
+
+	bdh = readb(sport->port.membase + UARTBDH);
+	bdh &= UARTBDH_SBR_MASK;
+	bdl = readb(sport->port.membase + UARTBDL);
+	sbr = bdh;
+	sbr <<= 8;
+	sbr |= bdl;
+	brfa = readb(sport->port.membase + UARTCR4);
+	brfa &= UARTCR4_BRFA_MASK;
+
+	uartclk = clk_get_rate(sport->clk);
+	/*
+	 * Baud = mod_clk/(16*(sbr[13]+(brfa)/32)
+	 */
+	baud_raw = uartclk / (16 * (sbr + brfa / 32));
+
+	if (*baud != baud_raw)
+		printk(KERN_INFO "Serial: Console lpuart rounded baud rate"
+				"from %d to %d\n", baud_raw, *baud);
+}
+
+static int __init lpuart_console_setup(struct console *co, char *options)
+{
+	struct lpuart_port *sport;
+	int baud = 115200;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	/*
+	 * 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(lpuart_ports))
+		co->index = 0;
+	sport = lpuart_ports[co->index];
+
+	if (sport == NULL)
+		return -ENODEV;
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+	else
+		lpuart_console_get_options(sport, &baud, &parity, &bits);
+
+	lpuart_setup_watermark(sport);
+
+	return uart_set_options(&sport->port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver lpuart_reg;
+static struct console lpuart_console = {
+	.name		= DEV_NAME,
+	.write		= lpuart_console_write,
+	.device		= uart_console_device,
+	.setup		= lpuart_console_setup,
+	.flags		= CON_PRINTBUFFER,
+	.index		= -1,
+	.data		= &lpuart_reg,
+};
+
+#define LPUART_CONSOLE	(&lpuart_console)
+#else
+#define LPUART_CONSOLE	NULL
+#endif
+
+static struct uart_driver lpuart_reg = {
+	.owner		= THIS_MODULE,
+	.driver_name	= DRIVER_NAME,
+	.dev_name	= DEV_NAME,
+	.nr		= ARRAY_SIZE(lpuart_ports),
+	.cons		= LPUART_CONSOLE,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int lpuart_uart_suspend(struct device *dev)
+{
+	struct lpuart_port *sport = dev_get_drvdata(dev);
+
+	uart_suspend_port(&lpuart_reg, &sport->port);
+
+	return 0;
+}
+
+static int lpuart_uart_resume(struct device *dev)
+{
+	struct lpuart_port *sport = dev_get_drvdata(dev);
+
+	uart_resume_port(&lpuart_reg, &sport->port);
+
+	return 0;
+}
+#endif
+
+static int lpuart_uart_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct lpuart_port *sport;
+	void __iomem *base;
+	struct resource *res;
+	int ret;
+
+	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
+	if (!sport)
+		return -ENOMEM;
+
+	pdev->dev.coherent_dma_mask = 0;
+
+	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;
+	}
+	sport->port.line = ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!base)
+		return -ENOMEM;
+
+	sport->port.dev = &pdev->dev;
+	sport->port.mapbase = res->start;
+	sport->port.membase = base;
+	sport->port.type = PORT_LPUART;
+	sport->port.iotype = UPIO_MEM;
+	sport->port.irq = platform_get_irq(pdev, 0);
+	sport->port.ops = &lpuart_pops;
+	sport->port.flags = UPF_BOOT_AUTOCONF;
+
+	sport->clk = devm_clk_get(&pdev->dev, "ipg");
+	if (IS_ERR(sport->clk)) {
+		ret = PTR_ERR(sport->clk);
+		dev_err(&pdev->dev, "failed to get uart clk: %d\n", ret);
+		return ret;
+	}
+
+	clk_prepare_enable(sport->clk);
+
+	sport->port.uartclk = clk_get_rate(sport->clk);
+
+	lpuart_ports[sport->port.line] = sport;
+
+	platform_set_drvdata(pdev, &sport->port);
+
+	ret = uart_add_one_port(&lpuart_reg, &sport->port);
+
+	if (ret) {
+		clk_disable_unprepare(sport->clk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int lpuart_uart_remove(struct platform_device *pdev)
+{
+	struct lpuart_port *sport = platform_get_drvdata(pdev);
+
+	uart_remove_one_port(&lpuart_reg, &sport->port);
+
+	clk_disable_unprepare(sport->clk);
+
+	return 0;
+}
+
+static const struct dev_pm_ops lpuart_uart_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(lpuart_uart_suspend, lpuart_uart_resume)
+};
+
+static struct platform_driver lpuart_uart_driver = {
+	.probe		= lpuart_uart_probe,
+	.remove		= lpuart_uart_remove,
+	.driver		= {
+		.name	= "lpuart-uart",
+		.owner	= THIS_MODULE,
+		.of_match_table = lpuart_uart_dt_ids,
+		.pm	= &lpuart_uart_pm_ops,
+	},
+};
+
+static int __init lpuart_serial_init(void)
+{
+	int ret;
+
+	pr_info("serial: Freescale lpuart driver\n");
+
+	ret = uart_register_driver(&lpuart_reg);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&lpuart_uart_driver);
+	if (ret)
+		uart_unregister_driver(&lpuart_reg);
+
+	return 0;
+}
+
+static void __exit lpuart_serial_exit(void)
+{
+	platform_driver_unregister(&lpuart_uart_driver);
+	uart_unregister_driver(&lpuart_reg);
+}
+
+module_init(lpuart_serial_init);
+module_exit(lpuart_serial_exit);
+
+MODULE_DESCRIPTION("Freescale LPUART serial driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 74c2bf7..df84fa2 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -226,4 +226,7 @@
 /* Rocketport EXPRESS/INFINITY */
 #define PORT_RP2	102
 
+/* Freescale lp-uart */
+#define PORT_LPUART	103
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */
-- 
1.8.0

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Shawn Guo]

On Thu, May 16, 2013 at 02:18:15PM +0800, Jingchang Lu wrote:
> Add Freescale lpuart driver support. The lpuart device
> can be founded on Vybrid MVF600 and Layerscape LS-1 SoCs.
> 
> Signed-off-by: Jingchang Lu <b35083@freescale.com>
> ---
> v3:
>   change the uart driver name from mvf to lpuart for further share between SoCs.

I do not think it's going to be a problem to name the IP with mvf family
name even though it will be used on LS-1 too.  That can simply telling
the history that the IP is firstly used on mvf family and then get
reused on LS-1.  But I'm fine you name the driver in a generic way,
purely using the IP name, if you feel like to do that strongly ...

>   add bind doc in Documentation/devicetree/bindings/tty/serial.
>   remove unused #include header lines.
>   clean up code.
> 
>  .../devicetree/bindings/tty/serial/fsl-lpuart.txt  |  14 +
>  drivers/tty/serial/Kconfig                         |  14 +
>  drivers/tty/serial/Makefile                        |   1 +
>  drivers/tty/serial/fsl_lpuart.c                    | 916 +++++++++++++++++++++
>  include/uapi/linux/serial_core.h                   |   3 +
>  5 files changed, 948 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt
>  create mode 100644 drivers/tty/serial/fsl_lpuart.c
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt
> new file mode 100644
> index 0000000..46ffdb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-lpuart.txt
> @@ -0,0 +1,14 @@
> +* Freescale Low Power Universal Asynchronous Receiver/Transmitter (LPUART)
> +
> +Required properties:
> +- compatible : Should be "fsl,lpuart"

... but I don't think purely using IP name as the compatible string is a
such a good idea, since it tells nothing about compatibility/version
info.  So please encode the SoC name (mvf600) in the string.

> +- reg : Address and length of the register set for the device
> +- interrupts : Should contain uart interrupt
> +
> +Example:
> +
> +uart0: serial at 40027000 {
> +	       compatible = "fsl,lpuart";
> +	       reg = <0x40027000 0x1000>;
> +	       interrupts = <0 61 0x00>;
> +       };
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 7e7006f..2cea1a0 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1484,6 +1484,20 @@ config SERIAL_RP2_NR_UARTS
>  	  If multiple cards are present, the default limit of 32 ports may
>  	  need to be increased.
>  
> +config SERIAL_FSL_LPUART
> +	bool "Freescale Lpuart serial port support"
> +	select SERIAL_CORE
> +	help
> +	  Support for the on-chip LPUARTs on some Freescal SOCs.
> +
> +config SERIAL_FSL_LPUART_CONSOLE
> +	bool "Console on Vybrid serial port"

This should be updated accordingly since the prompt text of
SERIAL_FSL_LPUART has changed?

Shawn

> +	depends on SERIAL_FSL_LPUART
> +	select SERIAL_CORE_CONSOLE
> +	help
> +	  If you have enabled the lpuart serial port on the Freescale SoCs,
> +	  you can make it the console by answering Y to this option.
> +
>  endmenu

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Jin Zhengxiong-R64188]

> -----Original Message-----
> From: Shawn Guo [mailto:shawn.guo at linaro.org]
> Sent: Monday, May 20, 2013 11:05 AM
> To: Lu Jingchang-B35083
> Cc: linux-serial at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> gregkh at linuxfoundation.org; s.hauer at pengutronix.de
> Subject: Re: [PATCH v3] tty: serial: add Freescale lpuart driver support
> 
> On Thu, May 16, 2013 at 02:18:15PM +0800, Jingchang Lu wrote:
> > Add Freescale lpuart driver support. The lpuart device can be founded
> > on Vybrid MVF600 and Layerscape LS-1 SoCs.
> >
> > Signed-off-by: Jingchang Lu <b35083@freescale.com>
> > ---
> > v3:
> >   change the uart driver name from mvf to lpuart for further share
> between SoCs.
> 
> I do not think it's going to be a problem to name the IP with mvf family
> name even though it will be used on LS-1 too.  That can simply telling
> the history that the IP is firstly used on mvf family and then get reused
> on LS-1.  But I'm fine you name the driver in a generic way, purely using
> the IP name, if you feel like to do that strongly ...
> 
[Jason Jin-R64188] Reuse the same LPUART on LS-1 with the compatible strings 'fsl,mvf600-lpuart' is technically OK, But it will fuss the route map of the product Line. The general name can show that the IP is shared between several product lines, and the history for which SOC firstly used the IP is not very important. With general compatible name, We can setup the general dts for the shared IPs.

That's also the case for the IPs used on Power platform, Take the IFC for example, this IP implemented on Power platform will also be reused LS-1, The compatible string is set as  "fsl,ifc", "simple-bus" but not the soc name on which the IP first used.

 Another example, though a little different, the nor flash driver, which is used for many platform, the compatible "cfi-flash" will be more reasonable than the soc/board name.

With the general name for the compatible string in the driver, if there is minor difference for the different implementation of the same IP, we can add the soc related compatible string to the driver to pass different .data structure for that soc.  

Jason

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Sharma Bhupesh-B45370]

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces at lists.infradead.org] On Behalf Of Jin Zhengxiong-R64188
> Sent: Wednesday, May 22, 2013 12:10 PM
> To: Shawn Guo; Lu Jingchang-B35083
> Cc: gregkh at linuxfoundation.org; s.hauer at pengutronix.de; linux-arm-
> kernel at lists.infradead.org; linux-serial at vger.kernel.org
> Subject: RE: [PATCH v3] tty: serial: add Freescale lpuart driver support
> 
> > -----Original Message-----
> > From: Shawn Guo [mailto:shawn.guo at linaro.org]
> > Sent: Monday, May 20, 2013 11:05 AM
> > To: Lu Jingchang-B35083
> > Cc: linux-serial at vger.kernel.org;
> > linux-arm-kernel at lists.infradead.org;
> > gregkh at linuxfoundation.org; s.hauer at pengutronix.de
> > Subject: Re: [PATCH v3] tty: serial: add Freescale lpuart driver
> > support
> >
> > On Thu, May 16, 2013 at 02:18:15PM +0800, Jingchang Lu wrote:
> > > Add Freescale lpuart driver support. The lpuart device can be
> > > founded on Vybrid MVF600 and Layerscape LS-1 SoCs.
> > >
> > > Signed-off-by: Jingchang Lu <b35083@freescale.com>
> > > ---
> > > v3:
> > >   change the uart driver name from mvf to lpuart for further share
> > between SoCs.
> >
> > I do not think it's going to be a problem to name the IP with mvf
> > family name even though it will be used on LS-1 too.  That can simply
> > telling the history that the IP is firstly used on mvf family and then
> > get reused on LS-1.  But I'm fine you name the driver in a generic
> > way, purely using the IP name, if you feel like to do that strongly ...
> >
> [Jason Jin-R64188] Reuse the same LPUART on LS-1 with the compatible
> strings 'fsl,mvf600-lpuart' is technically OK, But it will fuss the route
> map of the product Line. The general name can show that the IP is shared
> between several product lines, and the history for which SOC firstly used
> the IP is not very important. With general compatible name, We can setup
> the general dts for the shared IPs.
> 
> That's also the case for the IPs used on Power platform, Take the IFC for
> example, this IP implemented on Power platform will also be reused LS-1,
> The compatible string is set as  "fsl,ifc", "simple-bus" but not the soc
> name on which the IP first used.
> 
>  Another example, though a little different, the nor flash driver, which
> is used for many platform, the compatible "cfi-flash" will be more
> reasonable than the soc/board name.
> 
> With the general name for the compatible string in the driver, if there
> is minor difference for the different implementation of the same IP, we
> can add the soc related compatible string to the driver to pass different
> .data structure for that soc.
> 
> Jason
> 

It's better to use the generic name lpuart in the compatible string as it provides
much closer association with the lpuart driver which can be generic across SoCs.

As Jason pointed out, minor differences in IP implementation across SoCs can be
handled by passing the soc related compatible string.

Regards,
Bhupesh

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Shawn Guo]

On 22 May 2013 14:40, Jin Zhengxiong-R64188 <R64188@freescale.com> wrote:
> [Jason Jin-R64188] Reuse the same LPUART on LS-1 with the compatible strings 'fsl,mvf600-lpuart' is technically OK, But it will fuss the route map of the product Line. The general name can show that the IP is shared between several product lines, and the history for which SOC firstly used the IP is not very important. With general compatible name, We can setup the general dts for the shared IPs.
>
> That's also the case for the IPs used on Power platform, Take the IFC for example, this IP implemented on Power platform will also be reused LS-1, The compatible string is set as  "fsl,ifc", "simple-bus" but not the soc name on which the IP first used.
>
>  Another example, though a little different, the nor flash driver, which is used for many platform, the compatible "cfi-flash" will be more reasonable than the soc/board name.
>
> With the general name for the compatible string in the driver, if there is minor difference for the different implementation of the same IP, we can add the soc related compatible string to the driver to pass different .data structure for that soc.
>

As I already said, a generic compatible string does not specify
anything about compatibility/version, and hence it's not a good
compatible for IP block which is possibly to be integrated on multiple
SoCs with slight differences.

For example, if there is a new SoC mvf900 integrating the IP with
everything compatible to the version used on mvf600,

  compatible = <fsl,mvf900-uart>, <fsl,mvf600-uart>;
  compatible = <fsl,mvf900-uart>, <fsl,lpuart>;

which one do you think is better to tell the compatibility/version?

Shawn

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Jin Zhengxiong-R64188]

> Subject: Re: [PATCH v3] tty: serial: add Freescale lpuart driver support
> 
> On 22 May 2013 14:40, Jin Zhengxiong-R64188 <R64188@freescale.com> wrote:
> > [Jason Jin-R64188] Reuse the same LPUART on LS-1 with the compatible
> strings 'fsl,mvf600-lpuart' is technically OK, But it will fuss the route
> map of the product Line. The general name can show that the IP is shared
> between several product lines, and the history for which SOC firstly used
> the IP is not very important. With general compatible name, We can setup
> the general dts for the shared IPs.
> >
> > That's also the case for the IPs used on Power platform, Take the IFC
> for example, this IP implemented on Power platform will also be reused
> LS-1, The compatible string is set as  "fsl,ifc", "simple-bus" but not
> the soc name on which the IP first used.
> >
> >  Another example, though a little different, the nor flash driver,
> which is used for many platform, the compatible "cfi-flash" will be more
> reasonable than the soc/board name.
> >
> > With the general name for the compatible string in the driver, if there
> is minor difference for the different implementation of the same IP, we
> can add the soc related compatible string to the driver to pass
> different .data structure for that soc.
> >
> 
> As I already said, a generic compatible string does not specify anything
> about compatibility/version, and hence it's not a good compatible for IP
> block which is possibly to be integrated on multiple SoCs with slight
> differences.
> 
> For example, if there is a new SoC mvf900 integrating the IP with
> everything compatible to the version used on mvf600,
> 
>   compatible = <fsl,mvf900-uart>, <fsl,mvf600-uart>;
>   compatible = <fsl,mvf900-uart>, <fsl,lpuart>;
> 
> which one do you think is better to tell the compatibility/version?
> 
> Shawn
[Jason Jin-R64188] For the lpuart itself, I do not think it will have different version IP. The general name is sufficient and soc related name can introduce the minor difference. If the there are update version, we can name it lpuart2.0 or something like that.

For the compatible property itself, the ePAPR described it as:
---- 
The compatible property value consists of one or more strings that define the specific programming model for the device. This list of strings should be used by a client program for device driver selection. The property value consists of a concatenated list of null terminated strings, from most specific to most general. 
----
So for the IPs especially for the peripheral IPs, the compatible just used to select the driver for the device. It should not used to describe the version information. If the version changed and the driver cannot be used for the device then another compatible strings should be used.

As I said, Another reason for the general strings used is the across platform issue, It's maybe reasonably to name the compatible strings as you recommended for the IPs share in the same SoC family. But for the IPs shared across platform, general name sound more reasonably to avoid the product line confusing. 

The compatible strings once named as you said for Power platform, but more general strings were used later as IPs shared for more SoCs.

I was wondering what's kind of strings should be used for this kind of IPs across different platforms. I added Grant Likely for comments, Thanks.

Jason 

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Shawn Guo]

On Wed, May 22, 2013 at 08:59:20AM +0000, Jin Zhengxiong-R64188 wrote:
> [Jason Jin-R64188] For the lpuart itself, I do not think it will have different version IP.

You can never tell in 5 or 10 years.  Also no major version doesn't
necessarily mean no minor updates or revise.

> The general name is sufficient and soc related name can introduce the minor difference. If the there are update version, we can name it lpuart2.0 or something like that.
> 

Yeah, if IP designers name it lpuart2.0, it surely works.  But from my
experience on IMX, hardware guys are not always properly updating IP
name.  Then the version 2.0 becomes a versioning given by software
people which is improper to be used in compatible string.

> For the compatible property itself, the ePAPR described it as:
> ---- 
> The compatible property value consists of one or more strings that define the specific programming model for the device. This list of strings should be used by a client program for device driver selection. The property value consists of a concatenated list of null terminated strings, from most specific to most general. 
> ----
> So for the IPs especially for the peripheral IPs, the compatible just used to select the driver for the device.

Not only select the driver but also the programing model implemented in
the driver.  Take a look at drivers/mmc/host/sdhci-esdhc-imx.c or
drivers/net/ethernet/freescale/fec_main.c, multiple compatible strings
select the same driver but the different programing model implemented in
the driver.

> It should not used to describe the version information.

Seriously?  The "version" equally means the compatibility here.  How can
a compatible string be not used to describe "version" - compatibility?

> If the version changed and the driver cannot be used for the device then another compatible strings should be used.
> 

If the IP gets a revise, the driver should be updated to support a new
programing model with a new compatible string.

> As I said, Another reason for the general strings used is the across platform issue, It's maybe reasonably to name the compatible strings as you recommended for the IPs share in the same SoC family. But for the IPs shared across platform, general name sound more reasonably to avoid the product line confusing. 
> 

Think about it from "compatibility" point of view.  It does not cause
confusion but just help understand the compatible history.  Saying the
same IP is reused on LS-1 SoC, I do not see a problem with putting the
following compatible into LS-1 <soc>.dtsi under UART node, to tell that
the UART block on LS-1 is completely compatible with the one used on
MVF600.

  compatible = "fsl,ls1-uart", "fsl,mvf600-uart";

> The compatible strings once named as you said for Power platform, but more general strings were used later as IPs shared for more SoCs.
> 

I think IMX device tree support is added more recently than Power.
Encoding SoC name in IP compatible string is a general practise for IMX
device tree bindings.

Shawn

> I was wondering what's kind of strings should be used for this kind of IPs across different platforms. I added Grant Likely for comments, Thanks.
> 
> Jason 
> 

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Jin Zhengxiong-R64188]

> On Wed, May 22, 2013 at 08:59:20AM +0000, Jin Zhengxiong-R64188 wrote:
> > [Jason Jin-R64188] For the lpuart itself, I do not think it will have
> different version IP.
> 
> You can never tell in 5 or 10 years.  Also no major version doesn't
> necessarily mean no minor updates or revise.
> 
> > The general name is sufficient and soc related name can introduce the
> minor difference. If the there are update version, we can name it
> lpuart2.0 or something like that.
> >
> 
> Yeah, if IP designers name it lpuart2.0, it surely works.  But from my
> experience on IMX, hardware guys are not always properly updating IP name.
> Then the version 2.0 becomes a versioning given by software people which
> is improper to be used in compatible string.
> 
[Jason Jin-R64188] We had some internal discussion on this. For the lpuart itself, as we cannot get the version information at run time from the version register and lpuart is not publicly versioned, we agree to use the SoC name for lpuart. 
However, if the version information can be got from the register at run time, or the version was publicly versioned. the general name still useful than the SoC name. 
on the other side, the SoC itself may also have new versions, So the SoC name is also not a perfect solution.

> >
> 
> Think about it from "compatibility" point of view.  It does not cause
> confusion but just help understand the compatible history.  Saying the
> same IP is reused on LS-1 SoC, I do not see a problem with putting the
> following compatible into LS-1 <soc>.dtsi under UART node, to tell that
> the UART block on LS-1 is completely compatible with the one used on
> MVF600.
> 
>   compatible = "fsl,ls1-uart", "fsl,mvf600-uart";
> 
[Jason Jin-R64188] We'll leverage this compatible strings for compatible history. Thanks.
By the way, we'll change the SoC name from mvf600 to vf610 to align the description in the document. 

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Shawn Guo]

On Mon, May 27, 2013 at 01:27:04AM +0000, Jin Zhengxiong-R64188 wrote:
> By the way, we'll change the SoC name from mvf600 to vf610 to align the description in the document. 
> 

Such product renaming in documents happen on IMX as well, but we do
not bother to churn the source tree (file, function renaming) to catch
up with it, as long as we know what it is.

For this mvf600 to vf610 renaming, if you want to catch up with it, you
will need to rename a bunch of files functions, compatibles in Jingchang's
series, and it's probably fine even though it's not really necessary
since they haven't been merged.  But for those which have been merged,
like fec compatible string, doing this will probably piss off
maintainers.  So I would suggest that we stick to name mvf600 in the
source tree, and just be aware of it's the equivalent of vf610.

Shawn

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Jin Zhengxiong-R64188]

> -----Original Message-----
> From: Shawn Guo [mailto:shawn.guo at linaro.org]
> Sent: Monday, May 27, 2013 1:58 PM
> To: Jin Zhengxiong-R64188
> Cc: grant.likely at linaro.org; Lu Jingchang-B35083; linux-
> serial at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> gregkh at linuxfoundation.org; s.hauer at pengutronix.de
> Subject: Re: [PATCH v3] tty: serial: add Freescale lpuart driver support
> 
> On Mon, May 27, 2013 at 01:27:04AM +0000, Jin Zhengxiong-R64188 wrote:
> > By the way, we'll change the SoC name from mvf600 to vf610 to align the
> description in the document.
> >
> 
> Such product renaming in documents happen on IMX as well, but we do not
> bother to churn the source tree (file, function renaming) to catch up
> with it, as long as we know what it is.
> 
> For this mvf600 to vf610 renaming, if you want to catch up with it, you
> will need to rename a bunch of files functions, compatibles in
> Jingchang's series, and it's probably fine even though it's not really
> necessary since they haven't been merged.  But for those which have been
> merged, like fec compatible string, doing this will probably piss off
> maintainers.  So I would suggest that we stick to name mvf600 in the
> source tree, and just be aware of it's the equivalent of vf610.
> 
> Shawn
[Jason Jin-R64188] I think we need to align with the document as it's posted at Freescale website, otherwise, the inconsistency will confuse the customer. 
please refer to:
http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=VF6xx&fsrch=1&sr=5
http://www.freescale.com/webapp/sps/site/homepage.jsp?code=VYBRID
As the development work is ahead of the formal name posted outside, we continued to use the name for upstreaming patches. Sorry for taking into the confusion. For those patches aren't merged, please help to review them based on the new SoC name.

For the merged FEC patch, we'd like to explain it to the maintainer, Or can we just keep the FEC compatible string as before("fsl, mvf600-fec")? And be aware of it's the equivalent of "fsl,vf610-fec"?

Jason

[PATCH v3] tty: serial: add Freescale lpuart driver support

[Shawn Guo]

On Mon, May 27, 2013 at 09:12:25AM +0000, Jin Zhengxiong-R64188 wrote:
> [Jason Jin-R64188] I think we need to align with the document as it's posted at Freescale website, otherwise, the inconsistency will confuse the customer. 
> please refer to:
> http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=VF6xx&fsrch=1&sr=5
> http://www.freescale.com/webapp/sps/site/homepage.jsp?code=VYBRID
> As the development work is ahead of the formal name posted outside, we continued to use the name for upstreaming patches. Sorry for taking into the confusion. For those patches aren't merged, please help to review them based on the new SoC name.
> 

Okay.

> For the merged FEC patch, we'd like to explain it to the maintainer, Or can we just keep the FEC compatible string as before("fsl, mvf600-fec")? And be aware of it's the equivalent of "fsl,vf610-fec"?
> 

I'm fine with either way.  But if you do want to change it, please
change it as early as possible.

Shawn