Skip to contentSkip to conversation

tty: Fix comments that reference BKL, eventd, old paths

PATCH In review Kevin Cernekee 2 participants 6 messages Dec 27, 2012 – Dec 28, 2012

Subsystems (3)
Documentation TTY layer and serial drivers Rocketport express/infinity driver
lore
Share & tools
lore

Kevin Cernekee

author maintainer
#0 lore raw

PATCH 1/3 tty: Fix comments that reference BKL, eventd, old paths

Signed-off-by Kevin Cernekee <[email protected]>
---
 drivers/tty/serial/serial_core.c |   18 +++++++++---------
 drivers/tty/tty_io.c             |    2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)
drivers/tty/serial/serial_core.c +9 -9
index 2c7230a..83d127f 100644
@@ -1308,9 +1308,10 @@ static void uart_set_termios(struct tty_struct *tty,
 }
 
 /*
- * In 2.4.5, calls to this will be serialized via the BKL in
- *  linux/drivers/char/tty_io.c:tty_release()
- *  linux/drivers/char/tty_io.c:do_tty_handup()
+ * Calls to uart_close() are serialised via the tty_lock in
+ *   drivers/tty/tty_io.c:tty_release()
+ *   drivers/tty/tty_io.c:do_tty_hangup()
+ * This runs from a workqueue and can sleep for a _short_ time only.
  */
 static void uart_close(struct tty_struct *tty, struct file *filp)
 {

@@ -1437,10 +1438,9 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
 }
 
 /*
- * This is called with the BKL held in
- *  linux/drivers/char/tty_io.c:do_tty_hangup()
- * We're called from the eventd thread, so we can sleep for
- * a _short_ time only.
+ * Calls to uart_hangup() are serialised by the tty_lock in
+ *   drivers/tty/tty_io.c:do_tty_hangup()
+ * This runs from a workqueue and can sleep for a _short_ time only.
  */
 static void uart_hangup(struct tty_struct *tty)
 {

@@ -1521,8 +1521,8 @@ static void uart_dtr_rts(struct tty_port *port, int onoff)
 }
 
 /*
- * calls to uart_open are serialised by the BKL in
- *   fs/char_dev.c:chrdev_open()
+ * Calls to uart_open are serialised by the tty_lock in
+ *   drivers/tty/tty_io.c:tty_open()
  * Note that if this fails, then uart_close() _will_ be called.
  *
  * In time, we want to scrap the "opening nonpresent ports"
drivers/tty/tty_io.c +1 -1
index da9fde8..54a254a 100644
@@ -536,7 +536,7 @@ EXPORT_SYMBOL_GPL(tty_wakeup);
  *	__tty_hangup		-	actual handler for hangup events
  *	@work: tty device
  *
- *	This can be called by the "eventd" kernel thread.  That is process
+ *	This can be called by a "kworker" kernel thread.  That is process
  *	synchronous but doesn't hold any locks, so we need to make sure we
  *	have the appropriate locks for what we're doing.
  *
Signature · 1 line
1.7.10.4
author maintainer
#1 lore raw

PATCH 2/3 tty: Update serial core API documentation

Signed-off-by Kevin Cernekee <[email protected]>
---
 Documentation/serial/driver |   39 +++++++++++++++++++++++++++++++++++++++
 include/linux/serial_core.h |    8 ++++----
 2 files changed, 43 insertions(+), 4 deletions(-)
Documentation/serial/driver +39 -0
index 0a25a91..c680678 100644
@@ -133,6 +133,16 @@ hardware.
 	Interrupts: locally disabled.
 	This call must not sleep
 
+  send_xchar(port,ch)
+	Transmit a high priority character, even if the port is stopped.
+	This is used to implement XON/XOFF flow control and tcflow().  If
+	the serial driver does not implement this function, the tty core
+	will append the character to the circular buffer and then call
+	start_tx() / stop_tx() to flush the data out.
+
+	Locking: none.
+	Interrupts: caller dependent.
+
   stop_rx(port)
 	Stop receiving characters; the port is in the process of
 	being closed.

@@ -255,6 +265,10 @@ hardware.
 	Locking: none.
 	Interrupts: caller dependent.
 
+  set_wake(port,state)
+	Enable/disable power management wakeup on serial activity.  Not
+	currently implemented.
+
   type(port)
 	Return a pointer to a string constant describing the specified
 	port, or return NULL, in which case the string 'unknown' is

@@ -307,6 +321,31 @@ hardware.
 	Locking: none.
 	Interrupts: caller dependent.
 
+  poll_init(port)
+	Called by kgdb to perform the minimal hardware initialization needed
+	to support poll_put_char() and poll_get_char().  Unlike ->startup()
+	this should not request interrupts.
+
+	Locking: tty_mutex and tty_port->mutex taken.
+	Interrupts: n/a.
+
+  poll_put_char(port,ch)
+	Called by kgdb to write a single character directly to the serial
+	port.  It can and should block until there is space in the TX FIFO.
+
+	Locking: none.
+	Interrupts: caller dependent.
+	This call must not sleep
+
+  poll_get_char(port)
+	Called by kgdb to read a single character directly from the serial
+	port.  If data is available, it should be returned; otherwise
+	the function should return NO_POLL_CHAR immediately.
+
+	Locking: none.
+	Interrupts: caller dependent.
+	This call must not sleep
+
 Other functions
 ---------------
include/linux/serial_core.h +4 -4
index c6690a2..ccfeab2 100644
@@ -37,8 +37,8 @@ struct serial_struct;
 struct device;
 
 /*
- * This structure describes all the operations that can be
- * done on the physical hardware.
+ * This structure describes all the operations that can be done on the
+ * physical hardware.  See Documentation/serial/driver for details.
  */
 struct uart_ops {
 	unsigned int	(*tx_empty)(struct uart_port *);

@@ -65,7 +65,7 @@ struct uart_ops {
 	/*
 	 * Return a string describing the type of the port
 	 */
-	const char *(*type)(struct uart_port *);
+	const char	*(*type)(struct uart_port *);
 
 	/*
 	 * Release IO and memory resources used by the port.

@@ -83,7 +83,7 @@ struct uart_ops {
 	int		(*ioctl)(struct uart_port *, unsigned int, unsigned long);
 #ifdef CONFIG_CONSOLE_POLL
 	int		(*poll_init)(struct uart_port *);
-	void	(*poll_put_char)(struct uart_port *, unsigned char);
+	void		(*poll_put_char)(struct uart_port *, unsigned char);
 	int		(*poll_get_char)(struct uart_port *);
 #endif
 };
Signature · 1 line
1.7.10.4
author maintainer
#3 lore raw

PATCH 3/3 serial: rp2: New driver for Comtrol RocketPort 2 cards

In reply to Kevin Cernekee
Signed-off-by Kevin Cernekee <[email protected]>
---
 drivers/tty/serial/Kconfig       |   24 ++
 drivers/tty/serial/Makefile      |    1 +
 drivers/tty/serial/rp2.c         |  828 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |    3 +
 4 files changed, 856 insertions(+)
 create mode 100644 drivers/tty/serial/rp2.c

I am able to run basic TX/RX serial console traffic, zmodem, etc. on a RocketPort Express Octa DB-9 card.

It would be helpful to get test coverage on other configurations like:

- Dual ASIC (32-port) cards
- Systems with multiple cards installed
- Multiple-card systems with more than CONFIG_SERIAL_RP2_NR_UARTS ports
- Anything involving heavy load, terminal servers, etc.
- Anything that uses the modem status lines or flow control
- Systems that have a combination of RocketPort 1 + RocketPort 2 cards (the PCI ID table in rocket.c actually matches anything with Comtrol's vendor ID)

Non-RS232 interfaces, such as RS422/RS485/SMPTE, are not currently supported. They could be, if somebody has a use for them.

drivers/tty/serial/Kconfig +24 -0
index 59c23d0..555f3ca 100644
@@ -1447,4 +1447,28 @@ config SERIAL_ARC_NR_PORTS
 	  Set this to the number of serial ports you want the driver
 	  to support.
 
+config SERIAL_RP2
+	tristate "Comtrol RocketPort EXPRESS/INFINITY support"
+	depends on PCI
+	select SERIAL_CORE
+	help
+	  This driver supports the Comtrol RocketPort EXPRESS and
+	  RocketPort INFINITY families of PCI/PCIe multiport serial adapters.
+	  These adapters use a "RocketPort 2" ASIC that is not compatible
+	  with the original RocketPort driver (CONFIG_ROCKETPORT).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rp2.
+
+	  If you want to compile this driver into the kernel, say Y here.  If
+	  you don't have a suitable RocketPort card installed, say N.
+
+config SERIAL_RP2_NR_UARTS
+	int "Maximum number of RocketPort EXPRESS/INFINITY ports"
+	depends on SERIAL_RP2
+	default "32"
+	help
+	  If multiple cards are present, the default limit of 32 ports may
+	  need to be increased.
+
 endmenu
drivers/tty/serial/Makefile +1 -0
index df1b998..a582ee9 100644
@@ -83,3 +83,4 @@ obj-$(CONFIG_SERIAL_SIRFSOC) += sirfsoc_uart.o
 obj-$(CONFIG_SERIAL_AR933X)   += ar933x_uart.o
 obj-$(CONFIG_SERIAL_EFM32_UART) += efm32-uart.o
 obj-$(CONFIG_SERIAL_ARC)	+= arc_uart.o
+obj-$(CONFIG_SERIAL_RP2)	+= rp2.o
drivers/tty/serial/rp2.c new file +828 -0

828 lines not on this page · raw message

include/uapi/linux/serial_core.h +3 -0
index 78f99d9..9dd47a5 100644
@@ -219,4 +219,7 @@
 /* ARC (Synopsys) on-chip UART */
 #define PORT_ARC       101
 
+/* Rocketport EXPRESS/INFINITY */
+#define PORT_RP2	102
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */
Signature · 1 line
1.7.10.4
#4 lore raw

Re: [PATCH 3/3] serial: rp2: New driver for Comtrol RocketPort 2 cards

Quoted 3 lines

- Systems that have a combination of RocketPort 1 + RocketPort 2 cards (the PCI ID table in rocket.c actually matches anything with Comtrol's vendor ID)

In which case that will need updating to only include the identifiers for the original rocket port, or ensuring that if it sees an rp2 it returns an error on the probe so the other probe occurs. The first is probably preferable as it avoids an un-needed driver load.

drivers/tty/serial/rp2.c line 181

Quoted 8 lines
drivers/tty/serial/rp2.c new file +8 -0
+static const u8 rp2_ucode[] = {
+	0xF6, 0x8B, 0xE2, 0x86, 0x30, 0x40, 0x66, 0x19, 0x0A, 0x31, 0x40, 0x67,
+	0x19, 0x0A, 0x86, 0x01, 0x40, 0x11, 0x19, 0x0A, 0x00, 0x40, 0x13, 0x19,
+	0x0A, 0xFA, 0x83, 0x01, 0x0A, 0x00, 0x0A, 0x41, 0xFF, 0x89, 0xC2, 0x66,
+	0x86, 0x81, 0x91, 0x40, 0x65, 0x8E, 0x89, 0xC2, 0x66, 0x86, 0x81, 0x88,
+	0x40, 0x65, 0x85, 0x84, 0x00, 0x82, 0x0A, 0x08, 0x0A, 0x0A, 0x0A, 0x0A,
+	0x0A, 0x08, 0x0A,
+};

If that is firmware so is actually code executed by the hardware on the RocketPort 2 then it should be dynamically loaded with request_firmware. That is preferred as it avoids some potential licence funnies with GPLv2

drivers/tty/serial/rp2.c line 405

Quoted 4 lines
drivers/tty/serial/rp2.c new file +4 -0
+static void rp2_rx_chars(struct rp2_uart_port *up)
+{
+	u16 bytes = readw(up->base + RP2_RX_FIFO_COUNT);
+	struct tty_struct *tty = up->port.state->port.tty;

You need to take a reference here and allow for tty being NULL so

	tty = tty_port_tty_get(&up->port.state->port);

	if (tty)

drivers/tty/serial/rp2.c line 429

Quoted 1 line
drivers/tty/serial/rp2.c new file +1 -0
+	tty_flip_buffer_push(tty);

tty_kref_put()

(you may need to flush the bytes to clear the interrupt in the tty == NULL case). This allows for a parallel close/hangup clearing the tty and ensures the tty itself won't be deleted until your tty_kref_put

drivers/tty/serial/rp2.c line 518

Quoted 8 lines
drivers/tty/serial/rp2.c new file +8 -0
+static inline void rp2_flush_fifos(struct rp2_uart_port *up)
+{
+	rp2_rmw_set(up, RP2_UART_CTL,
+		    RP2_UART_CTL_FLUSH_RX_m | RP2_UART_CTL_FLUSH_TX_m);
+	udelay(10);
+	rp2_rmw_clr(up, RP2_UART_CTL,
+		    RP2_UART_CTL_FLUSH_RX_m | RP2_UART_CTL_FLUSH_TX_m);
+}

This looks wrong. A writel can be posted by the PCI bridge. That means it may occur after the udelay(10) finishes. However it cannot pass another read to the same device.

You probably want

	rp2_rmw_set(....)
	readl(some harmless register)
	udelay(10);
	rpm2_rmw_clr(....)

drivers/tty/serial/rp2.c line 604

Quoted 8 lines
drivers/tty/serial/rp2.c new file +8 -0
+static void rp2_reset_asic(struct rp2_card *card, unsigned int asic_id)
+{
+	void __iomem *base = card->bar1 + RP2_ASIC_OFFSET(asic_id);
+	u32 clk_cfg;
+
+	writew(1, base + RP2_GLOBAL_CMD);
+	msleep(100);
+	writel(0, base + RP2_CLK_PRESCALER);

Same problem here

drivers/tty/serial/rp2.c line 637

Quoted 10 lines
drivers/tty/serial/rp2.c new file +10 -0
+static void rp2_init_port(struct rp2_uart_port *up)
+{
+	int i;
+
+	writel(RP2_UART_CTL_RESET_CH_m, up->base + RP2_UART_CTL);
+	udelay(1);
+
+	writel(0, up->base + RP2_TXRX_CTL);
+	writel(0, up->base + RP2_UART_CTL);
+	udelay(1);

And probably here

Alan