Re: [PATCH] serial: DCC(JTAG) serial and console emulation support

[matthieu castet <castet.matthieu@free.fr>]

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

Daniel Walker a écrit :
> On Fri, 2010-10-08 at 21:34 +0200, matthieu castet wrote:
>> Hi,
>>
>> for the record I attach a modified version of the original samsung one that I did.
>>
>> I improved a bit the reactivity, and did some hack to make polling in idle loop.
>> With recent kernel this should be done in a thread with SCHED_IDLE policy.
>>
>> I also supported board with irq for rx and tx (not sure if the original driver supported it).
> 
> Do you use the CONFIG_SERIAL_DCC_STDSERIAL feature?
> 
No, I never used it

Matthieu

PS : I attached a diff against the original version. Looking at it, I think the really interesting stuff is
the dcc_idle that should called in a thread with low priority. The reactivity of the workqueue was very bad IIRC.

Irq stuff is only supported by few cpu, so I am not sure it worth to merge it.

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

--- drivers/serial/dcc.c	2010-10-08 22:21:57.000000000 +0200
+++ drivers/parrot/serial/dcc.c	2010-10-08 22:24:07.000000000 +0200
@@ -6,6 +6,7 @@
  * DCC(JTAG1) protocol version for JTAG ICE/ICD Debuggers:
  * 	Copyright (C) 2003, 2004, 2005 Hyok S. Choi (hyok.choi@samsung.com)
  * 	SAMSUNG ELECTRONICS Co.,Ltd.
+ *  Copyright (C) 2008 Parrot SA by matthieu.castet@parrot.com
  *
  * 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
@@ -19,7 +20,6 @@
  *
  */
 
-#include <linux/config.h>
 #include <linux/module.h>
 #include <linux/tty.h>
 #include <linux/ioport.h>
@@ -35,21 +35,21 @@
 
 #include <linux/serial_core.h>
 
-/*
- * if real irq interrupt used for receiving character,
- * uncomment below line and modify the DCC_IRQ to correct one.
- * Unless polling method id used.
- */
-//#define DCC_IRQ_USED
-//#define DCC_IRQ	(INT_N_EXT0)
+/* for PORT_DCC_JTAG1 */
+#include <mach/parrot_kernel_ids.h>
+
+//#undef CONFIG_ARCH_PARROT
+/* XXX may be use a platform driver for this */
+#ifdef CONFIG_ARCH_PARROT
+#define DCC_IRQ_USED
+#define IRQ_RX 27
+#define IRQ_TX 28
+#else
+/* polling mode */
+#define IRQ_RX 0x12345678
+#define IRQ_TX 0
 
-#ifndef  DCC_IRQ_USED
-#define DCC_POLL_RUN		0
-#define DCC_POLL_STOP		1
-#define DCC_POLL_STOPPED	2
-static struct work_struct dcc_poll_task;
-static void dcc_poll(void *data);
-static int dcc_poll_state = DCC_POLL_STOPPED;
+#warning polling mode
 #endif
 
 #define UART_NR			1	/* we have only one JTAG port */
@@ -57,240 +57,359 @@
 #ifdef CONFIG_SERIAL_DCC_STDSERIAL
 /* use ttyS for emulation of standard serial driver */
 #define SERIAL_DCC_NAME		"ttyS"
+#define SERIAL_DCC_MAJOR	TTY_MAJOR
 #define SERIAL_DCC_MINOR	64
 #else
 /* use ttyJ0(128) */
 #define SERIAL_DCC_NAME		"ttyJ"
-#define SERIAL_DCC_MINOR	(64 + 64)
+#define SERIAL_DCC_MAJOR	204
+#define SERIAL_DCC_MINOR	186
 #endif
-#define SERIAL_DCC_MAJOR	TTY_MAJOR
 
 /* DCC Status Bits */
 #define DCC_STATUS_RX		(1 << 0)
 #define DCC_STATUS_TX		(1 << 1)
 
+/* end of JTAG1 dependencies */
 
-static void
-dcc_putchar(struct uart_port *port, int ch)
-{
-	while (__dcc_getstatus() & DCC_STATUS_TX)
-		cpu_relax();
-	__dcc_putchar((char)(ch & 0xFF));
+static void dcc_tx_chars(struct uart_port *port, int max_count);
+static void dcc_rx_chars(struct uart_port *port);
+#ifdef DCC_IRQ_USED /* real IRQ used */
+/* check if there is a char to read and when don't read too much char */
+#define dcc_tx_ready(port) \
+	((__dcc_getstatus() & DCC_STATUS_TX) == 0)
+
+enum {
+	TX_OFF,
+	TX_ON,
+};
+static int dcc_tx_enable;
+enum {
+	RX_OFF,
+	RX_ON,
+	RX_PAUSE,
+};
+static int dcc_rx_enable;
+/* port locked, interrupts locally disabled */
+static void dcc_start_tx(struct uart_port *port)
+{
+	if (dcc_tx_enable == TX_OFF) {
+		enable_irq(port->irq);
+		dcc_tx_enable = TX_ON;
+	}
 }
 
-static inline void
-xmit_string(struct uart_port *port, char *p, int len)
+/* port locked, interrupts locally disabled */
+static void dcc_stop_tx(struct uart_port *port)
 {
-	for ( ; len; len--, p++)
-		dcc_putchar(port, *p);
+	if (dcc_tx_enable == TX_ON) {
+		disable_irq(port->irq);
+		dcc_tx_enable = TX_OFF;
+	}
 }
 
-static inline void
-dcc_transmit_buffer(struct uart_port *port)
+/* port locked, interrupts locally disabled */
+static void dcc_stop_rx(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->info->xmit;
-	int pendings = uart_circ_chars_pending(xmit);
+	if (dcc_rx_enable == RX_ON) {
+		disable_irq((int)port->membase);
+	}
+	dcc_rx_enable = RX_OFF;
+}
 
-	if(pendings + xmit->tail > UART_XMIT_SIZE)
-	{
-		xmit_string(port, &(xmit->buf[xmit->tail]),
-			UART_XMIT_SIZE - xmit->tail);
-		xmit_string(port, &(xmit->buf[0]), xmit->head);
-	} else
-		xmit_string(port, &(xmit->buf[xmit->tail]), pendings);
+/* port locked, interrupts locally disabled */
+static void dcc_throttle_rx(struct uart_port *port, int stop)
+{
+	if (stop && dcc_rx_enable == RX_ON) {
+		disable_irq((int)port->membase);
+		dcc_rx_enable = RX_PAUSE;
+	}
+	else if (stop == 0 && dcc_rx_enable == RX_PAUSE) {
+		enable_irq((int)port->membase);
+		dcc_rx_enable = RX_ON;
+	}
+}
 	
-	xmit->tail = (xmit->tail + pendings) & (UART_XMIT_SIZE-1);
-	port->icount.tx += pendings;
+#infdef CONFIG_SERIAL_DCC_IDLE_POLL
+int dcc_idle(void)
+{
+	return 0;
+}
+#endif
 
-static inline void
-dcc_transmit_x_char(struct uart_port *port)
+static irqreturn_t dcc_int_rx(int irq, void *dev_id)
 {
-	dcc_putchar(port, port->x_char);
-	port->icount.tx++;
-	port->x_char = 0;
+	struct uart_port *port = dev_id;
+	spin_lock(&port->lock);
+	dcc_rx_chars(port);
+	spin_unlock(&port->lock);
+
+	return IRQ_HANDLED;
 }
 
-static void
-dcc_start_tx(struct uart_port *port)
+static irqreturn_t dcc_int_tx(int irq, void *dev_id)
 {
-	dcc_transmit_buffer(port);
+	struct uart_port *port = dev_id;
+	spin_lock(&port->lock);
+
+	dcc_tx_chars(port, 64);
+	spin_unlock(&port->lock);
+
+	return IRQ_HANDLED;
 }
 
-static inline void
-dcc_rx_chars(struct uart_port *port)
+static int dcc_startup(struct uart_port *port)
 {
-	unsigned char ch;
-	struct tty_struct *tty = port->info->tty;
+	int retval;
 
-	/*
-	 * check input.
-	 * checking JTAG flag is better to resolve the status test.
-	 * incount is NOT used for JTAG1 protocol.
-	 */
+	dcc_tx_enable = TX_ON;
+	dcc_rx_enable = RX_ON;
+	/* Allocate the IRQ */
+	retval = request_irq(port->irq, dcc_int_tx, IRQF_DISABLED,
+			     "serial_dcc_tx", port);
+	if (retval)
+		return retval;
 
-	if (__dcc_getstatus() & DCC_STATUS_RX)
-	{
-		/* for JTAG 1 protocol, incount is always 1. */
-		ch = __dcc_getchar();
+	/* Allocate the IRQ */
+	retval = request_irq((int)port->membase, dcc_int_rx, IRQF_DISABLED,
+			     "serial_dcc_rx", port);
+	if (retval)
+		return retval;
 
-		if (tty) {
-			tty_insert_flip_char(tty, ch, TTY_NORMAL);
-			port->icount.rx++;
-			tty_flip_buffer_push(tty);
-		}
-	}
+	return 0;
 }
 
-static inline void
-dcc_overrun_chars(struct uart_port *port)
+static void dcc_shutdown(struct uart_port *port)
 {
-	port->icount.overrun++;
+	free_irq(port->irq, port);
+	free_irq((int)port->membase, port);
+}
+#else /* emulation by scheduled work */
+static void dcc_poll(struct work_struct *work);
+static DECLARE_DELAYED_WORK(dcc_poll_task, dcc_poll);
+static struct uart_port dcc_port;
+static int dcc_active;
+
+/* in polling mode, we wait for the next character */
+static int dcc_tx_ready(struct uart_port *port) {
+	while (__dcc_getstatus() & DCC_STATUS_TX) {
+		/* while waiting, we can check rx : it is free */
+		dcc_rx_chars(port);
+		cpu_relax();
+	}
+	return 1;
 }
 
-static inline void
-dcc_tx_chars(struct uart_port *port)
+static void dcc_start_tx(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->info->xmit;
-
-	if (port->x_char) {
-		dcc_transmit_x_char(port);
-		return; 
-	}
+	dcc_tx_chars(port, 0xFFFF);
+}
 
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
-		return;
+static void dcc_stop_tx(struct uart_port *port)
+{
+}
 
-	dcc_transmit_buffer(port);
+static void dcc_throttle_rx(struct uart_port *port, int stop)
+{
+}
 
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
+static void dcc_stop_rx(struct uart_port *port)
+{
 }
 
-#ifdef DCC_IRQ_USED /* real IRQ used */
-static irqreturn_t
-dcc_int(int irq, void *dev_id, struct pt_regs *regs)
+/* you can call this on your idle loop instead of sleeping
+ */
+#ifdef CONFIG_SERIAL_DCC_IDLE_POLL
+int dcc_idle(void)
 {
-	struct uart_port *port = dev_id;
-	int handled = 0;
+	struct uart_port *port = &dcc_port;
+	if (dcc_active == 0)
+		return 0;
 
 	spin_lock(&port->lock);
 	
 	dcc_rx_chars(port);
-	dcc_tx_chars(port);
+	dcc_tx_chars(port, 64);
+	dcc_rx_chars(port);
 
-	handled = 1;
 	spin_unlock(&port->lock);
-	
-	return IRQ_RETVAL(handled);
+	return 0;
 }
-#else /* emulation by scheduled work */
-static void
-dcc_poll(void *data)
+#endif
+
+/* we poll dcc every jiffies */
+static void dcc_poll(struct work_struct *work)
 {
-	struct uart_port *port = data;
+	struct uart_port *port = &dcc_port;
 
 	spin_lock(&port->lock);
 	
-	if (dcc_poll_state != DCC_POLL_RUN) {
-		dcc_poll_state = DCC_POLL_STOPPED;
-		goto dcc_poll_stop;
-	}
-
 	dcc_rx_chars(port);
-	dcc_tx_chars(port);
+	dcc_tx_chars(port, 0xFFFF);
+	dcc_rx_chars(port);
 
 	schedule_delayed_work(&dcc_poll_task, 1);
 
-dcc_poll_stop:
 	spin_unlock(&port->lock);
 }
+static int dcc_startup(struct uart_port *port)
+{
+	/* shcedule the polling work */
+	schedule_delayed_work(&dcc_poll_task, 1);
+	dcc_active = 1;
+
+	return 0;
+}
+
+static void dcc_shutdown(struct uart_port *port)
+{
+	cancel_rearming_delayed_work(&dcc_poll_task);
+	dcc_active = 0;
+}
 #endif /* end of DCC_IRQ_USED */
 
-static unsigned int
-dcc_tx_empty(struct uart_port *port)
+
+static void dcc_putchar(struct uart_port *port, int ch)
 {
-	return TIOCSER_TEMT;
+	while (__dcc_getstatus() & DCC_STATUS_TX)
+		cpu_relax();
+	__dcc_putchar((char)(ch & 0xFF));
 }
 
-static unsigned int
-dcc_get_mctrl(struct uart_port *port)
+static void dcc_rx_chars(struct uart_port *port)
 {
-	return TIOCM_CTS | TIOCM_DSR | TIOCM_CD;
+	unsigned char ch;
+	/* max char to send */
+	int count = 64;
+	struct tty_struct *tty = port->info->port.tty;
+
+	/*
+	 * check input.
+	 * checking JTAG flag is better to resolve the status test.
+	 * incount is NOT used for JTAG1 protocol.
+	 */
+
+	while (__dcc_getstatus() & DCC_STATUS_RX && count-- > 0) {
+		/* for JTAG 1 protocol, incount is always 1. */
+		ch = __dcc_getchar();
+
+		tty_insert_flip_char(tty, ch, TTY_NORMAL);
+		port->icount.rx++;
+	}
+	tty_flip_buffer_push(tty);
 }
 
-static int
-dcc_startup(struct uart_port *port)
+static void dcc_tx_chars(struct uart_port *port, int max_count)
 {
-#ifdef DCC_IRQ_USED /* real IRQ used */
-	int retval;
+	struct circ_buf *xmit = &port->info->xmit;
 
-	/* Allocate the IRQ */
-	retval = request_irq(port->irq, dcc_int, SA_INTERRUPT,
-			     "serial_dcc", port);
-	if (retval)
-		return retval;
-#else /* emulation */
-	/* Initialize the work, and shcedule it. */
-	INIT_WORK(&dcc_poll_task, dcc_poll, port);
-	spin_lock(&port->lock);
-	if (dcc_poll_state != DCC_POLL_RUN)
-		dcc_poll_state = DCC_POLL_RUN;
-	schedule_delayed_work(&dcc_poll_task, 1);
-	spin_unlock(&port->lock);
-#endif
+	if (port->x_char) {
+		dcc_putchar(port, port->x_char);
+		port->icount.tx++;
+		port->x_char = 0;
+		goto out;
+	}
+	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
+		dcc_stop_tx(port);
+		goto out;
+	}
 
-	return 0;
+	while (!uart_circ_empty(xmit) && dcc_tx_ready(port)
+			&& max_count-- > 0) {
+		__dcc_putchar(xmit->buf[xmit->tail]);
+		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 (uart_circ_empty(xmit)) {
+		dcc_stop_tx(port);
+	}
+
+out:
+	return;
 }
 
-static void
-dcc_shutdown(struct uart_port *port)
+static unsigned int dcc_tx_empty(struct uart_port *port)
 {
-#ifdef DCC_IRQ_USED /* real IRQ used */
-	free_irq(port->irq, port);
-#else
-	spin_lock(&port->lock);
-	dcc_poll_state = DCC_POLL_STOP;
-	spin_unlock(&port->lock);
-#endif
+	return (__dcc_getstatus() & DCC_STATUS_TX) ? 0:TIOCSER_TEMT;
+}
+
+static unsigned int dcc_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CTS | TIOCM_DSR | TIOCM_CD;
+}
+
+/* need for throttle/unthrottle */
+static void dcc_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* throttle clear TIOCM_RTS, unthrottle set it */
+	dcc_throttle_rx(port, !(mctrl & TIOCM_RTS));
 }
 
-static void
-dcc_set_termios(struct uart_port *port, struct termios *termios,
-		   struct termios *old)
+static void dcc_set_termios(struct uart_port *port, struct ktermios *termios,
+		   struct ktermios *old)
 {
-#ifdef DCC_IRQ_USED
 	unsigned long flags;
-#endif
 	unsigned int baud, quot;
 
 	/*
@@ -311,25 +430,19 @@
 	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16); 
 	quot = uart_get_divisor(port, baud);
 
-#ifdef DCC_IRQ_USED
 	spin_lock_irqsave(&port->lock, flags);
-#endif
 
 	uart_update_timeout(port, termios->c_cflag, baud);
 
-#ifdef DCC_IRQ_USED
 	spin_unlock_irqrestore(&port->lock, flags);
-#endif
 }
 
@@ -363,28 +474,23 @@
 }
 
 /* dummy operation handlers for uart_ops */
-static void
-dcc_dummy_ops(struct uart_port *port)
+static void dcc_dummy_ops(struct uart_port *port)
 {
 }
-static void
-dcc_dummy_ops_ui(struct uart_port *port, unsigned int ui)
-{
-}
-static void
-dcc_dummy_ops_i(struct uart_port *port, int i)
+
+static void dcc_break_ctl(struct uart_port *port, int i)
 {
 }
 
 static struct uart_ops dcc_pops = {
 	.tx_empty	= dcc_tx_empty,
-	.set_mctrl	= dcc_dummy_ops_ui,
+	.set_mctrl	= dcc_set_mctrl,
 	.get_mctrl	= dcc_get_mctrl,
-	.stop_tx	= dcc_dummy_ops,
+	.stop_tx	= dcc_stop_tx,
 	.start_tx	= dcc_start_tx,
-	.stop_rx	= dcc_dummy_ops,
+	.stop_rx	= dcc_stop_rx,
 	.enable_ms	= dcc_dummy_ops,
-	.break_ctl	= dcc_dummy_ops_i,
+	.break_ctl	= dcc_break_ctl,
 	.startup	= dcc_startup,
 	.shutdown	= dcc_shutdown,
 	.set_termios	= dcc_set_termios,
@@ -396,14 +502,10 @@
 };
 
 static struct uart_port dcc_port = {
-	.membase	= (char*)0x12345678,	/* we need these garbages */
 	.mapbase	= 0x12345678,		/* for serial_core.c */
 	.iotype		= UPIO_MEM,	
-#ifdef DCC_IRQ_USED
-	.irq		= DCC_IRQ,
-#else
-	.irq		= 0,
-#endif
+	.membase	= (char*)IRQ_RX,	/* we need these garbages */
+	.irq		= IRQ_TX,
 	.uartclk	= 14745600,
 	.fifosize	= 0,
 	.ops		= &dcc_pops,