All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 0/2] serial: Drivers for Altera UARTs
@ 2010-03-05 16:52 Tobias Klauser
  2010-03-05 16:52 ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Tobias Klauser
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Klauser @ 2010-03-05 16:52 UTC (permalink / raw)
  To: linux-serial; +Cc: gregkh, nios2-dev, linux-kernel, Tobias Klauser

This is the third version of the patchset to add the serial drivers for the
Altera UARTs. I made the corrections suggested by Alan Cox on top of the second
version submitted on 01 Mar 2010.

Changes from v2 to v3:

 - altera_uart: Copy old termios settings back and set baud rate
 - altera_uart: Remove board specific DTR/DCD macros

Changes from v1 to v2:

 - altera_jtaguart: Implement altera_jtaguart_set_termios
 - altera_jtaguart: Protect port.tty in altera_jtaguart_rx_chars
 - altera_uart, altera_jtaguart: Don't depend on EMBEDDED
 - altera_jtaguart: Correct check for co->index
 - altera_uart: Correct check for co->index

Thanks,
Tobias

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART
  2010-03-05 16:52 [PATCHv3 0/2] serial: Drivers for Altera UARTs Tobias Klauser
@ 2010-03-05 16:52 ` Tobias Klauser
  2010-03-05 16:52   ` [PATCHv3 2/2] serial: Add driver for the Altera UART Tobias Klauser
  2010-03-12 20:48   ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Andrew Morton
  0 siblings, 2 replies; 15+ messages in thread
From: Tobias Klauser @ 2010-03-05 16:52 UTC (permalink / raw)
  To: linux-serial; +Cc: gregkh, nios2-dev, linux-kernel, Tobias Klauser

Add an UART driver for the JTAG UART component available as a SOPC
(System on Programmable Chip) component for Altera FPGAs.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/serial/Kconfig           |   21 ++
 drivers/serial/Makefile          |    1 +
 drivers/serial/altera_jtaguart.c |  504 ++++++++++++++++++++++++++++++++++++++
 include/linux/altera_jtaguart.h  |   16 ++
 include/linux/serial_core.h      |    3 +
 5 files changed, 545 insertions(+), 0 deletions(-)
 create mode 100644 drivers/serial/altera_jtaguart.c
 create mode 100644 include/linux/altera_jtaguart.h

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 888a0ce..7e67283 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -1490,4 +1490,25 @@ config SERIAL_GRLIB_GAISLER_APBUART_CONSOLE
 	help
 	Support for running a console on the GRLIB APBUART
 
+config SERIAL_ALTERA_JTAGUART
+	bool "Altera JTAG UART support"
+	select SERIAL_CORE
+	help
+	  This driver supports the Altera JTAG UART port.
+
+config SERIAL_ALTERA_JTAGUART_CONSOLE
+	bool "Altera JTAG UART console support"
+	depends on SERIAL_ALTERA_JTAGUART
+	select SERIAL_CORE_CONSOLE
+	help
+	  Enable a Altera JTAG UART port to be the system console.
+
+config SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS
+	bool "Bypass output when no connection"
+	depends on SERIAL_ALTERA_JTAGUART_CONSOLE
+	select SERIAL_CORE_CONSOLE
+	help
+	  Bypass console output and keep going even if there is no
+	  JTAG terminal connection with the host.
+
 endmenu
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 5548fe7..6228b6e 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -82,3 +82,4 @@ 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
diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
new file mode 100644
index 0000000..f9b49b5
--- /dev/null
+++ b/drivers/serial/altera_jtaguart.c
@@ -0,0 +1,504 @@
+/*
+ * altera_jtaguart.c -- Altera JTAG UART driver
+ *
+ * Based on mcf.c -- Freescale ColdFire UART driver
+ *
+ * (C) Copyright 2003-2007, Greg Ungerer <gerg@snapgear.com>
+ * (C) Copyright 2008, Thomas Chou <thomas@wytron.com.tw>
+ * (C) Copyright 2010, Tobias Klauser <tklauser@distanz.ch>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/console.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/altera_jtaguart.h>
+
+#define DRV_NAME "altera_jtaguart"
+
+/*
+ * Altera JTAG UART register definitions according to the Altera JTAG UART
+ * datasheet: http://www.altera.com/literature/hb/nios2/n2cpu_nii51009.pdf
+ */
+
+#define ALTERA_JTAGUART_SIZE			8
+
+#define ALTERA_JTAGUART_DATA_REG		0
+
+#define ALTERA_JTAGUART_DATA_DATA_MSK		0x000000FF
+#define ALTERA_JTAGUART_DATA_RVALID_MSK		0x00008000
+#define ALTERA_JTAGUART_DATA_RAVAIL_MSK		0xFFFF0000
+#define ALTERA_JTAGUART_DATA_RAVAIL_OFF		16
+
+#define ALTERA_JTAGUART_CONTROL_REG		4
+
+#define ALTERA_JTAGUART_CONTROL_RE_MSK		0x00000001
+#define ALTERA_JTAGUART_CONTROL_WE_MSK		0x00000002
+#define ALTERA_JTAGUART_CONTROL_RI_MSK		0x00000100
+#define ALTERA_JTAGUART_CONTROL_RI_OFF		8
+#define ALTERA_JTAGUART_CONTROL_WI_MSK		0x00000200
+#define ALTERA_JTAGUART_CONTROL_AC_MSK		0x00000400
+#define ALTERA_JTAGUART_CONTROL_WSPACE_MSK	0xFFFF0000
+#define ALTERA_JTAGUART_CONTROL_WSPACE_OFF	16
+
+/*
+ * Local per-uart structure.
+ */
+struct altera_jtaguart {
+	struct uart_port port;
+	unsigned int sigs;	/* Local copy of line sigs */
+	unsigned long imr;	/* Local IMR mirror */
+};
+
+static unsigned int altera_jtaguart_tx_empty(struct uart_port *port)
+{
+	return (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
+		ALTERA_JTAGUART_CONTROL_WSPACE_MSK) ? TIOCSER_TEMT : 0;
+}
+
+static unsigned int altera_jtaguart_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
+}
+
+static void altera_jtaguart_set_mctrl(struct uart_port *port, unsigned int sigs)
+{
+}
+
+static void altera_jtaguart_start_tx(struct uart_port *port)
+{
+	struct altera_jtaguart *pp =
+	    container_of(port, struct altera_jtaguart, port);
+
+	pp->imr |= ALTERA_JTAGUART_CONTROL_WE_MSK;
+	writel(pp->imr, port->membase + ALTERA_JTAGUART_CONTROL_REG);
+}
+
+static void altera_jtaguart_stop_tx(struct uart_port *port)
+{
+	struct altera_jtaguart *pp =
+	    container_of(port, struct altera_jtaguart, port);
+
+	pp->imr &= ~ALTERA_JTAGUART_CONTROL_WE_MSK;
+	writel(pp->imr, port->membase + ALTERA_JTAGUART_CONTROL_REG);
+}
+
+static void altera_jtaguart_stop_rx(struct uart_port *port)
+{
+	struct altera_jtaguart *pp =
+	    container_of(port, struct altera_jtaguart, port);
+
+	pp->imr &= ~ALTERA_JTAGUART_CONTROL_RE_MSK;
+	writel(pp->imr, port->membase + ALTERA_JTAGUART_CONTROL_REG);
+}
+
+static void altera_jtaguart_break_ctl(struct uart_port *port, int break_state)
+{
+}
+
+static void altera_jtaguart_enable_ms(struct uart_port *port)
+{
+}
+
+static void altera_jtaguart_set_termios(struct uart_port *port,
+					struct ktermios *termios,
+					struct ktermios *old)
+{
+	/* Just copy the old termios settings back */
+	if (old)
+		tty_termios_copy_hw(termios, old);
+}
+
+static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp)
+{
+	struct uart_port *port = &pp->port;
+	unsigned char ch, flag;
+	unsigned long status;
+
+	while ((status = readl(port->membase + ALTERA_JTAGUART_DATA_REG)) &
+	       ALTERA_JTAGUART_DATA_RVALID_MSK) {
+		ch = status & ALTERA_JTAGUART_DATA_DATA_MSK;
+		flag = TTY_NORMAL;
+		port->icount.rx++;
+
+		if (uart_handle_sysrq_char(port, ch))
+			continue;
+		uart_insert_char(port, 0, 0, ch, flag);
+	}
+
+	tty_flip_buffer_push(port->state->port.tty);
+}
+
+static void altera_jtaguart_tx_chars(struct altera_jtaguart *pp)
+{
+	struct uart_port *port = &pp->port;
+	struct circ_buf *xmit = &port->state->xmit;
+	unsigned int pending, count;
+
+	if (port->x_char) {
+		/* Send special char - probably flow control */
+		writel(port->x_char, port->membase + ALTERA_JTAGUART_DATA_REG);
+		port->x_char = 0;
+		port->icount.tx++;
+		return;
+	}
+
+	pending = uart_circ_chars_pending(xmit);
+	if (pending > 0) {
+		count = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
+				ALTERA_JTAGUART_CONTROL_WSPACE_MSK) >>
+			ALTERA_JTAGUART_CONTROL_WSPACE_OFF;
+		if (count > pending)
+			count = pending;
+		if (count > 0) {
+			pending -= count;
+			while (count--) {
+				writel(xmit->buf[xmit->tail],
+				       port->membase + ALTERA_JTAGUART_DATA_REG);
+				xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+				port->icount.tx++;
+			}
+			if (pending < WAKEUP_CHARS)
+				uart_write_wakeup(port);
+		}
+	}
+
+	if (pending == 0) {
+		pp->imr &= ~ALTERA_JTAGUART_CONTROL_WE_MSK;
+		writel(pp->imr, port->membase + ALTERA_JTAGUART_CONTROL_REG);
+	}
+}
+
+static irqreturn_t altera_jtaguart_interrupt(int irq, void *data)
+{
+	struct uart_port *port = data;
+	struct altera_jtaguart *pp =
+	    container_of(port, struct altera_jtaguart, port);
+	unsigned int isr;
+
+	isr = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) >>
+	       ALTERA_JTAGUART_CONTROL_RI_OFF) & pp->imr;
+
+	spin_lock(&port->lock);
+
+	if (isr & ALTERA_JTAGUART_CONTROL_RE_MSK)
+		altera_jtaguart_rx_chars(pp);
+	if (isr & ALTERA_JTAGUART_CONTROL_WE_MSK)
+		altera_jtaguart_tx_chars(pp);
+
+	spin_unlock(&port->lock);
+
+	return IRQ_RETVAL(isr);
+}
+
+static void altera_jtaguart_config_port(struct uart_port *port, int flags)
+{
+	port->type = PORT_ALTERA_JTAGUART;
+
+	/* Clear mask, so no surprise interrupts. */
+	writel(0, port->membase + ALTERA_JTAGUART_CONTROL_REG);
+}
+
+static int altera_jtaguart_startup(struct uart_port *port)
+{
+	struct altera_jtaguart *pp =
+	    container_of(port, struct altera_jtaguart, port);
+	unsigned long flags;
+	int ret;
+
+	ret = request_irq(port->irq, altera_jtaguart_interrupt, IRQF_DISABLED,
+			DRV_NAME, port);
+	if (ret) {
+		pr_err(DRV_NAME ": unable to attach Altera JTAG UART %d "
+		       "interrupt vector=%d\n", port->line, port->irq);
+		return ret;
+	}
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	/* Enable RX interrupts now */
+	pp->imr = ALTERA_JTAGUART_CONTROL_RE_MSK;
+	writel(pp->imr, port->membase + ALTERA_JTAGUART_CONTROL_REG);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	return 0;
+}
+
+static void altera_jtaguart_shutdown(struct uart_port *port)
+{
+	struct altera_jtaguart *pp =
+	    container_of(port, struct altera_jtaguart, port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	/* Disable all interrupts now */
+	pp->imr = 0;
+	writel(pp->imr, port->membase + ALTERA_JTAGUART_CONTROL_REG);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	free_irq(port->irq, port);
+}
+
+static const char *altera_jtaguart_type(struct uart_port *port)
+{
+	return (port->type == PORT_ALTERA_JTAGUART) ? "Altera JTAG UART" : NULL;
+}
+
+static int altera_jtaguart_request_port(struct uart_port *port)
+{
+	/* UARTs always present */
+	return 0;
+}
+
+static void altera_jtaguart_release_port(struct uart_port *port)
+{
+	/* Nothing to release... */
+}
+
+static int altera_jtaguart_verify_port(struct uart_port *port,
+				       struct serial_struct *ser)
+{
+	if (ser->type != PORT_UNKNOWN && ser->type != PORT_ALTERA_JTAGUART)
+		return -EINVAL;
+	return 0;
+}
+
+/*
+ *	Define the basic serial functions we support.
+ */
+static struct uart_ops altera_jtaguart_ops = {
+	.tx_empty	= altera_jtaguart_tx_empty,
+	.get_mctrl	= altera_jtaguart_get_mctrl,
+	.set_mctrl	= altera_jtaguart_set_mctrl,
+	.start_tx	= altera_jtaguart_start_tx,
+	.stop_tx	= altera_jtaguart_stop_tx,
+	.stop_rx	= altera_jtaguart_stop_rx,
+	.enable_ms	= altera_jtaguart_enable_ms,
+	.break_ctl	= altera_jtaguart_break_ctl,
+	.startup	= altera_jtaguart_startup,
+	.shutdown	= altera_jtaguart_shutdown,
+	.set_termios	= altera_jtaguart_set_termios,
+	.type		= altera_jtaguart_type,
+	.request_port	= altera_jtaguart_request_port,
+	.release_port	= altera_jtaguart_release_port,
+	.config_port	= altera_jtaguart_config_port,
+	.verify_port	= altera_jtaguart_verify_port,
+};
+
+#define ALTERA_JTAGUART_MAXPORTS 1
+static struct altera_jtaguart altera_jtaguart_ports[ALTERA_JTAGUART_MAXPORTS];
+
+#if defined(CONFIG_SERIAL_ALTERA_JTAGUART_CONSOLE)
+
+int __init early_altera_jtaguart_setup(struct altera_jtaguart_platform_uart
+				       *platp)
+{
+	struct uart_port *port;
+	int i;
+
+	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
+		port = &altera_jtaguart_ports[i].port;
+
+		port->line = i;
+		port->type = PORT_ALTERA_JTAGUART;
+		port->mapbase = platp[i].mapbase;
+		port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
+		port->iotype = SERIAL_IO_MEM;
+		port->irq = platp[i].irq;
+		port->flags = ASYNC_BOOT_AUTOCONF;
+		port->ops = &altera_jtaguart_ops;
+	}
+
+	return 0;
+}
+
+#if defined(CONFIG_SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS)
+static void altera_jtaguart_console_putc(struct console *co, const char c)
+{
+	struct uart_port *port = &(altera_jtaguart_ports + co->index)->port;
+	unsigned long status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	while (((status = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG)) &
+		ALTERA_JTAGUART_CONTROL_WSPACE_MSK) == 0) {
+		if ((status & ALTERA_JTAGUART_CONTROL_AC_MSK) == 0) {
+			spin_unlock_irqrestore(&port->lock, flags);
+			return;	/* no connection activity */
+		}
+		spin_unlock_irqrestore(&port->lock, flags);
+		cpu_relax();
+		spin_lock_irqsave(&port->lock, flags);
+	}
+	writel(c, port->membase + ALTERA_JTAGUART_DATA_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+#else
+static void altera_jtaguart_console_putc(struct console *co, const char c)
+{
+	struct uart_port *port = &(altera_jtaguart_ports + co->index)->port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	while ((readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
+		ALTERA_JTAGUART_CONTROL_WSPACE_MSK) == 0) {
+		spin_unlock_irqrestore(&port->lock, flags);
+		cpu_relax();
+		spin_lock_irqsave(&port->lock, flags);
+	}
+	writel(c, port->membase + ALTERA_JTAGUART_DATA_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+#endif
+
+static void altera_jtaguart_console_write(struct console *co, const char *s,
+					  unsigned int count)
+{
+	for (; count; count--, s++) {
+		altera_jtaguart_console_putc(co, *s);
+		if (*s == '\n')
+			altera_jtaguart_console_putc(co, '\r');
+	}
+}
+
+static int __init altera_jtaguart_console_setup(struct console *co,
+						char *options)
+{
+	struct uart_port *port;
+
+	if (co->index < 0 || co->index >= ALTERA_JTAGUART_MAXPORTS)
+		return -EINVAL;
+	port = &altera_jtaguart_ports[co->index].port;
+	if (port->membase == 0)
+		return -ENODEV;
+	return 0;
+}
+
+static struct uart_driver altera_jtaguart_driver;
+
+static struct console altera_jtaguart_console = {
+	.name	= "ttyJ",
+	.write	= altera_jtaguart_console_write,
+	.device	= uart_console_device,
+	.setup	= altera_jtaguart_console_setup,
+	.flags	= CON_PRINTBUFFER,
+	.index	= -1,
+	.data	= &altera_jtaguart_driver,
+};
+
+static int __init altera_jtaguart_console_init(void)
+{
+	register_console(&altera_jtaguart_console);
+	return 0;
+}
+
+console_initcall(altera_jtaguart_console_init);
+
+#define	ALTERA_JTAGUART_CONSOLE	(&altera_jtaguart_console)
+
+#else
+
+#define	ALTERA_JTAGUART_CONSOLE	NULL
+
+#endif /* CONFIG_ALTERA_JTAGUART_CONSOLE */
+
+static struct uart_driver altera_jtaguart_driver = {
+	.owner		= THIS_MODULE,
+	.driver_name	= "altera_jtaguart",
+	.dev_name	= "ttyJ",
+	.major		= ALTERA_JTAGUART_MAJOR,
+	.minor		= ALTERA_JTAGUART_MINOR,
+	.nr		= ALTERA_JTAGUART_MAXPORTS,
+	.cons		= ALTERA_JTAGUART_CONSOLE,
+};
+
+static int __devinit altera_jtaguart_probe(struct platform_device *pdev)
+{
+	struct altera_jtaguart_platform_uart *platp = pdev->dev.platform_data;
+	struct uart_port *port;
+	int i;
+
+	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS && platp[i].mapbase; i++) {
+		port = &altera_jtaguart_ports[i].port;
+
+		port->line = i;
+		port->type = PORT_ALTERA_JTAGUART;
+		port->mapbase = platp[i].mapbase;
+		port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
+		port->iotype = SERIAL_IO_MEM;
+		port->irq = platp[i].irq;
+		port->ops = &altera_jtaguart_ops;
+		port->flags = ASYNC_BOOT_AUTOCONF;
+
+		uart_add_one_port(&altera_jtaguart_driver, port);
+	}
+
+	return 0;
+}
+
+static int __devexit altera_jtaguart_remove(struct platform_device *pdev)
+{
+	struct uart_port *port;
+	int i;
+
+	for (i = 0; i < ALTERA_JTAGUART_MAXPORTS; i++) {
+		port = &altera_jtaguart_ports[i].port;
+		if (port)
+			uart_remove_one_port(&altera_jtaguart_driver, port);
+	}
+
+	return 0;
+}
+
+static struct platform_driver altera_jtaguart_platform_driver = {
+	.probe	= altera_jtaguart_probe,
+	.remove	= __devexit_p(altera_jtaguart_remove),
+	.driver	= {
+		.name	= DRV_NAME,
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init altera_jtaguart_init(void)
+{
+	int rc;
+
+	rc = uart_register_driver(&altera_jtaguart_driver);
+	if (rc)
+		return rc;
+	rc = platform_driver_register(&altera_jtaguart_platform_driver);
+	if (rc) {
+		uart_unregister_driver(&altera_jtaguart_driver);
+		return rc;
+	}
+	return 0;
+}
+
+static void __exit altera_jtaguart_exit(void)
+{
+	platform_driver_unregister(&altera_jtaguart_platform_driver);
+	uart_unregister_driver(&altera_jtaguart_driver);
+}
+
+module_init(altera_jtaguart_init);
+module_exit(altera_jtaguart_exit);
+
+MODULE_DESCRIPTION("Altera JTAG UART driver");
+MODULE_AUTHOR("Thomas Chou <thomas@wytron.com.tw>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/altera_jtaguart.h b/include/linux/altera_jtaguart.h
new file mode 100644
index 0000000..953b178
--- /dev/null
+++ b/include/linux/altera_jtaguart.h
@@ -0,0 +1,16 @@
+/*
+ * altera_jtaguart.h -- Altera JTAG UART driver defines.
+ */
+
+#ifndef	__ALTJUART_H
+#define	__ALTJUART_H
+
+#define ALTERA_JTAGUART_MAJOR	204
+#define ALTERA_JTAGUART_MINOR	186
+
+struct altera_jtaguart_platform_uart {
+	unsigned long mapbase;	/* Physical address base */
+	unsigned int irq;	/* Interrupt vector */
+};
+
+#endif /* __ALTJUART_H */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 8c3dd36..28aa7c9 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -182,6 +182,9 @@
 /* Aeroflex Gaisler GRLIB APBUART */
 #define PORT_APBUART    90
 
+/* Altera UARTs */
+#define PORT_ALTERA_JTAGUART	91
+
 #ifdef __KERNEL__
 
 #include <linux/compiler.h>
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-05 16:52 ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Tobias Klauser
@ 2010-03-05 16:52   ` Tobias Klauser
  2010-03-23 21:54     ` Andrew Morton
  2010-03-12 20:48   ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Andrew Morton
  1 sibling, 1 reply; 15+ messages in thread
From: Tobias Klauser @ 2010-03-05 16:52 UTC (permalink / raw)
  To: linux-serial; +Cc: gregkh, nios2-dev, linux-kernel, Tobias Klauser

Add an UART driver for the UART component available as a SOPC (System on
Programmable Chip) component for Altera FPGAs.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/serial/Kconfig       |   31 +++
 drivers/serial/Makefile      |    1 +
 drivers/serial/altera_uart.c |  573 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/altera_uart.h  |   14 +
 include/linux/serial_core.h  |    1 +
 5 files changed, 620 insertions(+), 0 deletions(-)
 create mode 100644 drivers/serial/altera_uart.c
 create mode 100644 include/linux/altera_uart.h

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 7e67283..eaae8bb 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -1511,4 +1511,35 @@ config SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS
 	  Bypass console output and keep going even if there is no
 	  JTAG terminal connection with the host.
 
+config SERIAL_ALTERA_UART
+	bool "Altera UART support"
+	select SERIAL_CORE
+	help
+	  This driver supports the Altera softcore UART port.
+
+config SERIAL_ALTERA_UART_MAXPORTS
+	int "Maximum number of Altera UART ports"
+	depends on SERIAL_ALTERA_UART
+	default 4
+	help
+	  This setting lets you define the maximum number of the Altera
+	  UART ports. The usual default varies from board to board, and
+	  this setting is a way of catering for that.
+
+config SERIAL_ALTERA_UART_BAUDRATE
+	int "Default baudrate for Altera UART ports"
+	depends on SERIAL_ALTERA_UART
+	default 115200
+	help
+	  This setting lets you define what the default baudrate is for the
+	  Altera UART ports. The usual default varies from board to board,
+	  and this setting is a way of catering for that.
+
+config SERIAL_ALTERA_UART_CONSOLE
+	bool "Altera UART console support"
+	depends on SERIAL_ALTERA_UART
+	select SERIAL_CORE_CONSOLE
+	help
+	  Enable a Altera UART port to be the system console.
+
 endmenu
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 6228b6e..39007e6 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -83,3 +83,4 @@ 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
diff --git a/drivers/serial/altera_uart.c b/drivers/serial/altera_uart.c
new file mode 100644
index 0000000..09fab15
--- /dev/null
+++ b/drivers/serial/altera_uart.c
@@ -0,0 +1,573 @@
+/*
+ * altera_uart.c -- Altera UART driver
+ *
+ * Based on mcf.c -- Freescale ColdFire UART driver
+ *
+ * (C) Copyright 2003-2007, Greg Ungerer <gerg@snapgear.com>
+ * (C) Copyright 2008, Thomas Chou <thomas@wytron.com.tw>
+ * (C) Copyright 2010, Tobias Klauser <tklauser@distanz.ch>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/console.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/altera_uart.h>
+
+#define DRV_NAME "altera_uart"
+
+/*
+ * Altera UART register definitions according to the Nios UART datasheet:
+ * http://www.altera.com/literature/ds/ds_nios_uart.pdf
+ */
+
+#define ALTERA_UART_SIZE		32
+
+#define ALTERA_UART_RXDATA_REG		0
+#define ALTERA_UART_TXDATA_REG		4
+#define ALTERA_UART_STATUS_REG		8
+#define ALTERA_UART_CONTROL_REG		12
+#define ALTERA_UART_DIVISOR_REG		16
+#define ALTERA_UART_EOP_REG		20
+
+#define ALTERA_UART_STATUS_PE_MSK	0x0001	/* parity error */
+#define ALTERA_UART_STATUS_FE_MSK	0x0002	/* framing error */
+#define ALTERA_UART_STATUS_BRK_MSK	0x0004	/* break */
+#define ALTERA_UART_STATUS_ROE_MSK	0x0008	/* RX overrun error */
+#define ALTERA_UART_STATUS_TOE_MSK	0x0010	/* TX overrun error */
+#define ALTERA_UART_STATUS_TMT_MSK	0x0020	/* TX shift register state */
+#define ALTERA_UART_STATUS_TRDY_MSK	0x0040	/* TX ready */
+#define ALTERA_UART_STATUS_RRDY_MSK	0x0080	/* RX ready */
+#define ALTERA_UART_STATUS_E_MSK	0x0100	/* exception condition */
+#define ALTERA_UART_STATUS_DCTS_MSK	0x0400	/* CTS logic-level change */
+#define ALTERA_UART_STATUS_CTS_MSK	0x0800	/* CTS logic state */
+#define ALTERA_UART_STATUS_EOP_MSK	0x1000	/* EOP written/read */
+
+						/* Enable interrupt on... */
+#define ALTERA_UART_CONTROL_PE_MSK	0x0001	/* ...parity error */
+#define ALTERA_UART_CONTROL_FE_MSK	0x0002	/* ...framing error */
+#define ALTERA_UART_CONTROL_BRK_MSK	0x0004	/* ...break */
+#define ALTERA_UART_CONTROL_ROE_MSK	0x0008	/* ...RX overrun */
+#define ALTERA_UART_CONTROL_TOE_MSK	0x0010	/* ...TX overrun */
+#define ALTERA_UART_CONTROL_TMT_MSK	0x0020	/* ...TX shift register empty */
+#define ALTERA_UART_CONTROL_TRDY_MSK	0x0040	/* ...TX ready */
+#define ALTERA_UART_CONTROL_RRDY_MSK	0x0080	/* ...RX ready */
+#define ALTERA_UART_CONTROL_E_MSK	0x0100	/* ...exception*/
+
+#define ALTERA_UART_CONTROL_TRBK_MSK	0x0200	/* TX break */
+#define ALTERA_UART_CONTROL_DCTS_MSK	0x0400	/* Interrupt on CTS change */
+#define ALTERA_UART_CONTROL_RTS_MSK	0x0800	/* RTS signal */
+#define ALTERA_UART_CONTROL_EOP_MSK	0x1000	/* Interrupt on EOP */
+
+/*
+ * Local per-uart structure.
+ */
+struct altera_uart {
+	struct uart_port port;
+	unsigned int sigs;	/* Local copy of line sigs */
+	unsigned short imr;	/* Local IMR mirror */
+};
+
+static unsigned int altera_uart_tx_empty(struct uart_port *port)
+{
+	return (readl(port->membase + ALTERA_UART_STATUS_REG) &
+		ALTERA_UART_STATUS_TMT_MSK) ? TIOCSER_TEMT : 0;
+}
+
+static unsigned int altera_uart_get_mctrl(struct uart_port *port)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+	unsigned int sigs;
+
+	spin_lock_irqsave(&port->lock, flags);
+	sigs =
+	    (readl(port->membase + ALTERA_UART_STATUS_REG) &
+	     ALTERA_UART_STATUS_CTS_MSK) ? TIOCM_CTS : 0;
+	sigs |= (pp->sigs & TIOCM_RTS);
+	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
+	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	return sigs;
+}
+
+static void altera_uart_set_mctrl(struct uart_port *port, unsigned int sigs)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	pp->sigs = sigs;
+	altera_uart_setppdtr(port->line, (sigs & TIOCM_DTR));
+	if (sigs & TIOCM_RTS)
+		pp->imr |= ALTERA_UART_CONTROL_RTS_MSK;
+	else
+		pp->imr &= ~ALTERA_UART_CONTROL_RTS_MSK;
+	writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void altera_uart_start_tx(struct uart_port *port)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	pp->imr |= ALTERA_UART_CONTROL_TRDY_MSK;
+	writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void altera_uart_stop_tx(struct uart_port *port)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	pp->imr &= ~ALTERA_UART_CONTROL_TRDY_MSK;
+	writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void altera_uart_stop_rx(struct uart_port *port)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	pp->imr &= ~ALTERA_UART_CONTROL_RRDY_MSK;
+	writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void altera_uart_break_ctl(struct uart_port *port, int break_state)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	if (break_state == -1)
+		pp->imr |= ALTERA_UART_CONTROL_TRBK_MSK;
+	else
+		pp->imr &= ~ALTERA_UART_CONTROL_TRBK_MSK;
+	writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void altera_uart_enable_ms(struct uart_port *port)
+{
+}
+
+static void altera_uart_set_termios(struct uart_port *port,
+				    struct ktermios *termios,
+				    struct ktermios *old)
+{
+	unsigned long flags;
+	unsigned int baud, baudclk;
+
+	baud = uart_get_baud_rate(port, termios, old, 0, 4000000);
+	baudclk = port->uartclk / baud;
+
+	if (old)
+		tty_termios_copy_hw(termios, old);
+	tty_termios_encode_baud_rate(termios, baud, baud);
+
+	spin_lock_irqsave(&port->lock, flags);
+	writel(baudclk, port->membase + ALTERA_UART_DIVISOR_REG);
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void altera_uart_rx_chars(struct altera_uart *pp)
+{
+	struct uart_port *port = &pp->port;
+	unsigned char ch, flag;
+	unsigned short status;
+
+	while ((status = readl(port->membase + ALTERA_UART_STATUS_REG)) &
+	       ALTERA_UART_STATUS_RRDY_MSK) {
+		ch = readl(port->membase + ALTERA_UART_RXDATA_REG);
+		flag = TTY_NORMAL;
+		port->icount.rx++;
+
+		if (status & ALTERA_UART_STATUS_E_MSK) {
+			writel(status, port->membase + ALTERA_UART_STATUS_REG);
+
+			if (status & ALTERA_UART_STATUS_BRK_MSK) {
+				port->icount.brk++;
+				if (uart_handle_break(port))
+					continue;
+			} else if (status & ALTERA_UART_STATUS_PE_MSK) {
+				port->icount.parity++;
+			} else if (status & ALTERA_UART_STATUS_ROE_MSK) {
+				port->icount.overrun++;
+			} else if (status & ALTERA_UART_STATUS_FE_MSK) {
+				port->icount.frame++;
+			}
+
+			status &= port->read_status_mask;
+
+			if (status & ALTERA_UART_STATUS_BRK_MSK)
+				flag = TTY_BREAK;
+			else if (status & ALTERA_UART_STATUS_PE_MSK)
+				flag = TTY_PARITY;
+			else if (status & ALTERA_UART_STATUS_FE_MSK)
+				flag = TTY_FRAME;
+		}
+
+		if (uart_handle_sysrq_char(port, ch))
+			continue;
+		uart_insert_char(port, status, ALTERA_UART_STATUS_ROE_MSK, ch,
+				 flag);
+	}
+
+	tty_flip_buffer_push(port->state->port.tty);
+}
+
+static void altera_uart_tx_chars(struct altera_uart *pp)
+{
+	struct uart_port *port = &pp->port;
+	struct circ_buf *xmit = &port->state->xmit;
+
+	if (port->x_char) {
+		/* Send special char - probably flow control */
+		writel(port->x_char, port->membase + ALTERA_UART_TXDATA_REG);
+		port->x_char = 0;
+		port->icount.tx++;
+		return;
+	}
+
+	while (readl(port->membase + ALTERA_UART_STATUS_REG) &
+	       ALTERA_UART_STATUS_TRDY_MSK) {
+		if (xmit->head == xmit->tail)
+			break;
+		writel(xmit->buf[xmit->tail],
+		       port->membase + ALTERA_UART_TXDATA_REG);
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		port->icount.tx++;
+	}
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(port);
+
+	if (xmit->head == xmit->tail) {
+		pp->imr &= ~ALTERA_UART_CONTROL_TRDY_MSK;
+		writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+	}
+}
+
+static irqreturn_t altera_uart_interrupt(int irq, void *data)
+{
+	struct uart_port *port = data;
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned int isr;
+
+	isr = readl(port->membase + ALTERA_UART_STATUS_REG) & pp->imr;
+	if (isr & ALTERA_UART_STATUS_RRDY_MSK)
+		altera_uart_rx_chars(pp);
+	if (isr & ALTERA_UART_STATUS_TRDY_MSK)
+		altera_uart_tx_chars(pp);
+	return IRQ_RETVAL(isr);
+}
+
+static void altera_uart_config_port(struct uart_port *port, int flags)
+{
+	port->type = PORT_ALTERA_UART;
+
+	/* Clear mask, so no surprise interrupts. */
+	writel(0, port->membase + ALTERA_UART_CONTROL_REG);
+	/* Clear status register */
+	writel(0, port->membase + ALTERA_UART_STATUS_REG);
+}
+
+static int altera_uart_startup(struct uart_port *port)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+	int ret;
+
+	ret = request_irq(port->irq, altera_uart_interrupt, IRQF_DISABLED,
+			DRV_NAME, port);
+	if (ret) {
+		pr_err(DRV_NAME ": unable to attach Altera UART %d "
+		       "interrupt vector=%d\n", port->line, port->irq);
+		return ret;
+	}
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	/* Enable RX interrupts now */
+	pp->imr = ALTERA_UART_CONTROL_RRDY_MSK;
+	writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	return 0;
+}
+
+static void altera_uart_shutdown(struct uart_port *port)
+{
+	struct altera_uart *pp = container_of(port, struct altera_uart, port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	/* Disable all interrupts now */
+	pp->imr = 0;
+	writel(pp->imr, port->membase + ALTERA_UART_CONTROL_REG);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	free_irq(port->irq, port);
+}
+
+static const char *altera_uart_type(struct uart_port *port)
+{
+	return (port->type == PORT_ALTERA_UART) ? "Altera UART" : NULL;
+}
+
+static int altera_uart_request_port(struct uart_port *port)
+{
+	/* UARTs always present */
+	return 0;
+}
+
+static void altera_uart_release_port(struct uart_port *port)
+{
+	/* Nothing to release... */
+}
+
+static int altera_uart_verify_port(struct uart_port *port,
+				   struct serial_struct *ser)
+{
+	if ((ser->type != PORT_UNKNOWN) && (ser->type != PORT_ALTERA_UART))
+		return -EINVAL;
+	return 0;
+}
+
+/*
+ *	Define the basic serial functions we support.
+ */
+static struct uart_ops altera_uart_ops = {
+	.tx_empty	= altera_uart_tx_empty,
+	.get_mctrl	= altera_uart_get_mctrl,
+	.set_mctrl	= altera_uart_set_mctrl,
+	.start_tx	= altera_uart_start_tx,
+	.stop_tx	= altera_uart_stop_tx,
+	.stop_rx	= altera_uart_stop_rx,
+	.enable_ms	= altera_uart_enable_ms,
+	.break_ctl	= altera_uart_break_ctl,
+	.startup	= altera_uart_startup,
+	.shutdown	= altera_uart_shutdown,
+	.set_termios	= altera_uart_set_termios,
+	.type		= altera_uart_type,
+	.request_port	= altera_uart_request_port,
+	.release_port	= altera_uart_release_port,
+	.config_port	= altera_uart_config_port,
+	.verify_port	= altera_uart_verify_port,
+};
+
+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 = ASYNC_BOOT_AUTOCONF;
+		port->ops = &altera_uart_ops;
+	}
+
+	return 0;
+}
+
+static void altera_uart_console_putc(struct console *co, const char c)
+{
+	struct uart_port *port = &(altera_uart_ports + co->index)->port;
+	int i;
+
+	for (i = 0; i < 0x10000; i++) {
+		if (readl(port->membase + ALTERA_UART_STATUS_REG) &
+		    ALTERA_UART_STATUS_TRDY_MSK)
+			break;
+	}
+	writel(c, port->membase + ALTERA_UART_TXDATA_REG);
+	for (i = 0; i < 0x10000; i++) {
+		if (readl(port->membase + ALTERA_UART_STATUS_REG) &
+		    ALTERA_UART_STATUS_TRDY_MSK)
+			break;
+	}
+}
+
+static void altera_uart_console_write(struct console *co, const char *s,
+				      unsigned int count)
+{
+	for (; count; count--, s++) {
+		altera_uart_console_putc(co, *s);
+		if (*s == '\n')
+			altera_uart_console_putc(co, '\r');
+	}
+}
+
+static int __init altera_uart_console_setup(struct console *co, char *options)
+{
+	struct uart_port *port;
+	int baud = CONFIG_SERIAL_ALTERA_UART_BAUDRATE;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	if (co->index < 0 || co->index >= CONFIG_SERIAL_ALTERA_UART_MAXPORTS)
+		return -EINVAL;
+	port = &altera_uart_ports[co->index].port;
+	if (port->membase == 0)
+		return -ENODEV;
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+	return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver altera_uart_driver;
+
+static struct console altera_uart_console = {
+	.name	= "ttyS",
+	.write	= altera_uart_console_write,
+	.device	= uart_console_device,
+	.setup	= altera_uart_console_setup,
+	.flags	= CON_PRINTBUFFER,
+	.index	= -1,
+	.data	= &altera_uart_driver,
+};
+
+static int __init altera_uart_console_init(void)
+{
+	register_console(&altera_uart_console);
+	return 0;
+}
+
+console_initcall(altera_uart_console_init);
+
+#define	ALTERA_UART_CONSOLE	(&altera_uart_console)
+
+#else
+
+#define	ALTERA_UART_CONSOLE	NULL
+
+#endif /* CONFIG_ALTERA_UART_CONSOLE */
+
+/*
+ *	Define the altera_uart UART driver structure.
+ */
+static struct uart_driver altera_uart_driver = {
+	.owner		= THIS_MODULE,
+	.driver_name	= DRV_NAME,
+	.dev_name	= "ttyS",
+	.major		= TTY_MAJOR,
+	.minor		= 64,
+	.nr		= CONFIG_SERIAL_ALTERA_UART_MAXPORTS,
+	.cons		= ALTERA_UART_CONSOLE,
+};
+
+static int __devinit altera_uart_probe(struct platform_device *pdev)
+{
+	struct altera_uart_platform_uart *platp = pdev->dev.platform_data;
+	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->ops = &altera_uart_ops;
+		port->flags = ASYNC_BOOT_AUTOCONF;
+
+		uart_add_one_port(&altera_uart_driver, port);
+	}
+
+	return 0;
+}
+
+static int altera_uart_remove(struct platform_device *pdev)
+{
+	struct uart_port *port;
+	int i;
+
+	for (i = 0; i < CONFIG_SERIAL_ALTERA_UART_MAXPORTS; i++) {
+		port = &altera_uart_ports[i].port;
+		if (port)
+			uart_remove_one_port(&altera_uart_driver, port);
+	}
+
+	return 0;
+}
+
+static struct platform_driver altera_uart_platform_driver = {
+	.probe	= altera_uart_probe,
+	.remove	= __devexit_p(altera_uart_remove),
+	.driver	= {
+		.name	= DRV_NAME,
+		.owner	= THIS_MODULE,
+		.pm	= NULL,
+	},
+};
+
+static int __init altera_uart_init(void)
+{
+	int rc;
+
+	rc = uart_register_driver(&altera_uart_driver);
+	if (rc)
+		return rc;
+	rc = platform_driver_register(&altera_uart_platform_driver);
+	if (rc) {
+		uart_unregister_driver(&altera_uart_driver);
+		return rc;
+	}
+	return 0;
+}
+
+static void __exit altera_uart_exit(void)
+{
+	platform_driver_unregister(&altera_uart_platform_driver);
+	uart_unregister_driver(&altera_uart_driver);
+}
+
+module_init(altera_uart_init);
+module_exit(altera_uart_exit);
+
+MODULE_DESCRIPTION("Altera UART driver");
+MODULE_AUTHOR("Thomas Chou <thomas@wytron.com.tw>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/altera_uart.h b/include/linux/altera_uart.h
new file mode 100644
index 0000000..8d44106
--- /dev/null
+++ b/include/linux/altera_uart.h
@@ -0,0 +1,14 @@
+/*
+ * altera_uart.h -- Altera UART driver defines.
+ */
+
+#ifndef	__ALTUART_H
+#define	__ALTUART_H
+
+struct altera_uart_platform_uart {
+	unsigned long mapbase;	/* Physical address base */
+	unsigned int irq;	/* Interrupt vector */
+	unsigned int uartclk;	/* UART clock rate */
+};
+
+#endif /* __ALTUART_H */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 28aa7c9..9206120 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -184,6 +184,7 @@
 
 /* Altera UARTs */
 #define PORT_ALTERA_JTAGUART	91
+#define PORT_ALTERA_UART	92
 
 #ifdef __KERNEL__
 
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART
  2010-03-05 16:52 ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Tobias Klauser
  2010-03-05 16:52   ` [PATCHv3 2/2] serial: Add driver for the Altera UART Tobias Klauser
@ 2010-03-12 20:48   ` Andrew Morton
  2010-03-15 10:20     ` Tobias Klauser
  2010-03-16  0:47       ` Thomas Chou
  1 sibling, 2 replies; 15+ messages in thread
From: Andrew Morton @ 2010-03-12 20:48 UTC (permalink / raw)
  To: Tobias Klauser; +Cc: linux-serial, gregkh, nios2-dev, linux-kernel

On Fri,  5 Mar 2010 17:52:22 +0100
Tobias Klauser <tklauser@distanz.ch> wrote:

> Add an UART driver for the JTAG UART component available as a SOPC
> (System on Programmable Chip) component for Altera FPGAs.
> 
> ...
>
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -1490,4 +1490,25 @@ config SERIAL_GRLIB_GAISLER_APBUART_CONSOLE
>  	help
>  	Support for running a console on the GRLIB APBUART
>  
> +config SERIAL_ALTERA_JTAGUART
> +	bool "Altera JTAG UART support"
> +	select SERIAL_CORE
> +	help
> +	  This driver supports the Altera JTAG UART port.
> +
> +config SERIAL_ALTERA_JTAGUART_CONSOLE
> +	bool "Altera JTAG UART console support"
> +	depends on SERIAL_ALTERA_JTAGUART
> +	select SERIAL_CORE_CONSOLE
> +	help
> +	  Enable a Altera JTAG UART port to be the system console.
> +
> +config SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS
> +	bool "Bypass output when no connection"
> +	depends on SERIAL_ALTERA_JTAGUART_CONSOLE
> +	select SERIAL_CORE_CONSOLE
> +	help
> +	  Bypass console output and keep going even if there is no
> +	  JTAG terminal connection with the host.

So this driver will be available on all CPU architectures.

I'm guessing that the hardware _isn't_ available on all CPU
architectures?  Maybe that's wrong.

If this hardware is only available on certain CPUs then we face
tradeoffs.  By making it universally available, the code gets better
compile tesst coverage and that can detect problems (usually minor
ones).  otoh, it can increase your maintenance load a bit, and adds a
risk that people will ship unusable kernel modules and will see
increased build times.  I have no particular opinion either way.


>  endmenu
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index 5548fe7..6228b6e 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -82,3 +82,4 @@ 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
> diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
> new file mode 100644
> index 0000000..f9b49b5
> --- /dev/null
> +++ b/drivers/serial/altera_jtaguart.c
> @@ -0,0 +1,504 @@
> +/*
> + * altera_jtaguart.c -- Altera JTAG UART driver
> + *
> + * Based on mcf.c -- Freescale ColdFire UART driver
> + *
> + * (C) Copyright 2003-2007, Greg Ungerer <gerg@snapgear.com>
> + * (C) Copyright 2008, Thomas Chou <thomas@wytron.com.tw>
> + * (C) Copyright 2010, Tobias Klauser <tklauser@distanz.ch>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/console.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/serial.h>
> +#include <linux/serial_core.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/altera_jtaguart.h>

Does it make sense to put altera_jtaguart.h into include/linux?  Could
we put it in drivers/serial/?

>
> ...
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART
  2010-03-12 20:48   ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Andrew Morton
@ 2010-03-15 10:20     ` Tobias Klauser
  2010-03-15 10:24       ` [Nios2-dev] " waltergoossens
  2010-03-16  0:47       ` Thomas Chou
  1 sibling, 1 reply; 15+ messages in thread
From: Tobias Klauser @ 2010-03-15 10:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-serial, gregkh, nios2-dev, linux-kernel

On 2010-03-12 at 21:48:35 +0100, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri,  5 Mar 2010 17:52:22 +0100
> Tobias Klauser <tklauser@distanz.ch> wrote:
> 
> > Add an UART driver for the JTAG UART component available as a SOPC
> > (System on Programmable Chip) component for Altera FPGAs.
> > 
> > ...
> >
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -1490,4 +1490,25 @@ config SERIAL_GRLIB_GAISLER_APBUART_CONSOLE
> >  	help
> >  	Support for running a console on the GRLIB APBUART
> >  
> > +config SERIAL_ALTERA_JTAGUART
> > +	bool "Altera JTAG UART support"
> > +	select SERIAL_CORE
> > +	help
> > +	  This driver supports the Altera JTAG UART port.
> > +
> > +config SERIAL_ALTERA_JTAGUART_CONSOLE
> > +	bool "Altera JTAG UART console support"
> > +	depends on SERIAL_ALTERA_JTAGUART
> > +	select SERIAL_CORE_CONSOLE
> > +	help
> > +	  Enable a Altera JTAG UART port to be the system console.
> > +
> > +config SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS
> > +	bool "Bypass output when no connection"
> > +	depends on SERIAL_ALTERA_JTAGUART_CONSOLE
> > +	select SERIAL_CORE_CONSOLE
> > +	help
> > +	  Bypass console output and keep going even if there is no
> > +	  JTAG terminal connection with the host.
> 
> So this driver will be available on all CPU architectures.
> 
> I'm guessing that the hardware _isn't_ available on all CPU
> architectures?  Maybe that's wrong.

In principle it is available on all CPU. I can think of a hardware
component with an Altera FPGA containing this UART. But at the moment no
such implementation is known to me and the UART is only usable on the
nios2 CPU architecture  (which is not in mainline yet - we're planning
to change that, submitting the arch-sepcific drivers is the first step
towards this).

> If this hardware is only available on certain CPUs then we face
> tradeoffs.  By making it universally available, the code gets better
> compile tesst coverage and that can detect problems (usually minor
> ones).  otoh, it can increase your maintenance load a bit, and adds a
> risk that people will ship unusable kernel modules and will see
> increased build times.  I have no particular opinion either way.

I'd like to have this universally available if no one objects. It'd be
nice to get better test coverage and I don't really mind the additional
maintanance load. AFAICS other UART driver are only available on one
single architecture too, so I hoped it would be fine to go the same way
with the altera_{jtag,}uart drivers.

> >  endmenu
> > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> > index 5548fe7..6228b6e 100644
> > --- a/drivers/serial/Makefile
> > +++ b/drivers/serial/Makefile
> > @@ -82,3 +82,4 @@ 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
> > diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
> > new file mode 100644
> > index 0000000..f9b49b5
> > --- /dev/null
> > +++ b/drivers/serial/altera_jtaguart.c
> > @@ -0,0 +1,504 @@
> > +/*
> > + * altera_jtaguart.c -- Altera JTAG UART driver
> > + *
> > + * Based on mcf.c -- Freescale ColdFire UART driver
> > + *
> > + * (C) Copyright 2003-2007, Greg Ungerer <gerg@snapgear.com>
> > + * (C) Copyright 2008, Thomas Chou <thomas@wytron.com.tw>
> > + * (C) Copyright 2010, Tobias Klauser <tklauser@distanz.ch>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/console.h>
> > +#include <linux/tty.h>
> > +#include <linux/tty_flip.h>
> > +#include <linux/serial.h>
> > +#include <linux/serial_core.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +#include <linux/altera_jtaguart.h>
> 
> Does it make sense to put altera_jtaguart.h into include/linux?  Could
> we put it in drivers/serial/?

It is stored in include/linux and not in drivers/serial because we need
to include it from arch/nios2 for the platform device setup. Including
<linux/altera_jtaguart.h> looks nicer than including
"../drivers/serial/altera_jtaguart.h". The same holds true for the
altera_uart driver.

Thanks a lot,
Tobias

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Nios2-dev] [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART
  2010-03-15 10:20     ` Tobias Klauser
@ 2010-03-15 10:24       ` waltergoossens
  0 siblings, 0 replies; 15+ messages in thread
From: waltergoossens @ 2010-03-15 10:24 UTC (permalink / raw)
  To: nios2-dev; +Cc: Andrew Morton, gregkh, linux-kernel, linux-serial

----- Origineel bericht -----
Van: Tobias Klauser <tklauser@distanz.ch>
Datum: maandag, maart 15, 2010 10:21
Onderwerp: Re: [Nios2-dev] [PATCHv3 1/2] serial: Add driver for the Altera	JTAG UART
Aan: Andrew Morton <akpm@linux-foundation.org>
Cc: nios2-dev@sopc.et.ntust.edu.tw, gregkh@suse.de, linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org


> On 2010-03-12 at 21:48:35 +0100, Andrew Morton 
> <akpm@linux-foundation.org> wrote:
> > On Fri,  5 Mar 2010 17:52:22 +0100
> > Tobias Klauser <tklauser@distanz.ch> wrote:
> > 
> > > Add an UART driver for the JTAG UART component available as a SOPC
> > > (System on Programmable Chip) component for Altera FPGAs.
> > > 
> > > ...
> > >
> > > --- a/drivers/serial/Kconfig
> > > +++ b/drivers/serial/Kconfig
> > > @@ -1490,4 +1490,25 @@ config SERIAL_GRLIB_GAISLER_APBUART_CONSOLE
> > >  	help
> > >  	Support for running a console on the GRLIB APBUART
> > >  
> > > +config SERIAL_ALTERA_JTAGUART
> > > +	bool "Altera JTAG UART support"
> > > +	select SERIAL_CORE
> > > +	help
> > > +          This driver supports the Altera JTAG UART port.
> > > +
> > > +config SERIAL_ALTERA_JTAGUART_CONSOLE
> > > +	bool "Altera JTAG UART console support"
> > > +	depends on SERIAL_ALTERA_JTAGUART
> > > +	select SERIAL_CORE_CONSOLE
> > > +	help
> > > +          Enable a Altera JTAG UART port to be the system console.
> > > +
> > > +config SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS
> > > +	bool "Bypass output when no connection"
> > > +	depends on SERIAL_ALTERA_JTAGUART_CONSOLE
> > > +	select SERIAL_CORE_CONSOLE
> > > +	help
> > > +          Bypass console output and keep going even if there is no
> > > +          JTAG terminal connection with the host.
> > 
> > So this driver will be available on all CPU architectures.
> > 
> > I'm guessing that the hardware _isn't_ available on all CPU
> > architectures?  Maybe that's wrong.
> 
> In principle it is available on all CPU. I can think of a hardware
> component with an Altera FPGA containing this UART. But at the moment 
> no
> such implementation is known to me and the UART is only usable on the
> nios2 CPU architecture  (which is not in mainline yet - we're planning
> to change that, submitting the arch-sepcific drivers is the first step
> towards this).
> 
We are actually working on a project without a Nios2 where a PCIe core connected to a powerPC is the avalon-master. So I'm in favor of not getting everything too nios2 specific.

> > If this hardware is only available on certain CPUs then we face
> > tradeoffs.  By making it universally available, the code gets better
> > compile tesst coverage and that can detect problems (usually minor
> > ones).  otoh, it can increase your maintenance load a bit, and adds 
> a
> > risk that people will ship unusable kernel modules and will see
> > increased build times.  I have no particular opinion either way.
> 
> I'd like to have this universally available if no one objects. It'd be
> nice to get better test coverage and I don't really mind the additional
> maintanance load. AFAICS other UART driver are only available on one
> single architecture too, so I hoped it would be fine to go the same way
> with the altera_{jtag,}uart drivers.
> 
> > >  endmenu
> > > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> > > index 5548fe7..6228b6e 100644
> > > --- a/drivers/serial/Makefile
> > > +++ b/drivers/serial/Makefile
> > > @@ -82,3 +82,4 @@ 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
> > > diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c
> > > new file mode 100644
> > > index 0000000..f9b49b5
> > > --- /dev/null
> > > +++ b/drivers/serial/altera_jtaguart.c
> > > @@ -0,0 +1,504 @@
> > > +/*
> > > + * altera_jtaguart.c -- Altera JTAG UART driver
> > > + *
> > > + * Based on mcf.c -- Freescale ColdFire UART driver
> > > + *
> > > + * (C) Copyright 2003-2007, Greg Ungerer <gerg@snapgear.com>
> > > + * (C) Copyright 2008, Thomas Chou <thomas@wytron.com.tw>
> > > + * (C) Copyright 2010, Tobias Klauser <tklauser@distanz.ch>
> > > + *
> > > + * This program is free software; you can redistribute it and/or 
> modify
> > > + * it under the terms of the GNU General Public License as 
> published by
> > > + * the Free Software Foundation; either version 2 of the License, 
> or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/init.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/module.h>
> > > +#include <linux/console.h>
> > > +#include <linux/tty.h>
> > > +#include <linux/tty_flip.h>
> > > +#include <linux/serial.h>
> > > +#include <linux/serial_core.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/io.h>
> > > +#include <linux/altera_jtaguart.h>
> > 
> > Does it make sense to put altera_jtaguart.h into include/linux?  Could
> > we put it in drivers/serial/?
> 
> It is stored in include/linux and not in drivers/serial because we need
> to include it from arch/nios2 for the platform device setup. Including
> <linux/altera_jtaguart.h> looks nicer than including
> "../drivers/serial/altera_jtaguart.h". The same holds true for the
> altera_uart driver.
> 
> Thanks a lot,
> Tobias
> _______________________________________________
> Nios2-dev mailing list
> Nios2-dev@sopc.et.ntust.edu.tw
> http://sopc.et.ntust.edu.tw/cgi-bin/mailman/listinfo/nios2-dev

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Nios2-dev] [PATCHv3 1/2] serial: Add driver for the Altera  JTAG UART
  2010-03-12 20:48   ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Andrew Morton
@ 2010-03-16  0:47       ` Thomas Chou
  2010-03-16  0:47       ` Thomas Chou
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Chou @ 2010-03-16  0:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: nios2-dev, Tobias Klauser, gregkh, linux-kernel, linux-serial

On 03/13/2010 04:48 AM, Andrew Morton wrote:
>> +config SERIAL_ALTERA_JTAGUART
>> +	bool "Altera JTAG UART support"
>> +	select SERIAL_CORE
>> +	help
>> +	  This driver supports the Altera JTAG UART port.
>> +
> So this driver will be available on all CPU architectures.
>
> I'm guessing that the hardware _isn't_ available on all CPU
> architectures?  Maybe that's wrong.
>
>    
It is true that this driver is used mostly with nios2, soft-core 
coldfire and arm. But peripherals on FPGA can be made available to 
almost any CPU architecture. So I think it might make sense to let it 
available to all. Or should we put a "default n" ?

>> +#include<linux/platform_device.h>
>> +#include<linux/io.h>
>> +#include<linux/altera_jtaguart.h>
>>      
>
> Does it make sense to put altera_jtaguart.h into include/linux?  Could
> we put it in drivers/serial/?
>
>
>    
There is only a platform data structure declaration in the header, which 
should be passed through platform device data to the driver. If we put 
it in drivers/serial/, then we will need to include 
"../../../drivers/serial/altera_jtaguart.h" .

I have written this driver in parallel with the altera_uart driver. We 
could have passed the data ( base, irq, freq ) using platform resources. 
But I don't know if there is a way to pass ( freq )? Please let me know 
if you have any suggestions.

- Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Nios2-dev] [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART
@ 2010-03-16  0:47       ` Thomas Chou
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Chou @ 2010-03-16  0:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: nios2-dev, Tobias Klauser, gregkh, linux-kernel, linux-serial

On 03/13/2010 04:48 AM, Andrew Morton wrote:
>> +config SERIAL_ALTERA_JTAGUART
>> +	bool "Altera JTAG UART support"
>> +	select SERIAL_CORE
>> +	help
>> +	  This driver supports the Altera JTAG UART port.
>> +
> So this driver will be available on all CPU architectures.
>
> I'm guessing that the hardware _isn't_ available on all CPU
> architectures?  Maybe that's wrong.
>
>    
It is true that this driver is used mostly with nios2, soft-core 
coldfire and arm. But peripherals on FPGA can be made available to 
almost any CPU architecture. So I think it might make sense to let it 
available to all. Or should we put a "default n" ?

>> +#include<linux/platform_device.h>
>> +#include<linux/io.h>
>> +#include<linux/altera_jtaguart.h>
>>      
>
> Does it make sense to put altera_jtaguart.h into include/linux?  Could
> we put it in drivers/serial/?
>
>
>    
There is only a platform data structure declaration in the header, which 
should be passed through platform device data to the driver. If we put 
it in drivers/serial/, then we will need to include 
"../../../drivers/serial/altera_jtaguart.h" .

I have written this driver in parallel with the altera_uart driver. We 
could have passed the data ( base, irq, freq ) using platform resources. 
But I don't know if there is a way to pass ( freq )? Please let me know 
if you have any suggestions.

- Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-05 16:52   ` [PATCHv3 2/2] serial: Add driver for the Altera UART Tobias Klauser
@ 2010-03-23 21:54     ` Andrew Morton
  2010-03-24  6:47       ` Tobias Klauser
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2010-03-23 21:54 UTC (permalink / raw)
  To: Tobias Klauser; +Cc: linux-serial, gregkh, nios2-dev, linux-kernel

On Fri,  5 Mar 2010 17:52:23 +0100
Tobias Klauser <tklauser@distanz.ch> wrote:

> +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
> +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);

We seem to be missing a few things here.

drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'                                                                                                                                                                              

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-23 21:54     ` Andrew Morton
@ 2010-03-24  6:47       ` Tobias Klauser
  2010-03-24 11:05         ` Andrew Morton
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Klauser @ 2010-03-24  6:47 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-serial, gregkh, nios2-dev, linux-kernel, Alan Cox

On 2010-03-23 at 22:54:59 +0100, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri,  5 Mar 2010 17:52:23 +0100
> Tobias Klauser <tklauser@distanz.ch> wrote:
> 
> > +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
> > +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
> 
> We seem to be missing a few things here.
> 
> drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
> drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
> drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
> drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
> drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'

These should usually be declared in a board specific header. There were
compatibility macros in altera_uart.c which defined them to NOPs in case
the board header did not properly define them. But I remove them as per
request by Alan Cox (Message-ID: 20100301181920.3952c3e7@lxorguk.ukuu.org.uk).

Should we add them again (maybe to altera_uart.h)? Or would it be better
to define a config symbol which is set in the board specific Kconfig and
altera_uart depends on it?

Thanks
Tobias

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-24  6:47       ` Tobias Klauser
@ 2010-03-24 11:05         ` Andrew Morton
  2010-03-24 16:24           ` Tobias Klauser
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2010-03-24 11:05 UTC (permalink / raw)
  To: Tobias Klauser; +Cc: linux-serial, gregkh, nios2-dev, linux-kernel, Alan Cox

On Wed, 24 Mar 2010 07:47:47 +0100 Tobias Klauser <tklauser@distanz.ch> wrote:

> On 2010-03-23 at 22:54:59 +0100, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Fri,  5 Mar 2010 17:52:23 +0100
> > Tobias Klauser <tklauser@distanz.ch> wrote:
> > 
> > > +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
> > > +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
> > 
> > We seem to be missing a few things here.
> > 
> > drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
> > drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
> > drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
> > drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
> > drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'
> 
> These should usually be declared in a board specific header. There were
> compatibility macros in altera_uart.c which defined them to NOPs in case
> the board header did not properly define them. But I remove them as per
> request by Alan Cox (Message-ID: 20100301181920.3952c3e7@lxorguk.ukuu.org.uk).
> 
> Should we add them again (maybe to altera_uart.h)? Or would it be better
> to define a config symbol which is set in the board specific Kconfig and
> altera_uart depends on it?

I guess the latter.

There should have been a real implementation of these in the patchset -
otherwise the code can't be used or tested.  Confused.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-24 11:05         ` Andrew Morton
@ 2010-03-24 16:24           ` Tobias Klauser
  2010-03-25  0:54             ` Thomas Chou
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Klauser @ 2010-03-24 16:24 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-serial, gregkh, nios2-dev, linux-kernel, Alan Cox

On 2010-03-24 at 12:05:27 +0100, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 24 Mar 2010 07:47:47 +0100 Tobias Klauser <tklauser@distanz.ch> wrote:
> > On 2010-03-23 at 22:54:59 +0100, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Fri,  5 Mar 2010 17:52:23 +0100
> > > Tobias Klauser <tklauser@distanz.ch> wrote:
> > > 
> > > > +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
> > > > +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
> > > 
> > > We seem to be missing a few things here.
> > > 
> > > drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
> > > drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
> > > drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
> > > drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
> > > drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'
> > 
> > These should usually be declared in a board specific header. There were
> > compatibility macros in altera_uart.c which defined them to NOPs in case
> > the board header did not properly define them. But I remove them as per
> > request by Alan Cox (Message-ID: 20100301181920.3952c3e7@lxorguk.ukuu.org.uk).
> > 
> > Should we add them again (maybe to altera_uart.h)? Or would it be better
> > to define a config symbol which is set in the board specific Kconfig and
> > altera_uart depends on it?
> 
> I guess the latter.
> 
> There should have been a real implementation of these in the patchset -
> otherwise the code can't be used or tested.  Confused.

Sorry for the confusion.

The last patchset I submitted (with the functions removed from
altera_uart.c) was tested on a local branch, where I added the macros to
a global board specific header. I didn't include that one in the patch.

After looking at the code and it's history a bit closer (and also on the
nios2 specific part) I realised that this macro was probably added
because the driver was originally based on drivers/serial/mcf.c (the
macros are present there too).

Also there are currently no board configurations known to me that define
these macros. So I'd suggest to remove the usage of these macros
alltogether. We could still add them again (to the board specific part
and with the config option then) in case there will be a board
configuration implementing DTR/DCD lines on GPIOs.

Could anyone on nios2-dev verify that there are currently no such board
configurations?

I'd remove the usage of the macros then and post an updated patch.

  Tobias

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-24 16:24           ` Tobias Klauser
@ 2010-03-25  0:54             ` Thomas Chou
  2010-03-29 13:29               ` Tobias Klauser
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Chou @ 2010-03-25  0:54 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: Andrew Morton, linux-serial, gregkh, nios2-dev, linux-kernel, Alan Cox

On 03/25/2010 12:24 AM, Tobias Klauser wrote:
> On 2010-03-24 at 12:05:27 +0100, Andrew Morton<akpm@linux-foundation.org>  wrote:
>    
>> On Wed, 24 Mar 2010 07:47:47 +0100 Tobias Klauser<tklauser@distanz.ch>  wrote:
>>      
>>> On 2010-03-23 at 22:54:59 +0100, Andrew Morton<akpm@linux-foundation.org>  wrote:
>>>        
>>>> On Fri,  5 Mar 2010 17:52:23 +0100
>>>> Tobias Klauser<tklauser@distanz.ch>  wrote:
>>>>
>>>>          
>>>>> +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
>>>>> +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
>>>>>            
>>>> We seem to be missing a few things here.
>>>>
>>>> drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
>>>> drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
>>>> drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
>>>> drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
>>>> drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'
>>>>          
>>> These should usually be declared in a board specific header. There were
>>> compatibility macros in altera_uart.c which defined them to NOPs in case
>>> the board header did not properly define them. But I remove them as per
>>> request by Alan Cox (Message-ID: 20100301181920.3952c3e7@lxorguk.ukuu.org.uk).
>>>
>>> Should we add them again (maybe to altera_uart.h)? Or would it be better
>>> to define a config symbol which is set in the board specific Kconfig and
>>> altera_uart depends on it?
>>>        
>> I guess the latter.
>>
>> There should have been a real implementation of these in the patchset -
>> otherwise the code can't be used or tested.  Confused.
>>      
> Sorry for the confusion.
>
> The last patchset I submitted (with the functions removed from
> altera_uart.c) was tested on a local branch, where I added the macros to
> a global board specific header. I didn't include that one in the patch.
>
> After looking at the code and it's history a bit closer (and also on the
> nios2 specific part) I realised that this macro was probably added
> because the driver was originally based on drivers/serial/mcf.c (the
> macros are present there too).
>
> Also there are currently no board configurations known to me that define
> these macros. So I'd suggest to remove the usage of these macros
> alltogether. We could still add them again (to the board specific part
> and with the config option then) in case there will be a board
> configuration implementing DTR/DCD lines on GPIOs.
>
> Could anyone on nios2-dev verify that there are currently no such board
> configurations?
>
> I'd remove the usage of the macros then and post an updated patch.
>    
Maybe we can add pointers to functions for the DCD/DTR in the struct 
altera_uart_platform_uart. Then board config file can define them if 
they implement these pins, NULL/0 otherwise.

struct altera_uart_platform_uart {
...
     int (*getppdcd)(...);    /* get DCD status */
...

};


static unsigned int altera_uart_get_mctrl(struct uart_port *port)
{
...
     if (port->getppdcd)
         sigs |= (port->getppdcd(...)) ? TIOCM_CD : 0);


- Thomas


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-25  0:54             ` Thomas Chou
@ 2010-03-29 13:29               ` Tobias Klauser
  2010-03-30  0:18                 ` Thomas Chou
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Klauser @ 2010-03-29 13:29 UTC (permalink / raw)
  To: Thomas Chou
  Cc: Andrew Morton, linux-serial, gregkh, nios2-dev, linux-kernel, Alan Cox

On 2010-03-25 at 01:54:09 +0100, Thomas Chou <thomas@wytron.com.tw> wrote:
> On 03/25/2010 12:24 AM, Tobias Klauser wrote:
>> On 2010-03-24 at 12:05:27 +0100, Andrew Morton<akpm@linux-foundation.org>  wrote:
>>    
>>> On Wed, 24 Mar 2010 07:47:47 +0100 Tobias Klauser<tklauser@distanz.ch>  wrote:
>>>      
>>>> On 2010-03-23 at 22:54:59 +0100, Andrew Morton<akpm@linux-foundation.org>  wrote:
>>>>        
>>>>> On Fri,  5 Mar 2010 17:52:23 +0100
>>>>> Tobias Klauser<tklauser@distanz.ch>  wrote:
>>>>>
>>>>>          
>>>>>> +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
>>>>>> +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
>>>>>>            
>>>>> We seem to be missing a few things here.
>>>>>
>>>>> drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
>>>>> drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
>>>>> drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
>>>>> drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
>>>>> drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'
>>>>>          
>>>> These should usually be declared in a board specific header. There were
>>>> compatibility macros in altera_uart.c which defined them to NOPs in case
>>>> the board header did not properly define them. But I remove them as per
>>>> request by Alan Cox (Message-ID: 20100301181920.3952c3e7@lxorguk.ukuu.org.uk).
>>>>
>>>> Should we add them again (maybe to altera_uart.h)? Or would it be better
>>>> to define a config symbol which is set in the board specific Kconfig and
>>>> altera_uart depends on it?
>>>>        
>>> I guess the latter.
>>>
>>> There should have been a real implementation of these in the patchset -
>>> otherwise the code can't be used or tested.  Confused.
>>>      
>> Sorry for the confusion.
>>
>> The last patchset I submitted (with the functions removed from
>> altera_uart.c) was tested on a local branch, where I added the macros to
>> a global board specific header. I didn't include that one in the patch.
>>
>> After looking at the code and it's history a bit closer (and also on the
>> nios2 specific part) I realised that this macro was probably added
>> because the driver was originally based on drivers/serial/mcf.c (the
>> macros are present there too).
>>
>> Also there are currently no board configurations known to me that define
>> these macros. So I'd suggest to remove the usage of these macros
>> alltogether. We could still add them again (to the board specific part
>> and with the config option then) in case there will be a board
>> configuration implementing DTR/DCD lines on GPIOs.
>>
>> Could anyone on nios2-dev verify that there are currently no such board
>> configurations?
>>
>> I'd remove the usage of the macros then and post an updated patch.
>>    
> Maybe we can add pointers to functions for the DCD/DTR in the struct  
> altera_uart_platform_uart. Then board config file can define them if  
> they implement these pins, NULL/0 otherwise.

Unfortunately we then need to define them in struct altera_uart too and
set them in the probe function using the platform data.

For me that seems like a bit too much overhead just to support the
possibility of someone defining these functions in the future. I'd
prefer to get rid of the macros now for the mainline submission and then
add it later if someone would actually use these DCD/DTR get/set
functions. Hope that's OK for you too, Thomas.

-- Tobias

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 2/2] serial: Add driver for the Altera UART
  2010-03-29 13:29               ` Tobias Klauser
@ 2010-03-30  0:18                 ` Thomas Chou
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Chou @ 2010-03-30  0:18 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: Andrew Morton, linux-serial, gregkh, nios2-dev, linux-kernel, Alan Cox

On 03/29/2010 09:29 PM, Tobias Klauser wrote:
> On 2010-03-25 at 01:54:09 +0100, Thomas Chou<thomas@wytron.com.tw>  wrote:
>    
>> On 03/25/2010 12:24 AM, Tobias Klauser wrote:
>>      
>>> On 2010-03-24 at 12:05:27 +0100, Andrew Morton<akpm@linux-foundation.org>   wrote:
>>>
>>>        
>>>> On Wed, 24 Mar 2010 07:47:47 +0100 Tobias Klauser<tklauser@distanz.ch>   wrote:
>>>>
>>>>          
>>>>> On 2010-03-23 at 22:54:59 +0100, Andrew Morton<akpm@linux-foundation.org>   wrote:
>>>>>
>>>>>            
>>>>>> On Fri,  5 Mar 2010 17:52:23 +0100
>>>>>> Tobias Klauser<tklauser@distanz.ch>   wrote:
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
>>>>>>> +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
>>>>>>>
>>>>>>>                
>>>>>> We seem to be missing a few things here.
>>>>>>
>>>>>> drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
>>>>>> drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
>>>>>> drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
>>>>>> drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
>>>>>> drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'
>>>>>>
>>>>>>              
>>>>> These should usually be declared in a board specific header. There were
>>>>> compatibility macros in altera_uart.c which defined them to NOPs in case
>>>>> the board header did not properly define them. But I remove them as per
>>>>> request by Alan Cox (Message-ID: 20100301181920.3952c3e7@lxorguk.ukuu.org.uk).
>>>>>
>>>>> Should we add them again (maybe to altera_uart.h)? Or would it be better
>>>>> to define a config symbol which is set in the board specific Kconfig and
>>>>> altera_uart depends on it?
>>>>>
>>>>>            
>>>> I guess the latter.
>>>>
>>>> There should have been a real implementation of these in the patchset -
>>>> otherwise the code can't be used or tested.  Confused.
>>>>
>>>>          
>>> Sorry for the confusion.
>>>
>>> The last patchset I submitted (with the functions removed from
>>> altera_uart.c) was tested on a local branch, where I added the macros to
>>> a global board specific header. I didn't include that one in the patch.
>>>
>>> After looking at the code and it's history a bit closer (and also on the
>>> nios2 specific part) I realised that this macro was probably added
>>> because the driver was originally based on drivers/serial/mcf.c (the
>>> macros are present there too).
>>>
>>> Also there are currently no board configurations known to me that define
>>> these macros. So I'd suggest to remove the usage of these macros
>>> alltogether. We could still add them again (to the board specific part
>>> and with the config option then) in case there will be a board
>>> configuration implementing DTR/DCD lines on GPIOs.
>>>
>>> Could anyone on nios2-dev verify that there are currently no such board
>>> configurations?
>>>
>>> I'd remove the usage of the macros then and post an updated patch.
>>>
>>>        
>> Maybe we can add pointers to functions for the DCD/DTR in the struct
>> altera_uart_platform_uart. Then board config file can define them if
>> they implement these pins, NULL/0 otherwise.
>>      
> Unfortunately we then need to define them in struct altera_uart too and
> set them in the probe function using the platform data.
>
> For me that seems like a bit too much overhead just to support the
> possibility of someone defining these functions in the future. I'd
> prefer to get rid of the macros now for the mainline submission and then
> add it later if someone would actually use these DCD/DTR get/set
> functions. Hope that's OK for you too, Thomas.
>    
Hi Tobias,

OK. Please drop them. That would be easier.

Best regards,
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-03-30  0:15 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-05 16:52 [PATCHv3 0/2] serial: Drivers for Altera UARTs Tobias Klauser
2010-03-05 16:52 ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Tobias Klauser
2010-03-05 16:52   ` [PATCHv3 2/2] serial: Add driver for the Altera UART Tobias Klauser
2010-03-23 21:54     ` Andrew Morton
2010-03-24  6:47       ` Tobias Klauser
2010-03-24 11:05         ` Andrew Morton
2010-03-24 16:24           ` Tobias Klauser
2010-03-25  0:54             ` Thomas Chou
2010-03-29 13:29               ` Tobias Klauser
2010-03-30  0:18                 ` Thomas Chou
2010-03-12 20:48   ` [PATCHv3 1/2] serial: Add driver for the Altera JTAG UART Andrew Morton
2010-03-15 10:20     ` Tobias Klauser
2010-03-15 10:24       ` [Nios2-dev] " waltergoossens
2010-03-16  0:47     ` Thomas Chou
2010-03-16  0:47       ` Thomas Chou

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.