Message-ID: <54B92C4F.8020009@hurleysoftware.com>
Date:	Fri, 16 Jan 2015 10:20:47 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	Chunyan Zhang <chunyan.zhang@...eadtrum.com>
CC:	gregkh@...uxfoundation.org, mark.rutland@....com, arnd@...db.de,
	gnomes@...rguk.ukuu.org.uk, broonie@...nel.org, robh+dt@...nel.org,
	pawel.moll@....com, ijc+devicetree@...lion.org.uk,
	galak@...eaurora.org, will.deacon@....com, catalin.marinas@....com,
	jslaby@...e.cz, jason@...edaemon.net, heiko@...ech.de,
	florian.vaussard@...l.ch, andrew@...n.ch, rrichter@...ium.com,
	hytszk@...il.com, grant.likely@...aro.org, antonynpavlov@...il.com,
	Joel.Schopp@....com, Suravee.Suthikulpanit@....com,
	shawn.guo@...aro.org, lea.yan@...aro.org,
	jorge.ramirez-ortiz@...aro.org, lee.jones@...aro.org,
	orsonzhai@...il.com, geng.ren@...eadtrum.com,
	zhizhou.zhang@...eadtrum.com, lanqing.liu@...eadtrum.com,
	zhang.lyra@...il.com, wei.qiao@...eadtrum.com,
	devicetree@...r.kernel.org, linux-api@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v5 5/5] tty/serial: Add Spreadtrum sc9836-uart driver
 support

On 01/16/2015 05:00 AM, Chunyan Zhang wrote:
> Add a full sc9836-uart driver for SC9836 SoC which is based on the
> spreadtrum sharkl64 platform.
> This driver also support earlycon.
> This patch also replaced the spaces between the macros and their
> values with the tabs in serial_core.h

The locking doesn't look correct. Specific notations below.

> Signed-off-by: Chunyan Zhang <chunyan.zhang@...eadtrum.com>
> Signed-off-by: Orson Zhai <orson.zhai@...eadtrum.com>
> Originally-by: Lanqing Liu <lanqing.liu@...eadtrum.com>
> ---
>  drivers/tty/serial/Kconfig       |   18 +
>  drivers/tty/serial/Makefile      |    1 +
>  drivers/tty/serial/sprd_serial.c |  772 ++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |    3 +
>  4 files changed, 794 insertions(+)
>  create mode 100644 drivers/tty/serial/sprd_serial.c
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index c79b43c..969d3cd 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1577,6 +1577,24 @@ config SERIAL_MEN_Z135
>  	  This driver can also be build as a module. If so, the module will be called
>  	  men_z135_uart.ko
>  
> +config SERIAL_SPRD
> +	tristate "Support for SPRD serial"
> +	depends on ARCH_SPRD
> +	select SERIAL_CORE
> +	help
> +	  This enables the driver for the Spreadtrum's serial.
> +
> +config SERIAL_SPRD_CONSOLE
> +	bool "SPRD UART console support"
> +	depends on SERIAL_SPRD=y
> +	select SERIAL_CORE_CONSOLE
> +	select SERIAL_EARLYCON
> +	help
> +	  Support for early debug console using Spreadtrum's serial. This enables
> +	  the console before standard serial driver is probed. This is enabled
> +	  with "earlycon" on the kernel command line. The console is
> +	  enabled when early_param is processed.
> +
>  endmenu
>  
>  config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 9a548ac..4801aca 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_ARC)	+= arc_uart.o
>  obj-$(CONFIG_SERIAL_RP2)	+= rp2.o
>  obj-$(CONFIG_SERIAL_FSL_LPUART)	+= fsl_lpuart.o
>  obj-$(CONFIG_SERIAL_MEN_Z135)	+= men_z135_uart.o
> +obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>  
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> new file mode 100644
> index 0000000..81839e4
> --- /dev/null
> +++ b/drivers/tty/serial/sprd_serial.c
> @@ -0,0 +1,772 @@
> +/*
> + * Copyright (C) 2012 Spreadtrum Communications Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/console.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/serial_core.h>
> +#include <linux/serial.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +
> +/* device name */
> +#define UART_NR_MAX		8
> +#define SPRD_TTY_NAME		"ttySPX"
> +#define SPRD_FIFO_SIZE		128
> +#define SPRD_DEF_RATE		26000000
> +#define SPRD_TIMEOUT		2048
> +
> +/* the offset of serial registers and BITs for them */
> +/* data registers */
> +#define SPRD_TXD		0x0000
> +#define SPRD_RXD		0x0004
> +
> +/* line status register and its BITs  */
> +#define SPRD_LSR		0x0008
> +#define SPRD_LSR_OE		BIT(4)
> +#define SPRD_LSR_FE		BIT(3)
> +#define SPRD_LSR_PE		BIT(2)
> +#define SPRD_LSR_BI		BIT(7)
> +#define SPRD_LSR_TX_OVER	BIT(15)
> +
> +/* data number in TX and RX fifo */
> +#define SPRD_STS1		0x000C
> +
> +/* interrupt enable register and its BITs */
> +#define SPRD_IEN		0x0010
> +#define SPRD_IEN_RX_FULL	BIT(0)
> +#define SPRD_IEN_TX_EMPTY	BIT(1)
> +#define SPRD_IEN_BREAK_DETECT	BIT(7)
> +#define SPRD_IEN_TIMEOUT	BIT(13)
> +
> +/* interrupt clear register */
> +#define SPRD_ICLR		0x0014
> +
> +/* line control register */
> +#define SPRD_LCR		0x0018
> +#define SPRD_LCR_STOP_1BIT	0x10
> +#define SPRD_LCR_STOP_2BIT	0x30
> +#define SPRD_LCR_DATA_LEN	(BIT(2) | BIT(3))
> +#define SPRD_LCR_DATA_LEN5	0x0
> +#define SPRD_LCR_DATA_LEN6	0x4
> +#define SPRD_LCR_DATA_LEN7	0x8
> +#define SPRD_LCR_DATA_LEN8	0xc
> +#define SPRD_LCR_PARITY		(BIT(0) | BIT(1))
> +#define SPRD_LCR_PARITY_EN	0x2
> +#define SPRD_LCR_EVEN_PAR	0x0
> +#define SPRD_LCR_ODD_PAR	0x1
> +
> +/* control register 1 */
> +#define SPRD_CTL1		0x001C
> +#define RX_HW_FLOW_CTL_THLD	BIT(6)
> +#define RX_HW_FLOW_CTL_EN	BIT(7)
> +#define TX_HW_FLOW_CTL_EN	BIT(8)
> +
> +/* fifo threshold register */
> +#define SPRD_CTL2		0x0020
> +#define THLD_TX_EMPTY		0x40
> +#define THLD_RX_FULL		0x40
> +
> +/* config baud rate register */
> +#define SPRD_CLKD0		0x0024
> +#define SPRD_CLKD1		0x0028
> +
> +/* interrupt mask status register */
> +#define SPRD_IMSR		0x002C
> +#define SPRD_IMSR_RX_FIFO_FULL	BIT(0)
> +#define SPRD_IMSR_TX_FIFO_EMPTY	BIT(1)
> +#define SPRD_IMSR_BREAK_DETECT	BIT(7)
> +#define SPRD_IMSR_TIMEOUT	BIT(13)
> +
> +struct reg_backup {
> +	uint32_t ien;
> +	uint32_t ctrl0;
> +	uint32_t ctrl1;
> +	uint32_t ctrl2;
> +	uint32_t clkd0;
> +	uint32_t clkd1;
> +	uint32_t dspwait;
> +};
> +
> +struct sprd_uart_port {
> +	struct uart_port port;
> +	struct reg_backup reg_bak;
> +	char name[16];
> +};
> +
> +static struct sprd_uart_port *sprd_port[UART_NR_MAX] = { NULL };
> +
> +static inline unsigned int serial_in(struct uart_port *port, int offset)
> +{
> +	return readl_relaxed(port->membase + offset);
> +}
> +
> +static inline void serial_out(struct uart_port *port, int offset, int value)
> +{
> +	writel_relaxed(value, port->membase + offset);
> +}
> +
> +static unsigned int sprd_tx_empty(struct uart_port *port)
> +{
> +	if (serial_in(port, SPRD_STS1) & 0xff00)
> +		return 0;
> +	else
> +		return TIOCSER_TEMT;
> +}
> +
> +static unsigned int sprd_get_mctrl(struct uart_port *port)
> +{
> +	return TIOCM_DSR | TIOCM_CTS;
> +}
> +
> +static void sprd_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +	/* nothing to do */
> +}
> +
> +static void sprd_stop_tx(struct uart_port *port)
> +{
> +	unsigned int ien, iclr;
> +
> +	iclr = serial_in(port, SPRD_ICLR);
> +	ien = serial_in(port, SPRD_IEN);
> +
> +	iclr |= SPRD_IEN_TX_EMPTY;
> +	ien &= ~SPRD_IEN_TX_EMPTY;
> +
> +	serial_out(port, SPRD_ICLR, iclr);
> +	serial_out(port, SPRD_IEN, ien);
> +}
> +
> +static void sprd_start_tx(struct uart_port *port)
> +{
> +	unsigned int ien;
> +
> +	ien = serial_in(port, SPRD_IEN);
> +	if (!(ien & SPRD_IEN_TX_EMPTY)) {
> +		ien |= SPRD_IEN_TX_EMPTY;
> +		serial_out(port, SPRD_IEN, ien);
> +	}
> +}
> +
> +static void sprd_stop_rx(struct uart_port *port)
> +{
> +	unsigned int ien, iclr;
> +
> +	iclr = serial_in(port, SPRD_ICLR);
> +	ien = serial_in(port, SPRD_IEN);
> +
> +	ien &= ~(SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT);
> +	iclr |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT;
> +
> +	serial_out(port, SPRD_IEN, ien);
> +	serial_out(port, SPRD_ICLR, iclr);
> +}
> +
> +/* The Sprd serial does not support this function. */
> +static void sprd_break_ctl(struct uart_port *port, int break_state)
> +{
> +	/* nothing to do */
> +}
> +
> +static inline int handle_lsr_errors(struct uart_port *port,
> +				    unsigned int *flag,
> +				    unsigned int *lsr)
> +{
> +	int ret = 0;
> +
> +	/* statistics */
> +	if (*lsr & SPRD_LSR_BI) {
> +		*lsr &= ~(SPRD_LSR_FE | SPRD_LSR_PE);
> +		port->icount.brk++;
> +		ret = uart_handle_break(port);
> +		if (ret)
> +			return ret;
> +	} else if (*lsr & SPRD_LSR_PE)
> +		port->icount.parity++;
> +	else if (*lsr & SPRD_LSR_FE)
> +		port->icount.frame++;
> +	if (*lsr & SPRD_LSR_OE)
> +		port->icount.overrun++;
> +
> +	/* mask off conditions which should be ignored */
> +	*lsr &= port->read_status_mask;
> +	if (*lsr & SPRD_LSR_BI)
> +		*flag = TTY_BREAK;
> +	else if (*lsr & SPRD_LSR_PE)
> +		*flag = TTY_PARITY;
> +	else if (*lsr & SPRD_LSR_FE)
> +		*flag = TTY_FRAME;
> +
> +	return ret;
> +}
> +
> +static inline void sprd_rx(int irq, void *dev_id)
> +{
> +	struct uart_port *port = dev_id;
> +	struct tty_port *tty = &port->state->port;
> +	unsigned int ch, flag, lsr, max_count = SPRD_TIMEOUT;
> +
> +	while ((serial_in(port, SPRD_STS1) & 0x00ff) && max_count--) {
> +		lsr = serial_in(port, SPRD_LSR);
> +		ch = serial_in(port, SPRD_RXD);
> +		flag = TTY_NORMAL;
> +		port->icount.rx++;
> +
> +		if (lsr & (SPRD_LSR_BI | SPRD_LSR_PE
> +			| SPRD_LSR_FE | SPRD_LSR_OE))
> +			if (handle_lsr_errors(port, &lsr, &flag))
> +				continue;
> +		if (uart_handle_sysrq_char(port, ch))
> +			continue;
> +
> +		uart_insert_char(port, lsr, SPRD_LSR_OE, ch, flag);
> +	}
> +
> +	tty_flip_buffer_push(tty);
> +}
> +
> +static inline void sprd_tx(int irq, void *dev_id)
> +{
> +	struct uart_port *port = dev_id;
> +	struct circ_buf *xmit = &port->state->xmit;
> +	int count;
> +
> +	if (port->x_char) {
> +		serial_out(port, SPRD_TXD, port->x_char);
> +		port->icount.tx++;
> +		port->x_char = 0;
> +		return;
> +	}
> +
> +	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
> +		sprd_stop_tx(port);
> +		return;
> +	}
> +
> +	count = THLD_TX_EMPTY;
> +	do {
> +		serial_out(port, SPRD_TXD, xmit->buf[xmit->tail]);
> +		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> +		port->icount.tx++;
> +		if (uart_circ_empty(xmit))
> +			break;
> +	} while (--count > 0);
> +
> +	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +		uart_write_wakeup(port);
> +
> +	if (uart_circ_empty(xmit))
> +		sprd_stop_tx(port);
> +}
> +
> +/* this handles the interrupt from one port */
> +static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
> +{
> +	struct uart_port *port = (struct uart_port *)dev_id;
> +	unsigned int ims;

Why does your isr not have to take port->lock ?

> +	ims = serial_in(port, SPRD_IMSR);
> +
> +	if (!ims)
> +		return IRQ_NONE;
> +
> +	serial_out(port, SPRD_ICLR, ~0);
> +
> +	if (ims & (SPRD_IMSR_RX_FIFO_FULL |
> +		SPRD_IMSR_BREAK_DETECT | SPRD_IMSR_TIMEOUT))
> +		sprd_rx(irq, port);
> +
> +	if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
> +		sprd_tx(irq, port);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int sprd_startup(struct uart_port *port)
> +{
> +	int ret = 0;
> +	unsigned int ien, ctrl1;
> +	unsigned int timeout;
> +	struct sprd_uart_port *sp;
> +
> +	serial_out(port, SPRD_CTL2, ((THLD_TX_EMPTY << 8) | THLD_RX_FULL));
> +
> +	/* clear rx fifo */
> +	timeout = SPRD_TIMEOUT;
> +	while (timeout-- && serial_in(port, SPRD_STS1) & 0x00ff)
> +		serial_in(port, SPRD_RXD);
> +
> +	/* clear tx fifo */
> +	timeout = SPRD_TIMEOUT;
> +	while (timeout-- && serial_in(port, SPRD_STS1) & 0xff00)
> +		cpu_relax();
> +
> +	/* clear interrupt */
> +	serial_out(port, SPRD_IEN, 0x0);
> +	serial_out(port, SPRD_ICLR, ~0);
> +
> +	/* allocate irq */
> +	sp = container_of(port, struct sprd_uart_port, port);
> +	snprintf(sp->name, sizeof(sp->name), "sprd_serial%d", port->line);
> +	ret = devm_request_irq(port->dev, port->irq, sprd_handle_irq,
> +				IRQF_SHARED, sp->name, port);
> +	if (ret) {
> +		dev_err(port->dev, "fail to request serial irq %d, ret=%d\n",
> +			port->irq, ret);
> +		return ret;
> +	}
> +	ctrl1 = serial_in(port, SPRD_CTL1);
> +	ctrl1 |= 0x3e00 | THLD_RX_FULL;
> +	serial_out(port, SPRD_CTL1, ctrl1);
> +
> +	/* enable interrupt */
> +	spin_lock(&port->lock);
> +	ien = serial_in(port, SPRD_IEN);
> +	ien |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
> +	serial_out(port, SPRD_IEN, ien);
> +	spin_unlock(&port->lock);
> +
> +	return 0;
> +}
> +
> +static void sprd_shutdown(struct uart_port *port)
> +{
> +	serial_out(port, SPRD_IEN, 0x0);
> +	serial_out(port, SPRD_ICLR, ~0);
> +	devm_free_irq(port->dev, port->irq, port);
> +}
> +
> +static void sprd_set_termios(struct uart_port *port,
> +				    struct ktermios *termios,
> +				    struct ktermios *old)
> +{
> +	unsigned int baud, quot;
> +	unsigned int lcr, fc;
> +
> +	/* ask the core to calculate the divisor for us */
> +	baud = uart_get_baud_rate(port, termios, old, 1200, 3000000);
> +
> +	quot = (unsigned int)((port->uartclk + baud / 2) / baud);
> +
> +	/* set data length */
> +	lcr = serial_in(port, SPRD_LCR);
> +	lcr &= ~SPRD_LCR_DATA_LEN;
> +	switch (termios->c_cflag & CSIZE) {
> +	case CS5:
> +		lcr |= SPRD_LCR_DATA_LEN5;
> +		break;
> +	case CS6:
> +		lcr |= SPRD_LCR_DATA_LEN6;
> +		break;
> +	case CS7:
> +		lcr |= SPRD_LCR_DATA_LEN7;
> +		break;
> +	case CS8:
> +	default:
> +		lcr |= SPRD_LCR_DATA_LEN8;
> +		break;
> +	}
> +
> +	/* calculate stop bits */
> +	lcr &= ~(SPRD_LCR_STOP_1BIT | SPRD_LCR_STOP_2BIT);
> +	if (termios->c_cflag & CSTOPB)
> +		lcr |= SPRD_LCR_STOP_2BIT;
> +	else
> +		lcr |= SPRD_LCR_STOP_1BIT;
> +
> +	/* calculate parity */
> +	lcr &= ~SPRD_LCR_PARITY;
> +	termios->c_cflag &= ~CMSPAR;	/* no support mark/space */
> +	if (termios->c_cflag & PARENB) {
> +		lcr |= SPRD_LCR_PARITY_EN;
> +		if (termios->c_cflag & PARODD)
> +			lcr |= SPRD_LCR_ODD_PAR;
> +		else
> +			lcr |= SPRD_LCR_EVEN_PAR;
> +	}
> +
> +	/* change the port state. */
           ^^^^^^^^^^^^^^^^^^^^^^

This means you should be taking the port->lock here... (and disabling
local interrupts if your isr takes the port->lock)


> +	/* update the per-port timeout */
> +	uart_update_timeout(port, termios->c_cflag, baud);
> +
> +	port->read_status_mask = SPRD_LSR_OE;
> +	if (termios->c_iflag & INPCK)
> +		port->read_status_mask |= SPRD_LSR_FE | SPRD_LSR_PE;
> +	if (termios->c_iflag & (BRKINT | PARMRK))
> +		port->read_status_mask |= SPRD_LSR_BI;
> +
> +	/* characters to ignore */
> +	port->ignore_status_mask = 0;
> +	if (termios->c_iflag & IGNPAR)
> +		port->ignore_status_mask |= SPRD_LSR_PE | SPRD_LSR_FE;
> +	if (termios->c_iflag & IGNBRK) {
> +		port->ignore_status_mask |= SPRD_LSR_BI;
> +		/*
> +		 * If we're ignoring parity and break indicators,
> +		 * ignore overruns too (for real raw support).
> +		 */
> +		if (termios->c_iflag & IGNPAR)
> +			port->ignore_status_mask |= SPRD_LSR_OE;
> +	}
> +
> +	/* flow control */
> +	fc = serial_in(port, SPRD_CTL1);
> +	fc &= ~(RX_HW_FLOW_CTL_THLD | RX_HW_FLOW_CTL_EN | TX_HW_FLOW_CTL_EN);
> +	if (termios->c_cflag & CRTSCTS) {
> +		fc |= RX_HW_FLOW_CTL_THLD;
> +		fc |= RX_HW_FLOW_CTL_EN;
> +		fc |= TX_HW_FLOW_CTL_EN;
> +	}
> +
> +	/* clock divider bit0~bit15 */
> +	serial_out(port, SPRD_CLKD0, quot & 0xffff);
> +
> +	/* clock divider bit16~bit20 */
> +	serial_out(port, SPRD_CLKD1, (quot & 0x1f0000) >> 16);
> +	serial_out(port, SPRD_LCR, lcr);
> +	fc |= 0x3e00 | THLD_RX_FULL;
> +	serial_out(port, SPRD_CTL1, fc);

and dropping it here.

> +	/* Don't rewrite B0 */
> +	if (tty_termios_baud_rate(termios))
> +		tty_termios_encode_baud_rate(termios, baud, baud);
> +}
> +
> +static const char *sprd_type(struct uart_port *port)
> +{
> +	return "SPX";
> +}
> +
> +static void sprd_release_port(struct uart_port *port)
> +{
> +	/* nothing to do */
> +}
> +
> +static int sprd_request_port(struct uart_port *port)
> +{
> +	return 0;
> +}
> +
> +static void sprd_config_port(struct uart_port *port, int flags)
> +{
> +	if (flags & UART_CONFIG_TYPE)
> +		port->type = PORT_SPRD;
> +}
> +
> +static int sprd_verify_port(struct uart_port *port,
> +				   struct serial_struct *ser)
> +{
> +	if (ser->type != PORT_SPRD)
> +		return -EINVAL;
> +	if (port->irq != ser->irq)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +static struct uart_ops serial_sprd_ops = {
> +	.tx_empty = sprd_tx_empty,
> +	.get_mctrl = sprd_get_mctrl,
> +	.set_mctrl = sprd_set_mctrl,
> +	.stop_tx = sprd_stop_tx,
> +	.start_tx = sprd_start_tx,
> +	.stop_rx = sprd_stop_rx,
> +	.break_ctl = sprd_break_ctl,
> +	.startup = sprd_startup,
> +	.shutdown = sprd_shutdown,
> +	.set_termios = sprd_set_termios,
> +	.type = sprd_type,
> +	.release_port = sprd_release_port,
> +	.request_port = sprd_request_port,
> +	.config_port = sprd_config_port,
> +	.verify_port = sprd_verify_port,
> +};
> +
> +#ifdef CONFIG_SERIAL_SPRD_CONSOLE
> +static inline void wait_for_xmitr(struct uart_port *port)
> +{
> +	unsigned int status, tmout = 10000;
> +
> +	/* wait up to 10ms for the character(s) to be sent */
> +	do {
> +		status = serial_in(port, SPRD_STS1);
> +		if (--tmout == 0)
> +			break;
> +		udelay(1);
> +	} while (status & 0xff00);
> +}
> +
> +static void sprd_console_putchar(struct uart_port *port, int ch)
> +{
> +	wait_for_xmitr(port);
> +	serial_out(port, SPRD_TXD, ch);
> +}
> +
> +static void sprd_console_write(struct console *co, const char *s,
> +				      unsigned int count)
> +{
> +	struct uart_port *port = &sprd_port[co->index]->port;
> +	int ien;
> +	int locked = 1;
> +
> +	if (oops_in_progress)
> +		locked = spin_trylock(&port->lock);
> +	else
> +		spin_lock(&port->lock);

If you do need to take the port->lock in your isr, then you need to
disable local irq here.

> +	/* save the IEN then disable the interrupts */
> +	ien = serial_in(port, SPRD_IEN);
> +	serial_out(port, SPRD_IEN, 0x0);
> +
> +	uart_console_write(port, s, count, sprd_console_putchar);
> +
> +	/* wait for transmitter to become empty and restore the IEN */
> +	wait_for_xmitr(port);
> +	serial_out(port, SPRD_IEN, ien);
> +	if (locked)
> +		spin_unlock(&port->lock);
> +}
> +
> +static int __init sprd_console_setup(struct console *co, char *options)
> +{
> +	struct uart_port *port;
> +	int baud = 115200;
> +	int bits = 8;
> +	int parity = 'n';
> +	int flow = 'n';
> +
> +	if (co->index >= UART_NR_MAX || co->index < 0)
> +		co->index = 0;
> +
> +	port = &sprd_port[co->index]->port;
> +	if (port == NULL) {
> +		pr_info("serial port %d not yet initialized\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 sprd_uart_driver;
> +static struct console sprd_console = {
> +	.name = SPRD_TTY_NAME,
> +	.write = sprd_console_write,
> +	.device = uart_console_device,
> +	.setup = sprd_console_setup,
> +	.flags = CON_PRINTBUFFER,
> +	.index = -1,
> +	.data = &sprd_uart_driver,
> +};
> +
> +#define SPRD_CONSOLE	(&sprd_console)
> +
> +/* Support for earlycon */
> +static void sprd_putc(struct uart_port *port, int c)
> +{
> +	unsigned int timeout = SPRD_TIMEOUT;
> +
> +	while (timeout-- &&
> +		   !(readl(port->membase + SPRD_LSR) & SPRD_LSR_TX_OVER))
> +		cpu_relax();
> +
> +	writeb(c, port->membase + SPRD_TXD);
> +}
> +
> +static void sprd_early_write(struct console *con, const char *s,
> +				    unsigned n)
> +{
> +	struct earlycon_device *dev = con->data;
> +
> +	uart_console_write(&dev->port, s, n, sprd_putc);
> +}
> +
> +static int __init sprd_early_console_setup(
> +				struct earlycon_device *device,
> +				const char *opt)
> +{
> +	if (!device->port.membase)
> +		return -ENODEV;
> +
> +	device->con->write = sprd_early_write;
> +	return 0;
> +}
> +
> +EARLYCON_DECLARE(sprd_serial, sprd_early_console_setup);
> +OF_EARLYCON_DECLARE(sprd_serial, "sprd,sc9836-uart",
> +		    sprd_early_console_setup);
> +
> +#else /* !CONFIG_SERIAL_SPRD_CONSOLE */
> +#define SPRD_CONSOLE		NULL
> +#endif
> +
> +static struct uart_driver sprd_uart_driver = {
> +	.owner = THIS_MODULE,
> +	.driver_name = "sprd_serial",
> +	.dev_name = SPRD_TTY_NAME,
> +	.major = 0,
> +	.minor = 0,
> +	.nr = UART_NR_MAX,
> +	.cons = SPRD_CONSOLE,
> +};
> +
> +static int sprd_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct uart_port *up;
> +	struct clk *clk;
> +	int irq;
> +
> +	if (np)
> +		pdev->id = of_alias_get_id(np, "serial");
> +
> +	if (pdev->id < 0 || pdev->id >= UART_NR_MAX) {
> +		dev_err(&pdev->dev, "does not support id %d\n", pdev->id);
> +		return -ENXIO;
> +	}
> +
> +	sprd_port[pdev->id] = devm_kzalloc(&pdev->dev,
> +		sizeof(*sprd_port[pdev->id]), GFP_KERNEL);
> +	if (!sprd_port[pdev->id])
> +		return -ENOMEM;
> +
> +	up = &sprd_port[pdev->id]->port;
> +	up->dev = &pdev->dev;
> +	up->line = pdev->id;
> +	up->type = PORT_SPRD;
> +	up->iotype = SERIAL_IO_PORT;
> +	up->uartclk = SPRD_DEF_RATE;
> +	up->fifosize = SPRD_FIFO_SIZE;
> +	up->ops = &serial_sprd_ops;
> +	up->flags = ASYNC_BOOT_AUTOCONF;
> +
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (!IS_ERR(clk))
> +		up->uartclk = clk_get_rate(clk);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "not provide mem resource\n");
> +		return -ENODEV;
> +	}
> +	up->mapbase = res->start;
> +	up->membase = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(up->membase))
> +		return PTR_ERR(up->membase);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "not provide irq resource\n");
> +		return -ENODEV;
> +	}
> +	up->irq = irq;
> +
> +	platform_set_drvdata(pdev, up);
> +
> +	return uart_add_one_port(&sprd_uart_driver, up);
> +}
> +
> +static int sprd_remove(struct platform_device *dev)
> +{
> +	struct uart_port *up = platform_get_drvdata(dev);
> +
> +	return uart_remove_one_port(&sprd_uart_driver, up);
> +}
> +
> +static int sprd_suspend(struct device *dev)
> +{
> +	int id = to_platform_device(dev)->id;
> +	struct uart_port *port = &sprd_port[id]->port;
> +	struct reg_backup *reg_bak = &sprd_port[id]->reg_bak;
> +
> +	reg_bak->ien = serial_in(port, SPRD_IEN);
> +	reg_bak->ctrl0 = serial_in(port, SPRD_LCR);
> +	reg_bak->ctrl1 = serial_in(port, SPRD_CTL1);
> +	reg_bak->ctrl2 = serial_in(port, SPRD_CTL2);
> +	reg_bak->clkd0 = serial_in(port, SPRD_CLKD0);
> +	reg_bak->clkd1 = serial_in(port, SPRD_CLKD1);

Why are you saving and restoring these register states
across suspend/resume?

The serial core calls your set_termios() handler upon
resume (either for the console or if a tty is open)
so you should be reprogramming the hardware there
based on the termios settings.

Regards,
Peter Hurley

> +
> +	uart_suspend_port(&sprd_uart_driver, port);
> +
> +	return 0;
> +}
> +
> +static int sprd_resume(struct device *dev)
> +{
> +	int id = to_platform_device(dev)->id;
> +	struct uart_port *port = &sprd_port[id]->port;
> +	struct reg_backup *reg_bak = &sprd_port[id]->reg_bak;
> +
> +	serial_out(port, SPRD_LCR, reg_bak->ctrl0);
> +	serial_out(port, SPRD_CTL1, reg_bak->ctrl1);
> +	serial_out(port, SPRD_CTL2, reg_bak->ctrl2);
> +	serial_out(port, SPRD_CLKD0, reg_bak->clkd0);
> +	serial_out(port, SPRD_CLKD1, reg_bak->clkd1);
> +	serial_out(port, SPRD_IEN, reg_bak->ien);
> +
> +	uart_resume_port(&sprd_uart_driver, port);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id serial_ids[] = {
> +	{.compatible = "sprd,sc9836-uart",},
> +	{}
> +};
> +
> +static SIMPLE_DEV_PM_OPS(sprd_pm_ops, sprd_suspend, sprd_resume);
> +
> +static struct platform_driver sprd_platform_driver = {
> +	.probe		= sprd_probe,
> +	.remove		= sprd_remove,
> +	.driver 	= {
> +		.name	= "sprd_serial",
> +		.of_match_table = of_match_ptr(serial_ids),
> +		.pm	= &sprd_pm_ops,
> +	},
> +};
> +
> +static int __init sprd_serial_init(void)
> +{
> +	int ret = 0;
> +
> +	ret = uart_register_driver(&sprd_uart_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&sprd_platform_driver);
> +	if (ret)
> +		uart_unregister_driver(&sprd_uart_driver);
> +
> +	return ret;
> +}
> +
> +static void __exit sprd_serial_exit(void)
> +{
> +	platform_driver_unregister(&sprd_platform_driver);
> +	uart_unregister_driver(&sprd_uart_driver);
> +}
> +
> +module_init(sprd_serial_init);
> +module_exit(sprd_serial_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Spreadtrum SoC serial driver series");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index c172180..7e6eb39 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -248,4 +248,7 @@
>  /* MESON */
>  #define PORT_MESON	109
>  
> +/* SPRD SERIAL  */
> +#define PORT_SPRD	110
> +
>  #endif /* _UAPILINUX_SERIAL_CORE_H */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists