Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-04 20:07 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202041211150.9453@utopia.booyaka.com>

On Sat, Feb 04, 2012 at 12:24:07PM -0700, Paul Walmsley wrote:
> On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:
> 
> > On Sat, Feb 04, 2012 at 10:22:27AM -0700, Paul Walmsley wrote:
> > > No, that is not an example of a protocol with a retry.  That is an example 
> > > of a protocol that has no provision for reliable data delivery.  Sending a 
> > > new data string one second later is not a retry.
> > > 
> > > In such situations, the system integrator would just use the UART in the 
> > > default (lossless) mode.  And if they don't, they'll have to deal with the 
> > > consequences that they chose.  Those of us who ship battery-powered Linux 
> > > devices are indeed capable of making this choice.
> > 
> > Okay, lets see.  You're making a battery powered Linux device.  It has
> > a standard RS232 serial port available, and you allow users to load
> > 'apps' onto it.
> > 
> > Do you run the serial ports in lossless mode?
> 
> Not every serial port is available to arbitrary 'apps.'.  Not every 
> battery-powered Linux device allows users to run arbitrary 'apps.'
> 
> On devices that do allow users to load arbitrary 'apps,' and that allow 
> those 'apps' to have direct access to the serial ports, I personally 
> believe that system integrators should not change the default OMAP serial 
> setting, which is to run the serial ports in lossless mode.
> 
> Here is another example.  Suppose someone builds a GPS receiver with an 
> OMAP that is capable of sending NMEA position sentences, once per second, 
> to a remotely connected serial device.  No receive traffic is expected on 
> that port.
> 
> The position you seem to be advocating is that the mainline Linux kernel 
> should not support any ability to allow the system integrator to 
> affirmatively instruct the SoC to enter device idle between those position 
> sentences.  This will cause the SoC to consume energy to losslessly 
> handle an incoming serial character that will never come.  Is that really 
> what you're advocating?

Stop procrastinating.  Please answer my question.  Then I'll answer yours.

^ permalink raw reply

* [PATCH] Add sc16is7x2 driver
From: Thomas Weber @ 2012-02-05  6:55 UTC (permalink / raw)
  To: linux-serial; +Cc: Thomas Weber, Manuel Stahl, Thomas Weber

This patch adds support for the sc16is7x2 SPI to UART chips.
Each chip has two UARTs and 8 GPIOs.

Signed-off-by: Manuel Stahl <manuel.stahl@iis.fraunhofer.de>

[weber@corscience.de: Rebased to actual kernel]
Signed-off-by: Thomas Weber <weber@corscience.de>
---
 drivers/tty/serial/Kconfig       |   10 +
 drivers/tty/serial/Makefile      |    1 +
 drivers/tty/serial/sc16is7x2.c   | 1065 ++++++++++++++++++++++++++++++++++++++
 include/linux/serial_core.h      |    3 +
 include/linux/serial_sc16is7x2.h |   17 +
 5 files changed, 1096 insertions(+), 0 deletions(-)
 create mode 100644 drivers/tty/serial/sc16is7x2.c
 create mode 100755 include/linux/serial_sc16is7x2.h

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 2de9924..ab15542 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -9,6 +9,16 @@ source "drivers/tty/serial/8250/Kconfig"
 
 comment "Non-8250 serial port support"
 
+config SERIAL_SC16IS7X2
+	tristate "SC16IS7X2 support"
+	depends on SPI_MASTER
+	select SERIAL_CORE
+	help
+	  Selecting this option will add support for SC16IS7X2 SPI UARTs.
+	  The GPIOs are exported via gpiolib interface.
+
+	  If unsure, say N.
+
 config SERIAL_AMBA_PL010
 	tristate "ARM AMBA PL010 serial port support"
 	depends on ARM_AMBA && (BROKEN || !ARCH_VERSATILE)
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index fef32e1..a6cc55e 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_SERIAL_SUNSAB) += sunsab.o
 # Now bring in any enabled 8250/16450/16550 type drivers.
 obj-$(CONFIG_SERIAL_8250) += 8250/
 
+obj-$(CONFIG_SERIAL_SC16IS7X2) += sc16is7x2.o
 obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o
 obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o
 obj-$(CONFIG_SERIAL_CLPS711X) += clps711x.o
diff --git a/drivers/tty/serial/sc16is7x2.c b/drivers/tty/serial/sc16is7x2.c
new file mode 100644
index 0000000..215874f
--- /dev/null
+++ b/drivers/tty/serial/sc16is7x2.c
@@ -0,0 +1,1065 @@
+/**
+ * drivers/tty/serial/sc16is7x2.c
+ *
+ * Copyright (C) 2009 Manuel Stahl <manuel.stahl@iis.fraunhofer.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * The SC16IS7x2 device is a SPI driven dual UART with GPIOs.
+ *
+ * The driver exports two uarts and a gpiochip interface.
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/workqueue.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/serial_reg.h>
+#include <linux/serial_core.h>
+#include <linux/serial.h>
+#include <linux/gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/freezer.h>
+#include <linux/serial_sc16is7x2.h>
+
+#define MAX_SC16IS7X2		8
+#define FIFO_SIZE		64
+
+#define DRIVER_NAME		"sc16is7x2"
+#define TYPE_NAME		"SC16IS7x2"
+
+
+#define REG_READ	0x80
+#define REG_WRITE	0x00
+
+/* Special registers */
+#define REG_TXLVL	0x08	/* Transmitter FIFO Level register */
+#define REG_RXLVL	0x09	/* Receiver FIFO Level register */
+#define REG_IOD		0x0A	/* IO Direction register */
+#define REG_IOS		0x0B	/* IO State register */
+#define REG_IOI		0x0C	/* IO Interrupt Enable register */
+#define REG_IOC		0x0E	/* IO Control register */
+
+#define IOC_SRESET	0x08	/* Software reset */
+#define IOC_GPIO30	0x04	/* GPIO 3:0 unset: as IO, set: as modem pins */
+#define IOC_GPIO74	0x02	/* GPIO 7:4 unset: as IO, set: as modem pins */
+#define IOC_IOLATCH	0x01	/* Unset: input unlatched, set: input latched */
+
+struct sc16is7x2_chip;
+
+/*
+ * Some registers must be read back to modify.
+ * To save time we cache them here in memory.
+ */
+struct sc16is7x2_channel {
+	struct sc16is7x2_chip	*chip;	/* back link */
+	struct uart_port	uart;
+
+	/* Workqueue that does all the magic */
+	struct workqueue_struct *workqueue;
+	struct work_struct	work;
+
+	u16		quot;		/* baud rate divisor */
+	u8		iir;		/* state of IIR register */
+	u8		lsr;		/* state of LSR register */
+	u8		msr;		/* state of MSR register */
+	u8		ier;		/* cache for IER register */
+	u8		fcr;		/* cache for FCR register */
+	u8		lcr;		/* cache for LCR register */
+	u8		mcr;		/* cache for MCR register */
+	u8		efr;		/* cache for EFR register */
+#ifdef DEBUG
+	bool		handle_irq;
+#endif
+	bool		handle_baud;	/* baud rate needs update */
+	bool		handle_regs;	/* other regs need update */
+	u8		buf[FIFO_SIZE+1]; /* fifo transfer buffer */
+};
+
+struct sc16is7x2_chip {
+	struct spi_device *spi;
+	struct sc16is7x2_channel channel[2];
+
+#ifdef CONFIG_GPIOLIB
+	struct gpio_chip gpio;
+	struct mutex	io_lock;	/* lock for GPIO functions */
+	u8		io_dir;		/* cache for IODir register */
+	u8		io_state;	/* cache for IOState register */
+	u8		io_gpio;	/* PIN is GPIO */
+	u8		io_control;	/* cache for IOControl register */
+#endif
+};
+
+/* ******************************** SPI ********************************* */
+
+static inline u8 write_cmd(u8 reg, u8 ch)
+{
+	return REG_WRITE | (reg & 0xf) << 3 | (ch & 0x1) << 1;
+}
+
+static inline u8 read_cmd(u8 reg, u8 ch)
+{
+	return REG_READ  | (reg & 0xf) << 3 | (ch & 0x1) << 1;
+}
+
+/*
+ * sc16is7x2_write - Write a new register content (sync)
+ * @reg: Register offset
+ * @ch:  Channel (0 or 1)
+ */
+static int sc16is7x2_write(struct sc16is7x2_chip *ts, u8 reg, u8 ch, u8 val)
+{
+	u8 out[2];
+
+	out[0] = write_cmd(reg, ch);
+	out[1] = val;
+	return spi_write(ts->spi, out, sizeof(out));
+}
+
+/**
+ * sc16is7x2_read - Read back register content
+ * @reg: Register offset
+ * @ch:  Channel (0 or 1)
+ *
+ * Returns positive 8 bit value from the device if successful or a
+ * negative value on error
+ */
+static int sc16is7x2_read(struct sc16is7x2_chip *ts, unsigned reg, unsigned ch)
+{
+	return spi_w8r8(ts->spi, read_cmd(reg, ch));
+}
+
+/* ******************************** IRQ ********************************* */
+
+static void sc16is7x2_handle_rx(struct sc16is7x2_chip *ts, unsigned ch)
+{
+	struct sc16is7x2_channel *chan = &ts->channel[ch];
+	struct uart_port *uart = &chan->uart;
+	struct tty_struct *tty = uart->state->port.tty;
+	struct spi_message message;
+	struct spi_transfer t[2];
+	unsigned long flags;
+	u8 lsr = chan->lsr;
+	int rxlvl;
+
+	rxlvl = sc16is7x2_read(ts, REG_RXLVL, ch);
+	if (rxlvl <= 0) {
+		return;
+	} else if (rxlvl > FIFO_SIZE) {
+		/* Ensure sanity of RX level */
+		rxlvl = FIFO_SIZE;
+	}
+
+	dev_dbg(&ts->spi->dev, " %s (%i) %d bytes\n", __func__, ch, rxlvl);
+
+	memset(t, 0, sizeof t);
+	chan->buf[0] = read_cmd(UART_RX, ch);
+	t[0].len = 1;
+	t[0].tx_buf = &chan->buf[0];
+	t[1].len = rxlvl;
+	t[1].rx_buf = &chan->buf[1];
+
+	spi_message_init(&message);
+	spi_message_add_tail(&t[0], &message);
+	spi_message_add_tail(&t[1], &message);
+
+	if (spi_sync(ts->spi, &message)) {
+		dev_err(&ts->spi->dev, " SPI transfer RX handling failed\n");
+		return;
+	}
+	chan->buf[rxlvl + 1] = '\0';
+	dev_dbg(&ts->spi->dev, "%s\n", &chan->buf[1]);
+
+	spin_lock_irqsave(&uart->lock, flags);
+
+	if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
+		/*
+		 * For statistics only
+		 */
+		if (lsr & UART_LSR_BI) {
+			lsr &= ~(UART_LSR_FE | UART_LSR_PE);
+			chan->uart.icount.brk++;
+			/*
+			 * We do the SysRQ and SAK checking
+			 * here because otherwise the break
+			 * may get masked by ignore_status_mask
+			 * or read_status_mask.
+			 */
+			if (uart_handle_break(&chan->uart))
+				goto ignore_char;
+		} else if (lsr & UART_LSR_PE)
+			chan->uart.icount.parity++;
+		else if (lsr & UART_LSR_FE)
+			chan->uart.icount.frame++;
+		if (lsr & UART_LSR_OE)
+			chan->uart.icount.overrun++;
+	}
+
+	/* Insert received data */
+	tty_insert_flip_string(tty, &chan->buf[1], rxlvl);
+	/* Update RX counter */
+	uart->icount.rx += rxlvl;
+
+ignore_char:
+	spin_unlock_irqrestore(&uart->lock, flags);
+
+	/* Push the received data to receivers */
+	if (rxlvl)
+		tty_flip_buffer_push(tty);
+}
+
+static void sc16is7x2_handle_tx(struct sc16is7x2_chip *ts, unsigned ch)
+{
+	struct sc16is7x2_channel *chan = &ts->channel[ch];
+	struct uart_port *uart = &chan->uart;
+	struct circ_buf *xmit = &uart->state->xmit;
+	unsigned long flags;
+	unsigned i, len;
+	int txlvl;
+
+	if (chan->uart.x_char && chan->lsr & UART_LSR_THRE) {
+		dev_dbg(&ts->spi->dev, " tx: x-char\n");
+		sc16is7x2_write(ts, UART_TX, ch, uart->x_char);
+		uart->icount.tx++;
+		uart->x_char = 0;
+		return;
+	}
+	if (uart_circ_empty(xmit) || uart_tx_stopped(&chan->uart))
+		/* No data to send or TX is stopped */
+		return;
+
+	txlvl = sc16is7x2_read(ts, REG_TXLVL, ch);
+	if (txlvl <= 0) {
+		dev_dbg(&ts->spi->dev, " %s (%i) fifo full\n", __func__, ch);
+		return;
+	}
+
+	/* number of bytes to transfer to the fifo */
+	len = min(txlvl, (int)uart_circ_chars_pending(xmit));
+
+	dev_dbg(&ts->spi->dev, " %s (%i) %d bytes\n", __func__, ch, len);
+
+	spin_lock_irqsave(&uart->lock, flags);
+	for (i = 1; i <= len ; i++) {
+		chan->buf[i] = xmit->buf[xmit->tail];
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+	}
+	uart->icount.tx += len;
+	spin_unlock_irqrestore(&uart->lock, flags);
+
+	chan->buf[0] = write_cmd(UART_TX, ch);
+	if (spi_write(ts->spi, chan->buf, len + 1))
+		dev_err(&ts->spi->dev, " SPI transfer TX handling failed\n");
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(uart);
+}
+
+static void sc16is7x2_handle_baud(struct sc16is7x2_chip *ts, unsigned ch)
+{
+	struct sc16is7x2_channel *chan = &ts->channel[ch];
+
+	if (!chan->handle_baud)
+		return;
+
+	dev_dbg(&ts->spi->dev, "%s\n", __func__);
+
+	sc16is7x2_write(ts, UART_IER, ch, 0);
+	sc16is7x2_write(ts, UART_LCR, ch, UART_LCR_DLAB); /* access DLL&DLM */
+	sc16is7x2_write(ts, UART_DLL, ch, chan->quot & 0xff);
+	sc16is7x2_write(ts, UART_DLM, ch, chan->quot >> 8);
+	sc16is7x2_write(ts, UART_LCR, ch, chan->lcr);     /* reset DLAB */
+
+	chan->handle_baud = false;
+}
+
+static void sc16is7x2_handle_regs(struct sc16is7x2_chip *ts, unsigned ch)
+{
+	struct sc16is7x2_channel *chan = &ts->channel[ch];
+
+	if (!chan->handle_regs)
+		return;
+
+	dev_dbg(&ts->spi->dev, "%s\n", __func__);
+
+        sc16is7x2_write(ts, UART_LCR, ch, 0xBF);  /* access EFR */
+	sc16is7x2_write(ts, UART_EFR, ch, chan->efr);
+	sc16is7x2_write(ts, UART_LCR, ch, chan->lcr);
+	sc16is7x2_write(ts, UART_FCR, ch, chan->fcr);
+	sc16is7x2_write(ts, UART_MCR, ch, chan->mcr);
+	sc16is7x2_write(ts, UART_IER, ch, chan->ier);
+
+	chan->handle_regs = false;
+}
+
+static void sc16is7x2_read_status(struct sc16is7x2_chip *ts, unsigned ch)
+{
+	struct sc16is7x2_channel *chan = &(ts->channel[ch]);
+/*	struct spi_message m;
+	struct spi_transfer t;
+	u8 *buf = chan->buf; */
+	u8 ier;
+
+#ifdef DEBUG
+	ier = sc16is7x2_read(ts, UART_IER, ch);
+#endif
+	chan->iir = sc16is7x2_read(ts, UART_IIR, ch);
+	chan->msr = sc16is7x2_read(ts, UART_MSR, ch);
+	chan->lsr = sc16is7x2_read(ts, UART_LSR, ch);
+/*
+	buf[0] = read_cmd(UART_IER, ch);
+	buf[1] = read_cmd(UART_IIR, ch);
+	buf[2] = read_cmd(UART_MSR, ch);
+	buf[3] = read_cmd(UART_LSR, ch);
+
+	t.tx_buf = buf;
+	t.rx_buf = &buf[16];
+	t.len = 5;
+
+	spi_message_init(&m);
+	spi_message_add_tail(&t, &m);
+	spi_sync(ts->spi, &m); */
+
+	dev_dbg(&ts->spi->dev, " %s ier=0x%02x iir=0x%02x msr=0x%02x lsr=0x%02x\n",
+			__func__, ier, chan->iir, chan->msr, chan->lsr);
+/*
+	dev_dbg(&ts->spi->dev, " %s ier=0x%02x iir=0x%02x msr=0x%02x lsr=0x%02x\n",
+			__func__, buf[17], buf[18], buf[19], buf[20]);
+*/
+}
+
+static void sc16is7x2_handle_channel(struct work_struct *w)
+{
+	struct sc16is7x2_channel *chan =
+			container_of(w, struct sc16is7x2_channel, work);
+	struct sc16is7x2_chip *ts = chan->chip;
+	unsigned ch = (chan == ts->channel) ? 0 : 1;
+
+#ifdef DEBUG
+	dev_dbg(&ts->spi->dev, "%s (%i) %s\n", __func__, ch,
+			chan->handle_irq ? "irq" : "");
+	chan->handle_irq = false;
+#endif
+
+	do {
+		sc16is7x2_handle_baud(ts, ch);
+		sc16is7x2_handle_regs(ts, ch);
+
+		sc16is7x2_read_status(ts, ch);
+		sc16is7x2_handle_tx(ts, ch);
+		sc16is7x2_handle_rx(ts, ch);
+	} while (!(chan->iir & UART_IIR_NO_INT));
+
+	dev_dbg(&ts->spi->dev, "%s finished\n", __func__);
+}
+
+/* Trigger work thread*/
+static void sc16is7x2_dowork(struct sc16is7x2_channel *chan)
+{
+	if (!freezing(current))
+		queue_work(chan->workqueue, &chan->work);
+}
+
+static irqreturn_t sc16is7x2_irq(int irq, void *data)
+{
+	struct sc16is7x2_channel *chan = data;
+
+#ifdef DEBUG
+	/* Indicate irq */
+	chan->handle_irq = true;
+#endif
+
+	/* Trigger work thread */
+	sc16is7x2_dowork(chan);
+
+	return IRQ_HANDLED;
+}
+
+/* ******************************** UART ********************************* */
+
+#define to_sc16is7x2_channel(port) \
+		container_of(port, struct sc16is7x2_channel, uart)
+
+
+static unsigned int sc16is7x2_tx_empty(struct uart_port *port)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+	unsigned lsr;
+
+	dev_dbg(&ts->spi->dev, "%s = %s\n", __func__,
+			chan->lsr & UART_LSR_TEMT ? "yes" : "no");
+
+	lsr = chan->lsr;
+	return lsr & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
+}
+
+static unsigned int sc16is7x2_get_mctrl(struct uart_port *port)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+	unsigned int status;
+	unsigned int ret;
+
+	dev_dbg(&ts->spi->dev, "%s (0x%02x)\n", __func__, chan->msr);
+
+	status = chan->msr;
+
+	ret = 0;
+	if (status & UART_MSR_DCD)
+		ret |= TIOCM_CAR;
+	if (status & UART_MSR_RI)
+		ret |= TIOCM_RNG;
+	if (status & UART_MSR_DSR)
+		ret |= TIOCM_DSR;
+	if (status & UART_MSR_CTS)
+		ret |= TIOCM_CTS;
+	return ret;
+}
+
+static void sc16is7x2_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+
+	dev_dbg(&ts->spi->dev, "%s (0x%02x)\n", __func__, mctrl);
+
+	/* TODO: set DCD and DSR
+	 * CTS/RTS is handled automatically
+	 */
+}
+
+static void sc16is7x2_stop_tx(struct uart_port *port)
+{
+	/* Do nothing */
+}
+
+static void sc16is7x2_start_tx(struct uart_port *port)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+
+	dev_dbg(&ts->spi->dev, "%s\n", __func__);
+
+	/* Trigger work thread for sending data */
+	sc16is7x2_dowork(chan);
+}
+
+static void sc16is7x2_stop_rx(struct uart_port *port)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+
+	dev_dbg(&ts->spi->dev, "%s\n", __func__);
+
+	chan->ier &= ~UART_IER_RLSI;
+	chan->uart.read_status_mask &= ~UART_LSR_DR;
+	chan->handle_regs = true;
+	/* Trigger work thread for doing the actual configuration change */
+	sc16is7x2_dowork(chan);
+}
+
+static void sc16is7x2_enable_ms(struct uart_port *port)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+
+	dev_dbg(&ts->spi->dev, "%s\n", __func__);
+
+	chan->ier |= UART_IER_MSI;
+	chan->handle_regs = true;
+	/* Trigger work thread for doing the actual configuration change */
+	sc16is7x2_dowork(chan);
+}
+
+static void sc16is7x2_break_ctl(struct uart_port *port, int break_state)
+{
+	/* We don't support break control yet, do nothing */
+}
+
+static int sc16is7x2_startup(struct uart_port *port)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+	unsigned ch = (&ts->channel[1] == chan) ? 1 : 0;
+	unsigned long flags;
+
+	dev_dbg(&ts->spi->dev, "\n%s (%d)\n", __func__, port->line);
+
+	/* Clear the interrupt registers. */
+	sc16is7x2_write(ts, UART_IER, ch, 0);
+	sc16is7x2_read_status(ts, ch);
+
+	/* Initialize work queue */
+	chan->workqueue = create_freezable_workqueue("sc16is7x2");
+	if (!chan->workqueue) {
+		dev_err(&ts->spi->dev, "Workqueue creation failed\n");
+		return -EBUSY;
+	}
+	INIT_WORK(&chan->work, sc16is7x2_handle_channel);
+
+	/* Setup IRQ. Actually we have a low active IRQ, but we want
+	 * one shot behaviour */
+	if (request_irq(ts->spi->irq, sc16is7x2_irq,
+			IRQF_TRIGGER_FALLING | IRQF_SHARED,
+			"sc16is7x2", chan)) {
+		dev_err(&ts->spi->dev, "IRQ request failed\n");
+		destroy_workqueue(chan->workqueue);
+		chan->workqueue = NULL;
+		return -EBUSY;
+	}
+
+
+	spin_lock_irqsave(&chan->uart.lock, flags);
+	chan->lcr = UART_LCR_WLEN8;
+	chan->mcr = 0;
+	chan->fcr = 0;
+	chan->ier = UART_IER_RLSI | UART_IER_RDI | UART_IER_THRI;
+	spin_unlock_irqrestore(&chan->uart.lock, flags);
+
+	sc16is7x2_write(ts, UART_FCR, ch, UART_FCR_ENABLE_FIFO |
+		       UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
+	sc16is7x2_write(ts, UART_FCR, ch, chan->fcr);
+	/* Now, initialize the UART */
+	sc16is7x2_write(ts, UART_LCR, ch, chan->lcr);
+	sc16is7x2_write(ts, UART_MCR, ch, chan->mcr);
+	sc16is7x2_write(ts, UART_IER, ch, chan->ier);
+
+	return 0;
+}
+
+static void sc16is7x2_shutdown(struct uart_port *port)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+	unsigned long flags;
+	unsigned ch = port->line & 0x01;
+
+	dev_dbg(&ts->spi->dev, "%s\n", __func__);
+
+	BUG_ON(!chan);
+	BUG_ON(!ts);
+
+	/* Free the interrupt */
+	free_irq(ts->spi->irq, chan);
+
+	if (chan->workqueue) {
+		/* Flush and destroy work queue */
+		flush_workqueue(chan->workqueue);
+		destroy_workqueue(chan->workqueue);
+		chan->workqueue = NULL;
+	}
+
+	/* Suspend HW */
+	spin_lock_irqsave(&chan->uart.lock, flags);
+	chan->ier = UART_IERX_SLEEP;
+	spin_unlock_irqrestore(&chan->uart.lock, flags);
+	sc16is7x2_write(ts, UART_IER, ch, chan->ier);
+}
+
+static void
+sc16is7x2_set_termios(struct uart_port *port, struct ktermios *termios,
+		       struct ktermios *old)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+	unsigned long flags;
+	unsigned int baud;
+	u8 lcr, fcr = 0;
+
+	/* Ask the core to calculate the divisor for us. */
+	baud = uart_get_baud_rate(port, termios, old,
+				  port->uartclk / 16 / 0xffff,
+				  port->uartclk / 16);
+	chan->quot = uart_get_divisor(port, baud);
+	chan->handle_baud = true;
+
+	dev_dbg(&ts->spi->dev, "%s (baud %u)\n", __func__, baud);
+
+	/* set word length */
+	switch (termios->c_cflag & CSIZE) {
+	case CS5:
+		lcr = UART_LCR_WLEN5;
+		break;
+	case CS6:
+		lcr = UART_LCR_WLEN6;
+		break;
+	case CS7:
+		lcr = UART_LCR_WLEN7;
+		break;
+	default:
+	case CS8:
+		lcr = UART_LCR_WLEN8;
+		break;
+	}
+
+	if (termios->c_cflag & CSTOPB)
+		lcr |= UART_LCR_STOP;
+	if (termios->c_cflag & PARENB)
+		lcr |= UART_LCR_PARITY;
+	if (!(termios->c_cflag & PARODD))
+		lcr |= UART_LCR_EPAR;
+#ifdef CMSPAR
+	if (termios->c_cflag & CMSPAR)
+		lcr |= UART_LCR_SPAR;
+#endif
+
+	fcr = UART_FCR_ENABLE_FIFO;
+	/* configure the fifo */
+	if (baud < 2400)
+		fcr |= UART_FCR_TRIGGER_1;
+	else
+		fcr |= UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_10;
+
+	chan->efr = UART_EFR_ECB;
+	chan->mcr |= UART_MCR_RTS;
+	if (termios->c_cflag & CRTSCTS)
+		chan->efr |= UART_EFR_CTS | UART_EFR_RTS;
+
+	/*
+	 * Ok, we're now changing the port state.  Do it with
+	 * interrupts disabled.
+	 */
+	spin_lock_irqsave(&chan->uart.lock, flags);
+
+	/* we are sending char from a workqueue so enable */
+	chan->uart.state->port.tty->low_latency = 1;
+
+	/* Update the per-port timeout. */
+	uart_update_timeout(port, termios->c_cflag, baud);
+
+	chan->uart.read_status_mask = UART_LSR_OE | UART_LSR_THRE | UART_LSR_DR;
+	if (termios->c_iflag & INPCK)
+		chan->uart.read_status_mask |= UART_LSR_FE | UART_LSR_PE;
+	if (termios->c_iflag & (BRKINT | PARMRK))
+		chan->uart.read_status_mask |= UART_LSR_BI;
+
+	/* Characters to ignore */
+	chan->uart.ignore_status_mask = 0;
+	if (termios->c_iflag & IGNPAR)
+		chan->uart.ignore_status_mask |= UART_LSR_PE | UART_LSR_FE;
+	if (termios->c_iflag & IGNBRK) {
+		chan->uart.ignore_status_mask |= UART_LSR_BI;
+		/*
+		 * If we're ignoring parity and break indicators,
+		 * ignore overruns too (for real raw support).
+		 */
+		if (termios->c_iflag & IGNPAR)
+			chan->uart.ignore_status_mask |= UART_LSR_OE;
+	}
+
+	/* ignore all characters if CREAD is not set */
+	if ((termios->c_cflag & CREAD) == 0)
+		chan->uart.ignore_status_mask |= UART_LSR_DR;
+
+	/* CTS flow control flag and modem status interrupts */
+	chan->ier &= ~UART_IER_MSI;
+	if (UART_ENABLE_MS(&chan->uart, termios->c_cflag))
+		chan->ier |= UART_IER_MSI;
+
+	chan->lcr = lcr;	/* Save LCR */
+	chan->fcr = fcr;	/* Save FCR */
+	chan->handle_regs = true;
+
+	spin_unlock_irqrestore(&chan->uart.lock, flags);
+
+	/* Trigger work thread for doing the actual configuration change */
+	sc16is7x2_dowork(chan);
+}
+
+static const char * sc16is7x2_type(struct uart_port *port)
+{
+	pr_debug("%s\n", __func__);
+	return TYPE_NAME;
+}
+
+static void sc16is7x2_release_port(struct uart_port *port)
+{
+	pr_debug("%s\n", __func__);
+}
+
+static int sc16is7x2_request_port(struct uart_port *port)
+{
+	pr_debug("%s\n", __func__);
+	return 0;
+}
+
+static void sc16is7x2_config_port(struct uart_port *port, int flags)
+{
+	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
+	struct sc16is7x2_chip *ts = chan->chip;
+
+	dev_dbg(&ts->spi->dev, "%s\n", __func__);
+	if (flags & UART_CONFIG_TYPE)
+		chan->uart.type = PORT_SC16IS7X2;
+}
+
+static int
+sc16is7x2_verify_port(struct uart_port *port, struct serial_struct *ser)
+{
+	if (ser->type == PORT_UNKNOWN || ser->type == PORT_SC16IS7X2)
+		return 0;
+
+	return -EINVAL;
+}
+
+static struct uart_ops sc16is7x2_uart_ops = {
+	.tx_empty	= sc16is7x2_tx_empty,
+	.set_mctrl	= sc16is7x2_set_mctrl,
+	.get_mctrl	= sc16is7x2_get_mctrl,
+	.stop_tx        = sc16is7x2_stop_tx,
+	.start_tx	= sc16is7x2_start_tx,
+	.stop_rx	= sc16is7x2_stop_rx,
+	.enable_ms      = sc16is7x2_enable_ms,
+	.break_ctl      = sc16is7x2_break_ctl,
+	.startup	= sc16is7x2_startup,
+	.shutdown	= sc16is7x2_shutdown,
+	.set_termios	= sc16is7x2_set_termios,
+	.type		= sc16is7x2_type,
+	.release_port   = sc16is7x2_release_port,
+	.request_port   = sc16is7x2_request_port,
+	.config_port	= sc16is7x2_config_port,
+	.verify_port	= sc16is7x2_verify_port,
+};
+
+
+/* ******************************** GPIO ********************************* */
+
+#ifdef CONFIG_GPIOLIB
+
+static int sc16is7x2_gpio_request(struct gpio_chip *gpio, unsigned offset)
+{
+	struct sc16is7x2_chip *ts =
+			container_of(gpio, struct sc16is7x2_chip, gpio);
+	int control = (offset < 4) ? IOC_GPIO30 : IOC_GPIO74;
+	int ret = 0;
+
+	BUG_ON(offset > 8);
+	dev_dbg(&ts->spi->dev, "%s: offset = %d\n", __func__, offset);
+
+	mutex_lock(&ts->io_lock);
+
+	/* GPIO 0:3 and 4:7 can only be controlled as block */
+	ts->io_gpio |= BIT(offset);
+	if (ts->io_control & control) {
+		dev_dbg(&ts->spi->dev, "activate GPIOs %s\n",
+				(offset < 4) ? "0-3" : "4-7");
+		ts->io_control &= ~control;
+		ret = sc16is7x2_write(ts, REG_IOC, 0, ts->io_control);
+	}
+
+	mutex_unlock(&ts->io_lock);
+
+	return ret;
+}
+
+static void sc16is7x2_gpio_free(struct gpio_chip *gpio, unsigned offset)
+{
+	struct sc16is7x2_chip *ts =
+			container_of(gpio, struct sc16is7x2_chip, gpio);
+	int control = (offset < 4) ? IOC_GPIO30 : IOC_GPIO74;
+	int mask = (offset < 4) ? 0x0f : 0xf0;
+
+	BUG_ON(offset > 8);
+
+	mutex_lock(&ts->io_lock);
+
+	/* GPIO 0:3 and 4:7 can only be controlled as block */
+	ts->io_gpio &= ~BIT(offset);
+	dev_dbg(&ts->spi->dev, "%s: io_gpio = 0x%02X\n", __func__, ts->io_gpio);
+	if (!(ts->io_control & control) && !(ts->io_gpio & mask)) {
+		dev_dbg(&ts->spi->dev, "deactivate GPIOs %s\n",
+				(offset < 4) ? "0-3" : "4-7");
+		ts->io_control |= control;
+		sc16is7x2_write(ts, REG_IOC, 0, ts->io_control);
+	}
+
+	mutex_unlock(&ts->io_lock);
+}
+
+static int sc16is7x2_direction_input(struct gpio_chip *gpio, unsigned offset)
+{
+	struct sc16is7x2_chip *ts =
+			container_of(gpio, struct sc16is7x2_chip, gpio);
+	unsigned io_dir;
+
+	BUG_ON(offset > 8);
+
+	mutex_lock(&ts->io_lock);
+
+	ts->io_dir &= ~BIT(offset);
+	io_dir = ts->io_dir;
+
+	mutex_unlock(&ts->io_lock);
+
+	return sc16is7x2_write(ts, REG_IOD, 0, io_dir);
+}
+
+static int sc16is7x2_direction_output(struct gpio_chip *gpio, unsigned offset,
+				    int value)
+{
+	struct sc16is7x2_chip *ts =
+			container_of(gpio, struct sc16is7x2_chip, gpio);
+
+	BUG_ON(offset > 8);
+
+	mutex_lock(&ts->io_lock);
+
+	if (value)
+		ts->io_state |= BIT(offset);
+	else
+		ts->io_state &= ~BIT(offset);
+
+	ts->io_dir |= BIT(offset);
+
+	mutex_unlock(&ts->io_lock);
+
+	sc16is7x2_write(ts, REG_IOS, 0, ts->io_state);
+	return sc16is7x2_write(ts, REG_IOD, 0, ts->io_dir);
+}
+
+static int sc16is7x2_get(struct gpio_chip *gpio, unsigned offset)
+{
+	struct sc16is7x2_chip *ts =
+			container_of(gpio, struct sc16is7x2_chip, gpio);
+	int level = -EINVAL;
+
+	BUG_ON(offset > 8);
+
+	mutex_lock(&ts->io_lock);
+
+	if (ts->io_dir & BIT(offset)) {
+		/* Output: return cached level */
+		level = (ts->io_state >> offset) & 0x01;
+	} else {
+		/* Input: read out all pins */
+		level = sc16is7x2_read(ts, REG_IOS, 0);
+		if (level >= 0) {
+			ts->io_state = level;
+			level = (ts->io_state >> offset) & 0x01;
+		}
+	}
+
+	mutex_unlock(&ts->io_lock);
+
+	return level;
+}
+
+static void sc16is7x2_set(struct gpio_chip *gpio, unsigned offset, int value)
+{
+	struct sc16is7x2_chip *ts =
+			container_of(gpio, struct sc16is7x2_chip, gpio);
+	unsigned io_state;
+
+	BUG_ON(offset > 8);
+
+	mutex_lock(&ts->io_lock);
+
+	if (value)
+		ts->io_state |= BIT(offset);
+	else
+		ts->io_state &= ~BIT(offset);
+	io_state = ts->io_state;
+
+	mutex_unlock(&ts->io_lock);
+
+	sc16is7x2_write(ts, REG_IOS, 0, io_state);
+}
+
+#endif /* CONFIG_GPIOLIB */
+
+/* ******************************** INIT ********************************* */
+
+static struct uart_driver sc16is7x2_uart_driver;
+
+static int sc16is7x2_register_gpio(struct sc16is7x2_chip *ts,
+		struct sc16is7x2_platform_data *pdata)
+{
+#ifdef CONFIG_GPIOLIB
+	ts->gpio.label = (pdata->label) ? pdata->label : DRIVER_NAME;
+	ts->gpio.request	= sc16is7x2_gpio_request;
+	ts->gpio.free		= sc16is7x2_gpio_free;
+	ts->gpio.get		= sc16is7x2_get;
+	ts->gpio.set		= sc16is7x2_set;
+	ts->gpio.direction_input = sc16is7x2_direction_input;
+	ts->gpio.direction_output = sc16is7x2_direction_output;
+
+	ts->gpio.base = pdata->gpio_base;
+	ts->gpio.names = pdata->names;
+	ts->gpio.ngpio = SC16IS7X2_NR_GPIOS;
+	ts->gpio.can_sleep = 1;
+	ts->gpio.dev = &ts->spi->dev;
+	ts->gpio.owner = THIS_MODULE;
+
+	mutex_init(&ts->io_lock);
+
+	/* disable all GPIOs, enable on request */
+	ts->io_gpio = 0;
+	ts->io_control = IOC_GPIO30 | IOC_GPIO74;
+	ts->io_state = 0;
+	ts->io_dir = 0;
+
+	sc16is7x2_write(ts, REG_IOI, 0, 0); /* no support for irqs yet */
+	sc16is7x2_write(ts, REG_IOC, 0, ts->io_control);
+	sc16is7x2_write(ts, REG_IOS, 0, ts->io_state);
+	sc16is7x2_write(ts, REG_IOD, 0, ts->io_dir);
+
+	return gpiochip_add(&ts->gpio);
+#else
+	return 0;
+#endif
+}
+
+static int sc16is7x2_register_uart_port(struct sc16is7x2_chip *ts,
+		struct sc16is7x2_platform_data *pdata, unsigned ch)
+{
+	struct sc16is7x2_channel *chan = &(ts->channel[ch]);
+	struct uart_port *uart = &chan->uart;
+
+	/* Disable irqs and go to sleep */
+	sc16is7x2_write(ts, UART_IER, ch, UART_IERX_SLEEP);
+
+	chan->chip = ts;
+
+	uart->irq = ts->spi->irq;
+	uart->uartclk = pdata->uartclk;
+	uart->fifosize = FIFO_SIZE;
+	uart->ops = &sc16is7x2_uart_ops;
+	uart->flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
+	uart->line = pdata->uart_base + ch;
+	uart->type = PORT_SC16IS7X2;
+	uart->dev = &ts->spi->dev;
+
+	return uart_add_one_port(&sc16is7x2_uart_driver, uart);
+}
+
+static int __devinit sc16is7x2_probe(struct spi_device *spi)
+{
+	struct sc16is7x2_chip *ts;
+	struct sc16is7x2_platform_data *pdata;
+	int ret;
+
+	/* Only even uart base numbers are supported */
+	pdata = spi->dev.platform_data;
+	if (!pdata || !pdata->gpio_base || pdata->uart_base & 1) {
+		dev_dbg(&spi->dev, "incorrect or missing platform data\n");
+		return -EINVAL;
+	}
+
+	ts = kzalloc(sizeof(struct sc16is7x2_chip), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, ts);
+	ts->spi = spi;
+
+	/* Reset the chip */
+	sc16is7x2_write(ts, REG_IOC, 0, IOC_SRESET);
+
+	ret = sc16is7x2_register_uart_port(ts, pdata, 0);
+	if (ret)
+		goto exit_destroy;
+	ret = sc16is7x2_register_uart_port(ts, pdata, 1);
+	if (ret)
+		goto exit_uart0;
+
+	ret = sc16is7x2_register_gpio(ts, pdata);
+	if (ret)
+		goto exit_uart1;
+
+	dev_info(&spi->dev, DRIVER_NAME " at CS%d (irq %d), 2 UARTs, 8 GPIOs\n"
+			"    eser%d, eser%d, gpiochip%d\n",
+			spi->chip_select, spi->irq,
+			pdata->uart_base, pdata->uart_base + 1,
+			pdata->gpio_base);
+
+	return 0;
+
+exit_uart1:
+	uart_remove_one_port(&sc16is7x2_uart_driver, &ts->channel[1].uart);
+
+exit_uart0:
+	uart_remove_one_port(&sc16is7x2_uart_driver, &ts->channel[0].uart);
+
+exit_destroy:
+	dev_set_drvdata(&spi->dev, NULL);
+	kfree(ts);
+	return ret;
+}
+
+static int __devexit sc16is7x2_remove(struct spi_device *spi)
+{
+	struct sc16is7x2_chip *ts = spi_get_drvdata(spi);
+	int ret;
+
+	if (ts == NULL)
+		return -ENODEV;
+
+	ret = uart_remove_one_port(&sc16is7x2_uart_driver, &ts->channel[0].uart);
+	if (ret)
+		return ret;
+
+	ret = uart_remove_one_port(&sc16is7x2_uart_driver, &ts->channel[1].uart);
+	if (ret)
+		return ret;
+
+#ifdef CONFIG_GPIOLIB
+	ret = gpiochip_remove(&ts->gpio);
+	if (ret)
+		return ret;
+#endif
+
+	kfree(ts);
+
+	return 0;
+}
+
+static struct uart_driver sc16is7x2_uart_driver = {
+	.owner          = THIS_MODULE,
+	.driver_name    = DRIVER_NAME,
+	.dev_name       = "eser",
+	.nr             = MAX_SC16IS7X2,
+};
+
+/* Spi driver data */
+static struct spi_driver sc16is7x2_spi_driver = {
+	.driver = {
+		.name		= DRIVER_NAME,
+		.bus		= &spi_bus_type,
+		.owner		= THIS_MODULE,
+	},
+	.probe		= sc16is7x2_probe,
+	.remove		= __devexit_p(sc16is7x2_remove),
+};
+
+/* Driver init function */
+static int __init sc16is7x2_init(void)
+{
+	int ret = uart_register_driver(&sc16is7x2_uart_driver);
+	if (ret)
+		return ret;
+
+	return spi_register_driver(&sc16is7x2_spi_driver);
+}
+
+/* Driver exit function */
+static void __exit sc16is7x2_exit(void)
+{
+	spi_unregister_driver(&sc16is7x2_spi_driver);
+	uart_unregister_driver(&sc16is7x2_uart_driver);
+}
+
+/* register after spi postcore initcall and before
+ * subsys initcalls that may rely on these GPIOs
+ */
+subsys_initcall(sc16is7x2_init);
+module_exit(sc16is7x2_exit);
+
+MODULE_AUTHOR("Manuel Stahl");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("SC16IS7x2 SPI based UART chip");
+MODULE_ALIAS("spi:" DRIVER_NAME);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c91ace7..e535142 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -210,6 +210,9 @@
 /* Atheros AR933X SoC */
 #define PORT_AR933X	99
 
+/* NXP SC16IS7X2 */
+#define PORT_SC16IS7X2 100
+
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/serial_sc16is7x2.h b/include/linux/serial_sc16is7x2.h
new file mode 100755
index 0000000..931fe50
--- /dev/null
+++ b/include/linux/serial_sc16is7x2.h
@@ -0,0 +1,17 @@
+#ifndef LINUX_SPI_SC16IS752_H
+#define LINUX_SPI_SC16IS752_H
+
+#define SC16IS7X2_NR_GPIOS 8
+
+struct sc16is7x2_platform_data {
+	unsigned int	uartclk;
+	/* uart line number of the first channel */
+	unsigned	uart_base;
+	/* number assigned to the first GPIO */
+	unsigned	gpio_base;
+	char		*label;
+	/* list of GPIO names (array length = SC16IS7X2_NR_GPIOS) */
+	const char	*const *names;
+};
+
+#endif
-- 
1.7.9


^ permalink raw reply related

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-05 12:16 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202041224170.9453@utopia.booyaka.com>

On Sat, Feb 04, 2012 at 12:37:06PM -0700, Paul Walmsley wrote:
> On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:
> 
> > On Sat, Feb 04, 2012 at 10:32:16AM -0700, Paul Walmsley wrote:
> > > On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:
> > > 
> > > > [detailed discussion of framing errors]
> > > 
> > > Thanks for the detailed description.  If the driver is in fact discarding 
> > > characters with framing errors -- which I have not personally verified -- 
> > > then taking further action there is pointless.
> > 
> > Paul, I know you don't particularly like me getting involved with OMAP
> > issues, but tough.  You don't seem to understand some of these issues so
> > you're going to get more explanation.
> 
> Hehe.  Oh, hurt me again with more explanation, please ;-)  I can't take 
> it ;-)  I happen to enjoy many of your technical explanations.  Doesn't 
> necessarily mean that we're always in agreement, though.
> 
> As for the part about not wanting you involved with OMAP, that's an 
> interesting perspective, considering I mentioned to you at ELC-E last year 
> my appreciation of your technical review on the lists.  And you'll 
> probably note, if you care to review the lists, that many of my E-mail 
> responses express gratitude for your comments... including the one you 
> quoted.
> 
> Of course the snarky, personal bits can be unnecessarily irritating, but 
> anyone who's in this line of work just has to deal with them, it seems.

Paul, you're being two faced.  You can see the reaction that your message
below gained:

"On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:
"> [detailed discussion of framing errors]
"
"Thanks for the detailed description.  If the driver is in fact discarding
"characters with framing errors -- which I have not personally verified --
"then taking further action there is pointless."

which, _to_ _me_, looks like you're trying to get rid of my input from
this problem, and _that_ is extremely irritating.  You talk of irritating,
but maybe you don't realise that you're _just_ as irritating too at times
- and that's not the first time you've attempted to cut off my input in
that way.

Especially when you start making suggestions like throwing away an entire
FIFO load of data when you get a framing error.  I think you have a
fundamental misunderstanding about UARTs or what's required from them.

Now, the fact is that POSIX allows user programs to tell the TTY drivers
what behaviour they want, and it's essentially one of the following:

1. Ignore errors and receive all characters from the UART whether
   they be in error or not.
2. Receive characters in the FIFO and mark characters in error.
3. Receive all properly received characters.

Which errors cause this behaviour can be controlled individually.  Here's
the extract straight from POSIX:

   If IGNBRK is set, a break condition detected on input shall be ignored;
   that is, not put on the input queue and therefore not read by any process.
   If IGNBRK is not set and BRKINT is set, the break condition shall flush
   the input and output queues, and if the terminal is the controlling
   terminal of a foreground process group, the break condition shall generate
   a single SIGINT signal to that foreground process group. If neither IGNBRK
   nor BRKINT is set, a break condition shall be read as a single 0x00, or if
   PARMRK is set, as 0xff 0x00 0x00.

   If IGNPAR is set, a byte with a framing or parity error (other than break)
   shall be ignored.

   If PARMRK is set, and IGNPAR is not set, a byte with a framing or parity
   error (other than break) shall be given to the application as the
   three-byte sequence 0xff 0x00 X, where 0xff 0x00 is a two-byte flag
   preceding each sequence and X is the data of the byte received in error.
   To avoid ambiguity in this case, if ISTRIP is not set, a valid byte of
   0xff is given to the application as 0xff 0xff. If neither PARMRK nor
   IGNPAR is set, a framing or parity error (other than break) shall be given
   to the application as a single byte 0x00.

So, as you can see, the serial driver does not have the option of throwing
characters away just because an error bit is set - the behaviour required
is left to userspace to decide, and are partly implemented by the tty
layers and partly the serial driver.  The requirements are well defined,
and 8250 follows them - and I've just checked omap-serial against 8250
and it does the same.

The fact is that the way OMAP implements the power management around UARTs
is broken, because it results in corrupted characters being received.
It's as plain and simple as that.

And there's nothing you can to do solve the problem of the broken PM
causing a badly received character which _may_ have a framing error
being passed to userspace.

If the termios settings are correct, the badly framed character shouldn't
be passed to a shell - but that doesn't stop characters which don't have
framing errors but still aren't received as they were transmitted being
passed.  And there's absolutely _nothing_ you can do about that as long
as the broken PM is enabled on the port.

As I illustrated in my 'detailed discussion of framing errors'.

^ permalink raw reply

* Re: [PATCH/RFC] usb: fix renesas_usbhs to not schedule in atomic context
From: Felipe Balbi @ 2012-02-05 14:54 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Shimoda, Yoshihiro, linux-kernel, linux-sh, Vinod Koul,
	Magnus Damm, linux-mmc, alsa-devel, linux-serial, Paul Mundt,
	linux-usb
In-Reply-To: <Pine.LNX.4.64.1202031623000.6211@axis700.grange>

[-- Attachment #1: Type: text/plain, Size: 2450 bytes --]

Hi,

On Fri, Feb 03, 2012 at 04:43:20PM +0100, Guennadi Liakhovetski wrote:
> The current renesas_usbhs driver triggers
> 
> BUG: scheduling while atomic: ksoftirqd/0/3/0x00000102
> 
> with enabled CONFIG_DEBUG_ATOMIC_SLEEP, by submitting DMA transfers from 
> an atomic (tasklet) context, which is not supported by the shdma dmaengine 
> driver. Fix it by switching to a work. Also simplify some list 
> manipulations.

you are doing much more than what you say. Also, instead of using a
workqueue, have you considered using threaded_irqs ?

(I didn't go over the driver again to see if it makes sense to use
threaded_irqs in this case, but doesn't hurt asking)

> Shimoda-san, this is the problem, that you were observing. However, it 
> exists with the present version of shdma just as well as with the new one 
> - on top of the simple DMA library. I marked it an RFC because (1) I only 
> lightly tested it with the gadget device on mackerel with the mass storage 
> gadget, and (2) I am somewhat concerned about races. Currently the work 
> function runs with no locking and there are no usual cancel_work_sync() 
> points in the patch. However, it has also been like this before with the 
> tasklet implementation, which is not much better, and it looks like there 
> are no asynchronous operations on the same packets like timeouts. Only 
> asynchronous events, that I can think about are things like unloading the 
> driver or unplugging the cable, but these have been there before too. It 
> would become worse on SMP, I think. Comments welcome.
> 
> diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
> index 72339bd..4d739ec 100644
> --- a/drivers/usb/renesas_usbhs/fifo.c
> +++ b/drivers/usb/renesas_usbhs/fifo.c
> @@ -75,8 +75,7 @@ void usbhs_pkt_push(struct usbhs_pipe *pipe, struct usbhs_pkt *pkt,
>  		pipe->handler = &usbhsf_null_handler;
>  	}
>  
> -	list_del_init(&pkt->node);
> -	list_add_tail(&pkt->node, &pipe->list);
> +	list_move_tail(&pkt->node, &pipe->list);
>  
>  	/*
>  	 * each pkt must hold own handler.
> @@ -106,7 +105,7 @@ static struct usbhs_pkt *__usbhsf_pkt_get(struct usbhs_pipe *pipe)
>  	if (list_empty(&pipe->list))
>  		return NULL;
>  
> -	return list_entry(pipe->list.next, struct usbhs_pkt, node);
> +	return list_first_entry(&pipe->list, struct usbhs_pkt, node);

these two hunks are not part of $SUBJECT

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* RE: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Woodruff, Richard @ 2012-02-05 15:37 UTC (permalink / raw)
  To: Russell King - ARM Linux, Grazvydas Ignotas
  Cc: Paul Walmsley, Hilman, Kevin, NeilBrown, R, Govindraj,
	Valkeinen, Tomi, linux-serial@vger.kernel.org,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20120204163931.GH1275@n2100.arm.linux.org.uk>


> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Russell King - ARM Linux
> Sent: Saturday, February 04, 2012 10:40 AM

> It is entirely unacceptable to drop characters on a serial port through
> the use of PM.  Many serial protocols just will not cope with that kind
> of behaviour - yes, serial protocols may have retry built-in, but will
> they retry _before_ the port re-idles and the PLL shuts down?  Can you
> be sure?

What is acceptable depends on the hardware and applications stack ups being used. There are ways to work around issues along whole path in SW and HW.  There is no single setup which makes all combinations work. Some old PC chassis may define 1 of many standards but they probably were not defined with very low power tradeoffs in mind.

If the board designer hooks up all possible serial signals for uart/rs232/rs422/standard-xyz or just rx/tx, or adds some glue logic, or adds smart peripheral, or ..., there are will be constraints and ways to cope with issue being discussed.

For most OMAP reference platforms the HW design links available UARTs with likely peripherals used in that timeframe. When it comes to making each UART work with PM some changes are usually needed per interface. Depending on the given stack up (to include bugs/limitations) some per interface tweak is always needed. It might be as you say you have to defeat PM on a port and only release after protocol handshaking with some modem firmware or it might be the debug UART expectation is lowered and allowed to work in a lossy mode so as not to destroy platform power for non-production port.

[x] What is acceptable depends is not black and white.  Is there some QOS mapping which can be set per channel which allows runtime PM to pick a best chose (which may allow for loss and frame issues)?.

Regards,
Richard W.


^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-05 16:03 UTC (permalink / raw)
  To: Woodruff, Richard
  Cc: Grazvydas Ignotas, Paul Walmsley, Hilman, Kevin, NeilBrown,
	R, Govindraj, Valkeinen, Tomi, linux-serial@vger.kernel.org,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <EF62F09C0797D947AD4180A1043C0DF734934E36@DLEE10.ent.ti.com>

On Sun, Feb 05, 2012 at 03:37:21PM +0000, Woodruff, Richard wrote:
> [x] What is acceptable depends is not black and white.  Is there some
> QOS mapping which can be set per channel which allows runtime PM to
> pick a best chose (which may allow for loss and frame issues)?.

What you're asking is whether there's anything in the kernel which can
predict when the next character is to arrive.

For many serial protocols, that would require a non-causal universe.  So
far, I'm afraid, physics hasn't managed to provably invent time travel
in any useful way.  Once physics has, then maybe this can be fixed.

But, the fact of the matter is that deriving the UART clocks from a PLL
which takes a finite time to lock, and the PLL is shut down during runtime
PM _is_ _going_ _to_ _cause_ _problems_.  There is absolutely no getting
away from that.

Let's take your modem example.  Modems today would typically be used with
some IP based protocol, probably PPP based.  Incoming traffic on IP is
entirely unpredictable.  You wouldn't know when the next character would
be received.

One solution to this is to transmit an 0xff character before your real
data to ensure that your end is awake and properly synchronized...

Therefore, if you're running PPP (or, shudder, SLIP) over serial, you'd
have to totally defeat the PM support for the port.  No, not just for the
initial negotiation, but all the time that the PPP connection was
established.

You can't rely on the remote end retrying, because of backoffs.

You'll find that setting a PM timeout of maybe 5 seconds works, but as
soon as you experience a flakey link somewhere, the TCP could backoff to
10 seconds or even minutes with its retry.  At that point you're into a
vicious circle because you're now putting the port into low power mode,
which will drop the first character.  That means that TCP's retry will
get dropped on the floor, which in turn will increase the TCP backoff.
And so the cycle continues with ever increasing TCP backoffs and no
forward progress.

... but the problem with the 0xff character approach here is that most
ISPs won't allow you to drop by and fiddle with their PPP termination to
add that workaround.  They'll rightfully tell you to go screw yourself
and get some hardware which works.

So, go ahead with having PM drop random characters if you want, but don't
expect anyone in their right mind to accept broken workarounds to the
kernel serial driver to try to drop maybe 16 characters or more at a time
on the floor when a framing error occurs just because the PM is broken.

And let's not forget the problem that current kernels have on OMAP34xx
platforms.  Literally minutes to get a dmesg out over a 115200 baud serial
port, 16 characters at a time.  I guess you're going to try to justify
that as 'acceptable behaviour' from the system.  Yes, of course it is.
If you're dealing with a 1960s computer which processes that slowly.

And I thought we were in 2012.

^ permalink raw reply

* RE: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Woodruff, Richard @ 2012-02-05 17:57 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grazvydas Ignotas, Paul Walmsley, Hilman, Kevin, NeilBrown,
	R, Govindraj, Valkeinen, Tomi, linux-serial@vger.kernel.org,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20120205160329.GO1275@n2100.arm.linux.org.uk>


> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
> Sent: Sunday, February 05, 2012 10:03 AM
> To: Woodruff, Richard

> On Sun, Feb 05, 2012 at 03:37:21PM +0000, Woodruff, Richard wrote:
> > [x] What is acceptable depends is not black and white.  Is there some
> > QOS mapping which can be set per channel which allows runtime PM to
> > pick a best chose (which may allow for loss and frame issues)?.
> 
> What you're asking is whether there's anything in the kernel which can
> predict when the next character is to arrive.

No, this was not the comment's intent.

> But, the fact of the matter is that deriving the UART clocks from a PLL
> which takes a finite time to lock, and the PLL is shut down during runtime
> PM _is_ _going_ _to_ _cause_ _problems_.  There is absolutely no getting
> away from that.

Yes this is one of the issues to be worked around.

> Let's take your modem example.  Modems today would typically be used with
> some IP based protocol, probably PPP based.  Incoming traffic on IP is
> entirely unpredictable.  You wouldn't know when the next character would
> be received.
> 
> One solution to this is to transmit an 0xff character before your real
> data to ensure that your end is awake and properly synchronized...

This approach as you say has issues.  This is solved in different ways for modems.

<sleep>My observation is modem software which many talk over ppp over ip over serial of some sort (might be uart, might be usb), will send a command to the modem to go into a low power mode. Now you can cut clocks with out hurting modem and getting SOC power.

<wake>When some event happens at modem or processor (timer near beacon or other) the modem or apps processor can signal the other with some wake event (maybe over gpio) which then puts system in a state where it can receive data in trusted manner.

The modem channel driver try's to inform kernel about entering/exiting modes to set expectation.

> So, go ahead with having PM drop random characters if you want, but don't
> expect anyone in their right mind to accept broken workarounds to the
> kernel serial driver to try to drop maybe 16 characters or more at a time
> on the floor when a framing error occurs just because the PM is broken.

No character was dropped in modem example.  On the UART-to-Debug console it may be ok to drop a character.  Ether needs a coordination hook.

Each stack needs some way to adjust expectations.  Finding a way to isolate to a sub-layer of stack and not break everything is always the quest.

> And let's not forget the problem that current kernels have on OMAP34xx
> platforms.  Literally minutes to get a dmesg out over a 115200 baud serial
> port, 16 characters at a time.  I guess you're going to try to justify
> that as 'acceptable behaviour' from the system.  Yes, of course it is.
> If you're dealing with a 1960s computer which processes that slowly.
> 
> And I thought we were in 2012.

This looks like a different issue.  Years back with custom kernels with lots of hacks hack this was not the case.  These days the job is harder to make it work more generally.  Evolving for PM tradeoffs is painful in HW and SW.

Regards,
Richard W.


^ permalink raw reply

* Re: [PATCH] Add sc16is7x2 driver
From: Alan Cox @ 2012-02-05 19:56 UTC (permalink / raw)
  To: Thomas Weber; +Cc: linux-serial, Manuel Stahl
In-Reply-To: <1328424925-645-1-git-send-email-thomas@tweber.de>

> +/* ******************************** IRQ ********************************* */
> +
> +static void sc16is7x2_handle_rx(struct sc16is7x2_chip *ts, unsigned ch)
> +{
> +	struct sc16is7x2_channel *chan = &ts->channel[ch];
> +	struct uart_port *uart = &chan->uart;
> +	struct tty_struct *tty = uart->state->port.tty;

What guarantees tty cannot go NULL during this ? - I think you need to
hold references and check ? (tty_port_tty_get)


> +/* Trigger work thread*/
> +static void sc16is7x2_dowork(struct sc16is7x2_channel *chan)
> +{
> +	if (!freezing(current))
> +		queue_work(chan->workqueue, &chan->work);
> +}

Threaded interrupt handler or is the single queue for work in tx and rx
actually a nicer way to do it - just asking the question.


> +static void sc16is7x2_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
> +	struct sc16is7x2_chip *ts = chan->chip;
> +
> +	dev_dbg(&ts->spi->dev, "%s (0x%02x)\n", __func__, mctrl);
> +
> +	/* TODO: set DCD and DSR
> +	 * CTS/RTS is handled automatically
> +	 */

Looks like it's not yet finished and ready for merging ?

> +static void sc16is7x2_stop_rx(struct uart_port *port)
> +{
> +	struct sc16is7x2_channel *chan = to_sc16is7x2_channel(port);
> +	struct sc16is7x2_chip *ts = chan->chip;
> +
> +	dev_dbg(&ts->spi->dev, "%s\n", __func__);
> +
> +	chan->ier &= ~UART_IER_RLSI;
> +	chan->uart.read_status_mask &= ~UART_LSR_DR;
> +	chan->handle_regs = true;
> +	/* Trigger work thread for doing the actual configuration change */
> +	sc16is7x2_dowork(chan);
> +}

> +static void sc16is7x2_break_ctl(struct uart_port *port, int break_state)
> +{
> +	/* We don't support break control yet, do nothing */
> +}

More unfinished bits

> +	/* Setup IRQ. Actually we have a low active IRQ, but we want
> +	 * one shot behaviour */
> +	if (request_irq(ts->spi->irq, sc16is7x2_irq,
> +			IRQF_TRIGGER_FALLING | IRQF_SHARED,
> +			"sc16is7x2", chan)) {

Is this a property of the device or of your board ?


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

> +#ifdef CMSPAR
> +	if (termios->c_cflag & CMSPAR)
> +		lcr |= UART_LCR_SPAR;
> +#endif

ifdef shouldn't be needed


termios should write back the actual baud (see how 8250.c does it)


> +#ifdef CONFIG_GPIOLIB

I wonder if this should be a separate driver and the device an MFD -
would anyone ever be using it as gpio only ?


Looks basically sound. The GPIO question I think needs figuring out, and
there are a few bits of unfinished code that need glancing it.

Alan

^ permalink raw reply

* Re: [PATCH 6/7 v2] ASoC: SIU: prepare for conversion to simple DMA
From: Vinod Koul @ 2012-02-06  2:33 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-kernel, linux-sh, Magnus Damm, Yoshihiro Shimoda, linux-mmc,
	alsa-devel, linux-serial, Paul Mundt
In-Reply-To: <1327589784-4287-7-git-send-email-g.liakhovetski@gmx.de>

On Thu, 2012-01-26 at 15:56 +0100, Guennadi Liakhovetski wrote:
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  sound/soc/sh/siu_pcm.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/sh/siu_pcm.c b/sound/soc/sh/siu_pcm.c
> index 0193e59..414bb35 100644
> --- a/sound/soc/sh/siu_pcm.c
> +++ b/sound/soc/sh/siu_pcm.c
> @@ -330,12 +330,12 @@ static bool filter(struct dma_chan *chan, void *slave)
>  {
>  	struct sh_dmae_slave *param = slave;
>  
> -	pr_debug("%s: slave ID %d\n", __func__, param->slave_id);
> +	pr_debug("%s: slave ID %d\n", __func__, param->simple_slave.slave_id);
>  
>  	if (unlikely(param->dma_dev != chan->device->dev))
>  		return false;
>  
> -	chan->private = param;
> +	chan->private = &param->simple_slave;
While at it, Can you fix the sh-mobile driver to NOT use chan->private?
>  	return true;
>  }
>  
> @@ -360,12 +360,12 @@ static int siu_pcm_open(struct snd_pcm_substream *ss)
>  	if (ss->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		siu_stream = &port_info->playback;
>  		param = &siu_stream->param;
> -		param->slave_id = port ? pdata->dma_slave_tx_b :
> +		param->simple_slave.slave_id = port ? pdata->dma_slave_tx_b :
>  			pdata->dma_slave_tx_a;
>  	} else {
>  		siu_stream = &port_info->capture;
>  		param = &siu_stream->param;
> -		param->slave_id = port ? pdata->dma_slave_rx_b :
> +		param->simple_slave.slave_id = port ? pdata->dma_slave_rx_b :
>  			pdata->dma_slave_rx_a;
>  	}
>  


-- 
~Vinod


^ permalink raw reply

* Re: [PATCH 1/7 v2] dmaengine: add a simple dma library
From: Vinod Koul @ 2012-02-06  2:54 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-kernel, linux-sh, Magnus Damm, Yoshihiro Shimoda, linux-mmc,
	alsa-devel, linux-serial, Paul Mundt
In-Reply-To: <1327589784-4287-2-git-send-email-g.liakhovetski@gmx.de>

On Thu, 2012-01-26 at 15:56 +0100, Guennadi Liakhovetski wrote:
> This patch adds a library of functions, helping to implement dmaengine
> drivers for hardware, unable to handle scatter-gather lists natively.
> The first version of this driver only supports memcpy and slave DMA
> operation.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> v2:
> 
> 1. switch from using a tasklet to threaded IRQ, which allowed to
...?
> 2. remove lock / unlock inline functions
> 3. remove __devinit, __devexit annotations
> 
>  drivers/dma/Kconfig        |    3 +
>  drivers/dma/Makefile       |    1 +
>  drivers/dma/dma-simple.c   |  873 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/dma-simple.h |  124 +++++++
>  4 files changed, 1001 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/dma/dma-simple.c
>  create mode 100644 include/linux/dma-simple.h
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index f1a2749..f7c583e 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -149,6 +149,9 @@ config TXX9_DMAC
>  	  Support the TXx9 SoC internal DMA controller.  This can be
>  	  integrated in chips such as the Toshiba TX4927/38/39.
>  
> +config DMA_SIMPLE
> +	tristate
> +
>  config SH_DMAE
>  	tristate "Renesas SuperH DMAC support"
>  	depends on (SUPERH && SH_DMA) || (ARM && ARCH_SHMOBILE)
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 009a222..d63f773 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -2,6 +2,7 @@ ccflags-$(CONFIG_DMADEVICES_DEBUG)  := -DDEBUG
>  ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
>  
>  obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
> +obj-$(CONFIG_DMA_SIMPLE) += dma-simple.o
>  obj-$(CONFIG_NET_DMA) += iovlock.o
>  obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
>  obj-$(CONFIG_DMATEST) += dmatest.o
> diff --git a/drivers/dma/dma-simple.c b/drivers/dma/dma-simple.c
> new file mode 100644
> index 0000000..49d8f7d
> --- /dev/null
> +++ b/drivers/dma/dma-simple.c
> @@ -0,0 +1,873 @@
> +/*
> + * Simple dmaengine driver library
> + *
> + * extracted from shdma.c
> + *
> + * Copyright (C) 2011-2012 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + * Copyright (C) 2009 Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
> + * Copyright (C) 2009 Renesas Solutions, Inc. All rights reserved.
> + * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/dma-simple.h>
> +#include <linux/dmaengine.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +/* DMA descriptor control */
> +enum simple_desc_status {
> +	DESC_IDLE,
> +	DESC_PREPARED,
> +	DESC_SUBMITTED,
> +	DESC_COMPLETED,	/* completed, have to call callback */
> +	DESC_WAITING,	/* callback called, waiting for ack / re-submit */
> +};
why do you need to keep track of descriptor status?
> +
> +#define NR_DESCS_PER_CHANNEL 32
> +
> +#define to_simple_chan(c) container_of(c, struct dma_simple_chan, dma_chan)
> +#define to_simple_dev(d) container_of(d, struct dma_simple_dev, dma_dev)
> +
> +/*
> + * For slave DMA we assume, that there is a finite number of DMA slaves in the
> + * system, and that each such slave can only use a finite number of channels.
> + * We use slave channel IDs to make sure, that no such slave channel ID is
> + * allocated more than once.
> + */
> +static unsigned int slave_num = 256;
> +module_param(slave_num, uint, 0444);
> +
> +/* A bitmask with slave_num bits */
> +static unsigned long *simple_slave_used;
> +
> +/* Called under spin_lock_irq(&schan->chan_lock") */
> +static void simple_chan_xfer_ld_queue(struct dma_simple_chan *schan)
> +{
> +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> +	const struct dma_simple_ops *ops = sdev->ops;
> +	struct dma_simple_desc *sdesc;
> +
> +	/* DMA work check */
> +	if (ops->channel_busy(schan))
> +		return;
> +
> +	/* Find the first not transferred descriptor */
> +	list_for_each_entry(sdesc, &schan->ld_queue, node)
> +		if (sdesc->mark == DESC_SUBMITTED) {
> +			ops->start_xfer(schan, sdesc);
> +			break;
> +		}
> +}
> +
> +static dma_cookie_t simple_tx_submit(struct dma_async_tx_descriptor *tx)
> +{
> +	struct dma_simple_desc *chunk, *c, *desc =
> +		container_of(tx, struct dma_simple_desc, async_tx),
> +		*last = desc;
> +	struct dma_simple_chan *schan = to_simple_chan(tx->chan);
> +	struct dma_simple_slave *slave = tx->chan->private;
prive is masked depricated, os this needs to be removed.
Any slave config should be extracted from dma_lsave_config only... Do
you anything more than which is provided there??

> +	dma_async_tx_callback callback = tx->callback;
> +	dma_cookie_t cookie;
> +	bool power_up;
> +
> +	spin_lock_irq(&schan->chan_lock);
> +
> +	power_up = list_empty(&schan->ld_queue);
> +
> +	cookie = schan->dma_chan.cookie + 1;
> +	if (cookie < 0)
> +		cookie = 1;
> +
> +	schan->dma_chan.cookie = cookie;
> +	tx->cookie = cookie;
> +
> +	/* Mark all chunks of this descriptor as submitted, move to the queue */
> +	list_for_each_entry_safe(chunk, c, desc->node.prev, node) {
> +		/*
> +		 * All chunks are on the global ld_free, so, we have to find
> +		 * the end of the chain ourselves
> +		 */
> +		if (chunk != desc && (chunk->mark == DESC_IDLE ||
> +				      chunk->async_tx.cookie > 0 ||
> +				      chunk->async_tx.cookie == -EBUSY ||
> +				      &chunk->node == &schan->ld_free))
> +			break;
> +		chunk->mark = DESC_SUBMITTED;
> +		/* Callback goes to the last chunk */
> +		chunk->async_tx.callback = NULL;
> +		chunk->cookie = cookie;
> +		list_move_tail(&chunk->node, &schan->ld_queue);
> +		last = chunk;
> +
> +		dev_dbg(schan->dev, "submit #%d@%p on %d\n",
> +			tx->cookie, &last->async_tx, schan->id);
> +	}
> +
> +	last->async_tx.callback = callback;
> +	last->async_tx.callback_param = tx->callback_param;
> +
> +	if (power_up) {
> +		int ret;
> +		schan->pm_state = DMA_SIMPLE_PM_BUSY;
> +
> +		ret = pm_runtime_get(schan->dev);
> +
> +		spin_unlock_irq(&schan->chan_lock);
> +		if (ret < 0)
> +			dev_err(schan->dev, "%s(): GET = %d\n", __func__, ret);
> +
> +		pm_runtime_barrier(schan->dev);
> +
> +		spin_lock_irq(&schan->chan_lock);
> +
> +		/* Have we been reset, while waiting? */
> +		if (schan->pm_state != DMA_SIMPLE_PM_ESTABLISHED) {
> +			struct dma_simple_dev *sdev =
> +				to_simple_dev(schan->dma_chan.device);
> +			const struct dma_simple_ops *ops = sdev->ops;
> +			dev_dbg(schan->dev, "Bring up channel %d\n",
> +				schan->id);
> +			/*
> +			 * TODO: .xfer_setup() might fail on some platforms.
> +			 * Make it int then, on error remove chunks from the
> +			 * queue again
> +			 */
> +			ops->setup_xfer(schan, slave);
> +
> +			if (schan->pm_state == DMA_SIMPLE_PM_PENDING)
> +				simple_chan_xfer_ld_queue(schan);
> +			schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> +		}
> +	} else {
> +		/*
> +		 * Tell .device_issue_pending() not to run the queue, interrupts
> +		 * will do it anyway
> +		 */
> +		schan->pm_state = DMA_SIMPLE_PM_PENDING;
> +	}
> +
> +	spin_unlock_irq(&schan->chan_lock);
> +
> +	return cookie;
> +}
> +
> +/* Called with desc_lock held */
> +static struct dma_simple_desc *simple_get_desc(struct dma_simple_chan *schan)
> +{
> +	struct dma_simple_desc *sdesc;
> +
> +	list_for_each_entry(sdesc, &schan->ld_free, node)
> +		if (sdesc->mark != DESC_PREPARED) {
> +			BUG_ON(sdesc->mark != DESC_IDLE);
> +			list_del(&sdesc->node);
> +			return sdesc;
> +		}
> +
> +	return NULL;
> +}
> +
> +static int simple_alloc_chan_resources(struct dma_chan *chan)
> +{
> +	struct dma_simple_chan *schan = to_simple_chan(chan);
> +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> +	const struct dma_simple_ops *ops = sdev->ops;
> +	struct dma_simple_desc *desc;
> +	struct dma_simple_slave *slave = chan->private;
> +	int ret, i;
> +
> +	/*
> +	 * This relies on the guarantee from dmaengine that alloc_chan_resources
> +	 * never runs concurrently with itself or free_chan_resources.
> +	 */
> +	if (slave) {
> +		if (test_and_set_bit(slave->slave_id, simple_slave_used)) {
> +			ret = -EBUSY;
> +			goto etestused;
> +		}
> +
> +		ret = ops->set_slave(schan, slave);
> +		if (ret < 0)
> +			goto esetslave;
> +	}
> +
> +	schan->desc = kcalloc(NR_DESCS_PER_CHANNEL,
> +			      sdev->desc_size, GFP_KERNEL);
> +	if (!schan->desc) {
> +		ret = -ENOMEM;
> +		goto edescalloc;
> +	}
> +	schan->desc_num = NR_DESCS_PER_CHANNEL;
> +
> +	for (i = 0; i < NR_DESCS_PER_CHANNEL; i++) {
> +		desc = ops->embedded_desc(schan->desc, i);
> +		dma_async_tx_descriptor_init(&desc->async_tx,
> +					     &schan->dma_chan);
> +		desc->async_tx.tx_submit = simple_tx_submit;
> +		desc->mark = DESC_IDLE;
> +
> +		list_add(&desc->node, &schan->ld_free);
> +	}
> +
> +	return NR_DESCS_PER_CHANNEL;
> +
> +edescalloc:
> +	if (slave)
> +esetslave:
> +		clear_bit(slave->slave_id, simple_slave_used);
> +etestused:
> +	chan->private = NULL;
> +	return ret;
> +}
Typically chan allocation involves some kind of hand shaking between the
client and dmac which typically is arch specfic. If we want to make this
a truly independent library, then I think we should move allocation to
driver and let them allocate the required channel. The library by
definition is to _help_ for sg transfers so it should just create a
library of APIs to call which manage the sg transfer when not supported
by dmac.
> +
> +static dma_async_tx_callback __ld_cleanup(struct dma_simple_chan *schan, bool all)
> +{
> +	struct dma_simple_desc *desc, *_desc;
> +	/* Is the "exposed" head of a chain acked? */
> +	bool head_acked = false;
> +	dma_cookie_t cookie = 0;
> +	dma_async_tx_callback callback = NULL;
> +	void *param = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&schan->chan_lock, flags);
> +	list_for_each_entry_safe(desc, _desc, &schan->ld_queue, node) {
> +		struct dma_async_tx_descriptor *tx = &desc->async_tx;
> +
> +		BUG_ON(tx->cookie > 0 && tx->cookie != desc->cookie);
> +		BUG_ON(desc->mark != DESC_SUBMITTED &&
> +		       desc->mark != DESC_COMPLETED &&
> +		       desc->mark != DESC_WAITING);
> +
> +		/*
> +		 * queue is ordered, and we use this loop to (1) clean up all
> +		 * completed descriptors, and to (2) update descriptor flags of
> +		 * any chunks in a (partially) completed chain
> +		 */
> +		if (!all && desc->mark == DESC_SUBMITTED &&
> +		    desc->cookie != cookie)
> +			break;
> +
> +		if (tx->cookie > 0)
> +			cookie = tx->cookie;
> +
> +		if (desc->mark == DESC_COMPLETED && desc->chunks == 1) {
> +			if (schan->completed_cookie != desc->cookie - 1)
> +				dev_dbg(schan->dev,
> +					"Completing cookie %d, expected %d\n",
> +					desc->cookie,
> +					schan->completed_cookie + 1);
> +			schan->completed_cookie = desc->cookie;
> +		}
> +
> +		/* Call callback on the last chunk */
> +		if (desc->mark == DESC_COMPLETED && tx->callback) {
> +			desc->mark = DESC_WAITING;
> +			callback = tx->callback;
> +			param = tx->callback_param;
> +			dev_dbg(schan->dev, "descriptor #%d@%p on %d callback\n",
> +				tx->cookie, tx, schan->id);
> +			BUG_ON(desc->chunks != 1);
> +			break;
> +		}
> +
> +		if (tx->cookie > 0 || tx->cookie == -EBUSY) {
> +			if (desc->mark == DESC_COMPLETED) {
> +				BUG_ON(tx->cookie < 0);
> +				desc->mark = DESC_WAITING;
> +			}
> +			head_acked = async_tx_test_ack(tx);
> +		} else {
> +			switch (desc->mark) {
> +			case DESC_COMPLETED:
> +				desc->mark = DESC_WAITING;
> +				/* Fall through */
> +			case DESC_WAITING:
> +				if (head_acked)
> +					async_tx_ack(&desc->async_tx);
> +			}
> +		}
> +
> +		dev_dbg(schan->dev, "descriptor %p #%d completed.\n",
> +			tx, tx->cookie);
> +
> +		if (((desc->mark == DESC_COMPLETED ||
> +		      desc->mark == DESC_WAITING) &&
> +		     async_tx_test_ack(&desc->async_tx)) || all) {
> +			/* Remove from ld_queue list */
> +			desc->mark = DESC_IDLE;
> +
> +			list_move(&desc->node, &schan->ld_free);
> +
> +			if (list_empty(&schan->ld_queue)) {
> +				dev_dbg(schan->dev, "Bring down channel %d\n", schan->id);
> +				pm_runtime_put(schan->dev);
> +				schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> +			}
> +		}
> +	}
> +
> +	if (all && !callback)
> +		/*
> +		 * Terminating and the loop completed normally: forgive
> +		 * uncompleted cookies
> +		 */
> +		schan->completed_cookie = schan->dma_chan.cookie;
> +
> +	spin_unlock_irqrestore(&schan->chan_lock, flags);
> +
> +	if (callback)
> +		callback(param);
> +
> +	return callback;
> +}
> +
> +/*
> + * simple_chan_ld_cleanup - Clean up link descriptors
> + *
> + * Clean up the ld_queue of DMA channel.
> + */
> +static void simple_chan_ld_cleanup(struct dma_simple_chan *schan, bool all)
> +{
> +	while (__ld_cleanup(schan, all))
> +		;
> +}
> +
> +/*
> + * simple_free_chan_resources - Free all resources of the channel.
> + */
> +static void simple_free_chan_resources(struct dma_chan *chan)
> +{
> +	struct dma_simple_chan *schan = to_simple_chan(chan);
> +	struct dma_simple_dev *sdev = to_simple_dev(chan->device);
> +	const struct dma_simple_ops *ops = sdev->ops;
> +	LIST_HEAD(list);
> +
> +	/* Protect against ISR */
> +	spin_lock_irq(&schan->chan_lock);
> +	ops->halt_channel(schan);
> +	spin_unlock_irq(&schan->chan_lock);
> +
> +	/* Now no new interrupts will occur */
> +
> +	/* Prepared and not submitted descriptors can still be on the queue */
> +	if (!list_empty(&schan->ld_queue))
> +		simple_chan_ld_cleanup(schan, true);
> +
> +	if (chan->private) {
> +		/* The caller is holding dma_list_mutex */
> +		struct dma_simple_slave *slave = chan->private;
> +		clear_bit(slave->slave_id, simple_slave_used);
> +		chan->private = NULL;
> +	}
> +
> +	spin_lock_irq(&schan->chan_lock);
> +
> +	list_splice_init(&schan->ld_free, &list);
> +	schan->desc_num = 0;
> +
> +	spin_unlock_irq(&schan->chan_lock);
> +
> +	kfree(schan->desc);
> +}
> +
> +/**
> + * simple_add_desc - get, set up and return one transfer descriptor
> + * @schan:	DMA channel
> + * @flags:	DMA transfer flags
> + * @dst:	destination DMA address, incremented when direction equals
> + *		DMA_DEV_TO_MEM or DMA_MEM_TO_MEM
> + * @src:	source DMA address, incremented when direction equals
> + *		DMA_MEM_TO_DEV or DMA_MEM_TO_MEM
> + * @len:	DMA transfer length
> + * @first:	if NULL, set to the current descriptor and cookie set to -EBUSY
> + * @direction:	needed for slave DMA to decide which address to keep constant,
> + *		equals DMA_MEM_TO_MEM for MEMCPY
> + * Returns 0 or an error
> + * Locks: called with desc_lock held
> + */
> +static struct dma_simple_desc *simple_add_desc(struct dma_simple_chan *schan,
> +	unsigned long flags, dma_addr_t *dst, dma_addr_t *src, size_t *len,
> +	struct dma_simple_desc **first, enum dma_transfer_direction direction)
> +{
> +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> +	const struct dma_simple_ops *ops = sdev->ops;
> +	struct dma_simple_desc *new;
> +	size_t copy_size = *len;
> +
> +	if (!copy_size)
> +		return NULL;
> +
> +	/* Allocate the link descriptor from the free list */
> +	new = simple_get_desc(schan);
> +	if (!new) {
> +		dev_err(schan->dev, "No free link descriptor available\n");
> +		return NULL;
> +	}
> +
> +	ops->desc_setup(schan, new, *src, *dst, &copy_size);
> +
> +	if (!*first) {
> +		/* First desc */
> +		new->async_tx.cookie = -EBUSY;
> +		*first = new;
> +	} else {
> +		/* Other desc - invisible to the user */
> +		new->async_tx.cookie = -EINVAL;
> +	}
> +
> +	dev_dbg(schan->dev,
> +		"chaining (%u/%u)@%x -> %x with %p, cookie %d\n",
> +		copy_size, *len, *src, *dst, &new->async_tx,
> +		new->async_tx.cookie);
> +
> +	new->mark = DESC_PREPARED;
> +	new->async_tx.flags = flags;
> +	new->direction = direction;
> +
> +	*len -= copy_size;
> +	if (direction == DMA_MEM_TO_MEM || direction == DMA_MEM_TO_DEV)
> +		*src += copy_size;
> +	if (direction == DMA_MEM_TO_MEM || direction == DMA_DEV_TO_MEM)
> +		*dst += copy_size;
> +
> +	return new;
> +}
> +
> +/*
> + * simple_prep_sg - prepare transfer descriptors from an SG list
> + *
> + * Common routine for public (MEMCPY) and slave DMA. The MEMCPY case is also
> + * converted to scatter-gather to guarantee consistent locking and a correct
> + * list manipulation. For slave DMA direction carries the usual meaning, and,
> + * logically, the SG list is RAM and the addr variable contains slave address,
> + * e.g., the FIFO I/O register. For MEMCPY direction equals DMA_MEM_TO_MEM
> + * and the SG list contains only one element and points at the source buffer.
> + */
> +static struct dma_async_tx_descriptor *simple_prep_sg(struct dma_simple_chan *schan,
> +	struct scatterlist *sgl, unsigned int sg_len, dma_addr_t *addr,
> +	enum dma_transfer_direction direction, unsigned long flags)
> +{
> +	struct scatterlist *sg;
> +	struct dma_simple_desc *first = NULL, *new = NULL /* compiler... */;
> +	LIST_HEAD(tx_list);
> +	int chunks = 0;
> +	unsigned long irq_flags;
> +	int i;
> +
> +	for_each_sg(sgl, sg, sg_len, i)
> +		chunks += DIV_ROUND_UP(sg_dma_len(sg), schan->max_xfer_len);
> +
> +	/* Have to lock the whole loop to protect against concurrent release */
> +	spin_lock_irqsave(&schan->chan_lock, irq_flags);
> +
> +	/*
> +	 * Chaining:
> +	 * first descriptor is what user is dealing with in all API calls, its
> +	 *	cookie is at first set to -EBUSY, at tx-submit to a positive
> +	 *	number
> +	 * if more than one chunk is needed further chunks have cookie = -EINVAL
> +	 * the last chunk, if not equal to the first, has cookie = -ENOSPC
> +	 * all chunks are linked onto the tx_list head with their .node heads
> +	 *	only during this function, then they are immediately spliced
> +	 *	back onto the free list in form of a chain
> +	 */
> +	for_each_sg(sgl, sg, sg_len, i) {
> +		dma_addr_t sg_addr = sg_dma_address(sg);
> +		size_t len = sg_dma_len(sg);
> +
> +		if (!len)
> +			goto err_get_desc;
> +
> +		do {
> +			dev_dbg(schan->dev, "Add SG #%d@%p[%d], dma %llx\n",
> +				i, sg, len, (unsigned long long)sg_addr);
> +
> +			if (direction == DMA_DEV_TO_MEM)
> +				new = simple_add_desc(schan, flags,
> +						&sg_addr, addr, &len, &first,
> +						direction);
> +			else
> +				new = simple_add_desc(schan, flags,
> +						addr, &sg_addr, &len, &first,
> +						direction);
> +			if (!new)
> +				goto err_get_desc;
> +
> +			new->chunks = chunks--;
> +			list_add_tail(&new->node, &tx_list);
> +		} while (len);
> +	}
> +
> +	if (new != first)
> +		new->async_tx.cookie = -ENOSPC;
> +
> +	/* Put them back on the free list, so, they don't get lost */
> +	list_splice_tail(&tx_list, &schan->ld_free);
> +
> +	spin_unlock_irqrestore(&schan->chan_lock, irq_flags);
> +
> +	return &first->async_tx;
> +
> +err_get_desc:
> +	list_for_each_entry(new, &tx_list, node)
> +		new->mark = DESC_IDLE;
> +	list_splice(&tx_list, &schan->ld_free);
> +
> +	spin_unlock_irqrestore(&schan->chan_lock, irq_flags);
> +
> +	return NULL;
> +}
> +
> +static struct dma_async_tx_descriptor *simple_prep_memcpy(
> +	struct dma_chan *chan, dma_addr_t dma_dest, dma_addr_t dma_src,
> +	size_t len, unsigned long flags)
> +{
> +	struct dma_simple_chan *schan = to_simple_chan(chan);
> +	struct scatterlist sg;
> +
> +	if (!chan || !len)
> +		return NULL;
> +
> +	BUG_ON(!schan->desc_num);
> +
> +	sg_init_table(&sg, 1);
> +	sg_set_page(&sg, pfn_to_page(PFN_DOWN(dma_src)), len,
> +		    offset_in_page(dma_src));
> +	sg_dma_address(&sg) = dma_src;
> +	sg_dma_len(&sg) = len;
> +
> +	return simple_prep_sg(schan, &sg, 1, &dma_dest, DMA_MEM_TO_MEM, flags);
> +}
mempcy is a single transfer why should this go thru library?
got sg_memcpy yes, but otherwise NO

> +
> +static struct dma_async_tx_descriptor *simple_prep_slave_sg(
> +	struct dma_chan *chan, struct scatterlist *sgl, unsigned int sg_len,
> +	enum dma_transfer_direction direction, unsigned long flags)
> +{
> +	struct dma_simple_chan *schan = to_simple_chan(chan);
> +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> +	const struct dma_simple_ops *ops = sdev->ops;
> +	struct dma_simple_slave *slave = chan->private;
> +	dma_addr_t slave_addr;
> +
> +	if (!chan)
> +		return NULL;
> +
> +	BUG_ON(!schan->desc_num);
> +
> +	/* Someone calling slave DMA on a generic channel? */
> +	if (!slave || !sg_len) {
> +		dev_warn(schan->dev, "%s: bad parameter: %p, %d, %d\n",
> +			 __func__, slave, sg_len, slave ? slave->slave_id : -1);
> +		return NULL;
> +	}
> +
> +	slave_addr = ops->slave_addr(schan);
> +
> +	return simple_prep_sg(schan, sgl, sg_len, &slave_addr,
> +			      direction, flags);
> +}
> +
> +static int simple_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> +			  unsigned long arg)
> +{
> +	struct dma_simple_chan *schan = to_simple_chan(chan);
> +	struct dma_simple_dev *sdev = to_simple_dev(chan->device);
> +	const struct dma_simple_ops *ops = sdev->ops;
> +	unsigned long flags;
> +
> +	/* Only supports DMA_TERMINATE_ALL */
> +	if (cmd != DMA_TERMINATE_ALL)
> +		return -ENXIO;
nope, you should check from respective dmac...

> +
> +	if (!chan)
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&schan->chan_lock, flags);
> +
> +	ops->halt_channel(schan);
> +
> +	spin_unlock_irqrestore(&schan->chan_lock, flags);
> +
> +	simple_chan_ld_cleanup(schan, true);
> +
> +	return 0;
> +}
> +
> +static void simple_issue_pending(struct dma_chan *chan)
> +{
> +	struct dma_simple_chan *schan = to_simple_chan(chan);
> +
> +	spin_lock_irq(&schan->chan_lock);
> +	if (schan->pm_state == DMA_SIMPLE_PM_ESTABLISHED)
> +		simple_chan_xfer_ld_queue(schan);
> +	else
> +		schan->pm_state = DMA_SIMPLE_PM_PENDING;
> +	spin_unlock_irq(&schan->chan_lock);
> +}
> +
> +static enum dma_status simple_tx_status(struct dma_chan *chan,
> +					dma_cookie_t cookie,
> +					struct dma_tx_state *txstate)
> +{
> +	struct dma_simple_chan *schan = to_simple_chan(chan);
> +	dma_cookie_t last_used;
> +	dma_cookie_t last_complete;
> +	enum dma_status status;
> +	unsigned long flags;
> +
> +	simple_chan_ld_cleanup(schan, false);
> +
> +	/* First read completed cookie to avoid a skew */
> +	last_complete = schan->completed_cookie;
> +	rmb();
> +	last_used = chan->cookie;
> +	BUG_ON(last_complete < 0);
> +	dma_set_tx_state(txstate, last_complete, last_used, 0);
> +
> +	spin_lock_irqsave(&schan->chan_lock, flags);
> +
> +	status = dma_async_is_complete(cookie, last_complete, last_used);
> +
> +	/*
> +	 * If we don't find cookie on the queue, it has been aborted and we have
> +	 * to report error
> +	 */
> +	if (status != DMA_SUCCESS) {
> +		struct dma_simple_desc *sdesc;
> +		status = DMA_ERROR;
> +		list_for_each_entry(sdesc, &schan->ld_queue, node)
> +			if (sdesc->cookie == cookie) {
> +				status = DMA_IN_PROGRESS;
> +				break;
> +			}
> +	}
> +
> +	spin_unlock_irqrestore(&schan->chan_lock, flags);
> +
> +	return status;
> +}
> +
> +/* Called from error IRQ or NMI */
> +bool dma_simple_reset(struct dma_simple_dev *sdev)
> +{
> +	const struct dma_simple_ops *ops = sdev->ops;
> +	struct dma_simple_chan *schan;
> +	unsigned int handled = 0;
> +	int i;
> +
> +	/* Reset all channels */
> +	dma_simple_for_each_chan(schan, sdev, i) {
> +		struct dma_simple_desc *sdesc;
> +		LIST_HEAD(dl);
> +
> +		if (!schan)
> +			continue;
> +
> +		spin_lock(&schan->chan_lock);
> +
> +		/* Stop the channel */
> +		ops->halt_channel(schan);
> +
> +		list_splice_init(&schan->ld_queue, &dl);
> +
> +		if (!list_empty(&dl)) {
> +			dev_dbg(schan->dev, "Bring down channel %d\n", schan->id);
> +			pm_runtime_put(schan->dev);
> +		}
> +		schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> +
> +		spin_unlock(&schan->chan_lock);
> +
> +		/* Complete all  */
> +		list_for_each_entry(sdesc, &dl, node) {
> +			struct dma_async_tx_descriptor *tx = &sdesc->async_tx;
> +			sdesc->mark = DESC_IDLE;
> +			if (tx->callback)
> +				tx->callback(tx->callback_param);
> +		}
> +
> +		spin_lock(&schan->chan_lock);
> +		list_splice(&dl, &schan->ld_free);
> +		spin_unlock(&schan->chan_lock);
> +
> +		handled++;
> +	}
> +
> +	return !!handled;
> +}
> +EXPORT_SYMBOL(dma_simple_reset);
> +
> +static irqreturn_t chan_irq(int irq, void *dev)
> +{
> +	struct dma_simple_chan *schan = dev;
> +	const struct dma_simple_ops *ops =
> +		to_simple_dev(schan->dma_chan.device)->ops;
> +	irqreturn_t ret;
> +
> +	spin_lock(&schan->chan_lock);
> +
> +	ret = ops->chan_irq(schan, irq) ? IRQ_WAKE_THREAD : IRQ_NONE;
> +
> +	spin_unlock(&schan->chan_lock);
> +
> +	return ret;
> +}
> +
> +static irqreturn_t chan_irqt(int irq, void *dev)
> +{
> +	struct dma_simple_chan *schan = dev;
> +	const struct dma_simple_ops *ops =
> +		to_simple_dev(schan->dma_chan.device)->ops;
> +	struct dma_simple_desc *sdesc;
> +
> +	spin_lock_irq(&schan->chan_lock);
> +	list_for_each_entry(sdesc, &schan->ld_queue, node) {
> +		if (sdesc->mark == DESC_SUBMITTED &&
> +		    ops->desc_completed(schan, sdesc)) {
> +			dev_dbg(schan->dev, "done #%d@%p\n",
> +				sdesc->async_tx.cookie, &sdesc->async_tx);
> +			sdesc->mark = DESC_COMPLETED;
> +			break;
> +		}
> +	}
> +	/* Next desc */
> +	simple_chan_xfer_ld_queue(schan);
> +	spin_unlock_irq(&schan->chan_lock);
> +
> +	simple_chan_ld_cleanup(schan, false);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +int dma_simple_request_irq(struct dma_simple_chan *schan, int irq,
> +			   unsigned long flags, const char *name)
> +{
> +	int ret = request_threaded_irq(irq, chan_irq, chan_irqt,
> +				       flags, name, schan);
> +
> +	schan->irq = ret < 0 ? ret : irq;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dma_simple_request_irq);
> +
> +void dma_simple_free_irq(struct dma_simple_chan *schan)
> +{
> +	if (schan->irq >= 0)
> +		free_irq(schan->irq, schan);
> +}
> +EXPORT_SYMBOL(dma_simple_free_irq);
why would you use the irq here?? That should be handled by respective
dmac. Also, the library should just setup callbacks for descriptor and
use them to manage the descriptors for sg mode.
> +
> +void dma_simple_chan_probe(struct dma_simple_dev *sdev,
> +			   struct dma_simple_chan *schan, int id)
> +{
> +	schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> +
> +	/* reference struct dma_device */
> +	schan->dma_chan.device = &sdev->dma_dev;
> +
> +	schan->dev = sdev->dma_dev.dev;
> +	schan->id = id;
> +
> +	if (!schan->max_xfer_len)
> +		schan->max_xfer_len = PAGE_SIZE;
> +
> +	spin_lock_init(&schan->chan_lock);
> +
> +	/* Init descripter manage list */
> +	INIT_LIST_HEAD(&schan->ld_queue);
> +	INIT_LIST_HEAD(&schan->ld_free);
> +
> +	/* Add the channel to DMA device channel list */
> +	list_add_tail(&schan->dma_chan.device_node,
> +			&sdev->dma_dev.channels);
> +	sdev->schan[sdev->dma_dev.chancnt++] = schan;
> +}
> +EXPORT_SYMBOL(dma_simple_chan_probe);
> +
> +void dma_simple_chan_remove(struct dma_simple_chan *schan)
> +{
> +	list_del(&schan->dma_chan.device_node);
> +}
> +EXPORT_SYMBOL(dma_simple_chan_remove);
> +
> +int dma_simple_init(struct device *dev, struct dma_simple_dev *sdev,
> +		    int chan_num)
> +{
> +	struct dma_device *dma_dev = &sdev->dma_dev;
> +
> +	/*
> +	 * Require all call-backs for now, they can trivially be made optional
> +	 * later as required
> +	 */
> +	if (!sdev->ops ||
> +	    !sdev->desc_size ||
> +	    !sdev->ops->embedded_desc ||
> +	    !sdev->ops->start_xfer ||
> +	    !sdev->ops->setup_xfer ||
> +	    !sdev->ops->set_slave ||
> +	    !sdev->ops->desc_setup ||
> +	    !sdev->ops->slave_addr ||
> +	    !sdev->ops->channel_busy ||
> +	    !sdev->ops->halt_channel ||
> +	    !sdev->ops->desc_completed)
> +		return -EINVAL;
> +
> +	sdev->schan = kcalloc(chan_num, sizeof(*sdev->schan), GFP_KERNEL);
> +	if (!sdev->schan)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&dma_dev->channels);
> +
> +	/* Common and MEMCPY operations */
> +	dma_dev->device_alloc_chan_resources
> +		= simple_alloc_chan_resources;
> +	dma_dev->device_free_chan_resources = simple_free_chan_resources;
> +	dma_dev->device_prep_dma_memcpy = simple_prep_memcpy;
> +	dma_dev->device_tx_status = simple_tx_status;
> +	dma_dev->device_issue_pending = simple_issue_pending;
> +
> +	/* Compulsory for DMA_SLAVE fields */
> +	dma_dev->device_prep_slave_sg = simple_prep_slave_sg;
> +	dma_dev->device_control = simple_control;
> +
> +	dma_dev->dev = dev;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(dma_simple_init);
> +
> +void dma_simple_cleanup(struct dma_simple_dev *sdev)
> +{
> +	kfree(sdev->schan);
> +}
> +EXPORT_SYMBOL(dma_simple_cleanup);
> +
> +static int __init dma_simple_enter(void)
> +{
> +	simple_slave_used = kzalloc(DIV_ROUND_UP(slave_num, BITS_PER_LONG) *
> +				    sizeof(long), GFP_KERNEL);
> +	if (!simple_slave_used)
> +		return -ENOMEM;
> +	return 0;
> +}
> +module_init(dma_simple_enter);
> +
> +static void __exit dma_simple_exit(void)
> +{
> +	kfree(simple_slave_used);
> +}
> +module_exit(dma_simple_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Simple dmaengine driver library");
> +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>");
> diff --git a/include/linux/dma-simple.h b/include/linux/dma-simple.h
> new file mode 100644
> index 0000000..5336674
> --- /dev/null
> +++ b/include/linux/dma-simple.h
> @@ -0,0 +1,124 @@
> +/*
> + * Simple dmaengine driver library
> + *
> + * extracted from shdma.c and headers
> + *
> + * Copyright (C) 2011-2012 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + * Copyright (C) 2009 Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
> + * Copyright (C) 2009 Renesas Solutions, Inc. All rights reserved.
> + * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef DMA_SIMPLE_H
> +#define DMA_SIMPLE_H
> +
> +#include <linux/dmaengine.h>
> +#include <linux/interrupt.h>
> +#include <linux/list.h>
> +#include <linux/types.h>
> +
> +/**
> + * dma_simple_pm_state - DMA channel PM state
> + * DMA_SIMPLE_PM_ESTABLISHED:	either idle or during data transfer
> + * DMA_SIMPLE_PM_BUSY:		during the transfer preparation, when we have to
> + *				drop the lock temporarily
> + * DMA_SIMPLE_PM_PENDING:	transfers pending
> + */
> +enum dma_simple_pm_state {
> +	DMA_SIMPLE_PM_ESTABLISHED,
> +	DMA_SIMPLE_PM_BUSY,
> +	DMA_SIMPLE_PM_PENDING,
> +};
> +
> +struct device;
> +
> +/*
> + * Drivers, using this library are expected to embed struct dma_simple_dev,
> + * struct dma_simple_chan, struct dma_simple_desc, and struct dma_simple_slave
> + * in their respective device, channel, descriptor and slave objects.
> + */
> +
> +struct dma_simple_slave {
> +	unsigned int slave_id;
> +};
> +
> +struct dma_simple_desc {
> +	struct list_head node;
> +	struct dma_async_tx_descriptor async_tx;
> +	enum dma_transfer_direction direction;
> +	dma_cookie_t cookie;
> +	int chunks;
> +	int mark;
> +};
> +
> +struct dma_simple_chan {
> +	dma_cookie_t completed_cookie;	/* The maximum cookie completed */
> +	spinlock_t chan_lock;		/* Channel operation lock */
> +	struct list_head ld_queue;	/* Link descriptors queue */
> +	struct list_head ld_free;	/* Free link descriptors */
> +	struct dma_chan dma_chan;	/* DMA channel */
> +	struct device *dev;		/* Channel device */
> +	void *desc;			/* buffer for descriptor array */
> +	int desc_num;			/* desc count */
> +	size_t max_xfer_len;		/* max transfer length */
> +	int id;				/* Raw id of this channel */
> +	int irq;			/* Channel IRQ */
> +	enum dma_simple_pm_state pm_state;
> +};
> +
> +/**
> + * struct dma_simple_ops - simple DMA driver operations
> + * desc_completed:	return true, if this is the descriptor, that just has
> + *			completed (atomic)
> + * halt_channel:	stop DMA channel operation (atomic)
> + * channel_busy:	return true, if the channel is busy (atomic)
> + * slave_addr:		return slave DMA address
> + * desc_setup:		set up the hardware specific descriptor portion (atomic)
> + * set_slave:		bind channel to a slave
> + * setup_xfer:		configure channel hardware for operation (atomic)
> + * start_xfer:		start the DMA transfer (atomic)
> + * embedded_desc:	return Nth struct dma_simple_desc pointer from the
> + *			descriptor array
> + * chan_irq:		process channel IRQ, return true if a transfer has
> + *			completed (atomic)
> + */
> +struct dma_simple_ops {
> +	bool (*desc_completed)(struct dma_simple_chan *, struct dma_simple_desc *);
> +	void (*halt_channel)(struct dma_simple_chan *);
> +	bool (*channel_busy)(struct dma_simple_chan *);
> +	dma_addr_t (*slave_addr)(struct dma_simple_chan *);
> +	int (*desc_setup)(struct dma_simple_chan *, struct dma_simple_desc *,
> +			  dma_addr_t, dma_addr_t, size_t *);
> +	int (*set_slave)(struct dma_simple_chan *, struct dma_simple_slave *);
> +	void (*setup_xfer)(struct dma_simple_chan *, struct dma_simple_slave *);
> +	void (*start_xfer)(struct dma_simple_chan *, struct dma_simple_desc *);
> +	struct dma_simple_desc *(*embedded_desc)(void *, int);
> +	bool (*chan_irq)(struct dma_simple_chan *, int);
> +};
again so many callbacks... are they really required!!
> +
> +struct dma_simple_dev {
> +	struct dma_device dma_dev;
> +	struct dma_simple_chan **schan;
> +	const struct dma_simple_ops *ops;
> +	size_t desc_size;
> +};
> +
> +#define dma_simple_for_each_chan(c, d, i) for (i = 0, c = (d)->schan[0]; \
> +				i < (d)->dma_dev.chancnt; c = (d)->schan[++i])
> +
> +int dma_simple_request_irq(struct dma_simple_chan *, int,
> +			   unsigned long, const char *);
> +void dma_simple_free_irq(struct dma_simple_chan *);
> +bool dma_simple_reset(struct dma_simple_dev *sdev);
> +void dma_simple_chan_probe(struct dma_simple_dev *sdev,
> +			   struct dma_simple_chan *schan, int id);
> +void dma_simple_chan_remove(struct dma_simple_chan *schan);
> +int dma_simple_init(struct device *dev, struct dma_simple_dev *sdev,
> +		    int chan_num);
> +void dma_simple_cleanup(struct dma_simple_dev *sdev);
> +
> +#endif

Now I am confused on the intent of this library. It was proposed for
helping dmacs like sh-mobile to support sg transfers in software which
are not supported by hardware, but it seems this library is doing _much_
more.
IMHO, it should get inserted between dmaengine APIs and client driver,
_only_ for sg transfers. The channel allocation etc belong to dmac.
Further, the library should get notified by dmac based on the callbacks
set for descriptor and then should call native driver while submitting
the next one in queue...
Rest of the stuff (if required) would not be generic and probably should
be in arch specific directory.


-- 
~Vinod


^ permalink raw reply

* Re: [PATCH/RFC] usb: fix renesas_usbhs to not schedule in atomic context
From: Shimoda, Yoshihiro @ 2012-02-06  8:52 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: alsa-devel, linux-sh, Vinod Koul, Magnus Damm, linux-usb,
	linux-mmc, linux-kernel, Paul Mundt, linux-serial
In-Reply-To: <Pine.LNX.4.64.1202031623000.6211@axis700.grange>

Hi Guennadi-san,

2012/02/04 0:43, Guennadi Liakhovetski wrote:
> The current renesas_usbhs driver triggers
> 
> BUG: scheduling while atomic: ksoftirqd/0/3/0x00000102
> 
> with enabled CONFIG_DEBUG_ATOMIC_SLEEP, by submitting DMA transfers from 
> an atomic (tasklet) context, which is not supported by the shdma dmaengine 
> driver. Fix it by switching to a work. Also simplify some list 
> manipulations.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> Shimoda-san, this is the problem, that you were observing. However, it 
> exists with the present version of shdma just as well as with the new one 
> - on top of the simple DMA library. I marked it an RFC because (1) I only 
> lightly tested it with the gadget device on mackerel with the mass storage 
> gadget, and (2) I am somewhat concerned about races. Currently the work 
> function runs with no locking and there are no usual cancel_work_sync() 
> points in the patch. However, it has also been like this before with the 
> tasklet implementation, which is not much better, and it looks like there 
> are no asynchronous operations on the same packets like timeouts. Only 
> asynchronous events, that I can think about are things like unloading the 
> driver or unplugging the cable, but these have been there before too. It 
> would become worse on SMP, I think. Comments welcome.

Thank you for the patch and comments.
I also tested this patch using the g_mass_storage and g_ether.
About your points, I think so, too.
So, I will write such a code.

Best regards,
Yoshihiro Shimoda

^ permalink raw reply

* Re: [PATCH 6/7 v2] ASoC: SIU: prepare for conversion to simple DMA
From: Guennadi Liakhovetski @ 2012-02-06  9:11 UTC (permalink / raw)
  To: Vinod Koul
  Cc: linux-kernel, linux-sh, Magnus Damm, Yoshihiro Shimoda, linux-mmc,
	alsa-devel, linux-serial, Paul Mundt
In-Reply-To: <1328495625.26182.55.camel@vkoul-udesk3>

Hi Vinod

On Mon, 6 Feb 2012, Vinod Koul wrote:

> On Thu, 2012-01-26 at 15:56 +0100, Guennadi Liakhovetski wrote:
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  sound/soc/sh/siu_pcm.c |    8 ++++----
> >  1 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/sound/soc/sh/siu_pcm.c b/sound/soc/sh/siu_pcm.c
> > index 0193e59..414bb35 100644
> > --- a/sound/soc/sh/siu_pcm.c
> > +++ b/sound/soc/sh/siu_pcm.c
> > @@ -330,12 +330,12 @@ static bool filter(struct dma_chan *chan, void *slave)
> >  {
> >  	struct sh_dmae_slave *param = slave;
> >  
> > -	pr_debug("%s: slave ID %d\n", __func__, param->slave_id);
> > +	pr_debug("%s: slave ID %d\n", __func__, param->simple_slave.slave_id);
> >  
> >  	if (unlikely(param->dma_dev != chan->device->dev))
> >  		return false;
> >  
> > -	chan->private = param;
> > +	chan->private = &param->simple_slave;
> While at it, Can you fix the sh-mobile driver to NOT use chan->private?

This is an independent change and for it I need my recent RFC PATCH 
"[PATCH/RFC] dmaengine: add a slave parameter to __dma_request_channel()"

http://marc.info/?l=linux-kernel&m=132811019930675&w=2

Thanks
Guennadi

> >  	return true;
> >  }
> >  
> > @@ -360,12 +360,12 @@ static int siu_pcm_open(struct snd_pcm_substream *ss)
> >  	if (ss->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> >  		siu_stream = &port_info->playback;
> >  		param = &siu_stream->param;
> > -		param->slave_id = port ? pdata->dma_slave_tx_b :
> > +		param->simple_slave.slave_id = port ? pdata->dma_slave_tx_b :
> >  			pdata->dma_slave_tx_a;
> >  	} else {
> >  		siu_stream = &port_info->capture;
> >  		param = &siu_stream->param;
> > -		param->slave_id = port ? pdata->dma_slave_rx_b :
> > +		param->simple_slave.slave_id = port ? pdata->dma_slave_rx_b :
> >  			pdata->dma_slave_rx_a;
> >  	}
> >  
> 
> 
> -- 
> ~Vinod
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* [PATCH 1/3] serial: pch_uart: add debugfs hook for register dump
From: Feng Tang @ 2012-02-06  9:24 UTC (permalink / raw)
  To: linux-serial, gregkh, alan, tomoya.rohm; +Cc: Feng Tang

This driver will be use as interfaces for multiple kinds of
devices like Bluetooth/GPS etc, this debug hook will make driver
debugging much easier.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 drivers/tty/serial/pch_uart.c |   84 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 17ae657..6d9ca29 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -29,6 +29,7 @@
 #include <linux/nmi.h>
 #include <linux/delay.h>
 
+#include <linux/debugfs.h>
 #include <linux/dmaengine.h>
 #include <linux/pch_dma.h>
 
@@ -144,6 +145,8 @@ enum {
 #define PCH_UART_DLL		0x00
 #define PCH_UART_DLM		0x01
 
+#define PCH_UART_BRCSR		0x0E
+
 #define PCH_UART_IID_RLS	(PCH_UART_IIR_REI)
 #define PCH_UART_IID_RDR	(PCH_UART_IIR_RRI)
 #define PCH_UART_IID_RDR_TO	(PCH_UART_IIR_RRI | PCH_UART_IIR_TOI)
@@ -243,6 +246,8 @@ struct eg20t_port {
 	int				tx_dma_use;
 	void				*rx_buf_virt;
 	dma_addr_t			rx_buf_dma;
+
+	struct dentry	*debugfs;
 };
 
 /**
@@ -292,6 +297,73 @@ static const int trigger_level_64[4] = { 1, 16, 32, 56 };
 static const int trigger_level_16[4] = { 1, 4, 8, 14 };
 static const int trigger_level_1[4] = { 1, 1, 1, 1 };
 
+#ifdef CONFIG_DEBUG_FS
+
+#define PCH_REGS_BUFSIZE	1024
+static int pch_show_regs_open(struct inode *inode, struct file *file)
+{
+	file->private_data = inode->i_private;
+	return 0;
+}
+
+static ssize_t port_show_regs(struct file *file, char __user *user_buf,
+				size_t count, loff_t *ppos)
+{
+	struct eg20t_port *priv = file->private_data;
+	char *buf;
+	u32 len = 0;
+	ssize_t ret;
+	unsigned char lcr;
+
+	buf = kzalloc(PCH_REGS_BUFSIZE, GFP_KERNEL);
+	if (!buf)
+		return 0;
+
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"PCH EG20T port[%d] regs:\n", priv->port.line);
+
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"=================================\n");
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"IER: \t0x%02x\n", ioread8(priv->membase + UART_IER));
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"IIR: \t0x%02x\n", ioread8(priv->membase + UART_IIR));
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"LCR: \t0x%02x\n", ioread8(priv->membase + UART_LCR));
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"MCR: \t0x%02x\n", ioread8(priv->membase + UART_MCR));
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"LSR: \t0x%02x\n", ioread8(priv->membase + UART_LSR));
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"MSR: \t0x%02x\n", ioread8(priv->membase + UART_MSR));
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"BRCSR: \t0x%02x\n",
+			ioread8(priv->membase + PCH_UART_BRCSR));
+
+	lcr = ioread8(priv->membase + UART_LCR);
+	iowrite8(PCH_UART_LCR_DLAB, priv->membase + UART_LCR);
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"DLL: \t0x%02x\n", ioread8(priv->membase + UART_DLL));
+	len += snprintf(buf + len, PCH_REGS_BUFSIZE - len,
+			"DLM: \t0x%02x\n", ioread8(priv->membase + UART_DLM));
+	iowrite8(lcr, priv->membase + UART_LCR);
+
+	if (len > PCH_REGS_BUFSIZE)
+		len = PCH_REGS_BUFSIZE;
+
+	ret =  simple_read_from_buffer(user_buf, count, ppos, buf, len);
+	kfree(buf);
+	return ret;
+}
+
+static const struct file_operations port_regs_ops = {
+	.owner		= THIS_MODULE,
+	.open		= pch_show_regs_open,
+	.read		= port_show_regs,
+	.llseek		= default_llseek,
+};
+#endif	/* CONFIG_DEBUG_FS */
+
 static void pch_uart_hal_request(struct pci_dev *pdev, int fifosize,
 				 int base_baud)
 {
@@ -1554,6 +1626,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	int port_type;
 	struct pch_uart_driver_data *board;
 	const char *board_name;
+	char name[32];	/* for debugfs file name */
 
 	board = &drv_dat[id->driver_data];
 	port_type = board->port_type;
@@ -1623,6 +1696,12 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	if (ret < 0)
 		goto init_port_hal_free;
 
+#ifdef CONFIG_DEBUG_FS
+	snprintf(name, sizeof(name), "uart%d_regs", board->line_no);
+	priv->debugfs = debugfs_create_file(name, S_IFREG | S_IRUGO,
+				NULL, priv, &port_regs_ops);
+#endif
+
 	return priv;
 
 init_port_hal_free:
@@ -1639,6 +1718,11 @@ init_port_alloc_err:
 
 static void pch_uart_exit_port(struct eg20t_port *priv)
 {
+
+#ifdef CONFIG_DEBUG_FS
+	if (priv->debugfs)
+		debugfs_remove(priv->debugfs);
+#endif
 	uart_remove_one_port(&pch_uart_driver, &priv->port);
 	pci_set_drvdata(priv->pdev, NULL);
 	free_page((unsigned long)priv->rxbuf.buf);
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/3] serial: pch_uart: trivial cleanup by removing the get_msr()
From: Feng Tang @ 2012-02-06  9:24 UTC (permalink / raw)
  To: linux-serial, gregkh, alan, tomoya.rohm; +Cc: Feng Tang
In-Reply-To: <1328520285-10826-1-git-send-email-feng.tang@intel.com>

The short get_msr() has some unnecessary code and only used once,
so merge it with its caller to make code cleaner. No functional
change at all.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 drivers/tty/serial/pch_uart.c |   29 ++++++++---------------------
 1 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 6d9ca29..811edcb 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -373,14 +373,6 @@ static void pch_uart_hal_request(struct pci_dev *pdev, int fifosize,
 	priv->fcr = 0;
 }
 
-static unsigned int get_msr(struct eg20t_port *priv, void __iomem *base)
-{
-	unsigned int msr = ioread8(base + UART_MSR);
-	priv->dmsr |= msr & PCH_UART_MSR_DELTA;
-
-	return msr;
-}
-
 static void pch_uart_hal_enable_interrupt(struct eg20t_port *priv,
 					  unsigned int flag)
 {
@@ -514,8 +506,9 @@ static int pch_uart_hal_set_fifo(struct eg20t_port *priv,
 
 static u8 pch_uart_hal_get_modem(struct eg20t_port *priv)
 {
-	priv->dmsr = 0;
-	return get_msr(priv, priv->membase);
+	unsigned int msr = ioread8(priv->membase + UART_MSR);
+	priv->dmsr = msr & PCH_UART_MSR_DELTA;
+	return (u8)msr;
 }
 
 static void pch_uart_hal_write(struct eg20t_port *priv,
@@ -596,7 +589,7 @@ static int push_rx(struct eg20t_port *priv, const unsigned char *buf,
 
 static int pop_tx_x(struct eg20t_port *priv, unsigned char *buf)
 {
-	int ret;
+	int ret = 0;
 	struct uart_port *port = &priv->port;
 
 	if (port->x_char) {
@@ -605,8 +598,6 @@ static int pop_tx_x(struct eg20t_port *priv, unsigned char *buf)
 		buf[0] = port->x_char;
 		port->x_char = 0;
 		ret = 1;
-	} else {
-		ret = 0;
 	}
 
 	return ret;
@@ -1104,14 +1095,12 @@ static irqreturn_t pch_uart_interrupt(int irq, void *dev_id)
 static unsigned int pch_uart_tx_empty(struct uart_port *port)
 {
 	struct eg20t_port *priv;
-	int ret;
+
 	priv = container_of(port, struct eg20t_port, port);
 	if (priv->tx_empty)
-		ret = TIOCSER_TEMT;
+		return TIOCSER_TEMT;
 	else
-		ret = 0;
-
-	return ret;
+		return 0;
 }
 
 /* Returns the current state of modem control inputs. */
@@ -1345,9 +1334,8 @@ static void pch_uart_set_termios(struct uart_port *port,
 		else
 			parity = PCH_UART_HAL_PARITY_EVEN;
 
-	} else {
+	} else
 		parity = PCH_UART_HAL_PARITY_NONE;
-	}
 
 	/* Only UART0 has auto hardware flow function */
 	if ((termios->c_cflag & CRTSCTS) && (priv->fifo_size == 256))
@@ -1519,7 +1507,6 @@ static void
 pch_console_write(struct console *co, const char *s, unsigned int count)
 {
 	struct eg20t_port *priv;
-
 	unsigned long flags;
 	u8 ier;
 	int locked = 1;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 3/3] serial: pch_uart: trivail cleanup by removing the pch_uart_hal_request()
From: Feng Tang @ 2012-02-06  9:24 UTC (permalink / raw)
  To: linux-serial, gregkh, alan, tomoya.rohm; +Cc: Feng Tang
In-Reply-To: <1328520285-10826-1-git-send-email-feng.tang@intel.com>

pch_uart_hal_request() has parameters which it never uses, also
it is very short, so merge it with its caller to make code cleaner.
No functional changes at all.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 drivers/tty/serial/pch_uart.c |   16 +++-------------
 1 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 811edcb..aa4d07b 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -364,15 +364,6 @@ static const struct file_operations port_regs_ops = {
 };
 #endif	/* CONFIG_DEBUG_FS */
 
-static void pch_uart_hal_request(struct pci_dev *pdev, int fifosize,
-				 int base_baud)
-{
-	struct eg20t_port *priv = pci_get_drvdata(pdev);
-
-	priv->trigger_level = 1;
-	priv->fcr = 0;
-}
-
 static void pch_uart_hal_enable_interrupt(struct eg20t_port *priv,
 					  unsigned int flag)
 {
@@ -1674,7 +1665,8 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	spin_lock_init(&priv->port.lock);
 
 	pci_set_drvdata(pdev, priv);
-	pch_uart_hal_request(pdev, fifosize, base_baud);
+	priv->trigger_level = 1;
+	priv->fcr = 0;
 
 #ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
 	pch_uart_ports[board->line_no] = priv;
@@ -1717,9 +1709,7 @@ static void pch_uart_exit_port(struct eg20t_port *priv)
 
 static void pch_uart_pci_remove(struct pci_dev *pdev)
 {
-	struct eg20t_port *priv;
-
-	priv = (struct eg20t_port *)pci_get_drvdata(pdev);
+	struct eg20t_port *priv = pci_get_drvdata(pdev);
 
 	pci_disable_msi(pdev);
 
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 1/7 v2] dmaengine: add a simple dma library
From: Guennadi Liakhovetski @ 2012-02-06  9:53 UTC (permalink / raw)
  To: Vinod Koul
  Cc: alsa-devel, linux-sh, Magnus Damm, Yoshihiro Shimoda, linux-mmc,
	linux-kernel, Paul Mundt, linux-serial
In-Reply-To: <1328496896.26182.76.camel@vkoul-udesk3>

On Mon, 6 Feb 2012, Vinod Koul wrote:

> On Thu, 2012-01-26 at 15:56 +0100, Guennadi Liakhovetski wrote:
> > This patch adds a library of functions, helping to implement dmaengine
> > drivers for hardware, unable to handle scatter-gather lists natively.
> > The first version of this driver only supports memcpy and slave DMA
> > operation.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > 
> > v2:
> > 
> > 1. switch from using a tasklet to threaded IRQ, which allowed to
> ...?

Sorry, what exactly is your question here? The unfinished sentence? It is 
finished below in item 2, so, it should read like "...allowed to remove 
lock..."

> > 2. remove lock / unlock inline functions
> > 3. remove __devinit, __devexit annotations
> > 
> >  drivers/dma/Kconfig        |    3 +
> >  drivers/dma/Makefile       |    1 +
> >  drivers/dma/dma-simple.c   |  873 ++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/dma-simple.h |  124 +++++++
> >  4 files changed, 1001 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/dma/dma-simple.c
> >  create mode 100644 include/linux/dma-simple.h
> > 
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index f1a2749..f7c583e 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -149,6 +149,9 @@ config TXX9_DMAC
> >  	  Support the TXx9 SoC internal DMA controller.  This can be
> >  	  integrated in chips such as the Toshiba TX4927/38/39.
> >  
> > +config DMA_SIMPLE
> > +	tristate
> > +
> >  config SH_DMAE
> >  	tristate "Renesas SuperH DMAC support"
> >  	depends on (SUPERH && SH_DMA) || (ARM && ARCH_SHMOBILE)
> > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > index 009a222..d63f773 100644
> > --- a/drivers/dma/Makefile
> > +++ b/drivers/dma/Makefile
> > @@ -2,6 +2,7 @@ ccflags-$(CONFIG_DMADEVICES_DEBUG)  := -DDEBUG
> >  ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
> >  
> >  obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
> > +obj-$(CONFIG_DMA_SIMPLE) += dma-simple.o
> >  obj-$(CONFIG_NET_DMA) += iovlock.o
> >  obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
> >  obj-$(CONFIG_DMATEST) += dmatest.o
> > diff --git a/drivers/dma/dma-simple.c b/drivers/dma/dma-simple.c
> > new file mode 100644
> > index 0000000..49d8f7d
> > --- /dev/null
> > +++ b/drivers/dma/dma-simple.c
> > @@ -0,0 +1,873 @@
> > +/*
> > + * Simple dmaengine driver library
> > + *
> > + * extracted from shdma.c
> > + *
> > + * Copyright (C) 2011-2012 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > + * Copyright (C) 2009 Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
> > + * Copyright (C) 2009 Renesas Solutions, Inc. All rights reserved.
> > + * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved.
> > + *
> > + * This is free software; you can redistribute it and/or modify
> > + * it under the terms of version 2 of the GNU General Public License as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/dma-simple.h>
> > +#include <linux/dmaengine.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +
> > +/* DMA descriptor control */
> > +enum simple_desc_status {
> > +	DESC_IDLE,
> > +	DESC_PREPARED,
> > +	DESC_SUBMITTED,
> > +	DESC_COMPLETED,	/* completed, have to call callback */
> > +	DESC_WAITING,	/* callback called, waiting for ack / re-submit */
> > +};
> why do you need to keep track of descriptor status?

Because descriptors in different states can be present on the same queue 
and you have to differentiate between them, when traversing the list.

> > +
> > +#define NR_DESCS_PER_CHANNEL 32
> > +
> > +#define to_simple_chan(c) container_of(c, struct dma_simple_chan, dma_chan)
> > +#define to_simple_dev(d) container_of(d, struct dma_simple_dev, dma_dev)
> > +
> > +/*
> > + * For slave DMA we assume, that there is a finite number of DMA slaves in the
> > + * system, and that each such slave can only use a finite number of channels.
> > + * We use slave channel IDs to make sure, that no such slave channel ID is
> > + * allocated more than once.
> > + */
> > +static unsigned int slave_num = 256;
> > +module_param(slave_num, uint, 0444);
> > +
> > +/* A bitmask with slave_num bits */
> > +static unsigned long *simple_slave_used;
> > +
> > +/* Called under spin_lock_irq(&schan->chan_lock") */
> > +static void simple_chan_xfer_ld_queue(struct dma_simple_chan *schan)
> > +{
> > +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> > +	const struct dma_simple_ops *ops = sdev->ops;
> > +	struct dma_simple_desc *sdesc;
> > +
> > +	/* DMA work check */
> > +	if (ops->channel_busy(schan))
> > +		return;
> > +
> > +	/* Find the first not transferred descriptor */
> > +	list_for_each_entry(sdesc, &schan->ld_queue, node)
> > +		if (sdesc->mark == DESC_SUBMITTED) {
> > +			ops->start_xfer(schan, sdesc);
> > +			break;
> > +		}
> > +}
> > +
> > +static dma_cookie_t simple_tx_submit(struct dma_async_tx_descriptor *tx)
> > +{
> > +	struct dma_simple_desc *chunk, *c, *desc =
> > +		container_of(tx, struct dma_simple_desc, async_tx),
> > +		*last = desc;
> > +	struct dma_simple_chan *schan = to_simple_chan(tx->chan);
> > +	struct dma_simple_slave *slave = tx->chan->private;
> prive is masked depricated, os this needs to be removed.

Right, it would be best to first merge patch "[PATCH/RFC] dmaengine: add a 
slave parameter to __dma_request_channel()" and then port this library on 
top of it, then private will not be used any more.

> Any slave config should be extracted from dma_lsave_config only... Do
> you anything more than which is provided there??

I don't think the dmaengine_slave_config() API is very well suitable for 
our situation. The problem is, that on sh-mobile not all DMA controllers 
support all functions. E.g., sh7372 has two dedicated USB DMA controllers, 
that otherwise are fully compatible with other DMA controllers on that 
platforms. If a client requests a channel for a USB slave and gets back a 
channel on one of other DMAC instances, issuing dmaengine_slave_config() 
with a USB configuration, obviously, will not work. Similarly, if a client 
would try to allocate a non-USB channel on a USB controller. So, it is 
best to be able to decide at dma_request_channel() time, whether and from 
which controller this slave channel request can be satisfied.

> > +	dma_async_tx_callback callback = tx->callback;
> > +	dma_cookie_t cookie;
> > +	bool power_up;
> > +
> > +	spin_lock_irq(&schan->chan_lock);
> > +
> > +	power_up = list_empty(&schan->ld_queue);
> > +
> > +	cookie = schan->dma_chan.cookie + 1;
> > +	if (cookie < 0)
> > +		cookie = 1;
> > +
> > +	schan->dma_chan.cookie = cookie;
> > +	tx->cookie = cookie;
> > +
> > +	/* Mark all chunks of this descriptor as submitted, move to the queue */
> > +	list_for_each_entry_safe(chunk, c, desc->node.prev, node) {
> > +		/*
> > +		 * All chunks are on the global ld_free, so, we have to find
> > +		 * the end of the chain ourselves
> > +		 */
> > +		if (chunk != desc && (chunk->mark == DESC_IDLE ||
> > +				      chunk->async_tx.cookie > 0 ||
> > +				      chunk->async_tx.cookie == -EBUSY ||
> > +				      &chunk->node == &schan->ld_free))
> > +			break;
> > +		chunk->mark = DESC_SUBMITTED;
> > +		/* Callback goes to the last chunk */
> > +		chunk->async_tx.callback = NULL;
> > +		chunk->cookie = cookie;
> > +		list_move_tail(&chunk->node, &schan->ld_queue);
> > +		last = chunk;
> > +
> > +		dev_dbg(schan->dev, "submit #%d@%p on %d\n",
> > +			tx->cookie, &last->async_tx, schan->id);
> > +	}
> > +
> > +	last->async_tx.callback = callback;
> > +	last->async_tx.callback_param = tx->callback_param;
> > +
> > +	if (power_up) {
> > +		int ret;
> > +		schan->pm_state = DMA_SIMPLE_PM_BUSY;
> > +
> > +		ret = pm_runtime_get(schan->dev);
> > +
> > +		spin_unlock_irq(&schan->chan_lock);
> > +		if (ret < 0)
> > +			dev_err(schan->dev, "%s(): GET = %d\n", __func__, ret);
> > +
> > +		pm_runtime_barrier(schan->dev);
> > +
> > +		spin_lock_irq(&schan->chan_lock);
> > +
> > +		/* Have we been reset, while waiting? */
> > +		if (schan->pm_state != DMA_SIMPLE_PM_ESTABLISHED) {
> > +			struct dma_simple_dev *sdev =
> > +				to_simple_dev(schan->dma_chan.device);
> > +			const struct dma_simple_ops *ops = sdev->ops;
> > +			dev_dbg(schan->dev, "Bring up channel %d\n",
> > +				schan->id);
> > +			/*
> > +			 * TODO: .xfer_setup() might fail on some platforms.
> > +			 * Make it int then, on error remove chunks from the
> > +			 * queue again
> > +			 */
> > +			ops->setup_xfer(schan, slave);
> > +
> > +			if (schan->pm_state == DMA_SIMPLE_PM_PENDING)
> > +				simple_chan_xfer_ld_queue(schan);
> > +			schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> > +		}
> > +	} else {
> > +		/*
> > +		 * Tell .device_issue_pending() not to run the queue, interrupts
> > +		 * will do it anyway
> > +		 */
> > +		schan->pm_state = DMA_SIMPLE_PM_PENDING;
> > +	}
> > +
> > +	spin_unlock_irq(&schan->chan_lock);
> > +
> > +	return cookie;
> > +}
> > +
> > +/* Called with desc_lock held */
> > +static struct dma_simple_desc *simple_get_desc(struct dma_simple_chan *schan)
> > +{
> > +	struct dma_simple_desc *sdesc;
> > +
> > +	list_for_each_entry(sdesc, &schan->ld_free, node)
> > +		if (sdesc->mark != DESC_PREPARED) {
> > +			BUG_ON(sdesc->mark != DESC_IDLE);
> > +			list_del(&sdesc->node);
> > +			return sdesc;
> > +		}
> > +
> > +	return NULL;
> > +}
> > +
> > +static int simple_alloc_chan_resources(struct dma_chan *chan)
> > +{
> > +	struct dma_simple_chan *schan = to_simple_chan(chan);
> > +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> > +	const struct dma_simple_ops *ops = sdev->ops;
> > +	struct dma_simple_desc *desc;
> > +	struct dma_simple_slave *slave = chan->private;
> > +	int ret, i;
> > +
> > +	/*
> > +	 * This relies on the guarantee from dmaengine that alloc_chan_resources
> > +	 * never runs concurrently with itself or free_chan_resources.
> > +	 */
> > +	if (slave) {
> > +		if (test_and_set_bit(slave->slave_id, simple_slave_used)) {
> > +			ret = -EBUSY;
> > +			goto etestused;
> > +		}
> > +
> > +		ret = ops->set_slave(schan, slave);
> > +		if (ret < 0)
> > +			goto esetslave;
> > +	}
> > +
> > +	schan->desc = kcalloc(NR_DESCS_PER_CHANNEL,
> > +			      sdev->desc_size, GFP_KERNEL);
> > +	if (!schan->desc) {
> > +		ret = -ENOMEM;
> > +		goto edescalloc;
> > +	}
> > +	schan->desc_num = NR_DESCS_PER_CHANNEL;
> > +
> > +	for (i = 0; i < NR_DESCS_PER_CHANNEL; i++) {
> > +		desc = ops->embedded_desc(schan->desc, i);
> > +		dma_async_tx_descriptor_init(&desc->async_tx,
> > +					     &schan->dma_chan);
> > +		desc->async_tx.tx_submit = simple_tx_submit;
> > +		desc->mark = DESC_IDLE;
> > +
> > +		list_add(&desc->node, &schan->ld_free);
> > +	}
> > +
> > +	return NR_DESCS_PER_CHANNEL;
> > +
> > +edescalloc:
> > +	if (slave)
> > +esetslave:
> > +		clear_bit(slave->slave_id, simple_slave_used);
> > +etestused:
> > +	chan->private = NULL;
> > +	return ret;
> > +}
> Typically chan allocation involves some kind of hand shaking between the
> client and dmac which typically is arch specfic. If we want to make this
> a truly independent library, then I think we should move allocation to
> driver and let them allocate the required channel. The library by
> definition is to _help_ for sg transfers so it should just create a
> library of APIs to call which manage the sg transfer when not supported
> by dmac.

Again, that's also something, that should be handled by the proposed 
patch. With it any additional information, required to configure the 
controller and / or the channel for the slave operation is passed already 
to the allocation routine. Then, hopefully, no additional handshaking 
would be needed.

> > +
> > +static dma_async_tx_callback __ld_cleanup(struct dma_simple_chan *schan, bool all)
> > +{
> > +	struct dma_simple_desc *desc, *_desc;
> > +	/* Is the "exposed" head of a chain acked? */
> > +	bool head_acked = false;
> > +	dma_cookie_t cookie = 0;
> > +	dma_async_tx_callback callback = NULL;
> > +	void *param = NULL;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&schan->chan_lock, flags);
> > +	list_for_each_entry_safe(desc, _desc, &schan->ld_queue, node) {
> > +		struct dma_async_tx_descriptor *tx = &desc->async_tx;
> > +
> > +		BUG_ON(tx->cookie > 0 && tx->cookie != desc->cookie);
> > +		BUG_ON(desc->mark != DESC_SUBMITTED &&
> > +		       desc->mark != DESC_COMPLETED &&
> > +		       desc->mark != DESC_WAITING);
> > +
> > +		/*
> > +		 * queue is ordered, and we use this loop to (1) clean up all
> > +		 * completed descriptors, and to (2) update descriptor flags of
> > +		 * any chunks in a (partially) completed chain
> > +		 */
> > +		if (!all && desc->mark == DESC_SUBMITTED &&
> > +		    desc->cookie != cookie)
> > +			break;
> > +
> > +		if (tx->cookie > 0)
> > +			cookie = tx->cookie;
> > +
> > +		if (desc->mark == DESC_COMPLETED && desc->chunks == 1) {
> > +			if (schan->completed_cookie != desc->cookie - 1)
> > +				dev_dbg(schan->dev,
> > +					"Completing cookie %d, expected %d\n",
> > +					desc->cookie,
> > +					schan->completed_cookie + 1);
> > +			schan->completed_cookie = desc->cookie;
> > +		}
> > +
> > +		/* Call callback on the last chunk */
> > +		if (desc->mark == DESC_COMPLETED && tx->callback) {
> > +			desc->mark = DESC_WAITING;
> > +			callback = tx->callback;
> > +			param = tx->callback_param;
> > +			dev_dbg(schan->dev, "descriptor #%d@%p on %d callback\n",
> > +				tx->cookie, tx, schan->id);
> > +			BUG_ON(desc->chunks != 1);
> > +			break;
> > +		}
> > +
> > +		if (tx->cookie > 0 || tx->cookie == -EBUSY) {
> > +			if (desc->mark == DESC_COMPLETED) {
> > +				BUG_ON(tx->cookie < 0);
> > +				desc->mark = DESC_WAITING;
> > +			}
> > +			head_acked = async_tx_test_ack(tx);
> > +		} else {
> > +			switch (desc->mark) {
> > +			case DESC_COMPLETED:
> > +				desc->mark = DESC_WAITING;
> > +				/* Fall through */
> > +			case DESC_WAITING:
> > +				if (head_acked)
> > +					async_tx_ack(&desc->async_tx);
> > +			}
> > +		}
> > +
> > +		dev_dbg(schan->dev, "descriptor %p #%d completed.\n",
> > +			tx, tx->cookie);
> > +
> > +		if (((desc->mark == DESC_COMPLETED ||
> > +		      desc->mark == DESC_WAITING) &&
> > +		     async_tx_test_ack(&desc->async_tx)) || all) {
> > +			/* Remove from ld_queue list */
> > +			desc->mark = DESC_IDLE;
> > +
> > +			list_move(&desc->node, &schan->ld_free);
> > +
> > +			if (list_empty(&schan->ld_queue)) {
> > +				dev_dbg(schan->dev, "Bring down channel %d\n", schan->id);
> > +				pm_runtime_put(schan->dev);
> > +				schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (all && !callback)
> > +		/*
> > +		 * Terminating and the loop completed normally: forgive
> > +		 * uncompleted cookies
> > +		 */
> > +		schan->completed_cookie = schan->dma_chan.cookie;
> > +
> > +	spin_unlock_irqrestore(&schan->chan_lock, flags);
> > +
> > +	if (callback)
> > +		callback(param);
> > +
> > +	return callback;
> > +}
> > +
> > +/*
> > + * simple_chan_ld_cleanup - Clean up link descriptors
> > + *
> > + * Clean up the ld_queue of DMA channel.
> > + */
> > +static void simple_chan_ld_cleanup(struct dma_simple_chan *schan, bool all)
> > +{
> > +	while (__ld_cleanup(schan, all))
> > +		;
> > +}
> > +
> > +/*
> > + * simple_free_chan_resources - Free all resources of the channel.
> > + */
> > +static void simple_free_chan_resources(struct dma_chan *chan)
> > +{
> > +	struct dma_simple_chan *schan = to_simple_chan(chan);
> > +	struct dma_simple_dev *sdev = to_simple_dev(chan->device);
> > +	const struct dma_simple_ops *ops = sdev->ops;
> > +	LIST_HEAD(list);
> > +
> > +	/* Protect against ISR */
> > +	spin_lock_irq(&schan->chan_lock);
> > +	ops->halt_channel(schan);
> > +	spin_unlock_irq(&schan->chan_lock);
> > +
> > +	/* Now no new interrupts will occur */
> > +
> > +	/* Prepared and not submitted descriptors can still be on the queue */
> > +	if (!list_empty(&schan->ld_queue))
> > +		simple_chan_ld_cleanup(schan, true);
> > +
> > +	if (chan->private) {
> > +		/* The caller is holding dma_list_mutex */
> > +		struct dma_simple_slave *slave = chan->private;
> > +		clear_bit(slave->slave_id, simple_slave_used);
> > +		chan->private = NULL;
> > +	}
> > +
> > +	spin_lock_irq(&schan->chan_lock);
> > +
> > +	list_splice_init(&schan->ld_free, &list);
> > +	schan->desc_num = 0;
> > +
> > +	spin_unlock_irq(&schan->chan_lock);
> > +
> > +	kfree(schan->desc);
> > +}
> > +
> > +/**
> > + * simple_add_desc - get, set up and return one transfer descriptor
> > + * @schan:	DMA channel
> > + * @flags:	DMA transfer flags
> > + * @dst:	destination DMA address, incremented when direction equals
> > + *		DMA_DEV_TO_MEM or DMA_MEM_TO_MEM
> > + * @src:	source DMA address, incremented when direction equals
> > + *		DMA_MEM_TO_DEV or DMA_MEM_TO_MEM
> > + * @len:	DMA transfer length
> > + * @first:	if NULL, set to the current descriptor and cookie set to -EBUSY
> > + * @direction:	needed for slave DMA to decide which address to keep constant,
> > + *		equals DMA_MEM_TO_MEM for MEMCPY
> > + * Returns 0 or an error
> > + * Locks: called with desc_lock held
> > + */
> > +static struct dma_simple_desc *simple_add_desc(struct dma_simple_chan *schan,
> > +	unsigned long flags, dma_addr_t *dst, dma_addr_t *src, size_t *len,
> > +	struct dma_simple_desc **first, enum dma_transfer_direction direction)
> > +{
> > +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> > +	const struct dma_simple_ops *ops = sdev->ops;
> > +	struct dma_simple_desc *new;
> > +	size_t copy_size = *len;
> > +
> > +	if (!copy_size)
> > +		return NULL;
> > +
> > +	/* Allocate the link descriptor from the free list */
> > +	new = simple_get_desc(schan);
> > +	if (!new) {
> > +		dev_err(schan->dev, "No free link descriptor available\n");
> > +		return NULL;
> > +	}
> > +
> > +	ops->desc_setup(schan, new, *src, *dst, &copy_size);
> > +
> > +	if (!*first) {
> > +		/* First desc */
> > +		new->async_tx.cookie = -EBUSY;
> > +		*first = new;
> > +	} else {
> > +		/* Other desc - invisible to the user */
> > +		new->async_tx.cookie = -EINVAL;
> > +	}
> > +
> > +	dev_dbg(schan->dev,
> > +		"chaining (%u/%u)@%x -> %x with %p, cookie %d\n",
> > +		copy_size, *len, *src, *dst, &new->async_tx,
> > +		new->async_tx.cookie);
> > +
> > +	new->mark = DESC_PREPARED;
> > +	new->async_tx.flags = flags;
> > +	new->direction = direction;
> > +
> > +	*len -= copy_size;
> > +	if (direction == DMA_MEM_TO_MEM || direction == DMA_MEM_TO_DEV)
> > +		*src += copy_size;
> > +	if (direction == DMA_MEM_TO_MEM || direction == DMA_DEV_TO_MEM)
> > +		*dst += copy_size;
> > +
> > +	return new;
> > +}
> > +
> > +/*
> > + * simple_prep_sg - prepare transfer descriptors from an SG list
> > + *
> > + * Common routine for public (MEMCPY) and slave DMA. The MEMCPY case is also
> > + * converted to scatter-gather to guarantee consistent locking and a correct
> > + * list manipulation. For slave DMA direction carries the usual meaning, and,
> > + * logically, the SG list is RAM and the addr variable contains slave address,
> > + * e.g., the FIFO I/O register. For MEMCPY direction equals DMA_MEM_TO_MEM
> > + * and the SG list contains only one element and points at the source buffer.
> > + */
> > +static struct dma_async_tx_descriptor *simple_prep_sg(struct dma_simple_chan *schan,
> > +	struct scatterlist *sgl, unsigned int sg_len, dma_addr_t *addr,
> > +	enum dma_transfer_direction direction, unsigned long flags)
> > +{
> > +	struct scatterlist *sg;
> > +	struct dma_simple_desc *first = NULL, *new = NULL /* compiler... */;
> > +	LIST_HEAD(tx_list);
> > +	int chunks = 0;
> > +	unsigned long irq_flags;
> > +	int i;
> > +
> > +	for_each_sg(sgl, sg, sg_len, i)
> > +		chunks += DIV_ROUND_UP(sg_dma_len(sg), schan->max_xfer_len);
> > +
> > +	/* Have to lock the whole loop to protect against concurrent release */
> > +	spin_lock_irqsave(&schan->chan_lock, irq_flags);
> > +
> > +	/*
> > +	 * Chaining:
> > +	 * first descriptor is what user is dealing with in all API calls, its
> > +	 *	cookie is at first set to -EBUSY, at tx-submit to a positive
> > +	 *	number
> > +	 * if more than one chunk is needed further chunks have cookie = -EINVAL
> > +	 * the last chunk, if not equal to the first, has cookie = -ENOSPC
> > +	 * all chunks are linked onto the tx_list head with their .node heads
> > +	 *	only during this function, then they are immediately spliced
> > +	 *	back onto the free list in form of a chain
> > +	 */
> > +	for_each_sg(sgl, sg, sg_len, i) {
> > +		dma_addr_t sg_addr = sg_dma_address(sg);
> > +		size_t len = sg_dma_len(sg);
> > +
> > +		if (!len)
> > +			goto err_get_desc;
> > +
> > +		do {
> > +			dev_dbg(schan->dev, "Add SG #%d@%p[%d], dma %llx\n",
> > +				i, sg, len, (unsigned long long)sg_addr);
> > +
> > +			if (direction == DMA_DEV_TO_MEM)
> > +				new = simple_add_desc(schan, flags,
> > +						&sg_addr, addr, &len, &first,
> > +						direction);
> > +			else
> > +				new = simple_add_desc(schan, flags,
> > +						addr, &sg_addr, &len, &first,
> > +						direction);
> > +			if (!new)
> > +				goto err_get_desc;
> > +
> > +			new->chunks = chunks--;
> > +			list_add_tail(&new->node, &tx_list);
> > +		} while (len);
> > +	}
> > +
> > +	if (new != first)
> > +		new->async_tx.cookie = -ENOSPC;
> > +
> > +	/* Put them back on the free list, so, they don't get lost */
> > +	list_splice_tail(&tx_list, &schan->ld_free);
> > +
> > +	spin_unlock_irqrestore(&schan->chan_lock, irq_flags);
> > +
> > +	return &first->async_tx;
> > +
> > +err_get_desc:
> > +	list_for_each_entry(new, &tx_list, node)
> > +		new->mark = DESC_IDLE;
> > +	list_splice(&tx_list, &schan->ld_free);
> > +
> > +	spin_unlock_irqrestore(&schan->chan_lock, irq_flags);
> > +
> > +	return NULL;
> > +}
> > +
> > +static struct dma_async_tx_descriptor *simple_prep_memcpy(
> > +	struct dma_chan *chan, dma_addr_t dma_dest, dma_addr_t dma_src,
> > +	size_t len, unsigned long flags)
> > +{
> > +	struct dma_simple_chan *schan = to_simple_chan(chan);
> > +	struct scatterlist sg;
> > +
> > +	if (!chan || !len)
> > +		return NULL;
> > +
> > +	BUG_ON(!schan->desc_num);
> > +
> > +	sg_init_table(&sg, 1);
> > +	sg_set_page(&sg, pfn_to_page(PFN_DOWN(dma_src)), len,
> > +		    offset_in_page(dma_src));
> > +	sg_dma_address(&sg) = dma_src;
> > +	sg_dma_len(&sg) = len;
> > +
> > +	return simple_prep_sg(schan, &sg, 1, &dma_dest, DMA_MEM_TO_MEM, flags);
> > +}
> mempcy is a single transfer why should this go thru library?
> got sg_memcpy yes, but otherwise NO

This allows to unify the transfer (descriptor) handling also for cases, 
when the user is requesting too large a transfer, that has to be split 
internally in the driver into several transfers.

> > +
> > +static struct dma_async_tx_descriptor *simple_prep_slave_sg(
> > +	struct dma_chan *chan, struct scatterlist *sgl, unsigned int sg_len,
> > +	enum dma_transfer_direction direction, unsigned long flags)
> > +{
> > +	struct dma_simple_chan *schan = to_simple_chan(chan);
> > +	struct dma_simple_dev *sdev = to_simple_dev(schan->dma_chan.device);
> > +	const struct dma_simple_ops *ops = sdev->ops;
> > +	struct dma_simple_slave *slave = chan->private;
> > +	dma_addr_t slave_addr;
> > +
> > +	if (!chan)
> > +		return NULL;
> > +
> > +	BUG_ON(!schan->desc_num);
> > +
> > +	/* Someone calling slave DMA on a generic channel? */
> > +	if (!slave || !sg_len) {
> > +		dev_warn(schan->dev, "%s: bad parameter: %p, %d, %d\n",
> > +			 __func__, slave, sg_len, slave ? slave->slave_id : -1);
> > +		return NULL;
> > +	}
> > +
> > +	slave_addr = ops->slave_addr(schan);
> > +
> > +	return simple_prep_sg(schan, sgl, sg_len, &slave_addr,
> > +			      direction, flags);
> > +}
> > +
> > +static int simple_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> > +			  unsigned long arg)
> > +{
> > +	struct dma_simple_chan *schan = to_simple_chan(chan);
> > +	struct dma_simple_dev *sdev = to_simple_dev(chan->device);
> > +	const struct dma_simple_ops *ops = sdev->ops;
> > +	unsigned long flags;
> > +
> > +	/* Only supports DMA_TERMINATE_ALL */
> > +	if (cmd != DMA_TERMINATE_ALL)
> > +		return -ENXIO;
> nope, you should check from respective dmac...

Well, right, some drivers might implement more. So, our choice is either 
to preemptively prepare code to handle those, or wait until such drivers 
surface and wish to use this library, then we can extend it to handle 
those too.

> > +
> > +	if (!chan)
> > +		return -EINVAL;
> > +
> > +	spin_lock_irqsave(&schan->chan_lock, flags);
> > +
> > +	ops->halt_channel(schan);
> > +
> > +	spin_unlock_irqrestore(&schan->chan_lock, flags);
> > +
> > +	simple_chan_ld_cleanup(schan, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static void simple_issue_pending(struct dma_chan *chan)
> > +{
> > +	struct dma_simple_chan *schan = to_simple_chan(chan);
> > +
> > +	spin_lock_irq(&schan->chan_lock);
> > +	if (schan->pm_state == DMA_SIMPLE_PM_ESTABLISHED)
> > +		simple_chan_xfer_ld_queue(schan);
> > +	else
> > +		schan->pm_state = DMA_SIMPLE_PM_PENDING;
> > +	spin_unlock_irq(&schan->chan_lock);
> > +}
> > +
> > +static enum dma_status simple_tx_status(struct dma_chan *chan,
> > +					dma_cookie_t cookie,
> > +					struct dma_tx_state *txstate)
> > +{
> > +	struct dma_simple_chan *schan = to_simple_chan(chan);
> > +	dma_cookie_t last_used;
> > +	dma_cookie_t last_complete;
> > +	enum dma_status status;
> > +	unsigned long flags;
> > +
> > +	simple_chan_ld_cleanup(schan, false);
> > +
> > +	/* First read completed cookie to avoid a skew */
> > +	last_complete = schan->completed_cookie;
> > +	rmb();
> > +	last_used = chan->cookie;
> > +	BUG_ON(last_complete < 0);
> > +	dma_set_tx_state(txstate, last_complete, last_used, 0);
> > +
> > +	spin_lock_irqsave(&schan->chan_lock, flags);
> > +
> > +	status = dma_async_is_complete(cookie, last_complete, last_used);
> > +
> > +	/*
> > +	 * If we don't find cookie on the queue, it has been aborted and we have
> > +	 * to report error
> > +	 */
> > +	if (status != DMA_SUCCESS) {
> > +		struct dma_simple_desc *sdesc;
> > +		status = DMA_ERROR;
> > +		list_for_each_entry(sdesc, &schan->ld_queue, node)
> > +			if (sdesc->cookie == cookie) {
> > +				status = DMA_IN_PROGRESS;
> > +				break;
> > +			}
> > +	}
> > +
> > +	spin_unlock_irqrestore(&schan->chan_lock, flags);
> > +
> > +	return status;
> > +}
> > +
> > +/* Called from error IRQ or NMI */
> > +bool dma_simple_reset(struct dma_simple_dev *sdev)
> > +{
> > +	const struct dma_simple_ops *ops = sdev->ops;
> > +	struct dma_simple_chan *schan;
> > +	unsigned int handled = 0;
> > +	int i;
> > +
> > +	/* Reset all channels */
> > +	dma_simple_for_each_chan(schan, sdev, i) {
> > +		struct dma_simple_desc *sdesc;
> > +		LIST_HEAD(dl);
> > +
> > +		if (!schan)
> > +			continue;
> > +
> > +		spin_lock(&schan->chan_lock);
> > +
> > +		/* Stop the channel */
> > +		ops->halt_channel(schan);
> > +
> > +		list_splice_init(&schan->ld_queue, &dl);
> > +
> > +		if (!list_empty(&dl)) {
> > +			dev_dbg(schan->dev, "Bring down channel %d\n", schan->id);
> > +			pm_runtime_put(schan->dev);
> > +		}
> > +		schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> > +
> > +		spin_unlock(&schan->chan_lock);
> > +
> > +		/* Complete all  */
> > +		list_for_each_entry(sdesc, &dl, node) {
> > +			struct dma_async_tx_descriptor *tx = &sdesc->async_tx;
> > +			sdesc->mark = DESC_IDLE;
> > +			if (tx->callback)
> > +				tx->callback(tx->callback_param);
> > +		}
> > +
> > +		spin_lock(&schan->chan_lock);
> > +		list_splice(&dl, &schan->ld_free);
> > +		spin_unlock(&schan->chan_lock);
> > +
> > +		handled++;
> > +	}
> > +
> > +	return !!handled;
> > +}
> > +EXPORT_SYMBOL(dma_simple_reset);
> > +
> > +static irqreturn_t chan_irq(int irq, void *dev)
> > +{
> > +	struct dma_simple_chan *schan = dev;
> > +	const struct dma_simple_ops *ops =
> > +		to_simple_dev(schan->dma_chan.device)->ops;
> > +	irqreturn_t ret;
> > +
> > +	spin_lock(&schan->chan_lock);
> > +
> > +	ret = ops->chan_irq(schan, irq) ? IRQ_WAKE_THREAD : IRQ_NONE;
> > +
> > +	spin_unlock(&schan->chan_lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static irqreturn_t chan_irqt(int irq, void *dev)
> > +{
> > +	struct dma_simple_chan *schan = dev;
> > +	const struct dma_simple_ops *ops =
> > +		to_simple_dev(schan->dma_chan.device)->ops;
> > +	struct dma_simple_desc *sdesc;
> > +
> > +	spin_lock_irq(&schan->chan_lock);
> > +	list_for_each_entry(sdesc, &schan->ld_queue, node) {
> > +		if (sdesc->mark == DESC_SUBMITTED &&
> > +		    ops->desc_completed(schan, sdesc)) {
> > +			dev_dbg(schan->dev, "done #%d@%p\n",
> > +				sdesc->async_tx.cookie, &sdesc->async_tx);
> > +			sdesc->mark = DESC_COMPLETED;
> > +			break;
> > +		}
> > +	}
> > +	/* Next desc */
> > +	simple_chan_xfer_ld_queue(schan);
> > +	spin_unlock_irq(&schan->chan_lock);
> > +
> > +	simple_chan_ld_cleanup(schan, false);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +int dma_simple_request_irq(struct dma_simple_chan *schan, int irq,
> > +			   unsigned long flags, const char *name)
> > +{
> > +	int ret = request_threaded_irq(irq, chan_irq, chan_irqt,
> > +				       flags, name, schan);
> > +
> > +	schan->irq = ret < 0 ? ret : irq;
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(dma_simple_request_irq);
> > +
> > +void dma_simple_free_irq(struct dma_simple_chan *schan)
> > +{
> > +	if (schan->irq >= 0)
> > +		free_irq(schan->irq, schan);
> > +}
> > +EXPORT_SYMBOL(dma_simple_free_irq);
> why would you use the irq here?? That should be handled by respective
> dmac. Also, the library should just setup callbacks for descriptor and
> use them to manage the descriptors for sg mode.

Please, see below.

> > +
> > +void dma_simple_chan_probe(struct dma_simple_dev *sdev,
> > +			   struct dma_simple_chan *schan, int id)
> > +{
> > +	schan->pm_state = DMA_SIMPLE_PM_ESTABLISHED;
> > +
> > +	/* reference struct dma_device */
> > +	schan->dma_chan.device = &sdev->dma_dev;
> > +
> > +	schan->dev = sdev->dma_dev.dev;
> > +	schan->id = id;
> > +
> > +	if (!schan->max_xfer_len)
> > +		schan->max_xfer_len = PAGE_SIZE;
> > +
> > +	spin_lock_init(&schan->chan_lock);
> > +
> > +	/* Init descripter manage list */
> > +	INIT_LIST_HEAD(&schan->ld_queue);
> > +	INIT_LIST_HEAD(&schan->ld_free);
> > +
> > +	/* Add the channel to DMA device channel list */
> > +	list_add_tail(&schan->dma_chan.device_node,
> > +			&sdev->dma_dev.channels);
> > +	sdev->schan[sdev->dma_dev.chancnt++] = schan;
> > +}
> > +EXPORT_SYMBOL(dma_simple_chan_probe);
> > +
> > +void dma_simple_chan_remove(struct dma_simple_chan *schan)
> > +{
> > +	list_del(&schan->dma_chan.device_node);
> > +}
> > +EXPORT_SYMBOL(dma_simple_chan_remove);
> > +
> > +int dma_simple_init(struct device *dev, struct dma_simple_dev *sdev,
> > +		    int chan_num)
> > +{
> > +	struct dma_device *dma_dev = &sdev->dma_dev;
> > +
> > +	/*
> > +	 * Require all call-backs for now, they can trivially be made optional
> > +	 * later as required
> > +	 */
> > +	if (!sdev->ops ||
> > +	    !sdev->desc_size ||
> > +	    !sdev->ops->embedded_desc ||
> > +	    !sdev->ops->start_xfer ||
> > +	    !sdev->ops->setup_xfer ||
> > +	    !sdev->ops->set_slave ||
> > +	    !sdev->ops->desc_setup ||
> > +	    !sdev->ops->slave_addr ||
> > +	    !sdev->ops->channel_busy ||
> > +	    !sdev->ops->halt_channel ||
> > +	    !sdev->ops->desc_completed)
> > +		return -EINVAL;
> > +
> > +	sdev->schan = kcalloc(chan_num, sizeof(*sdev->schan), GFP_KERNEL);
> > +	if (!sdev->schan)
> > +		return -ENOMEM;
> > +
> > +	INIT_LIST_HEAD(&dma_dev->channels);
> > +
> > +	/* Common and MEMCPY operations */
> > +	dma_dev->device_alloc_chan_resources
> > +		= simple_alloc_chan_resources;
> > +	dma_dev->device_free_chan_resources = simple_free_chan_resources;
> > +	dma_dev->device_prep_dma_memcpy = simple_prep_memcpy;
> > +	dma_dev->device_tx_status = simple_tx_status;
> > +	dma_dev->device_issue_pending = simple_issue_pending;
> > +
> > +	/* Compulsory for DMA_SLAVE fields */
> > +	dma_dev->device_prep_slave_sg = simple_prep_slave_sg;
> > +	dma_dev->device_control = simple_control;
> > +
> > +	dma_dev->dev = dev;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(dma_simple_init);
> > +
> > +void dma_simple_cleanup(struct dma_simple_dev *sdev)
> > +{
> > +	kfree(sdev->schan);
> > +}
> > +EXPORT_SYMBOL(dma_simple_cleanup);
> > +
> > +static int __init dma_simple_enter(void)
> > +{
> > +	simple_slave_used = kzalloc(DIV_ROUND_UP(slave_num, BITS_PER_LONG) *
> > +				    sizeof(long), GFP_KERNEL);
> > +	if (!simple_slave_used)
> > +		return -ENOMEM;
> > +	return 0;
> > +}
> > +module_init(dma_simple_enter);
> > +
> > +static void __exit dma_simple_exit(void)
> > +{
> > +	kfree(simple_slave_used);
> > +}
> > +module_exit(dma_simple_exit);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Simple dmaengine driver library");
> > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>");
> > diff --git a/include/linux/dma-simple.h b/include/linux/dma-simple.h
> > new file mode 100644
> > index 0000000..5336674
> > --- /dev/null
> > +++ b/include/linux/dma-simple.h
> > @@ -0,0 +1,124 @@
> > +/*
> > + * Simple dmaengine driver library
> > + *
> > + * extracted from shdma.c and headers
> > + *
> > + * Copyright (C) 2011-2012 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > + * Copyright (C) 2009 Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
> > + * Copyright (C) 2009 Renesas Solutions, Inc. All rights reserved.
> > + * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved.
> > + *
> > + * This is free software; you can redistribute it and/or modify
> > + * it under the terms of version 2 of the GNU General Public License as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef DMA_SIMPLE_H
> > +#define DMA_SIMPLE_H
> > +
> > +#include <linux/dmaengine.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/list.h>
> > +#include <linux/types.h>
> > +
> > +/**
> > + * dma_simple_pm_state - DMA channel PM state
> > + * DMA_SIMPLE_PM_ESTABLISHED:	either idle or during data transfer
> > + * DMA_SIMPLE_PM_BUSY:		during the transfer preparation, when we have to
> > + *				drop the lock temporarily
> > + * DMA_SIMPLE_PM_PENDING:	transfers pending
> > + */
> > +enum dma_simple_pm_state {
> > +	DMA_SIMPLE_PM_ESTABLISHED,
> > +	DMA_SIMPLE_PM_BUSY,
> > +	DMA_SIMPLE_PM_PENDING,
> > +};
> > +
> > +struct device;
> > +
> > +/*
> > + * Drivers, using this library are expected to embed struct dma_simple_dev,
> > + * struct dma_simple_chan, struct dma_simple_desc, and struct dma_simple_slave
> > + * in their respective device, channel, descriptor and slave objects.
> > + */
> > +
> > +struct dma_simple_slave {
> > +	unsigned int slave_id;
> > +};
> > +
> > +struct dma_simple_desc {
> > +	struct list_head node;
> > +	struct dma_async_tx_descriptor async_tx;
> > +	enum dma_transfer_direction direction;
> > +	dma_cookie_t cookie;
> > +	int chunks;
> > +	int mark;
> > +};
> > +
> > +struct dma_simple_chan {
> > +	dma_cookie_t completed_cookie;	/* The maximum cookie completed */
> > +	spinlock_t chan_lock;		/* Channel operation lock */
> > +	struct list_head ld_queue;	/* Link descriptors queue */
> > +	struct list_head ld_free;	/* Free link descriptors */
> > +	struct dma_chan dma_chan;	/* DMA channel */
> > +	struct device *dev;		/* Channel device */
> > +	void *desc;			/* buffer for descriptor array */
> > +	int desc_num;			/* desc count */
> > +	size_t max_xfer_len;		/* max transfer length */
> > +	int id;				/* Raw id of this channel */
> > +	int irq;			/* Channel IRQ */
> > +	enum dma_simple_pm_state pm_state;
> > +};
> > +
> > +/**
> > + * struct dma_simple_ops - simple DMA driver operations
> > + * desc_completed:	return true, if this is the descriptor, that just has
> > + *			completed (atomic)
> > + * halt_channel:	stop DMA channel operation (atomic)
> > + * channel_busy:	return true, if the channel is busy (atomic)
> > + * slave_addr:		return slave DMA address
> > + * desc_setup:		set up the hardware specific descriptor portion (atomic)
> > + * set_slave:		bind channel to a slave
> > + * setup_xfer:		configure channel hardware for operation (atomic)
> > + * start_xfer:		start the DMA transfer (atomic)
> > + * embedded_desc:	return Nth struct dma_simple_desc pointer from the
> > + *			descriptor array
> > + * chan_irq:		process channel IRQ, return true if a transfer has
> > + *			completed (atomic)
> > + */
> > +struct dma_simple_ops {
> > +	bool (*desc_completed)(struct dma_simple_chan *, struct dma_simple_desc *);
> > +	void (*halt_channel)(struct dma_simple_chan *);
> > +	bool (*channel_busy)(struct dma_simple_chan *);
> > +	dma_addr_t (*slave_addr)(struct dma_simple_chan *);
> > +	int (*desc_setup)(struct dma_simple_chan *, struct dma_simple_desc *,
> > +			  dma_addr_t, dma_addr_t, size_t *);
> > +	int (*set_slave)(struct dma_simple_chan *, struct dma_simple_slave *);
> > +	void (*setup_xfer)(struct dma_simple_chan *, struct dma_simple_slave *);
> > +	void (*start_xfer)(struct dma_simple_chan *, struct dma_simple_desc *);
> > +	struct dma_simple_desc *(*embedded_desc)(void *, int);
> > +	bool (*chan_irq)(struct dma_simple_chan *, int);
> > +};
> again so many callbacks... are they really required!!

Yes, they are all used, therefore they are required.

> > +
> > +struct dma_simple_dev {
> > +	struct dma_device dma_dev;
> > +	struct dma_simple_chan **schan;
> > +	const struct dma_simple_ops *ops;
> > +	size_t desc_size;
> > +};
> > +
> > +#define dma_simple_for_each_chan(c, d, i) for (i = 0, c = (d)->schan[0]; \
> > +				i < (d)->dma_dev.chancnt; c = (d)->schan[++i])
> > +
> > +int dma_simple_request_irq(struct dma_simple_chan *, int,
> > +			   unsigned long, const char *);
> > +void dma_simple_free_irq(struct dma_simple_chan *);
> > +bool dma_simple_reset(struct dma_simple_dev *sdev);
> > +void dma_simple_chan_probe(struct dma_simple_dev *sdev,
> > +			   struct dma_simple_chan *schan, int id);
> > +void dma_simple_chan_remove(struct dma_simple_chan *schan);
> > +int dma_simple_init(struct device *dev, struct dma_simple_dev *sdev,
> > +		    int chan_num);
> > +void dma_simple_cleanup(struct dma_simple_dev *sdev);
> > +
> > +#endif
> 
> Now I am confused on the intent of this library. It was proposed for
> helping dmacs like sh-mobile to support sg transfers in software which
> are not supported by hardware, but it seems this library is doing _much_
> more.
> IMHO, it should get inserted between dmaengine APIs and client driver,
> _only_ for sg transfers. The channel allocation etc belong to dmac.
> Further, the library should get notified by dmac based on the callbacks
> set for descriptor and then should call native driver while submitting
> the next one in queue...
> Rest of the stuff (if required) would not be generic and probably should
> be in arch specific directory.

Ok, let me explain a bit more the intensions of this library. You're 
right, it is indeed doing more than just descriptor list manipulations. 
But the list handling is the most complex part of the library, which is 
why I advertised it as a library for aiding in that.

As a matter of fact, this library appeared when an attempt has been made 
to extend the shdma library to support the SUDMAC controller:

http://marc.info/?l=linux-sh&m=132626708503808&w=2

As you can see there, the SUDMAC hardware is completely incompatible with 
the original sh-mobile DMAC engines, but the SUDMAC code was able to reuse 
the driver to 99% by only replacing hardware-specific parts. So, instead 
of doing that I proposed to extract the generic code to a library and only 
provide hardware-specific bits to handle DMAC and SUDMAC. Since descriptor 
management is the largest and most complex part of the library, that's 
also how I described it.

I think, it would be good to preserve the library design at large as is, 
maybe updating its description to more precisely explain what it does, 
port it on top of the slave-parameter in channel allocation patch, maybe 
add some cosmetic improvements. If you think as it stands it is not 
generic enough, because it takes too much freedom away from individual 
drivers, we can make a step back and make it sh-mobile specific to be used 
only by shdma and sudmac, and then see, whether any other drivers will 
want to use it and how it will then have to be adjusted.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH/RFC] usb: fix renesas_usbhs to not schedule in atomic context
From: Guennadi Liakhovetski @ 2012-02-06 10:11 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Shimoda, Yoshihiro, linux-kernel, linux-sh, Vinod Koul,
	Magnus Damm, linux-mmc, alsa-devel, linux-serial, Paul Mundt,
	linux-usb
In-Reply-To: <20120205145358.GA13762@legolas.emea.dhcp.ti.com>

Hi Felipe

On Sun, 5 Feb 2012, Felipe Balbi wrote:

> Hi,
> 
> On Fri, Feb 03, 2012 at 04:43:20PM +0100, Guennadi Liakhovetski wrote:
> > The current renesas_usbhs driver triggers
> > 
> > BUG: scheduling while atomic: ksoftirqd/0/3/0x00000102
> > 
> > with enabled CONFIG_DEBUG_ATOMIC_SLEEP, by submitting DMA transfers from 
> > an atomic (tasklet) context, which is not supported by the shdma dmaengine 
> > driver. Fix it by switching to a work. Also simplify some list 
> > manipulations.
> 
> you are doing much more than what you say.

Are those two list macro changes what you refer to as "a lot?" ;-) You're 
right in principle, they are not directly related to the purpose of this 
patch, they are just something that occurred to me, while tracking down 
DMA packets. But yes, it can be extracted to a separate cosmetic patch...

> Also, instead of using a
> workqueue, have you considered using threaded_irqs ?
> 
> (I didn't go over the driver again to see if it makes sense to use
> threaded_irqs in this case, but doesn't hurt asking)

>From a first glance these tasklets are not directly enough related to 
IRQs, so, doing that is either impossible, or would require a _much_ 
deeper change to the driver and _this_ would indeed be a much bigger 
change than just fixing the Oops.

Thanks
Guennadi

> > Shimoda-san, this is the problem, that you were observing. However, it 
> > exists with the present version of shdma just as well as with the new one 
> > - on top of the simple DMA library. I marked it an RFC because (1) I only 
> > lightly tested it with the gadget device on mackerel with the mass storage 
> > gadget, and (2) I am somewhat concerned about races. Currently the work 
> > function runs with no locking and there are no usual cancel_work_sync() 
> > points in the patch. However, it has also been like this before with the 
> > tasklet implementation, which is not much better, and it looks like there 
> > are no asynchronous operations on the same packets like timeouts. Only 
> > asynchronous events, that I can think about are things like unloading the 
> > driver or unplugging the cable, but these have been there before too. It 
> > would become worse on SMP, I think. Comments welcome.
> > 
> > diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
> > index 72339bd..4d739ec 100644
> > --- a/drivers/usb/renesas_usbhs/fifo.c
> > +++ b/drivers/usb/renesas_usbhs/fifo.c
> > @@ -75,8 +75,7 @@ void usbhs_pkt_push(struct usbhs_pipe *pipe, struct usbhs_pkt *pkt,
> >  		pipe->handler = &usbhsf_null_handler;
> >  	}
> >  
> > -	list_del_init(&pkt->node);
> > -	list_add_tail(&pkt->node, &pipe->list);
> > +	list_move_tail(&pkt->node, &pipe->list);
> >  
> >  	/*
> >  	 * each pkt must hold own handler.
> > @@ -106,7 +105,7 @@ static struct usbhs_pkt *__usbhsf_pkt_get(struct usbhs_pipe *pipe)
> >  	if (list_empty(&pipe->list))
> >  		return NULL;
> >  
> > -	return list_entry(pipe->list.next, struct usbhs_pkt, node);
> > +	return list_first_entry(&pipe->list, struct usbhs_pkt, node);
> 
> these two hunks are not part of $SUBJECT
> 
> -- 
> balbi
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH/RFC] usb: fix renesas_usbhs to not schedule in atomic context
From: Felipe Balbi @ 2012-02-06 10:31 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: alsa-devel, linux-sh, Vinod Koul, linux-mmc, Shimoda, Yoshihiro,
	linux-usb, Magnus Damm, linux-kernel, Felipe Balbi, Paul Mundt,
	linux-serial
In-Reply-To: <Pine.LNX.4.64.1202061058430.10363@axis700.grange>


[-- Attachment #1.1: Type: text/plain, Size: 1585 bytes --]

Hi,

On Mon, Feb 06, 2012 at 11:11:45AM +0100, Guennadi Liakhovetski wrote:
> Hi Felipe
> 
> On Sun, 5 Feb 2012, Felipe Balbi wrote:
> 
> > Hi,
> > 
> > On Fri, Feb 03, 2012 at 04:43:20PM +0100, Guennadi Liakhovetski wrote:
> > > The current renesas_usbhs driver triggers
> > > 
> > > BUG: scheduling while atomic: ksoftirqd/0/3/0x00000102
> > > 
> > > with enabled CONFIG_DEBUG_ATOMIC_SLEEP, by submitting DMA transfers from 
> > > an atomic (tasklet) context, which is not supported by the shdma dmaengine 
> > > driver. Fix it by switching to a work. Also simplify some list 
> > > manipulations.
> > 
> > you are doing much more than what you say.
> 
> Are those two list macro changes what you refer to as "a lot?" ;-) You're 
> right in principle, they are not directly related to the purpose of this 
> patch, they are just something that occurred to me, while tracking down 
> DMA packets. But yes, it can be extracted to a separate cosmetic patch...

please do so ;-)

> > Also, instead of using a
> > workqueue, have you considered using threaded_irqs ?
> > 
> > (I didn't go over the driver again to see if it makes sense to use
> > threaded_irqs in this case, but doesn't hurt asking)
> 
> From a first glance these tasklets are not directly enough related to 
> IRQs, so, doing that is either impossible, or would require a _much_ 
> deeper change to the driver and _this_ would indeed be a much bigger 
> change than just fixing the Oops.

I see.. so please just split the list changes to separate patch and
resend.

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: NeilBrown @ 2012-02-06 23:58 UTC (permalink / raw)
  To: Woodruff, Richard
  Cc: Russell King - ARM Linux, Grazvydas Ignotas, Paul Walmsley,
	Hilman, Kevin, R, Govindraj, Valkeinen, Tomi,
	linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <EF62F09C0797D947AD4180A1043C0DF734934E9D@DLEE10.ent.ti.com>

[-- Attachment #1: Type: text/plain, Size: 4328 bytes --]

On Sun, 5 Feb 2012 17:57:40 +0000 "Woodruff, Richard" <r-woodruff2@ti.com>
wrote:

> 
> > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
> > Sent: Sunday, February 05, 2012 10:03 AM
> > To: Woodruff, Richard
> 
> > On Sun, Feb 05, 2012 at 03:37:21PM +0000, Woodruff, Richard wrote:
> > > [x] What is acceptable depends is not black and white.  Is there some
> > > QOS mapping which can be set per channel which allows runtime PM to
> > > pick a best chose (which may allow for loss and frame issues)?.
> > 
> > What you're asking is whether there's anything in the kernel which can
> > predict when the next character is to arrive.
> 
> No, this was not the comment's intent.
> 
> > But, the fact of the matter is that deriving the UART clocks from a PLL
> > which takes a finite time to lock, and the PLL is shut down during runtime
> > PM _is_ _going_ _to_ _cause_ _problems_.  There is absolutely no getting
> > away from that.
> 
> Yes this is one of the issues to be worked around.
> 
> > Let's take your modem example.  Modems today would typically be used with
> > some IP based protocol, probably PPP based.  Incoming traffic on IP is
> > entirely unpredictable.  You wouldn't know when the next character would
> > be received.
> > 
> > One solution to this is to transmit an 0xff character before your real
> > data to ensure that your end is awake and properly synchronized...
> 
> This approach as you say has issues.  This is solved in different ways for modems.
> 
> <sleep>My observation is modem software which many talk over ppp over ip over serial of some sort (might be uart, might be usb), will send a command to the modem to go into a low power mode. Now you can cut clocks with out hurting modem and getting SOC power.
> 
> <wake>When some event happens at modem or processor (timer near beacon or other) the modem or apps processor can signal the other with some wake event (maybe over gpio) which then puts system in a state where it can receive data in trusted manner.
> 
> The modem channel driver try's to inform kernel about entering/exiting modes to set expectation.

I think the correct way to "inform the kernel" would be with the 'CREAD'
c_cflag in termios.  Clearing it indicates that we don't expect to read which
would allow the UART to go to sleep.
When the GPIO interrupt arrives, we would use termios to set CREAD and then
start talking to the modem.

However it is easy to imagine situations where this wouldn't be enough.
A fairly obvious way to wake a sleeping serial connection is with a 'break'.
I have a GPS which can be put to sleep and then woken by sending a 'break'.
Similarly the OMAP uart can reliably receive a break when asleep, but cannot
reliably receive any other input.

Though maybe if BRKINT is set in c_iflag, then we could still receive a break
even when CREAD is clear.  Then clearing CREAD would be enough to allow
low-power mode.

> 
> > So, go ahead with having PM drop random characters if you want, but don't
> > expect anyone in their right mind to accept broken workarounds to the
> > kernel serial driver to try to drop maybe 16 characters or more at a time
> > on the floor when a framing error occurs just because the PM is broken.
> 
> No character was dropped in modem example.  On the UART-to-Debug console it may be ok to drop a character.  Ether needs a coordination hook.

I don't think it is really OK to drop chars on the UART-to-Debug console.
However it is OK to drop the BAUD rate to 57600 where we can wake up in time
for to catch the first bit.  So if you want power saving, drop the console
buad rate.

So I would suggest:
 - remove the autosuspend timeouts
 - allow runtime_pm to shutdown the device when it is not open, or when
   rate is 57600 or below, or when 'CREAD' is clear
 - keep runtime_pm active whenever there are bytes in the output queue or
   fifo

The only case that wouldn't support is when a device will wake up the SOC by
sending a non-break character which it is OK to receive corrupted.  The tty
would have to be in !CREAD for that to happen, and then there would be no way
for the app to know that a non-break character was received.
Would it be reasonable to treat any input while CREAD is clear as a break?

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* RE: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Woodruff, Richard @ 2012-02-07  1:00 UTC (permalink / raw)
  To: NeilBrown
  Cc: Grazvydas Ignotas, Paul Walmsley, Greg KH, greg@kroah.com,
	Hilman, Kevin, R, Govindraj, Valkeinen, Tomi,
	linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20120204133101.1ab3500f@notabene.brown>


> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Friday, February 03, 2012 8:31 PM

> So... if flow control is available, then when we idle the uart we should
> set <snip> ...

Yes I think you can improve situation with such tricks.

Unfortunately there are a few types of low-power idle wakeups which muddy the water when trying to understand TRMs.

- The IOpad type wakes are the ones being discussed now. These are used in conjunction with isolation rings which stop external signals from propagating into chip and causing undefined things. This protection is used to enable OFF mode (but can be armed outside of from 34xx+ and beyond). The wakeup event here travels through pins to wakeup domain then to prcm which reactivates subdomains and signals can then reflow (if there are still valid). You get IOpad status which can map to function.

- A shade of idle up is module idle wakes. Here if the isolation latch is not enabled you need to program omap-ocp&ip wake-function wrappers in uart itself. Here the wake signal comes through pads to uart-ip then it signals prcm to reflow signals.

- A wakeup which no one seems to use above this is the 16x50 UART has some internal sleep/wake features. The generic linux driver might know these but they are rarely used.
 
> > Outside of debug console, this loss has not been huge. Protocols like
> irda would retransmit their magic wake packets. You can move between DMA
> and interrupt modes with activity. So far there has been a work around per
> attached device.
> 
> What about bluetooth?  HCI/UART doesn't seem to have a lot of error
> handling.  Maybe it has enough though.
> (I have bluetooth on UART1 ... of course we might not have the same
> problems
> on UART1 .. I haven't played with bluetooth much yet).

I heard of solutions but don't recall as I personally didn't donate blood to get them working. I recall some activity timer + wake packets but would have to dig up old PPTs.

Regards,
Richard W.


^ permalink raw reply

* RE: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Woodruff, Richard @ 2012-02-07  1:13 UTC (permalink / raw)
  To: NeilBrown
  Cc: Russell King - ARM Linux, Grazvydas Ignotas, Paul Walmsley,
	Hilman, Kevin, R, Govindraj, Valkeinen, Tomi,
	linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20120207105820.367361c5@notabene.brown>


> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Monday, February 06, 2012 5:58 PM
> To: Woodruff, Richard

Apologies for mangled mails... I am years over due ditching current method.

> I don't think it is really OK to drop chars on the UART-to-Debug console.
> However it is OK to drop the BAUD rate to 57600 where we can wake up in
> time
> for to catch the first bit.  So if you want power saving, drop the console
> buad rate.
> 
> So I would suggest:
>  - remove the autosuspend timeouts
>  - allow runtime_pm to shutdown the device when it is not open, or when
>    rate is 57600 or below, or when 'CREAD' is clear
>  - keep runtime_pm active whenever there are bytes in the output queue or
>    fifo

Yes slower baud + use of flow control should help. OMAP4 is like 10x better than OMAP2 with OMAP3 in middle.

> The only case that wouldn't support is when a device will wake up the SOC
> by
> sending a non-break character which it is OK to receive corrupted.  The
> tty
> would have to be in !CREAD for that to happen, and then there would be no
> way
> for the app to know that a non-break character was received.
> Would it be reasonable to treat any input while CREAD is clear as a break?

Others would have to comment on this.  I never took this step as I was ok with degradation on debug console.

If ever I wanted better logs I tended to telnet/ssh in on a network port which was better at retry at higher level.

Regards,
Richard W.




^ permalink raw reply

* Re: [PATCH] serial: relocate remaining serial drivers from tty/ to tty/serial/
From: Jiri Slaby @ 2012-02-08 10:30 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Alan Cox, Sam Ravnborg, Arnd Bergmann, Greg KH, Jiri Slaby,
	linux-serial, linux-kernel
In-Reply-To: <20120105232142.GA10870@windriver.com>

On 01/06/2012 12:21 AM, Paul Gortmaker wrote:
> --- /dev/null
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -0,0 +1,20 @@
> +#
> +# Makefile for the 8250 serial device drivers.
> +#
> +
> +obj-$(CONFIG_SERIAL_8250)		+= 8250.o
> +obj-$(CONFIG_SERIAL_8250_PNP)		+= 8250_pnp.o
> +obj-$(CONFIG_SERIAL_8250_GSC)		+= 8250_gsc.o
> +obj-$(CONFIG_SERIAL_8250_PCI)		+= 8250_pci.o
> +obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
> +obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
> +obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
> +obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
> +obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
> +obj-$(CONFIG_SERIAL_8250_ACCENT)	+= 8250_accent.o
> +obj-$(CONFIG_SERIAL_8250_BOCA)		+= 8250_boca.o
> +obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554)	+= 8250_exar_st16c554.o
> +obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
> +obj-$(CONFIG_SERIAL_8250_MCA)		+= 8250_mca.o
> +obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
> +obj-$(CONFIG_SERIAL_8250_DW)		+= 8250_dw.o

...

> diff --git a/drivers/tty/serial/m32r_sio.c b/drivers/tty/serial/8250/m32r_sio.c
> similarity index 100%
> rename from drivers/tty/serial/m32r_sio.c
> rename to drivers/tty/serial/8250/m32r_sio.c
> diff --git a/drivers/tty/serial/m32r_sio.h b/drivers/tty/serial/8250/m32r_sio.h
> similarity index 100%
> rename from drivers/tty/serial/m32r_sio.h
> rename to drivers/tty/serial/8250/m32r_sio.h
> diff --git a/drivers/tty/serial/m32r_sio_reg.h b/drivers/tty/serial/8250/m32r_sio_reg.h
> similarity index 100%
> rename from drivers/tty/serial/m32r_sio_reg.h
> rename to drivers/tty/serial/8250/m32r_sio_reg.h

Hmm, these cannot be built now :(. Why did you move them?

It looks like they should be moved back.

regards,
-- 
js

^ permalink raw reply

* Re: [PATCH] serial: relocate remaining serial drivers from tty/ to tty/serial/
From: Jiri Slaby @ 2012-02-08 10:32 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Alan Cox, Sam Ravnborg, Arnd Bergmann, Greg Kroah-Hartman,
	Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <4F324ECB.7040102@gmail.com>

Damnit! They didn't manage to forward Greg's email. What a crap.

On 02/08/2012 11:30 AM, Jiri Slaby wrote:
> On 01/06/2012 12:21 AM, Paul Gortmaker wrote:
>> --- /dev/null
>> +++ b/drivers/tty/serial/8250/Makefile
>> @@ -0,0 +1,20 @@
>> +#
>> +# Makefile for the 8250 serial device drivers.
>> +#
>> +
>> +obj-$(CONFIG_SERIAL_8250)		+= 8250.o
>> +obj-$(CONFIG_SERIAL_8250_PNP)		+= 8250_pnp.o
>> +obj-$(CONFIG_SERIAL_8250_GSC)		+= 8250_gsc.o
>> +obj-$(CONFIG_SERIAL_8250_PCI)		+= 8250_pci.o
>> +obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
>> +obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
>> +obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
>> +obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
>> +obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
>> +obj-$(CONFIG_SERIAL_8250_ACCENT)	+= 8250_accent.o
>> +obj-$(CONFIG_SERIAL_8250_BOCA)		+= 8250_boca.o
>> +obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554)	+= 8250_exar_st16c554.o
>> +obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
>> +obj-$(CONFIG_SERIAL_8250_MCA)		+= 8250_mca.o
>> +obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
>> +obj-$(CONFIG_SERIAL_8250_DW)		+= 8250_dw.o
> 
> ...
> 
>> diff --git a/drivers/tty/serial/m32r_sio.c b/drivers/tty/serial/8250/m32r_sio.c
>> similarity index 100%
>> rename from drivers/tty/serial/m32r_sio.c
>> rename to drivers/tty/serial/8250/m32r_sio.c
>> diff --git a/drivers/tty/serial/m32r_sio.h b/drivers/tty/serial/8250/m32r_sio.h
>> similarity index 100%
>> rename from drivers/tty/serial/m32r_sio.h
>> rename to drivers/tty/serial/8250/m32r_sio.h
>> diff --git a/drivers/tty/serial/m32r_sio_reg.h b/drivers/tty/serial/8250/m32r_sio_reg.h
>> similarity index 100%
>> rename from drivers/tty/serial/m32r_sio_reg.h
>> rename to drivers/tty/serial/8250/m32r_sio_reg.h
> 
> Hmm, these cannot be built now :(. Why did you move them?
> 
> It looks like they should be moved back.
> 
> regards,
-- 
js

^ permalink raw reply

* [PATCH] tty: serial: altera_uart: Add CONSOLE_POLL support
From: Tobias Klauser @ 2012-02-08 13:36 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial

This allows altera_uart to be used for KGDB debugging over serial line.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/tty/serial/Makefile      |    2 +-
 drivers/tty/serial/altera_uart.c |   24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index fee0690..d469771 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -81,12 +81,12 @@ obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
 obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
 obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
 obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
+obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
 obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o
 obj-$(CONFIG_SERIAL_QE) += ucc_uart.o
 obj-$(CONFIG_SERIAL_TIMBERDALE)	+= timbuart.o
 obj-$(CONFIG_SERIAL_GRLIB_GAISLER_APBUART) += apbuart.o
 obj-$(CONFIG_SERIAL_ALTERA_JTAGUART) += altera_jtaguart.o
-obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
 obj-$(CONFIG_SERIAL_VT8500) += vt8500_serial.o
 obj-$(CONFIG_SERIAL_MRST_MAX3110)	+= mrst_max3110.o
 obj-$(CONFIG_SERIAL_MFD_HSU)	+= mfd.o
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 3a9cffa..fefaa4c 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -377,6 +377,26 @@ static int altera_uart_verify_port(struct uart_port *port,
 	return 0;
 }
 
+#ifdef CONFIG_CONSOLE_POLL
+static int altera_uart_poll_get_char(struct uart_port *port)
+{
+	while (!(altera_uart_readl(port, ALTERA_UART_STATUS_REG) &
+		 ALTERA_UART_STATUS_RRDY_MSK))
+		cpu_relax();
+
+	return altera_uart_readl(port, ALTERA_UART_RXDATA_REG);
+}
+
+static void altera_uart_poll_put_char(struct uart_port *port, unsigned char c)
+{
+	while (!(altera_uart_readl(port, ALTERA_UART_STATUS_REG) &
+		 ALTERA_UART_STATUS_TRDY_MSK))
+		cpu_relax();
+
+	altera_uart_writel(port, c, ALTERA_UART_TXDATA_REG);
+}
+#endif
+
 /*
  *	Define the basic serial functions we support.
  */
@@ -397,6 +417,10 @@ static struct uart_ops altera_uart_ops = {
 	.release_port	= altera_uart_release_port,
 	.config_port	= altera_uart_config_port,
 	.verify_port	= altera_uart_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+	.poll_get_char	= altera_uart_poll_get_char,
+	.poll_put_char	= altera_uart_poll_put_char,
+#endif
 };
 
 static struct altera_uart altera_uart_ports[CONFIG_SERIAL_ALTERA_UART_MAXPORTS];
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH] tty: serial: altera_uart: remove early_altera_uart_setup
From: Tobias Klauser @ 2012-02-08 13:35 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial

The function has no users inside the tree and the nios2
(out-of-mainline) port doesn't use it either (anymore).

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/tty/serial/altera_uart.c |   23 -----------------------
 include/linux/altera_uart.h      |    4 ----
 2 files changed, 0 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index bde59d1..5ef414c 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -406,29 +406,6 @@ static struct altera_uart altera_uart_ports[CONFIG_SERIAL_ALTERA_UART_MAXPORTS];
 
 #if defined(CONFIG_SERIAL_ALTERA_UART_CONSOLE)
 
-int __init early_altera_uart_setup(struct altera_uart_platform_uart *platp)
-{
-	struct uart_port *port;
-	int i;
-
-	for (i = 0; i < CONFIG_SERIAL_ALTERA_UART_MAXPORTS && platp[i].mapbase; i++) {
-		port = &altera_uart_ports[i].port;
-
-		port->line = i;
-		port->type = PORT_ALTERA_UART;
-		port->mapbase = platp[i].mapbase;
-		port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
-		port->iotype = SERIAL_IO_MEM;
-		port->irq = platp[i].irq;
-		port->uartclk = platp[i].uartclk;
-		port->flags = UPF_BOOT_AUTOCONF;
-		port->ops = &altera_uart_ops;
-		port->private_data = platp;
-	}
-
-	return 0;
-}
-
 static void altera_uart_console_putc(struct uart_port *port, const char c)
 {
 	while (!(altera_uart_readl(port, ALTERA_UART_STATUS_REG) &
diff --git a/include/linux/altera_uart.h b/include/linux/altera_uart.h
index a10a907..c022c82 100644
--- a/include/linux/altera_uart.h
+++ b/include/linux/altera_uart.h
@@ -5,8 +5,6 @@
 #ifndef	__ALTUART_H
 #define	__ALTUART_H
 
-#include <linux/init.h>
-
 struct altera_uart_platform_uart {
 	unsigned long mapbase;	/* Physical address base */
 	unsigned int irq;	/* Interrupt vector */
@@ -14,6 +12,4 @@ struct altera_uart_platform_uart {
 	unsigned int bus_shift;	/* Bus shift (address stride) */
 };
 
-int __init early_altera_uart_setup(struct altera_uart_platform_uart *platp);
-
 #endif /* __ALTUART_H */
-- 
1.7.5.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox