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

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

[Daniel Walker]

Many of JTAG debuggers for ARM support DCC protocol over JTAG
connection, which is very useful to debug hardwares which has no
serial port. This patch adds DCC serial emulation and the console
support. System timer based polling method is used for the
emulation of serial input interrupt handling.

Most of the code was taken from Hyok S. Choi original work, but the
inline assmebly needed some work and updating. It now supports ARMv7.
Also the description above is from Hyok also.

CC: Hyok S. Choi <hyok.choi@samsung.com>
CC: Tony Lindgren <tony@atomide.com>
Signed-off-by: Jeff Ohlstein <johlstei@quicinc.com>
Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
---
 drivers/serial/Kconfig      |   31 +++
 drivers/serial/Makefile     |    1 +
 drivers/serial/dcc.c        |  432 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/serial_core.h |    3 +
 4 files changed, 467 insertions(+), 0 deletions(-)
 create mode 100644 drivers/serial/dcc.c

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 12900f7..a54a57c 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -495,6 +495,37 @@ config SERIAL_S3C2400
 	help
 	  Serial port support for the Samsung S3C2400 SoC
 
+config SERIAL_DCC
+	bool "JTAG ICE/ICD DCC serial port emulation support"
+	depends on ARM
+	select SERIAL_CORE
+	help
+	  This selects serial port emulation driver for ICE/ICD JTAG debugger
+	  (e.g. Trace32) for ARM architecture. You should make an terminal with
+	  DCC(JTAG1) protocol.
+
+	  if unsure, say N.
+
+config SERIAL_DCC_CONSOLE
+	bool "Support for console on JTAG ICE/ICD DCC"
+	depends on SERIAL_DCC
+	select SERIAL_CORE_CONSOLE
+	help
+	  Say Y here if you wish to use ICE/ICD JTAG DCC serial port emulation
+	  as the system console.
+
+	  if unsure, say N.
+
+config SERIAL_DCC_STDSERIAL
+	bool "Install JTAG ICE/ICD DCC as standard serial"
+	default y
+	depends on !SERIAL_8250 && SERIAL_DCC
+	help
+	  Say Y here if you want to install DCC driver as a normal serial port
+	  /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
+	  (major 4, minor 128) and can co-exist with other UARTs, such as
+	  8250/16C550 compatibles.
+
 config SERIAL_S3C2410
 	tristate "Samsung S3C2410 Serial port support"
 	depends on SERIAL_SAMSUNG && CPU_S3C2410
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 1ca4fd5..896c3f6 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_SERIAL_8250_MCA) += 8250_mca.o
 obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o
 obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o
 obj-$(CONFIG_SERIAL_CLPS711X) += clps711x.o
+obj-$(CONFIG_SERIAL_DCC) += dcc.o
 obj-$(CONFIG_SERIAL_PXA) += pxa.o
 obj-$(CONFIG_SERIAL_PNX8XXX) += pnx8xxx_uart.o
 obj-$(CONFIG_SERIAL_SA1100) += sa1100.o
diff --git a/drivers/serial/dcc.c b/drivers/serial/dcc.c
new file mode 100644
index 0000000..233caf7
--- /dev/null
+++ b/drivers/serial/dcc.c
@@ -0,0 +1,432 @@
+/*
+ *  linux/drivers/serial/dcc.c
+ *
+ *  serial port emulation driver for the JTAG DCC Terminal.
+ *
+ * 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.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ *  Changelog:
+ *   Oct-2003 Hyok S. Choi	Created
+ *   Feb-2004 Hyok S. Choi	Updated for serial_core.c and 2.6 kernel
+ *   Mar-2005 Hyok S. Choi	renamed from T32 to DCC
+ *   Apr-2006 Hyok S. Choi	revised including the MAJOR number
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/tty.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/serial.h>
+#include <linux/console.h>
+#include <linux/sysrq.h>
+#include <linux/tty_flip.h>
+#include <linux/major.h>
+
+#include <linux/io.h>
+#include <linux/irq.h>
+
+#include <linux/serial_core.h>
+
+#define DCC_POLL_RUN		0
+#define DCC_POLL_STOP		1
+#define DCC_POLL_STOPPED	2
+
+static struct uart_port dcc_port;
+static struct delayed_work dcc_poll_task;
+static void dcc_poll(struct work_struct *work);
+static int dcc_poll_state = DCC_POLL_STOPPED;
+
+#define UART_NR			1	/* we have only one JTAG port */
+
+#ifdef CONFIG_SERIAL_DCC_STDSERIAL
+/* use ttyS for emulation of standard serial driver */
+#define SERIAL_DCC_NAME		"ttyS"
+#define SERIAL_DCC_MINOR	64
+#else
+/* use ttyJ0(128) */
+#define SERIAL_DCC_NAME		"ttyJ"
+#define SERIAL_DCC_MINOR	(64 + 64)
+#endif
+#define SERIAL_DCC_MAJOR	TTY_MAJOR
+
+/* DCC Status Bits */
+#define DCC_STATUS_RX		(1 << 30)
+#define DCC_STATUS_TX		(1 << 29)
+
+static inline u32 __dcc_getstatus(void)
+{
+	u32 __ret;
+
+	asm("mrc p14, 0, %0, c0, c1, 0	@ read comms ctrl reg"
+		: "=r" (__ret) : : "cc");
+
+	return __ret;
+}
+
+
+#if defined(CONFIG_CPU_V7)
+static inline char __dcc_getchar(void)
+{
+	char __c;
+
+	asm("get_wait:	mrc p14, 0, pc, c0, c1, 0                          \n\
+			bne get_wait                                       \n\
+			mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
+		: "=r" (__c) : : "cc");
+
+	return __c;
+}
+#else
+static inline char __dcc_getchar(void)
+{
+	char __c;
+
+	asm("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
+		: "=r" (__c));
+
+	return __c;
+}
+#endif
+
+#if defined(CONFIG_CPU_V7)
+static inline void __dcc_putchar(char c)
+{
+	asm("put_wait:	mrc p14, 0, pc, c0, c1, 0                 \n\
+			bcs put_wait                              \n\
+			mcr p14, 0, %0, c0, c5, 0                   "
+	: : "r" (c) : "cc");
+}
+#else
+static inline void __dcc_putchar(char c)
+{
+	asm("mcr p14, 0, %0, c0, c5, 0	@ write a char"
+		: /* no output register */
+		: "r" (c));
+}
+#endif
+
+static void dcc_putchar(struct uart_port *port, int ch)
+{
+	while (__dcc_getstatus() & DCC_STATUS_TX)
+		cpu_relax();
+	__dcc_putchar((char)(ch & 0xFF));
+}
+
+static inline void xmit_string(struct uart_port *port, char *p, int len)
+{
+	for ( ; len; len--, p++)
+		dcc_putchar(port, *p);
+}
+
+static inline void dcc_transmit_buffer(struct uart_port *port)
+{
+	struct circ_buf *xmit = &port->state->xmit;
+	int pendings = uart_circ_chars_pending(xmit);
+
+	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);
+
+	xmit->tail = (xmit->tail + pendings) & (UART_XMIT_SIZE-1);
+	port->icount.tx += pendings;
+}
+
+static inline void dcc_transmit_x_char(struct uart_port *port)
+{
+	dcc_putchar(port, port->x_char);
+	port->icount.tx++;
+	port->x_char = 0;
+}
+
+static void dcc_start_tx(struct uart_port *port)
+{
+	dcc_transmit_buffer(port);
+}
+
+static inline void dcc_rx_chars(struct uart_port *port)
+{
+	unsigned char ch;
+	struct tty_struct *tty = port->state->port.tty;
+
+	/*
+	 * check input.
+	 * checking JTAG flag is better to resolve the status test.
+	 * incount is NOT used for JTAG1 protocol.
+	 */
+
+	if (__dcc_getstatus() & DCC_STATUS_RX) {
+
+		/* for JTAG 1 protocol, incount is always 1. */
+		ch = __dcc_getchar();
+
+		if (tty) {
+			tty_insert_flip_char(tty, ch, TTY_NORMAL);
+			port->icount.rx++;
+			tty_flip_buffer_push(tty);
+		}
+	}
+}
+
+static inline void dcc_overrun_chars(struct uart_port *port)
+{
+	port->icount.overrun++;
+}
+
+static inline void dcc_tx_chars(struct uart_port *port)
+{
+	struct circ_buf *xmit = &port->state->xmit;
+
+	if (port->x_char) {
+		dcc_transmit_x_char(port);
+		return;
+	}
+
+	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
+		return;
+
+	dcc_transmit_buffer(port);
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(port);
+}
+
+static void dcc_poll(struct work_struct *work)
+{
+	spin_lock(&dcc_port.lock);
+
+	if (dcc_poll_state != DCC_POLL_RUN) {
+		dcc_poll_state = DCC_POLL_STOPPED;
+		goto dcc_poll_stop;
+	}
+
+	dcc_rx_chars(&dcc_port);
+	dcc_tx_chars(&dcc_port);
+
+	schedule_delayed_work(&dcc_poll_task, 1);
+
+dcc_poll_stop:
+	spin_unlock(&dcc_port.lock);
+}
+
+static unsigned int dcc_tx_empty(struct uart_port *port)
+{
+	return TIOCSER_TEMT;
+}
+
+static unsigned int dcc_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CTS | TIOCM_DSR | TIOCM_CD;
+}
+
+static int dcc_startup(struct uart_port *port)
+{
+	/* Initialize the work, and shcedule it. */
+	INIT_DELAYED_WORK(&dcc_poll_task, dcc_poll);
+	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);
+
+	return 0;
+}
+
+static void dcc_shutdown(struct uart_port *port)
+{
+	spin_lock(&port->lock);
+	dcc_poll_state = DCC_POLL_STOP;
+	spin_unlock(&port->lock);
+}
+
+static void
+dcc_set_termios(struct uart_port *port, struct ktermios *termios,
+		struct ktermios *old)
+{
+	unsigned int baud, quot;
+
+	/*
+	 * We don't support parity, stop bits, or anything other
+	 * than 8 bits, so clear these termios flags.
+	 */
+	termios->c_cflag &= ~(CSIZE | CSTOPB | PARENB | PARODD | CREAD);
+	termios->c_cflag |= CS8;
+
+	/*
+	 * We don't appear to support any error conditions either.
+	 */
+	termios->c_iflag &= ~(INPCK | IGNPAR | IGNBRK | BRKINT);
+
+	/*
+	 * Ask the core to calculate the divisor for us.
+	 */
+	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16);
+	quot = uart_get_divisor(port, baud);
+
+	uart_update_timeout(port, termios->c_cflag, baud);
+
+}
+
+static const char * dcc_type(struct uart_port *port)
+{
+	return port->type == PORT_DCC_JTAG1 ? "DCC" : NULL;
+}
+
+static int dcc_request_port(struct uart_port *port)
+{
+	return 0;
+}
+
+/*
+ * Configure/autoconfigure the port.
+ */
+static void dcc_config_port(struct uart_port *port, int flags)
+{
+	if (flags & UART_CONFIG_TYPE) {
+		port->type = PORT_DCC_JTAG1;
+		dcc_request_port(port);
+	}
+}
+
+/*
+ * verify the new serial_struct (for TIOCSSERIAL).
+ */
+static int dcc_verify_port(struct uart_port *port, struct serial_struct *ser)
+{
+	int ret = 0;
+	if (ser->type != PORT_UNKNOWN && ser->type != PORT_DCC_JTAG1)
+		ret = -EINVAL;
+	if (ser->irq < 0 || ser->irq >= NR_IRQS)
+		ret = -EINVAL;
+	if (ser->baud_base < 9600)
+		ret = -EINVAL;
+	return ret;
+}
+
+/* dummy operation handlers for uart_ops */
+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 struct uart_ops dcc_pops = {
+	.tx_empty	= dcc_tx_empty,
+	.set_mctrl	= dcc_dummy_ops_ui,
+	.get_mctrl	= dcc_get_mctrl,
+	.stop_tx	= dcc_dummy_ops,
+	.start_tx	= dcc_start_tx,
+	.stop_rx	= dcc_dummy_ops,
+	.enable_ms	= dcc_dummy_ops,
+	.break_ctl	= dcc_dummy_ops_i,
+	.startup	= dcc_startup,
+	.shutdown	= dcc_shutdown,
+	.set_termios	= dcc_set_termios,
+	.type		= dcc_type,
+	.release_port	= dcc_dummy_ops,
+	.request_port	= dcc_request_port,
+	.config_port	= dcc_config_port,
+	.verify_port	= dcc_verify_port,
+};
+
+static struct uart_port dcc_port = {
+	.membase	= (char *)0x12345678,	/* we need these garbages */
+	.mapbase	= 0x12345678,		/* for serial_core.c */
+	.iotype		= UPIO_MEM,
+	.irq		= 0,
+	.uartclk	= 14745600,
+	.fifosize	= 0,
+	.ops		= &dcc_pops,
+	.flags		= UPF_BOOT_AUTOCONF,
+	.line		= 0,
+};
+
+#ifdef CONFIG_SERIAL_DCC_CONSOLE
+static void
+dcc_console_write(struct console *co, const char *s, unsigned int count)
+{
+	uart_console_write(&dcc_port, s, count, dcc_putchar);
+}
+
+static int __init dcc_console_setup(struct console *co, char *options)
+{
+	struct uart_port *port = &dcc_port;
+	int baud = 9600;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+	return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver dcc_reg;
+static struct console dcc_console = {
+	.name		= SERIAL_DCC_NAME,
+	.write		= dcc_console_write,
+	.device		= uart_console_device,
+	.setup		= dcc_console_setup,
+	.flags		= CON_PRINTBUFFER,
+	.index		= -1,
+	.data		= &dcc_reg,
+};
+
+static int __init dcc_console_init(void)
+{
+	register_console(&dcc_console);
+	return 0;
+}
+console_initcall(dcc_console_init);
+
+#define DCC_CONSOLE	&dcc_console
+#else
+#define DCC_CONSOLE	NULL
+#endif
+
+static struct uart_driver dcc_reg = {
+	.owner		= THIS_MODULE,
+	.driver_name	= SERIAL_DCC_NAME,
+	.dev_name	= SERIAL_DCC_NAME,
+	.major		= SERIAL_DCC_MAJOR,
+	.minor		= SERIAL_DCC_MINOR,
+	.nr		= UART_NR,
+	.cons		= DCC_CONSOLE,
+};
+
+static int __init dcc_init(void)
+{
+	int ret;
+
+	printk(KERN_INFO "DCC: JTAG1 Serial emulation driver\n");
+
+	ret = uart_register_driver(&dcc_reg);
+
+	if (ret)
+		return ret;
+
+	uart_add_one_port(&dcc_reg, &dcc_port);
+
+	return 0;
+}
+
+device_initcall(dcc_init);
+
+MODULE_DESCRIPTION("DCC(JTAG1) JTAG debugger console emulation driver");
+MODULE_AUTHOR("Hyok S. Choi <hyok.choi@samsung.com>");
+MODULE_SUPPORTED_DEVICE(SERIAL_DCC_NAME);
+MODULE_LICENSE("GPL");
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 563e234..a360c3a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -196,6 +196,9 @@
 /* High Speed UART for Medfield */
 #define PORT_MFD	95
 
+/* DCC(JTAG) emulation port types */
+#define PORT_DCC_JTAG1	96
+
 #ifdef __KERNEL__
 
 #include <linux/compiler.h>
-- 
1.7.1

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 14:36, Daniel Walker wrote:
> Many of JTAG debuggers for ARM support DCC protocol over JTAG
> connection, which is very useful to debug hardwares which has no
> serial port. This patch adds DCC serial emulation and the console
> support. System timer based polling method is used for the
> emulation of serial input interrupt handling.

why use serial_core at all ?  seems like you could just use the tty
layer directly.  i did that with drivers/char/bfin_jtag_comm.c.

although, there's the suggestion in the air that my jtag driver could
be converted to the HVC framework to even further simplify things.
but no one thinks it should be serial_core :).
-mike

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

[Daniel Walker]

On Thu, 2010-10-07 at 15:25 -0400, Mike Frysinger wrote:
> On Thu, Oct 7, 2010 at 14:36, Daniel Walker wrote:
> > Many of JTAG debuggers for ARM support DCC protocol over JTAG
> > connection, which is very useful to debug hardwares which has no
> > serial port. This patch adds DCC serial emulation and the console
> > support. System timer based polling method is used for the
> > emulation of serial input interrupt handling.
> 
> why use serial_core at all ?  seems like you could just use the tty
> layer directly.  i did that with drivers/char/bfin_jtag_comm.c.
> 
> although, there's the suggestion in the air that my jtag driver could
> be converted to the HVC framework to even further simplify things.
> but no one thinks it should be serial_core :).
> -mike

It will actually create a serial port on ttyS* if you want it to.. It
could be debatable if that's useful, but I didn't rip that out because
Hyok had that in his original code (he thought it was useful) ..

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 15:39, Daniel Walker wrote:
> On Thu, 2010-10-07 at 15:25 -0400, Mike Frysinger wrote:
>> On Thu, Oct 7, 2010 at 14:36, Daniel Walker wrote:
>> > Many of JTAG debuggers for ARM support DCC protocol over JTAG
>> > connection, which is very useful to debug hardwares which has no
>> > serial port. This patch adds DCC serial emulation and the console
>> > support. System timer based polling method is used for the
>> > emulation of serial input interrupt handling.
>>
>> why use serial_core at all ?  seems like you could just use the tty
>> layer directly.  i did that with drivers/char/bfin_jtag_comm.c.
>>
>> although, there's the suggestion in the air that my jtag driver could
>> be converted to the HVC framework to even further simplify things.
>> but no one thinks it should be serial_core :).
>
> It will actually create a serial port on ttyS* if you want it to.. It
> could be debatable if that's useful, but I didn't rip that out because
> Hyok had that in his original code (he thought it was useful) ..

i dont think hijacking the "ttyS" namespace is acceptable, even if
it's behind a Kconfig.  other people have tried in the past (myself
included) and been shot down.  sounds like a shim for crappy userspace
apps that blindly assume /dev/ttyS*.  is that really an issue anymore
though now that things like /dev/ttyUSB* are so common and people are
used to picking between multiple serial sources ?
-mike

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

[Daniel Walker]

On Thu, 2010-10-07 at 15:48 -0400, Mike Frysinger wrote:

> i dont think hijacking the "ttyS" namespace is acceptable, even if
> it's behind a Kconfig.  other people have tried in the past (myself
> included) and been shot down.  sounds like a shim for crappy userspace
> apps that blindly assume /dev/ttyS*.  is that really an issue anymore
> though now that things like /dev/ttyUSB* are so common and people are
> used to picking between multiple serial sources ?

It also creates a ttyJ* which uses the same interfaces as the code for
ttyS* .. I don't think allowing a Kconfig option to shim this in as a
ttyS* is all that bad .. This driver is only going to be used in rare
cases for debugging , having a ttyS* is just a level of flexibility ..

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 15:58, Daniel Walker wrote:
> On Thu, 2010-10-07 at 15:48 -0400, Mike Frysinger wrote:
>> i dont think hijacking the "ttyS" namespace is acceptable, even if
>> it's behind a Kconfig.  other people have tried in the past (myself
>> included) and been shot down.  sounds like a shim for crappy userspace
>> apps that blindly assume /dev/ttyS*.  is that really an issue anymore
>> though now that things like /dev/ttyUSB* are so common and people are
>> used to picking between multiple serial sources ?
>
> It also creates a ttyJ* which uses the same interfaces as the code for
> ttyS* .. I don't think allowing a Kconfig option to shim this in as a
> ttyS* is all that bad .. This driver is only going to be used in rare
> cases for debugging , having a ttyS* is just a level of flexibility ..

how is that any different from:
ln -s ttyJ0 /dev/ttyS0
-mike

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

[Daniel Walker]

On Thu, 2010-10-07 at 16:02 -0400, Mike Frysinger wrote:

> how is that any different from:
> ln -s ttyJ0 /dev/ttyS0
> -mike

It has the same major and minors as ttyS* does. So you don't have to run
anything on the target.

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 16:06, Daniel Walker wrote:
> On Thu, 2010-10-07 at 16:02 -0400, Mike Frysinger wrote:
>> how is that any different from:
>> ln -s ttyJ0 /dev/ttyS0
>
> It has the same major and minors as ttyS* does. So you don't have to run
> anything on the target.

i dont see how those things are related.  the major/minor are
irrelevant, unless you've already hard coded these in some app that
creates device nodes manually (instead of mdev/udev), but even then
that's something that "needs to be run on the target".  and both
already have config support to transparently do something like
"symlink ttyS# to XXX" as XXX is created.
-mike

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

[Daniel Walker]

On Thu, 2010-10-07 at 16:47 -0400, Mike Frysinger wrote:
> On Thu, Oct 7, 2010 at 16:06, Daniel Walker wrote:
> > On Thu, 2010-10-07 at 16:02 -0400, Mike Frysinger wrote:
> >> how is that any different from:
> >> ln -s ttyJ0 /dev/ttyS0
> >
> > It has the same major and minors as ttyS* does. So you don't have to run
> > anything on the target.
> 
> i dont see how those things are related.  the major/minor are
> irrelevant, unless you've already hard coded these in some app that
> creates device nodes manually (instead of mdev/udev), but even then
> that's something that "needs to be run on the target".  and both
> already have config support to transparently do something like
> "symlink ttyS# to XXX" as XXX is created.

Something does ultimately run on all targets, the issues is does the
person debugging have the ability to change it .. It's not always the
case that they can change it. The userspace may not have udev at all.
The userspace may consist of a single binary .. I'm not making any
assumptions about the userspace here, it can be anything.

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 16:59, Daniel Walker wrote:
> On Thu, 2010-10-07 at 16:47 -0400, Mike Frysinger wrote:
>> On Thu, Oct 7, 2010 at 16:06, Daniel Walker wrote:
>> > On Thu, 2010-10-07 at 16:02 -0400, Mike Frysinger wrote:
>> >> how is that any different from:
>> >> ln -s ttyJ0 /dev/ttyS0
>> >
>> > It has the same major and minors as ttyS* does. So you don't have to run
>> > anything on the target.
>>
>> i dont see how those things are related.  the major/minor are
>> irrelevant, unless you've already hard coded these in some app that
>> creates device nodes manually (instead of mdev/udev), but even then
>> that's something that "needs to be run on the target".  and both
>> already have config support to transparently do something like
>> "symlink ttyS# to XXX" as XXX is created.
>
> Something does ultimately run on all targets, the issues is does the
> person debugging have the ability to change it .. It's not always the
> case that they can change it. The userspace may not have udev at all.
> The userspace may consist of a single binary .. I'm not making any
> assumptions about the userspace here, it can be anything.

then there is already a static rootfs with pre-populated /dev which
would be fixed, or the single binary fixed.  if the options are "my
system works" or "my system doesnt work", i cant imagine people would
continue to attempt to use "/dev/ttyS#" if it didnt exist or didnt
work.
-mike

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

[Daniel Walker]

On Thu, 2010-10-07 at 17:05 -0400, Mike Frysinger wrote:
> On Thu, Oct 7, 2010 at 16:59, Daniel Walker wrote:
> > On Thu, 2010-10-07 at 16:47 -0400, Mike Frysinger wrote:
> >> On Thu, Oct 7, 2010 at 16:06, Daniel Walker wrote:
> >> > On Thu, 2010-10-07 at 16:02 -0400, Mike Frysinger wrote:
> >> >> how is that any different from:
> >> >> ln -s ttyJ0 /dev/ttyS0
> >> >
> >> > It has the same major and minors as ttyS* does. So you don't have to run
> >> > anything on the target.
> >>
> >> i dont see how those things are related.  the major/minor are
> >> irrelevant, unless you've already hard coded these in some app that
> >> creates device nodes manually (instead of mdev/udev), but even then
> >> that's something that "needs to be run on the target".  and both
> >> already have config support to transparently do something like
> >> "symlink ttyS# to XXX" as XXX is created.
> >
> > Something does ultimately run on all targets, the issues is does the
> > person debugging have the ability to change it .. It's not always the
> > case that they can change it. The userspace may not have udev at all.
> > The userspace may consist of a single binary .. I'm not making any
> > assumptions about the userspace here, it can be anything.
> 
> then there is already a static rootfs with pre-populated /dev which
> would be fixed, or the single binary fixed.  if the options are "my
> system works" or "my system doesnt work", i cant imagine people would
> continue to attempt to use "/dev/ttyS#" if it didnt exist or didnt
> work.

Ideally you would want this driver to work in any situation .. If it's
setup to use ttyS0 for debugging, even if it doesn't exist, then this
driver would be able to stand in for that interface.

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 17:17, Daniel Walker wrote:
> On Thu, 2010-10-07 at 17:05 -0400, Mike Frysinger wrote:
>> On Thu, Oct 7, 2010 at 16:59, Daniel Walker wrote:
>> > On Thu, 2010-10-07 at 16:47 -0400, Mike Frysinger wrote:
>> >> On Thu, Oct 7, 2010 at 16:06, Daniel Walker wrote:
>> >> > On Thu, 2010-10-07 at 16:02 -0400, Mike Frysinger wrote:
>> >> >> how is that any different from:
>> >> >> ln -s ttyJ0 /dev/ttyS0
>> >> >
>> >> > It has the same major and minors as ttyS* does. So you don't have to run
>> >> > anything on the target.
>> >>
>> >> i dont see how those things are related.  the major/minor are
>> >> irrelevant, unless you've already hard coded these in some app that
>> >> creates device nodes manually (instead of mdev/udev), but even then
>> >> that's something that "needs to be run on the target".  and both
>> >> already have config support to transparently do something like
>> >> "symlink ttyS# to XXX" as XXX is created.
>> >
>> > Something does ultimately run on all targets, the issues is does the
>> > person debugging have the ability to change it .. It's not always the
>> > case that they can change it. The userspace may not have udev at all.
>> > The userspace may consist of a single binary .. I'm not making any
>> > assumptions about the userspace here, it can be anything.
>>
>> then there is already a static rootfs with pre-populated /dev which
>> would be fixed, or the single binary fixed.  if the options are "my
>> system works" or "my system doesnt work", i cant imagine people would
>> continue to attempt to use "/dev/ttyS#" if it didnt exist or didnt
>> work.
>
> Ideally you would want this driver to work in any situation .. If it's
> setup to use ttyS0 for debugging, even if it doesn't exist, then this
> driver would be able to stand in for that interface.

i dont think that's the case.  "any situation" is way too vague and
invites mounds of crap to be included in the kernel which really
should be in userspace.  ive never had a problem with my embedded work
using ttyBF# or ttyBFJC# or ttySS# for the Blackfin serial devices,
nor have i heard customers complain that the file absolutely must be
named "ttyS#".  ive found that simply informing them "to use ttyBF#"
has always been sufficient.
-mike

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

[Daniel Walker]

On Thu, 2010-10-07 at 17:32 -0400, Mike Frysinger wrote:

> > Ideally you would want this driver to work in any situation .. If it's
> > setup to use ttyS0 for debugging, even if it doesn't exist, then this
> > driver would be able to stand in for that interface.
> 
> i dont think that's the case.  "any situation" is way too vague and
> invites mounds of crap to be included in the kernel which really
> should be in userspace.  ive never had a problem with my embedded work
> using ttyBF# or ttyBFJC# or ttySS# for the Blackfin serial devices,
> nor have i heard customers complain that the file absolutely must be
> named "ttyS#".  ive found that simply informing them "to use ttyBF#"
> has always been sufficient.
> -mike

It's kind of a "your milage may vary" situation .. From my perspective
it doesn't hurt anything to have an exception for this, considering what
it is used for.

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Alan Cox]

On Thu, 7 Oct 2010 15:25:34 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> On Thu, Oct 7, 2010 at 14:36, Daniel Walker wrote:
> > Many of JTAG debuggers for ARM support DCC protocol over JTAG
> > connection, which is very useful to debug hardwares which has no
> > serial port. This patch adds DCC serial emulation and the console
> > support. System timer based polling method is used for the
> > emulation of serial input interrupt handling.
> 
> why use serial_core at all ?  seems like you could just use the tty
> layer directly.  i did that with drivers/char/bfin_jtag_comm.c.

I would agree 100% with this for the driver in question. The tty_port
helpers now make it trivial to do so and it'll be small and clean as a
result.

Alan

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

[Daniel Walker]

On Thu, 2010-10-07 at 21:52 +0100, Alan Cox wrote:
> On Thu, 7 Oct 2010 15:25:34 -0400
> Mike Frysinger <vapier@gentoo.org> wrote:
> 
> > On Thu, Oct 7, 2010 at 14:36, Daniel Walker wrote:
> > > Many of JTAG debuggers for ARM support DCC protocol over JTAG
> > > connection, which is very useful to debug hardwares which has no
> > > serial port. This patch adds DCC serial emulation and the console
> > > support. System timer based polling method is used for the
> > > emulation of serial input interrupt handling.
> > 
> > why use serial_core at all ?  seems like you could just use the tty
> > layer directly.  i did that with drivers/char/bfin_jtag_comm.c.
> 
> I would agree 100% with this for the driver in question. The tty_port
> helpers now make it trivial to do so and it'll be small and clean as a
> result.

Can you give some example of other drivers which have done this?

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Alan Cox]

> > I would agree 100% with this for the driver in question. The tty_port
> > helpers now make it trivial to do so and it'll be small and clean as a
> > result.
> 
> Can you give some example of other drivers which have done this?

The blackfin driver is probably the nicest example for a very simple
device but see also the USB serial drivers (complex example), the SDIO
serial support (shows how to do fancy hotpluggable device stuff with it).

Basically tty_port_open/tty_port_close/tty_port_hangup do all the icky
nasty POSIX open/close/hangup handling and provide you with simple
methods to implement activate/shutdown of a port and carrier detect etc
most of which you don't even need to supply.

You can also skip a lot of other stuff like modem lines - although I can
see why you might want to provide an emulated modem line response for a
console emulation.

Alan

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

[Alan Cox]

> +	  Say Y here if you want to install DCC driver as a normal serial port
> +	  /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
> +	  (major 4, minor 128) and can co-exist with other UARTs, such as
> +	  8250/16C550 compatibles.
> +

NAK to both

ttyJ0 is 204,186 for "JTAG1 DCC protocol based serial"

so there is an existing name and minor allocation, which as you won't be
using two of them should be quite usable.

If you want to be able to switch at runtime to pretend it is ttyS0 please
deal with that in your user space. The same rules apply to you as have
been applied to everyone else who has tried to implement this same crap
in their uart driver too (we'd have it in about 30 by now otherwise)

>
> +static inline void dcc_rx_chars(struct uart_port *port)
> +{
> +	unsigned char ch;
> +	struct tty_struct *tty = port->state->port.tty;

Not safe in the general case - you might get a hangup here then call
through a NULL pointer. Please use the proper tty_port helpers.


> +static inline void dcc_overrun_chars(struct uart_port *port)
> +{
> +	port->icount.overrun++;
> +}

Why make this a function?


> +static void
> +dcc_set_termios(struct uart_port *port, struct ktermios *termios,
> +		struct ktermios *old)
> +{
> +	unsigned int baud, quot;
> +
> +	/*
> +	 * We don't support parity, stop bits, or anything other
> +	 * than 8 bits, so clear these termios flags.
> +	 */
> +	termios->c_cflag &= ~(CSIZE | CSTOPB | PARENB | PARODD | CREAD);
> +	termios->c_cflag |= CS8;
> +
> +	/*
> +	 * We don't appear to support any error conditions either.
> +	 */
> +	termios->c_iflag &= ~(INPCK | IGNPAR | IGNBRK | BRKINT);

There is a helper function for keeping the hardware bits fixed. If your
hardware is fixed then please use it.

Alan

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

[Daniel Walker]

On Thu, 2010-10-07 at 21:50 +0100, Alan Cox wrote:
> > +	  Say Y here if you want to install DCC driver as a normal serial port
> > +	  /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
> > +	  (major 4, minor 128) and can co-exist with other UARTs, such as
> > +	  8250/16C550 compatibles.
> > +
> 
> NAK to both

Both what?

> ttyJ0 is 204,186 for "JTAG1 DCC protocol based serial"
> 
> so there is an existing name and minor allocation, which as you won't be
> using two of them should be quite usable.
> 
> If you want to be able to switch at runtime to pretend it is ttyS0 please
> deal with that in your user space. The same rules apply to you as have
> been applied to everyone else who has tried to implement this same crap
> in their uart driver too (we'd have it in about 30 by now otherwise)

I would agree if this wasn't strictly for debugging embedded devices in
difficult situations.. After talking to Mike, it's seems like it would
be useful to have this as a ttyS* specifically because embedded devices
won't always create a ttyJ* for you and ttyS* will likely already exist.

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Alan Cox]

On Thu, 07 Oct 2010 13:36:06 -0700
Daniel Walker <dwalker@codeaurora.org> wrote:

> On Thu, 2010-10-07 at 21:50 +0100, Alan Cox wrote:
> > > +	  Say Y here if you want to install DCC driver as a normal serial port
> > > +	  /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
> > > +	  (major 4, minor 128) and can co-exist with other UARTs, such as
> > > +	  8250/16C550 compatibles.
> > > +
> > 
> > NAK to both
> 
> Both what?

Both device allocations. Please use the 204,186 assigned for JTAG DCC.

> I would agree if this wasn't strictly for debugging embedded devices in
> difficult situations.. After talking to Mike, it's seems like it would
> be useful to have this as a ttyS* specifically because embedded devices
> won't always create a ttyJ* for you and ttyS* will likely already exist.

If you are debugging an embedded device you are capable of changing it
(otherwise you wouldn't debug it). Therefore the fact you might need to
tweak the tty creation on the device is not a problem. If you can't
change the device well there is no point debugging it is there !

We've said no over a period of about ten years to a lot of attempts to
just borrow the ttyS0 range. If we'd said yes it would have been a
complete mess by now.

So the answer is no.

Alan

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

[Daniel Walker]

On Thu, 2010-10-07 at 22:05 +0100, Alan Cox wrote:
> On Thu, 07 Oct 2010 13:36:06 -0700
> Daniel Walker <dwalker@codeaurora.org> wrote:
> 
> > On Thu, 2010-10-07 at 21:50 +0100, Alan Cox wrote:
> > > > +	  Say Y here if you want to install DCC driver as a normal serial port
> > > > +	  /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
> > > > +	  (major 4, minor 128) and can co-exist with other UARTs, such as
> > > > +	  8250/16C550 compatibles.
> > > > +
> > > 
> > > NAK to both
> > 
> > Both what?
> 
> Both device allocations. Please use the 204,186 assigned for JTAG DCC.
> 
> > I would agree if this wasn't strictly for debugging embedded devices in
> > difficult situations.. After talking to Mike, it's seems like it would
> > be useful to have this as a ttyS* specifically because embedded devices
> > won't always create a ttyJ* for you and ttyS* will likely already exist.
> 
> If you are debugging an embedded device you are capable of changing it
> (otherwise you wouldn't debug it). Therefore the fact you might need to
> tweak the tty creation on the device is not a problem. If you can't
> change the device well there is no point debugging it is there !

Your making too many assumptions .. You might be able to modify the
kernel, and not the userspace. So you couldn't tweak the device
creation .. It's much easier in the server world ..

Hyok should really be addressing this .. If he added for some random
useless reason , then for sure we can rip it out .. Otherwise I think it
deserves some more discussion ..

> We've said no over a period of about ten years to a lot of attempts to
> just borrow the ttyS0 range. If we'd said yes it would have been a
> complete mess by now.
> 
> So the answer is no.

Nothing can be unilateral, there's always room for exceptions. You
should say something more like "it's possible, but unlikely".

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 16:51, Daniel Walker wrote:
> On Thu, 2010-10-07 at 22:05 +0100, Alan Cox wrote:
>> On Thu, 07 Oct 2010 13:36:06 -0700 Daniel Walker wrote:
>> > On Thu, 2010-10-07 at 21:50 +0100, Alan Cox wrote:
>> > > > +         Say Y here if you want to install DCC driver as a normal serial port
>> > > > +         /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
>> > > > +         (major 4, minor 128) and can co-exist with other UARTs, such as
>> > > > +         8250/16C550 compatibles.
>> > > > +
>> > >
>> > > NAK to both
>> >
>> > I would agree if this wasn't strictly for debugging embedded devices in
>> > difficult situations.. After talking to Mike, it's seems like it would
>> > be useful to have this as a ttyS* specifically because embedded devices
>> > won't always create a ttyJ* for you and ttyS* will likely already exist.
>>
>> If you are debugging an embedded device you are capable of changing it
>> (otherwise you wouldn't debug it). Therefore the fact you might need to
>> tweak the tty creation on the device is not a problem. If you can't
>> change the device well there is no point debugging it is there !
>
> Your making too many assumptions .. You might be able to modify the
> kernel, and not the userspace. So you couldn't tweak the device
> creation .. It's much easier in the server world ..

this driver isnt in the kernel.  there is no ABI to preserve.  if
people want to use the mainline driver, they can fix their userspace.
i dont think the "binary userspace" argument is terribly relevant to
new driver submissions.
-mike

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

[Daniel Walker]

On Thu, 2010-10-07 at 17:03 -0400, Mike Frysinger wrote:
> On Thu, Oct 7, 2010 at 16:51, Daniel Walker wrote:
> > On Thu, 2010-10-07 at 22:05 +0100, Alan Cox wrote:
> >> On Thu, 07 Oct 2010 13:36:06 -0700 Daniel Walker wrote:
> >> > On Thu, 2010-10-07 at 21:50 +0100, Alan Cox wrote:
> >> > > > +         Say Y here if you want to install DCC driver as a normal serial port
> >> > > > +         /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
> >> > > > +         (major 4, minor 128) and can co-exist with other UARTs, such as
> >> > > > +         8250/16C550 compatibles.
> >> > > > +
> >> > >
> >> > > NAK to both
> >> >
> >> > I would agree if this wasn't strictly for debugging embedded devices in
> >> > difficult situations.. After talking to Mike, it's seems like it would
> >> > be useful to have this as a ttyS* specifically because embedded devices
> >> > won't always create a ttyJ* for you and ttyS* will likely already exist.
> >>
> >> If you are debugging an embedded device you are capable of changing it
> >> (otherwise you wouldn't debug it). Therefore the fact you might need to
> >> tweak the tty creation on the device is not a problem. If you can't
> >> change the device well there is no point debugging it is there !
> >
> > Your making too many assumptions .. You might be able to modify the
> > kernel, and not the userspace. So you couldn't tweak the device
> > creation .. It's much easier in the server world ..
> 
> this driver isnt in the kernel.  there is no ABI to preserve.  if
> people want to use the mainline driver, they can fix their userspace.
> i dont think the "binary userspace" argument is terribly relevant to
> new driver submissions.

It is , what's to stop any userspace from using ttyS0 ? .. There's lots
of binary userspaces in the embedded world, along with many strange
usages ..

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Alan Cox]

> It is , what's to stop any userspace from using ttyS0 ? .. There's lots
> of binary userspaces in the embedded world, along with many strange
> usages ..

So you change your file system, whoopee.

And if your hypothetical user isn't able to do this then maybe instead of
trying to screw up the kernel for everyone they should ask an
undergraduate student who is smart enough to help them.

Alan

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

[Daniel Walker]

On Fri, 2010-10-08 at 09:13 +0100, Alan Cox wrote:
> > It is , what's to stop any userspace from using ttyS0 ? .. There's lots
> > of binary userspaces in the embedded world, along with many strange
> > usages ..
> 
> So you change your file system, whoopee.

I'm assuming Hyok added this because it wasn't easy or possible to
change the file system .. I can imagine there are debugging situations
where that's true.

> And if your hypothetical user isn't able to do this then maybe instead of
> trying to screw up the kernel for everyone they should ask an
> undergraduate student who is smart enough to help them.

I don't think the kernel is going to implode if we allow an optional
ttyS override for debugging purposes.. I just don't see that "screwing"
up the kernel. I can even embedded that inside the C file, and not make
it a Kconfig option ..

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Greg KH]

On Fri, Oct 08, 2010 at 08:23:54AM -0700, Daniel Walker wrote:
> > And if your hypothetical user isn't able to do this then maybe instead of
> > trying to screw up the kernel for everyone they should ask an
> > undergraduate student who is smart enough to help them.
> 
> I don't think the kernel is going to implode if we allow an optional
> ttyS override for debugging purposes.. I just don't see that "screwing"
> up the kernel. I can even embedded that inside the C file, and not make
> it a Kconfig option ..

I admire your persistence, but perhaps you should take a step back and
look at the history here.  You just want this override.  Then someone
else does, and then, someone else.  Eventually we are all overridden.

Remember, you aren't unique here, as much as you might feel like you are
:)

Please listen to what Alan and Mike are saying, experience matters, and
they are trying to impart it to you, despite your most fervent attempts
to avoid it.

good luck,

greg k-h

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

[Daniel Walker]

On Fri, 2010-10-08 at 08:40 -0700, Greg KH wrote:
> On Fri, Oct 08, 2010 at 08:23:54AM -0700, Daniel Walker wrote:
> > > And if your hypothetical user isn't able to do this then maybe instead of
> > > trying to screw up the kernel for everyone they should ask an
> > > undergraduate student who is smart enough to help them.
> > 
> > I don't think the kernel is going to implode if we allow an optional
> > ttyS override for debugging purposes.. I just don't see that "screwing"
> > up the kernel. I can even embedded that inside the C file, and not make
> > it a Kconfig option ..
> 
> I admire your persistence, but perhaps you should take a step back and
> look at the history here.  You just want this override.  Then someone
> else does, and then, someone else.  Eventually we are all overridden.
> 
> Remember, you aren't unique here, as much as you might feel like you are
> :)

It's not that _I_ want it .. If you look at the code it's not that huge
a part of the source .. It's got it's own ttyJ already, and it's
ifdefe'd. It would be pretty easy to remove it. The other thing is that
it's not something I use, so for me it's kind of take it or leave it.

So why am I arguing to keep it? Well the original author added it for a
purpose, which I can't assume was useless .. I can also imagine
situations when it would be helpful to do this.

To me it's more a question of if the override is useful or not .. If
it's useful then it's just about organizing the code so the kernel
doesn't melt down if we allow it.

Thinking about it overrides are actually allowed right now, it's just
that you have to mod the source some to do it.

> Please listen to what Alan and Mike are saying, experience matters, and
> they are trying to impart it to you, despite your most fervent attempts
> to avoid it.

The whole "fervent" thing is kind of funny to me.. I'm just persistent,
that's what it's comes down to. Like I said above I'm not "in love" with
this feature. I am actually taking Mike and Alan's advice by the way, so
it's not like I'm not listening (or reading in this case)..

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Alan Cox]

> I'm assuming Hyok added this because it wasn't easy or possible to
> change the file system .. I can imagine there are debugging situations
> where that's true.

Well stop assuming. You cc'd Hyok so let him speak for himself if he
cares about it and has an example.

> I don't think the kernel is going to implode if we allow an optional
> ttyS override for debugging purposes.. I just don't see that "screwing"
> up the kernel.

That is perhaps why you are not a subsystem maintainer.

Alan

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

[Daniel Walker]

On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
> > I'm assuming Hyok added this because it wasn't easy or possible to
> > change the file system .. I can imagine there are debugging situations
> > where that's true.
> 
> Well stop assuming. You cc'd Hyok so let him speak for himself if he
> cares about it and has an example.

I've been waiting, but he's not lightning fast responding it seems.

> > I don't think the kernel is going to implode if we allow an optional
> > ttyS override for debugging purposes.. I just don't see that "screwing"
> > up the kernel.
> 
> That is perhaps why you are not a subsystem maintainer.

No, I'm sure it's not.

What I was telling Greg is that this type of override is allowed right
now, if you mod the source (everything is allowed if you mod the source
right?) ..

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Mike Frysinger]

On Fri, Oct 8, 2010 at 12:45, Daniel Walker wrote:
> On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
>> > I don't think the kernel is going to implode if we allow an optional
>> > ttyS override for debugging purposes.. I just don't see that "screwing"
>> > up the kernel.
>>
>> That is perhaps why you are not a subsystem maintainer.
>
> No, I'm sure it's not.
>
> What I was telling Greg is that this type of override is allowed right
> now, if you mod the source (everything is allowed if you mod the source
> right?) ..

just because someone can mod the source code to add a root shell
doesnt mean we should accept patches to do it
-mike

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

[Daniel Walker]

On Fri, 2010-10-08 at 14:38 -0400, Mike Frysinger wrote:
> On Fri, Oct 8, 2010 at 12:45, Daniel Walker wrote:
> > On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
> >> > I don't think the kernel is going to implode if we allow an optional
> >> > ttyS override for debugging purposes.. I just don't see that "screwing"
> >> > up the kernel.
> >>
> >> That is perhaps why you are not a subsystem maintainer.
> >
> > No, I'm sure it's not.
> >
> > What I was telling Greg is that this type of override is allowed right
> > now, if you mod the source (everything is allowed if you mod the source
> > right?) ..
> 
> just because someone can mod the source code to add a root shell
> doesnt mean we should accept patches to do it

That's not what I was getting at. If you can mod the source to add the
override, then adding in a patch which also requires you to mod the
source to add the override should be acceptable because it's no
different that what we currently have.

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Mike Frysinger]

On Fri, Oct 8, 2010 at 15:01, Daniel Walker wrote:
> On Fri, 2010-10-08 at 14:38 -0400, Mike Frysinger wrote:
>> On Fri, Oct 8, 2010 at 12:45, Daniel Walker wrote:
>> > On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
>> >> > I don't think the kernel is going to implode if we allow an optional
>> >> > ttyS override for debugging purposes.. I just don't see that "screwing"
>> >> > up the kernel.
>> >>
>> >> That is perhaps why you are not a subsystem maintainer.
>> >
>> > No, I'm sure it's not.
>> >
>> > What I was telling Greg is that this type of override is allowed right
>> > now, if you mod the source (everything is allowed if you mod the source
>> > right?) ..
>>
>> just because someone can mod the source code to add a root shell
>> doesnt mean we should accept patches to do it
>
> That's not what I was getting at. If you can mod the source to add the
> override, then adding in a patch which also requires you to mod the
> source to add the override should be acceptable because it's no
> different that what we currently have.

sorry, but you lost me.  all i see is "drop the hack code".
-mike

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

[Daniel Walker]

On Fri, 2010-10-08 at 15:20 -0400, Mike Frysinger wrote:
> On Fri, Oct 8, 2010 at 15:01, Daniel Walker wrote:
> > On Fri, 2010-10-08 at 14:38 -0400, Mike Frysinger wrote:
> >> On Fri, Oct 8, 2010 at 12:45, Daniel Walker wrote:
> >> > On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
> >> >> > I don't think the kernel is going to implode if we allow an optional
> >> >> > ttyS override for debugging purposes.. I just don't see that "screwing"
> >> >> > up the kernel.
> >> >>
> >> >> That is perhaps why you are not a subsystem maintainer.
> >> >
> >> > No, I'm sure it's not.
> >> >
> >> > What I was telling Greg is that this type of override is allowed right
> >> > now, if you mod the source (everything is allowed if you mod the source
> >> > right?) ..
> >>
> >> just because someone can mod the source code to add a root shell
> >> doesnt mean we should accept patches to do it
> >
> > That's not what I was getting at. If you can mod the source to add the
> > override, then adding in a patch which also requires you to mod the
> > source to add the override should be acceptable because it's no
> > different that what we currently have.
> 
> sorry, but you lost me.  all i see is "drop the hack code".

I can make it simpler for you. You add code into Linux, people can
modify it, and you can't control that.

Daniel


-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Mike Frysinger]

On Fri, Oct 8, 2010 at 15:50, Daniel Walker wrote:
> On Fri, 2010-10-08 at 15:20 -0400, Mike Frysinger wrote:
>> On Fri, Oct 8, 2010 at 15:01, Daniel Walker wrote:
>> > On Fri, 2010-10-08 at 14:38 -0400, Mike Frysinger wrote:
>> >> On Fri, Oct 8, 2010 at 12:45, Daniel Walker wrote:
>> >> > On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
>> >> >> > I don't think the kernel is going to implode if we allow an optional
>> >> >> > ttyS override for debugging purposes.. I just don't see that "screwing"
>> >> >> > up the kernel.
>> >> >>
>> >> >> That is perhaps why you are not a subsystem maintainer.
>> >> >
>> >> > No, I'm sure it's not.
>> >> >
>> >> > What I was telling Greg is that this type of override is allowed right
>> >> > now, if you mod the source (everything is allowed if you mod the source
>> >> > right?) ..
>> >>
>> >> just because someone can mod the source code to add a root shell
>> >> doesnt mean we should accept patches to do it
>> >
>> > That's not what I was getting at. If you can mod the source to add the
>> > override, then adding in a patch which also requires you to mod the
>> > source to add the override should be acceptable because it's no
>> > different that what we currently have.
>>
>> sorry, but you lost me.  all i see is "drop the hack code".
>
> I can make it simpler for you. You add code into Linux, people can
> modify it, and you can't control that.

while true, that lacks justification for merging hacks into mainline
-mike

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

[Daniel Walker]

On Fri, 2010-10-08 at 18:02 -0400, Mike Frysinger wrote:
> On Fri, Oct 8, 2010 at 15:50, Daniel Walker wrote:
> > On Fri, 2010-10-08 at 15:20 -0400, Mike Frysinger wrote:
> >> On Fri, Oct 8, 2010 at 15:01, Daniel Walker wrote:
> >> > On Fri, 2010-10-08 at 14:38 -0400, Mike Frysinger wrote:
> >> >> On Fri, Oct 8, 2010 at 12:45, Daniel Walker wrote:
> >> >> > On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
> >> >> >> > I don't think the kernel is going to implode if we allow an optional
> >> >> >> > ttyS override for debugging purposes.. I just don't see that "screwing"
> >> >> >> > up the kernel.
> >> >> >>
> >> >> >> That is perhaps why you are not a subsystem maintainer.
> >> >> >
> >> >> > No, I'm sure it's not.
> >> >> >
> >> >> > What I was telling Greg is that this type of override is allowed right
> >> >> > now, if you mod the source (everything is allowed if you mod the source
> >> >> > right?) ..
> >> >>
> >> >> just because someone can mod the source code to add a root shell
> >> >> doesnt mean we should accept patches to do it
> >> >
> >> > That's not what I was getting at. If you can mod the source to add the
> >> > override, then adding in a patch which also requires you to mod the
> >> > source to add the override should be acceptable because it's no
> >> > different that what we currently have.
> >>
> >> sorry, but you lost me.  all i see is "drop the hack code".
> >
> > I can make it simpler for you. You add code into Linux, people can
> > modify it, and you can't control that.
> 
> while true, that lacks justification for merging hacks into mainline

No one is merging hack, or giving justification for that.. So not sure
why your going down that thread again .

Daniel


-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Mike Frysinger]

On Fri, Oct 8, 2010 at 18:22, Daniel Walker wrote:
> On Fri, 2010-10-08 at 18:02 -0400, Mike Frysinger wrote:
>> On Fri, Oct 8, 2010 at 15:50, Daniel Walker wrote:
>> > On Fri, 2010-10-08 at 15:20 -0400, Mike Frysinger wrote:
>> >> On Fri, Oct 8, 2010 at 15:01, Daniel Walker wrote:
>> >> > On Fri, 2010-10-08 at 14:38 -0400, Mike Frysinger wrote:
>> >> >> On Fri, Oct 8, 2010 at 12:45, Daniel Walker wrote:
>> >> >> > On Fri, 2010-10-08 at 17:56 +0100, Alan Cox wrote:
>> >> >> >> > I don't think the kernel is going to implode if we allow an optional
>> >> >> >> > ttyS override for debugging purposes.. I just don't see that "screwing"
>> >> >> >> > up the kernel.
>> >> >> >>
>> >> >> >> That is perhaps why you are not a subsystem maintainer.
>> >> >> >
>> >> >> > No, I'm sure it's not.
>> >> >> >
>> >> >> > What I was telling Greg is that this type of override is allowed right
>> >> >> > now, if you mod the source (everything is allowed if you mod the source
>> >> >> > right?) ..
>> >> >>
>> >> >> just because someone can mod the source code to add a root shell
>> >> >> doesnt mean we should accept patches to do it
>> >> >
>> >> > That's not what I was getting at. If you can mod the source to add the
>> >> > override, then adding in a patch which also requires you to mod the
>> >> > source to add the override should be acceptable because it's no
>> >> > different that what we currently have.
>> >>
>> >> sorry, but you lost me.  all i see is "drop the hack code".
>> >
>> > I can make it simpler for you. You add code into Linux, people can
>> > modify it, and you can't control that.
>>
>> while true, that lacks justification for merging hacks into mainline
>
> No one is merging hack, or giving justification for that.. So not sure
> why your going down that thread again .

forget it; i'm completely bored with this whole driver now.  people
have been clear on what it will take for this to be merged, so ball is
in your court now.
-mike

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

[Greg KH]

On Thu, Oct 07, 2010 at 01:51:18PM -0700, Daniel Walker wrote:
> Your making too many assumptions .. You might be able to modify the
> kernel, and not the userspace. So you couldn't tweak the device
> creation .. It's much easier in the server world ..

You're saying it's easier to replace an embedded kernel than a userspace
file on an embedded system?  Heh, that's funny.

And no, it's not easier to repace a kernel on a "server" than a
userspace file, due to all sorts of good reasons (certifications,
support contracts, etc...)

> > We've said no over a period of about ten years to a lot of attempts to
> > just borrow the ttyS0 range. If we'd said yes it would have been a
> > complete mess by now.
> > 
> > So the answer is no.
> 
> Nothing can be unilateral, there's always room for exceptions. You
> should say something more like "it's possible, but unlikely".

Hm, how about this, as the TTY and serial driver[1] maintainer, I will
not accept this kind of patch at all.

Is that final enough for you?

thanks,

greg k-h

[1] http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-06-tty/serial-mark-the-8250-driver-as-maintained.patch

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

[Daniel Walker]

On Thu, 2010-10-07 at 14:15 -0700, Greg KH wrote:
> On Thu, Oct 07, 2010 at 01:51:18PM -0700, Daniel Walker wrote:
> > Your making too many assumptions .. You might be able to modify the
> > kernel, and not the userspace. So you couldn't tweak the device
> > creation .. It's much easier in the server world ..
> 
> You're saying it's easier to replace an embedded kernel than a userspace
> file on an embedded system?  Heh, that's funny.

I'm saying it _can_ happen that it's easier to replace the kernel.. I'm
not saying it's always the case. We're talking about debugging
situations in an embedded environment which don't always follow a normal
path.

> > > We've said no over a period of about ten years to a lot of attempts to
> > > just borrow the ttyS0 range. If we'd said yes it would have been a
> > > complete mess by now.
> > > 
> > > So the answer is no.
> > 
> > Nothing can be unilateral, there's always room for exceptions. You
> > should say something more like "it's possible, but unlikely".
> 
> Hm, how about this, as the TTY and serial driver[1] maintainer, I will
> not accept this kind of patch at all.
> 
> Is that final enough for you?

So you don't like it, that's fair enough .. <thinks>I wonder what other
maintainers I can send this too</thinks> ;)

Can you be more specific about your objections .. The discussion over
the ttyS* thing is still happening. I'm waiting for Hyok to give a good
reason for why he wrote that part of it. I can imagine good reasons for
why that part would exist, which is what I'm discussing with Alan and
Mike.

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Greg KH]

On Thu, Oct 07, 2010 at 02:47:18PM -0700, Daniel Walker wrote:
> On Thu, 2010-10-07 at 14:15 -0700, Greg KH wrote:
> > On Thu, Oct 07, 2010 at 01:51:18PM -0700, Daniel Walker wrote:
> > > Your making too many assumptions .. You might be able to modify the
> > > kernel, and not the userspace. So you couldn't tweak the device
> > > creation .. It's much easier in the server world ..
> > 
> > You're saying it's easier to replace an embedded kernel than a userspace
> > file on an embedded system?  Heh, that's funny.
> 
> I'm saying it _can_ happen that it's easier to replace the kernel.. I'm
> not saying it's always the case. We're talking about debugging
> situations in an embedded environment which don't always follow a normal
> path.

Nothing is "normal", so stop trying to say it is easier in the "server"
world please.

> > > > We've said no over a period of about ten years to a lot of attempts to
> > > > just borrow the ttyS0 range. If we'd said yes it would have been a
> > > > complete mess by now.
> > > > 
> > > > So the answer is no.
> > > 
> > > Nothing can be unilateral, there's always room for exceptions. You
> > > should say something more like "it's possible, but unlikely".
> > 
> > Hm, how about this, as the TTY and serial driver[1] maintainer, I will
> > not accept this kind of patch at all.
> > 
> > Is that final enough for you?
> 
> So you don't like it, that's fair enough .. <thinks>I wonder what other
> maintainers I can send this too</thinks> ;)
> 
> Can you be more specific about your objections

See everything that Mike and Alan wrote, do I need to repeat it?

greg k-h

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

[Daniel Walker]

On Thu, 2010-10-07 at 14:52 -0700, Greg KH wrote:
> On Thu, Oct 07, 2010 at 02:47:18PM -0700, Daniel Walker wrote:
> > On Thu, 2010-10-07 at 14:15 -0700, Greg KH wrote:
> > > On Thu, Oct 07, 2010 at 01:51:18PM -0700, Daniel Walker wrote:
> > > > Your making too many assumptions .. You might be able to modify the
> > > > kernel, and not the userspace. So you couldn't tweak the device
> > > > creation .. It's much easier in the server world ..
> > > 
> > > You're saying it's easier to replace an embedded kernel than a userspace
> > > file on an embedded system?  Heh, that's funny.
> > 
> > I'm saying it _can_ happen that it's easier to replace the kernel.. I'm
> > not saying it's always the case. We're talking about debugging
> > situations in an embedded environment which don't always follow a normal
> > path.
> 
> Nothing is "normal", so stop trying to say it is easier in the "server"
> world please.

Ok ..

> > > > > We've said no over a period of about ten years to a lot of attempts to
> > > > > just borrow the ttyS0 range. If we'd said yes it would have been a
> > > > > complete mess by now.
> > > > > 
> > > > > So the answer is no.
> > > > 
> > > > Nothing can be unilateral, there's always room for exceptions. You
> > > > should say something more like "it's possible, but unlikely".
> > > 
> > > Hm, how about this, as the TTY and serial driver[1] maintainer, I will
> > > not accept this kind of patch at all.
> > > 
> > > Is that final enough for you?
> > 
> > So you don't like it, that's fair enough .. <thinks>I wonder what other
> > maintainers I can send this too</thinks> ;)
> > 
> > Can you be more specific about your objections
> 
> See everything that Mike and Alan wrote, do I need to repeat it?

I'm not sure .. I would say "Yes" if I didn't get the feeling your
already to tear my head off.

I think the only disputed issue is the use of ttyS as options part of
the driver ..

Daniel

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Mike Frysinger]

On Thu, Oct 7, 2010 at 18:11, Daniel Walker wrote:
> On Thu, 2010-10-07 at 14:52 -0700, Greg KH wrote:
>> On Thu, Oct 07, 2010 at 02:47:18PM -0700, Daniel Walker wrote:
>> > On Thu, 2010-10-07 at 14:15 -0700, Greg KH wrote:
>> > > On Thu, Oct 07, 2010 at 01:51:18PM -0700, Daniel Walker wrote:
>> > > > > We've said no over a period of about ten years to a lot of attempts to
>> > > > > just borrow the ttyS0 range. If we'd said yes it would have been a
>> > > > > complete mess by now.
>> > > > >
>> > > > > So the answer is no.
>> > > >
>> > > > Nothing can be unilateral, there's always room for exceptions. You
>> > > > should say something more like "it's possible, but unlikely".
>> > >
>> > > Hm, how about this, as the TTY and serial driver[1] maintainer, I will
>> > > not accept this kind of patch at all.
>> > >
>> > > Is that final enough for you?
>> >
>> > So you don't like it, that's fair enough .. <thinks>I wonder what other
>> > maintainers I can send this too</thinks> ;)
>> >
>> > Can you be more specific about your objections
>>
>> See everything that Mike and Alan wrote, do I need to repeat it?
>
> I'm not sure .. I would say "Yes" if I didn't get the feeling your
> already to tear my head off.
>
> I think the only disputed issue is the use of ttyS as options part of
> the driver ..

and using "serial_core" at all in favor of HVC or tty_core (the latter
might be easier for you as the former is a bit of unknown atm)
-mike

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

[Alan Cox]

> Your making too many assumptions .. You might be able to modify the
> kernel, and not the userspace. So you couldn't tweak the device
> creation .. It's much easier in the server world ..

Spare me the embedded is different lecture. It's not. In fact it's
usually easier because the environment you are testing is usually
standalone, doesn't cause a million dollar an hour downtime if you get it
wrong and was designed for debug/test so actually has a jtag port.

If you can modify the file system you can make /dev/ttyS0 the major/minor
of the jtag port.

If you can load anything in front of the userspace you can rename the
device/move it

So I find your example case fictional. In fact the only time I can see
you having a box so tightly locked up you can only tweak the kernel is
jailbreaking it, in which case you are such a fringe usage case and
technically competent that you can just do custom patches as you'll be
doing anyway for other bits.

> > We've said no over a period of about ten years to a lot of attempts to
> > just borrow the ttyS0 range. If we'd said yes it would have been a
> > complete mess by now.
> > 
> > So the answer is no.
> 
> Nothing can be unilateral, there's always room for exceptions. You
> should say something more like "it's possible, but unlikely".

This argument gets regurgitated every so often and nobody has yet
provided a plausable reason to make a nasty mess.

Either way I would suggest the path forward is

- Look at the blackfin jtag using tty_port alone and see if it looks
  cleaner
- Fix the bugs noted
- Submit a driver that uses the existing allocated jtag major/minor only

and then have a debate about ttyS0 hackery separately.

Alan

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

[Daniel Walker]

On Thu, 2010-10-07 at 22:38 +0100, Alan Cox wrote:
> > Your making too many assumptions .. You might be able to modify the
> > kernel, and not the userspace. So you couldn't tweak the device
> > creation .. It's much easier in the server world ..
> 
> Spare me the embedded is different lecture. It's not. In fact it's
> usually easier because the environment you are testing is usually
> standalone, doesn't cause a million dollar an hour downtime if you get it
> wrong and was designed for debug/test so actually has a jtag port.

fair enough, it's hard for both of us.

> If you can modify the file system you can make /dev/ttyS0 the major/minor
> of the jtag port.
> 
> If you can load anything in front of the userspace you can rename the
> device/move it
> 
> So I find your example case fictional. In fact the only time I can see
> you having a box so tightly locked up you can only tweak the kernel is
> jailbreaking it, in which case you are such a fringe usage case and
> technically competent that you can just do custom patches as you'll be
> doing anyway for other bits.

There's lots of other situations that can come up. You can't in anyway
say that there is some certain set of situations where you'll always
have X, Y, and Z.. There have been enough strange situations that I
don't think it's right for you or me to assume we just know what they
all are.

> > > We've said no over a period of about ten years to a lot of attempts to
> > > just borrow the ttyS0 range. If we'd said yes it would have been a
> > > complete mess by now.
> > > 
> > > So the answer is no.
> > 
> > Nothing can be unilateral, there's always room for exceptions. You
> > should say something more like "it's possible, but unlikely".
> 
> This argument gets regurgitated every so often and nobody has yet
> provided a plausable reason to make a nasty mess.

There's plenty of nasty messes in the kernel, "they" must have had some
justification ..

> Either way I would suggest the path forward is
> 
> - Look at the blackfin jtag using tty_port alone and see if it looks
>   cleaner

Yeah ..

> - Fix the bugs noted

Check,

> - Submit a driver that uses the existing allocated jtag major/minor only

The driver does this already. It uses ttyJ* by default, and ttyS is an
option ..

> and then have a debate about ttyS0 hackery separately.

Fine with me..

-- 

Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Alan Cox]

> > - Submit a driver that uses the existing allocated jtag major/minor only
> 
> The driver does this already. It uses ttyJ* by default, and ttyS is an
> option ..

It didn't seem to be using the correct major/minor however ?

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

[Daniel Walker]

On Fri, 2010-10-08 at 09:18 +0100, Alan Cox wrote:
> > > - Submit a driver that uses the existing allocated jtag major/minor only
> > 
> > The driver does this already. It uses ttyJ* by default, and ttyS is an
> > option ..
> 
> It didn't seem to be using the correct major/minor however ?

yeah, it looks like it. ttyJ was registered specifically for this
driver..

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Daniel Walker]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1771 bytes --]

I'm not disputing the serial core part. I ok with converting it to tty.

Daniel

Mike Frysinger <vapier@gentoo.org> wrote:

>On Thu, Oct 7, 2010 at 18:11, Daniel Walker wrote:
>> On Thu, 2010-10-07 at 14:52 -0700, Greg KH wrote:
>>> On Thu, Oct 07, 2010 at 02:47:18PM -0700, Daniel Walker wrote:
>>> > On Thu, 2010-10-07 at 14:15 -0700, Greg KH wrote:
>>> > > On Thu, Oct 07, 2010 at 01:51:18PM -0700, Daniel Walker wrote:
>>> > > > > We've said no over a period of about ten years to a lot of attempts to
>>> > > > > just borrow the ttyS0 range. If we'd said yes it would have been a
>>> > > > > complete mess by now.
>>> > > > >
>>> > > > > So the answer is no.
>>> > > >
>>> > > > Nothing can be unilateral, there's always room for exceptions. You
>>> > > > should say something more like "it's possible, but unlikely".
>>> > >
>>> > > Hm, how about this, as the TTY and serial driver[1] maintainer, I will
>>> > > not accept this kind of patch at all.
>>> > >
>>> > > Is that final enough for you?
>>> >
>>> > So you don't like it, that's fair enough .. <thinks>I wonder what other
>>> > maintainers I can send this too</thinks> ;)
>>> >
>>> > Can you be more specific about your objections
>>>
>>> See everything that Mike and Alan wrote, do I need to repeat it?
>>
>> I'm not sure .. I would say "Yes" if I didn't get the feeling your
>> already to tear my head off.
>>
>> I think the only disputed issue is the use of ttyS as options part of
>> the driver ..
>
>and using "serial_core" at all in favor of HVC or tty_core (the latter
>might be easier for you as the former is a bit of unknown atm)
>-mike
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Mike Frysinger]

On Fri, Oct 8, 2010 at 00:59, Daniel Walker wrote:
> I'm not disputing the serial core part. I ok with converting it to tty.

sorry, i missed the distinction you were making with the "disputation" qualifier

please dont top post
-mike

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

[matthieu castet]

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

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).



Matthieu

[-- Attachment #2: dcc.diff --]
[-- Type: text/x-diff, Size: 14540 bytes --]

diff --git a/drivers/parrot/serial/dcc.c b/drivers/parrot/serial/dcc.c
new file mode 100644
index 0000000..e778b0c
--- /dev/null
+++ b/drivers/parrot/serial/dcc.c
@@ -0,0 +1,590 @@
+/*
+ *  linux/drivers/serial/dcc_serial.c
+ *
+ *  serial port emulation driver for the JTAG DCC Terminal.
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ *  Changelog:
+ *   Oct-2003 Hyok S. Choi	Created
+ *   Feb-2004 Hyok S. Choi	Updated for serial_core.c and 2.6 kernel
+ *   Mar-2005 Hyok S. Choi	renamed from T32 to DCC
+ *   Apr-2006 Hyok S. Choi	revised including the MAJOR number
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/tty.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/serial.h>
+#include <linux/console.h>
+#include <linux/sysrq.h>
+#include <linux/tty_flip.h>
+#include <linux/major.h>
+
+#include <asm/io.h>
+#include <asm/irq.h>
+
+#include <linux/serial_core.h>
+
+/* 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
+
+#warning polling mode
+#endif
+
+#define UART_NR			1	/* we have only one JTAG port */
+
+#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_MAJOR	204
+#define SERIAL_DCC_MINOR	186
+#endif
+
+/* DCC Status Bits */
+#define DCC_STATUS_RX		(1 << 0)
+#define DCC_STATUS_TX		(1 << 1)
+
+/* primitive JTAG1 protocol utilities */
+static inline u32 __dcc_getstatus(void)
+{
+	u32 ret;
+
+	asm __volatile__ ("mrc p14, 0, %0, c0, c0	@ read comms ctrl reg"
+		: "=r" (ret));
+
+	return ret;
+}
+
+static inline unsigned char __dcc_getchar(void)
+{
+	unsigned char c;
+
+	asm __volatile__ ("mrc p14, 0, %0, c1, c0	@ read comms data reg"
+		: "=r" (c));
+
+	return c;
+}
+
+static inline void __dcc_putchar(unsigned char c)
+{
+	asm __volatile__ ("mcr p14, 0, %0, c1, c0	@ write a char"
+		: /* no output register */
+		: "r" (c));
+}
+/* end of JTAG1 dependencies */
+
+static void dcc_serial_tx_chars(struct uart_port *port, int max_count);
+static void dcc_serial_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_serial_tx_ready(port) \
+	((__dcc_getstatus() & DCC_STATUS_TX) == 0)
+
+enum {
+	TX_OFF,
+	TX_ON,
+};
+static int dcc_serial_tx_enable;
+enum {
+	RX_OFF,
+	RX_ON,
+	RX_PAUSE,
+};
+static int dcc_serial_rx_enable;
+/* port locked, interrupts locally disabled */
+static void dcc_serial_start_tx(struct uart_port *port)
+{
+	if (dcc_serial_tx_enable == TX_OFF) {
+		enable_irq(port->irq);
+		dcc_serial_tx_enable = TX_ON;
+	}
+}
+
+/* port locked, interrupts locally disabled */
+static void dcc_serial_stop_tx(struct uart_port *port)
+{
+	if (dcc_serial_tx_enable == TX_ON) {
+		disable_irq(port->irq);
+		dcc_serial_tx_enable = TX_OFF;
+	}
+}
+
+/* port locked, interrupts locally disabled */
+static void dcc_serial_stop_rx(struct uart_port *port)
+{
+	if (dcc_serial_rx_enable == RX_ON) {
+		disable_irq((int)port->membase);
+	}
+	dcc_serial_rx_enable = RX_OFF;
+}
+
+/* port locked, interrupts locally disabled */
+static void dcc_serial_throttle_rx(struct uart_port *port, int stop)
+{
+	if (stop && dcc_serial_rx_enable == RX_ON) {
+		disable_irq((int)port->membase);
+		dcc_serial_rx_enable = RX_PAUSE;
+	}
+	else if (stop == 0 && dcc_serial_rx_enable == RX_PAUSE) {
+		enable_irq((int)port->membase);
+		dcc_serial_rx_enable = RX_ON;
+	}
+}
+
+#if 0
+static void dcc_serial_rx_mchars(struct uart_port *port)
+{
+	int timeout;
+	unsigned int read = 0;
+	unsigned int it = 0;
+	unsigned int it1 = 0;
+	do {
+		timeout = 1000;
+		/* wait for a char */
+		//while ((__dcc_getstatus() & DCC_STATUS_RX) == 0 && timeout-- > 0) {
+		while ((readl(0xfc000008) & 0x08000000) == 0) {
+			cpu_relax();
+		}
+		/* read it */
+		if (timeout >= 0) {
+			if (readl(0xfc000008) & 0x08000000)
+				it++;
+			read++;
+			if (read & 0x100) {
+				printk("dcc_serial stat %u %u %u\n", read, it, it1);
+				read = it = it1 = 0;
+			}
+			dcc_serial_rx_chars(port);
+			if (readl(0xfc000008) & 0x08000000)
+				it1++;
+		}
+	} while (/*timeout >= 0*/1);
+}
+#endif
+#ifdef CONFIG_SERIAL_DCC_IDLE_POLL
+int dcc_idle(void)
+{
+	return 0;
+}
+#endif
+
+static irqreturn_t dcc_serial_int_rx(int irq, void *dev_id)
+{
+	struct uart_port *port = dev_id;
+	spin_lock(&port->lock);
+	dcc_serial_rx_chars(port);
+	spin_unlock(&port->lock);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t dcc_serial_int_tx(int irq, void *dev_id)
+{
+	struct uart_port *port = dev_id;
+	spin_lock(&port->lock);
+
+	dcc_serial_tx_chars(port, 64);
+	spin_unlock(&port->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int dcc_serial_startup(struct uart_port *port)
+{
+	int retval;
+
+	dcc_serial_tx_enable = TX_ON;
+	dcc_serial_rx_enable = RX_ON;
+	/* Allocate the IRQ */
+	retval = request_irq(port->irq, dcc_serial_int_tx, IRQF_DISABLED,
+			     "serial_dcc_serial_tx", port);
+	if (retval)
+		return retval;
+
+	/* Allocate the IRQ */
+	retval = request_irq((int)port->membase, dcc_serial_int_rx, IRQF_DISABLED,
+			     "serial_dcc_serial_rx", port);
+	if (retval)
+		return retval;
+
+	return 0;
+}
+
+static void dcc_serial_shutdown(struct uart_port *port)
+{
+	free_irq(port->irq, port);
+	free_irq((int)port->membase, port);
+}
+#else /* emulation by scheduled work */
+static void dcc_serial_poll(struct work_struct *work);
+static DECLARE_DELAYED_WORK(dcc_serial_poll_task, dcc_serial_poll);
+static struct uart_port dcc_serial_port;
+static int dcc_serial_active;
+
+/* in polling mode, we wait for the next character */
+static int dcc_serial_tx_ready(struct uart_port *port) {
+	while (__dcc_getstatus() & DCC_STATUS_TX) {
+		/* while waiting, we can check rx : it is free */
+		dcc_serial_rx_chars(port);
+		cpu_relax();
+	}
+	return 1;
+}
+
+static void dcc_serial_start_tx(struct uart_port *port)
+{
+	dcc_serial_tx_chars(port, 0xFFFF);
+}
+
+static void dcc_serial_stop_tx(struct uart_port *port)
+{
+}
+
+static void dcc_serial_throttle_rx(struct uart_port *port, int stop)
+{
+}
+
+static void dcc_serial_stop_rx(struct uart_port *port)
+{
+}
+
+/* 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 = &dcc_serial_port;
+	if (dcc_serial_active == 0)
+		return 0;
+
+	spin_lock(&port->lock);
+
+	dcc_serial_rx_chars(port);
+	dcc_serial_tx_chars(port, 64);
+	dcc_serial_rx_chars(port);
+
+	spin_unlock(&port->lock);
+	return 0;
+}
+#endif
+
+/* we poll dcc every jiffies */
+static void dcc_serial_poll(struct work_struct *work)
+{
+	struct uart_port *port = &dcc_serial_port;
+
+	spin_lock(&port->lock);
+
+	dcc_serial_rx_chars(port);
+	dcc_serial_tx_chars(port, 0xFFFF);
+	dcc_serial_rx_chars(port);
+
+	schedule_delayed_work(&dcc_serial_poll_task, 1);
+
+	spin_unlock(&port->lock);
+}
+static int dcc_serial_startup(struct uart_port *port)
+{
+	/* shcedule the polling work */
+	schedule_delayed_work(&dcc_serial_poll_task, 1);
+	dcc_serial_active = 1;
+
+	return 0;
+}
+
+static void dcc_serial_shutdown(struct uart_port *port)
+{
+	cancel_rearming_delayed_work(&dcc_serial_poll_task);
+	dcc_serial_active = 0;
+}
+#endif /* end of DCC_IRQ_USED */
+
+
+static void dcc_serial_putchar(struct uart_port *port, int ch)
+{
+	while (__dcc_getstatus() & DCC_STATUS_TX)
+		cpu_relax();
+	__dcc_putchar((char)(ch & 0xFF));
+}
+
+static void dcc_serial_rx_chars(struct uart_port *port)
+{
+	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 void dcc_serial_tx_chars(struct uart_port *port, int max_count)
+{
+	struct circ_buf *xmit = &port->info->xmit;
+
+	if (port->x_char) {
+		dcc_serial_putchar(port, port->x_char);
+		port->icount.tx++;
+		port->x_char = 0;
+		goto out;
+	}
+	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
+		dcc_serial_stop_tx(port);
+		goto out;
+	}
+
+	while (!uart_circ_empty(xmit) && dcc_serial_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_serial_stop_tx(port);
+	}
+
+out:
+	return;
+}
+
+static unsigned int dcc_serial_tx_empty(struct uart_port *port)
+{
+	return (__dcc_getstatus() & DCC_STATUS_TX) ? 0:TIOCSER_TEMT;
+}
+
+static unsigned int dcc_serial_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CTS | TIOCM_DSR | TIOCM_CD;
+}
+
+/* need for throttle/unthrottle */
+static void dcc_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* throttle clear TIOCM_RTS, unthrottle set it */
+	dcc_serial_throttle_rx(port, !(mctrl & TIOCM_RTS));
+}
+
+static void dcc_serial_set_termios(struct uart_port *port, struct ktermios *termios,
+		   struct ktermios *old)
+{
+	unsigned long flags;
+	unsigned int baud, quot;
+
+	/*
+	 * We don't support parity, stop bits, or anything other
+	 * than 8 bits, so clear these termios flags.
+	 */
+	termios->c_cflag &= ~(CSIZE | CSTOPB | PARENB | PARODD | CREAD);
+	termios->c_cflag |= CS8;
+
+	/*
+	 * We don't appear to support any error conditions either.
+	 */
+	termios->c_iflag &= ~(INPCK | IGNPAR | IGNBRK | BRKINT);
+
+	/*
+	 * Ask the core to calculate the divisor for us.
+	 */
+	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16); 
+	quot = uart_get_divisor(port, baud);
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	uart_update_timeout(port, termios->c_cflag, baud);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static const char * dcc_serial_type(struct uart_port *port)
+{
+	return port->type == PORT_DCC_JTAG1 ? "DCC" : NULL;
+}
+
+static int dcc_serial_request_port(struct uart_port *port)
+{
+	return 0;
+}
+
+/*
+ * Configure/autoconfigure the port.
+ */
+static void dcc_serial_config_port(struct uart_port *port, int flags)
+{
+	if (flags & UART_CONFIG_TYPE) {
+		port->type = PORT_DCC_JTAG1;
+		dcc_serial_request_port(port);
+	}
+}
+
+/*
+ * verify the new serial_struct (for TIOCSSERIAL).
+ */
+static int dcc_serial_verify_port(struct uart_port *port, struct serial_struct *ser)
+{
+	int ret = 0;
+	if (ser->type != PORT_UNKNOWN && ser->type != PORT_DCC_JTAG1)
+		ret = -EINVAL;
+	if (ser->irq < 0 || ser->irq >= NR_IRQS)
+		ret = -EINVAL;
+	if (ser->baud_base < 9600)
+		ret = -EINVAL;
+	return ret;
+}
+
+/* dummy operation handlers for uart_ops */
+static void dcc_serial_dummy_ops(struct uart_port *port)
+{
+}
+
+static void dcc_serial_break_ctl(struct uart_port *port, int i)
+{
+}
+
+static struct uart_ops dcc_serial_pops = {
+	.tx_empty	= dcc_serial_tx_empty,
+	.set_mctrl	= dcc_serial_set_mctrl,
+	.get_mctrl	= dcc_serial_get_mctrl,
+	.stop_tx	= dcc_serial_stop_tx,
+	.start_tx	= dcc_serial_start_tx,
+	.stop_rx	= dcc_serial_stop_rx,
+	.enable_ms	= dcc_serial_dummy_ops,
+	.break_ctl	= dcc_serial_break_ctl,
+	.startup	= dcc_serial_startup,
+	.shutdown	= dcc_serial_shutdown,
+	.set_termios	= dcc_serial_set_termios,
+	.type		= dcc_serial_type,
+	.release_port	= dcc_serial_dummy_ops,
+	.request_port	= dcc_serial_request_port,
+	.config_port	= dcc_serial_config_port,
+	.verify_port	= dcc_serial_verify_port,
+};
+
+static struct uart_port dcc_serial_port = {
+	.mapbase	= 0x12345678,		/* for serial_core.c */
+	.iotype		= UPIO_MEM,	
+	.membase	= (char*)IRQ_RX,	/* we need these garbages */
+	.irq		= IRQ_TX,
+	.uartclk	= 14745600,
+	.fifosize	= 0,
+	.ops		= &dcc_serial_pops,
+	.flags		= UPF_BOOT_AUTOCONF,
+	.line		= 0,
+};
+
+#ifdef CONFIG_SERIAL_DCC_CONSOLE
+static void dcc_serial_console_write(struct console *co, const char *s, unsigned int count)
+{
+	uart_console_write(&dcc_serial_port, s, count, dcc_serial_putchar);
+}
+
+static int __init dcc_serial_console_setup(struct console *co, char *options)
+{
+	struct uart_port *port = &dcc_serial_port;
+	int baud = 9600;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+	return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver dcc_serial_reg;
+static struct console dcc_serial_console = {
+	.name		= SERIAL_DCC_NAME,
+	.write		= dcc_serial_console_write,
+	.device		= uart_console_device,
+	.setup		= dcc_serial_console_setup,
+	.flags		= CON_PRINTBUFFER,
+	.index		= -1,
+	.data		= &dcc_serial_reg,
+};
+
+static int __init dcc_serial_console_init(void)
+{
+	register_console(&dcc_serial_console);
+	return 0;
+}
+console_initcall(dcc_serial_console_init);
+
+#define DCC_CONSOLE	&dcc_serial_console
+#else
+#define DCC_CONSOLE	NULL
+#endif
+
+static struct uart_driver dcc_serial_reg = {
+	.owner		= THIS_MODULE,
+	.driver_name	= SERIAL_DCC_NAME,
+	.dev_name	= SERIAL_DCC_NAME,
+	.major		= SERIAL_DCC_MAJOR,
+	.minor		= SERIAL_DCC_MINOR,
+	.nr		= UART_NR,
+	.cons		= DCC_CONSOLE,
+};
+
+static int __init dcc_serial_init(void)
+{
+	int ret;
+
+	printk(KERN_INFO "DCC: JTAG1 Serial emulation driver driver $Revision: 1.9 $\n");
+
+	ret = uart_register_driver(&dcc_serial_reg);
+
+	if (ret)
+		return ret;
+
+	uart_add_one_port(&dcc_serial_reg, &dcc_serial_port);
+
+	return 0;
+}
+
+__initcall(dcc_serial_init);
+
+MODULE_DESCRIPTION("DCC(JTAG1) JTAG debugger console emulation driver");
+MODULE_AUTHOR("Hyok S. Choi <hyok.choi@samsung.com>");
+MODULE_SUPPORTED_DEVICE(SERIAL_DCC_NAME);
+MODULE_LICENSE("GPL");

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

[Daniel Walker]

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).

It's hard to see what you've changed here. Could you possible make this
a delta against the original version you started with?

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[Daniel Walker]

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?

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

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

[matthieu castet]

[-- 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,