All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] jsm: remove remaining flip buffer code
@ 2011-08-24 16:14 Thadeu Lima de Souza Cascardo
  2011-08-24 16:14 ` [PATCH 2/3] jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2011-08-24 16:14 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Alan Cox, linux-serial, linux-kernel, Thadeu Lima de Souza Cascardo

The flip buffer is not used anymore. Remove its allocation and
declaration in the board structure.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 drivers/tty/serial/jsm/jsm.h        |    3 ---
 drivers/tty/serial/jsm/jsm_driver.c |   18 ------------------
 2 files changed, 0 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/jsm/jsm.h b/drivers/tty/serial/jsm/jsm.h
index b704c8c..cd53bdd 100644
--- a/drivers/tty/serial/jsm/jsm.h
+++ b/drivers/tty/serial/jsm/jsm.h
@@ -88,7 +88,6 @@ enum {
 
 /* 4 extra for alignment play space */
 #define WRITEBUFLEN	((4096) + 4)
-#define MYFLIPLEN	N_TTY_BUF_SIZE
 
 #define JSM_VERSION	"jsm: 1.2-1-INKERNEL"
 #define JSM_PARTNUM	"40002438_A-INKERNEL"
@@ -150,7 +149,6 @@ struct jsm_board
 	u32		bd_uart_offset;	/* Space between each UART */
 
 	struct jsm_channel *channels[MAXPORTS]; /* array of pointers to our channels. */
-	char		*flipbuf;	/* Our flip buffer, alloced if board is found */
 
 	u32		bd_dividend;	/* Board/UARTs specific dividend */
 
@@ -177,7 +175,6 @@ struct jsm_board
 #define CH_TX_FIFO_LWM	0x0800		/* TX Fifo is below Low Water	*/
 #define CH_BREAK_SENDING 0x1000		/* Break is being sent		*/
 #define CH_LOOPBACK 0x2000		/* Channel is in lookback mode	*/
-#define CH_FLIPBUF_IN_USE 0x4000	/* Channel's flipbuf is in use	*/
 #define CH_BAUD0	0x08000		/* Used for checking B0 transitions */
 
 /* Our Read/Error/Write queue sizes */
diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
index 96da178..1cc8cf6 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -160,27 +160,10 @@ static int __devinit jsm_probe_one(struct pci_dev *pdev, const struct pci_device
 	dev_info(&pdev->dev, "board %d: Digi Neo (rev %d), irq %d\n",
 			adapter_count, brd->rev, brd->irq);
 
-	/*
-	 * allocate flip buffer for board.
-	 *
-	 * Okay to malloc with GFP_KERNEL, we are not at interrupt
-	 * context, and there are no locks held.
-	 */
-	brd->flipbuf = kzalloc(MYFLIPLEN, GFP_KERNEL);
-	if (!brd->flipbuf) {
-		/* XXX: leaking all resources from jsm_tty_init and
-		 	jsm_uart_port_init here! */
-		dev_err(&pdev->dev, "memory allocation for flipbuf failed\n");
-		rc = -ENOMEM;
-		goto out_free_uart;
-	}
-
 	pci_set_drvdata(pdev, brd);
 	pci_save_state(pdev);
 
 	return 0;
- out_free_uart:
-	jsm_remove_uart_port(brd);
  out_free_irq:
 	jsm_remove_uart_port(brd);
 	free_irq(brd->irq, brd);
@@ -218,7 +201,6 @@ static void __devexit jsm_remove_one(struct pci_dev *pdev)
 
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
-	kfree(brd->flipbuf);
 	kfree(brd);
 }
 
-- 
1.7.4.4


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

* [PATCH 2/3] jsm: remove buggy write queue
  2011-08-24 16:14 [PATCH 1/3] jsm: remove remaining flip buffer code Thadeu Lima de Souza Cascardo
@ 2011-08-24 16:14 ` Thadeu Lima de Souza Cascardo
  2011-08-24 16:14 ` [PATCH 3/3] jsm: print byte we are dequeing Thadeu Lima de Souza Cascardo
  2011-09-13 17:55 ` jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
  2 siblings, 0 replies; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2011-08-24 16:14 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Alan Cox, linux-serial, linux-kernel, Thadeu Lima de Souza Cascardo

jsm uses a write queue that copies from uart_core circular buffer. This
copying however has some bugs, like not wrapping the head counter. Since
this write queue is also a circular buffer, the consumer function is
ready to use the uart_core circular buffer directly.

This buggy copying function was making some bytes be dropped when
transmitting to a raw tty, doing something like this.

[root@hostname ~]$ cat /dev/ttyn1 > cascardo/dump &
[1] 2658
[root@hostname ~]$ cat /proc/tty/drivers > /dev/ttyn0
[root@hostname ~]$ cat /proc/tty/drivers
/dev/tty             /dev/tty        5       0 system:/dev/tty
/dev/console         /dev/console    5       1 system:console
/dev/ptmx            /dev/ptmx       5       2 system
/dev/vc/0            /dev/vc/0       4       0 system:vtmaster
jsm                  /dev/ttyn     250 0-31 serial
serial               /dev/ttyS       4 64-95 serial
hvc                  /dev/hvc      229 0-7 system
pty_slave            /dev/pts      136 0-1048575 pty:slave
pty_master           /dev/ptm      128 0-1048575 pty:master
unknown              /dev/tty        4 1-63 console
[root@hostname ~]$ cat cascardo/dump
/dev/tty             /dev/tty        5       0 system:/dev/tty
/dev/console         /dev/console    5       1 system:console
/dev/ptmx            /dev/ptmx       5       2 system
/dev/vc/0            /dev/vc/0       4       0 system:vtmaste[root@hostname ~]$

This patch drops the driver write queue entirely, using the circular
buffer from uart_core only.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 drivers/tty/serial/jsm/jsm.h        |    7 ---
 drivers/tty/serial/jsm/jsm_driver.c |    1 -
 drivers/tty/serial/jsm/jsm_neo.c    |   29 ++++++-----
 drivers/tty/serial/jsm/jsm_tty.c    |   94 +++++------------------------------
 4 files changed, 28 insertions(+), 103 deletions(-)

diff --git a/drivers/tty/serial/jsm/jsm.h b/drivers/tty/serial/jsm/jsm.h
index cd53bdd..529bec6 100644
--- a/drivers/tty/serial/jsm/jsm.h
+++ b/drivers/tty/serial/jsm/jsm.h
@@ -180,10 +180,8 @@ struct jsm_board
 /* Our Read/Error/Write queue sizes */
 #define RQUEUEMASK	0x1FFF		/* 8 K - 1 */
 #define EQUEUEMASK	0x1FFF		/* 8 K - 1 */
-#define WQUEUEMASK	0x0FFF		/* 4 K - 1 */
 #define RQUEUESIZE	(RQUEUEMASK + 1)
 #define EQUEUESIZE	RQUEUESIZE
-#define WQUEUESIZE	(WQUEUEMASK + 1)
 
 
 /************************************************************************
@@ -223,10 +221,6 @@ struct jsm_channel {
 	u16		ch_e_head;	/* Head location of the error queue */
 	u16		ch_e_tail;	/* Tail location of the error queue */
 
-	u8		*ch_wqueue;	/* Our write queue buffer - malloc'ed */
-	u16		ch_w_head;	/* Head location of the write queue */
-	u16		ch_w_tail;	/* Tail location of the write queue */
-
 	u64		ch_rxcount;	/* total of data received so far */
 	u64		ch_txcount;	/* total of data transmitted so far */
 
@@ -375,7 +369,6 @@ extern int	jsm_debug;
  * Prototypes for non-static functions used in more than one module
  *
  *************************************************************************/
-int jsm_tty_write(struct uart_port *port);
 int jsm_tty_init(struct jsm_board *);
 int jsm_uart_port_init(struct jsm_board *);
 int jsm_remove_uart_port(struct jsm_board *);
diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
index 1cc8cf6..648b6a3 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -194,7 +194,6 @@ static void __devexit jsm_remove_one(struct pci_dev *pdev)
 		if (brd->channels[i]) {
 			kfree(brd->channels[i]->ch_rqueue);
 			kfree(brd->channels[i]->ch_equeue);
-			kfree(brd->channels[i]->ch_wqueue);
 			kfree(brd->channels[i]);
 		}
 	}
diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
index 4538c3e..bd6e846 100644
--- a/drivers/tty/serial/jsm/jsm_neo.c
+++ b/drivers/tty/serial/jsm/jsm_neo.c
@@ -496,12 +496,15 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
 	int s;
 	int qlen;
 	u32 len_written = 0;
+	struct circ_buf *circ;
 
 	if (!ch)
 		return;
 
+	circ = &ch->uart_port.state->xmit;
+
 	/* No data to write to the UART */
-	if (ch->ch_w_tail == ch->ch_w_head)
+	if (uart_circ_empty(circ))
 		return;
 
 	/* If port is "stopped", don't send any data to the UART */
@@ -517,11 +520,10 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
 		if (ch->ch_cached_lsr & UART_LSR_THRE) {
 			ch->ch_cached_lsr &= ~(UART_LSR_THRE);
 
-			writeb(ch->ch_wqueue[ch->ch_w_tail], &ch->ch_neo_uart->txrx);
+			writeb(circ->buf[circ->tail], &ch->ch_neo_uart->txrx);
 			jsm_printk(WRITE, INFO, &ch->ch_bd->pci_dev,
-					"Tx data: %x\n", ch->ch_wqueue[ch->ch_w_head]);
-			ch->ch_w_tail++;
-			ch->ch_w_tail &= WQUEUEMASK;
+					"Tx data: %x\n", circ->buf[circ->head]);
+			circ->tail = (circ->tail + 1) & (UART_XMIT_SIZE - 1);
 			ch->ch_txcount++;
 		}
 		return;
@@ -536,36 +538,36 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
 	n = UART_17158_TX_FIFOSIZE - ch->ch_t_tlevel;
 
 	/* cache head and tail of queue */
-	head = ch->ch_w_head & WQUEUEMASK;
-	tail = ch->ch_w_tail & WQUEUEMASK;
-	qlen = (head - tail) & WQUEUEMASK;
+	head = circ->head & (UART_XMIT_SIZE - 1);
+	tail = circ->tail & (UART_XMIT_SIZE - 1);
+	qlen = uart_circ_chars_pending(circ);
 
 	/* Find minimum of the FIFO space, versus queue length */
 	n = min(n, qlen);
 
 	while (n > 0) {
 
-		s = ((head >= tail) ? head : WQUEUESIZE) - tail;
+		s = ((head >= tail) ? head : UART_XMIT_SIZE) - tail;
 		s = min(s, n);
 
 		if (s <= 0)
 			break;
 
-		memcpy_toio(&ch->ch_neo_uart->txrxburst, ch->ch_wqueue + tail, s);
+		memcpy_toio(&ch->ch_neo_uart->txrxburst, circ->buf + tail, s);
 		/* Add and flip queue if needed */
-		tail = (tail + s) & WQUEUEMASK;
+		tail = (tail + s) & (UART_XMIT_SIZE - 1);
 		n -= s;
 		ch->ch_txcount += s;
 		len_written += s;
 	}
 
 	/* Update the final tail */
-	ch->ch_w_tail = tail & WQUEUEMASK;
+	circ->tail = tail & (UART_XMIT_SIZE - 1);
 
 	if (len_written >= ch->ch_t_tlevel)
 		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 
-	if (!jsm_tty_write(&ch->uart_port))
+	if (uart_circ_empty(circ))
 		uart_write_wakeup(&ch->uart_port);
 }
 
@@ -946,7 +948,6 @@ static void neo_param(struct jsm_channel *ch)
 	if ((ch->ch_c_cflag & (CBAUD)) == 0) {
 		ch->ch_r_head = ch->ch_r_tail = 0;
 		ch->ch_e_head = ch->ch_e_tail = 0;
-		ch->ch_w_head = ch->ch_w_tail = 0;
 
 		neo_flush_uart_write(ch);
 		neo_flush_uart_read(ch);
diff --git a/drivers/tty/serial/jsm/jsm_tty.c b/drivers/tty/serial/jsm/jsm_tty.c
index 7a4a914..434bd88 100644
--- a/drivers/tty/serial/jsm/jsm_tty.c
+++ b/drivers/tty/serial/jsm/jsm_tty.c
@@ -118,6 +118,19 @@ static void jsm_tty_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	udelay(10);
 }
 
+/*
+ * jsm_tty_write()
+ *
+ * Take data from the user or kernel and send it out to the FEP.
+ * In here exists all the Transparent Print magic as well.
+ */
+static void jsm_tty_write(struct uart_port *port)
+{
+	struct jsm_channel *channel;
+	channel = container_of(port, struct jsm_channel, uart_port);
+	channel->ch_bd->bd_ops->copy_data_from_queue_to_uart(channel);
+}
+
 static void jsm_tty_start_tx(struct uart_port *port)
 {
 	struct jsm_channel *channel = (struct jsm_channel *)port;
@@ -216,14 +229,6 @@ static int jsm_tty_open(struct uart_port *port)
 			return -ENOMEM;
 		}
 	}
-	if (!channel->ch_wqueue) {
-		channel->ch_wqueue = kzalloc(WQUEUESIZE, GFP_KERNEL);
-		if (!channel->ch_wqueue) {
-			jsm_printk(INIT, ERR, &channel->ch_bd->pci_dev,
-				"unable to allocate write queue buf");
-			return -ENOMEM;
-		}
-	}
 
 	channel->ch_flags &= ~(CH_OPENING);
 	/*
@@ -237,7 +242,6 @@ static int jsm_tty_open(struct uart_port *port)
 	 */
 	channel->ch_r_head = channel->ch_r_tail = 0;
 	channel->ch_e_head = channel->ch_e_tail = 0;
-	channel->ch_w_head = channel->ch_w_tail = 0;
 
 	brd->bd_ops->flush_uart_write(channel);
 	brd->bd_ops->flush_uart_read(channel);
@@ -836,75 +840,3 @@ void jsm_check_queue_flow_control(struct jsm_channel *ch)
 		}
 	}
 }
-
-/*
- * jsm_tty_write()
- *
- * Take data from the user or kernel and send it out to the FEP.
- * In here exists all the Transparent Print magic as well.
- */
-int jsm_tty_write(struct uart_port *port)
-{
-	int bufcount;
-	int data_count = 0,data_count1 =0;
-	u16 head;
-	u16 tail;
-	u16 tmask;
-	u32 remain;
-	int temp_tail = port->state->xmit.tail;
-	struct jsm_channel *channel = (struct jsm_channel *)port;
-
-	tmask = WQUEUEMASK;
-	head = (channel->ch_w_head) & tmask;
-	tail = (channel->ch_w_tail) & tmask;
-
-	if ((bufcount = tail - head - 1) < 0)
-		bufcount += WQUEUESIZE;
-
-	bufcount = min(bufcount, 56);
-	remain = WQUEUESIZE - head;
-
-	data_count = 0;
-	if (bufcount >= remain) {
-		bufcount -= remain;
-		while ((port->state->xmit.head != temp_tail) &&
-		(data_count < remain)) {
-			channel->ch_wqueue[head++] =
-			port->state->xmit.buf[temp_tail];
-
-			temp_tail++;
-			temp_tail &= (UART_XMIT_SIZE - 1);
-			data_count++;
-		}
-		if (data_count == remain) head = 0;
-	}
-
-	data_count1 = 0;
-	if (bufcount > 0) {
-		remain = bufcount;
-		while ((port->state->xmit.head != temp_tail) &&
-			(data_count1 < remain)) {
-			channel->ch_wqueue[head++] =
-				port->state->xmit.buf[temp_tail];
-
-			temp_tail++;
-			temp_tail &= (UART_XMIT_SIZE - 1);
-			data_count1++;
-
-		}
-	}
-
-	port->state->xmit.tail = temp_tail;
-
-	data_count += data_count1;
-	if (data_count) {
-		head &= tmask;
-		channel->ch_w_head = head;
-	}
-
-	if (data_count) {
-		channel->ch_bd->bd_ops->copy_data_from_queue_to_uart(channel);
-	}
-
-	return data_count;
-}
-- 
1.7.4.4


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

* [PATCH 3/3] jsm: print byte we are dequeing
  2011-08-24 16:14 [PATCH 1/3] jsm: remove remaining flip buffer code Thadeu Lima de Souza Cascardo
  2011-08-24 16:14 ` [PATCH 2/3] jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
@ 2011-08-24 16:14 ` Thadeu Lima de Souza Cascardo
  2011-09-13 17:55 ` jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
  2 siblings, 0 replies; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2011-08-24 16:14 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Alan Cox, linux-serial, linux-kernel, Thadeu Lima de Souza Cascardo

Instead of printing the head of the buffer, we should print the tail,
which is the byte we are sending to the device.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
 drivers/tty/serial/jsm/jsm_neo.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
index bd6e846..81dfafa 100644
--- a/drivers/tty/serial/jsm/jsm_neo.c
+++ b/drivers/tty/serial/jsm/jsm_neo.c
@@ -522,7 +522,7 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
 
 			writeb(circ->buf[circ->tail], &ch->ch_neo_uart->txrx);
 			jsm_printk(WRITE, INFO, &ch->ch_bd->pci_dev,
-					"Tx data: %x\n", circ->buf[circ->head]);
+					"Tx data: %x\n", circ->buf[circ->tail]);
 			circ->tail = (circ->tail + 1) & (UART_XMIT_SIZE - 1);
 			ch->ch_txcount++;
 		}
-- 
1.7.4.4


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

* jsm: remove buggy write queue
  2011-08-24 16:14 [PATCH 1/3] jsm: remove remaining flip buffer code Thadeu Lima de Souza Cascardo
  2011-08-24 16:14 ` [PATCH 2/3] jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
  2011-08-24 16:14 ` [PATCH 3/3] jsm: print byte we are dequeing Thadeu Lima de Souza Cascardo
@ 2011-09-13 17:55 ` Thadeu Lima de Souza Cascardo
  2011-09-22 22:38   ` Greg KH
  2 siblings, 1 reply; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2011-09-13 17:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: stable, Breno Leitao, lucaskt, Alan Cox, linux-serial,
	linux-kernel, Thadeu Lima de Souza Cascardo

jsm uses a write queue that copies from uart_core circular buffer. This
copying however has some bugs, like not wrapping the head counter. Since
this write queue is also a circular buffer, the consumer function is
ready to use the uart_core circular buffer directly.

This buggy copying function was making some bytes be dropped when
transmitting to a raw tty, doing something like this.

[root@hostname ~]$ cat /dev/ttyn1 > cascardo/dump &
[1] 2658
[root@hostname ~]$ cat /proc/tty/drivers > /dev/ttyn0
[root@hostname ~]$ cat /proc/tty/drivers
/dev/tty             /dev/tty        5       0 system:/dev/tty
/dev/console         /dev/console    5       1 system:console
/dev/ptmx            /dev/ptmx       5       2 system
/dev/vc/0            /dev/vc/0       4       0 system:vtmaster
jsm                  /dev/ttyn     250 0-31 serial
serial               /dev/ttyS       4 64-95 serial
hvc                  /dev/hvc      229 0-7 system
pty_slave            /dev/pts      136 0-1048575 pty:slave
pty_master           /dev/ptm      128 0-1048575 pty:master
unknown              /dev/tty        4 1-63 console
[root@hostname ~]$ cat cascardo/dump
/dev/tty             /dev/tty        5       0 system:/dev/tty
/dev/console         /dev/console    5       1 system:console
/dev/ptmx            /dev/ptmx       5       2 system
/dev/vc/0            /dev/vc/0       4       0 system:vtmaste[root@hostname ~]$

This patch drops the driver write queue entirely, using the circular
buffer from uart_core only.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---

Hello, Greg.

This should probably go to 3.1 and stable releases, since it fixes a
bug.

Regards,
Cascardo.


---
 drivers/tty/serial/jsm/jsm.h        |    7 ---
 drivers/tty/serial/jsm/jsm_driver.c |    1 -
 drivers/tty/serial/jsm/jsm_neo.c    |   29 ++++++-----
 drivers/tty/serial/jsm/jsm_tty.c    |   94 +++++------------------------------
 4 files changed, 28 insertions(+), 103 deletions(-)

diff --git a/drivers/tty/serial/jsm/jsm.h b/drivers/tty/serial/jsm/jsm.h
index cd53bdd..529bec6 100644
--- a/drivers/tty/serial/jsm/jsm.h
+++ b/drivers/tty/serial/jsm/jsm.h
@@ -180,10 +180,8 @@ struct jsm_board
 /* Our Read/Error/Write queue sizes */
 #define RQUEUEMASK	0x1FFF		/* 8 K - 1 */
 #define EQUEUEMASK	0x1FFF		/* 8 K - 1 */
-#define WQUEUEMASK	0x0FFF		/* 4 K - 1 */
 #define RQUEUESIZE	(RQUEUEMASK + 1)
 #define EQUEUESIZE	RQUEUESIZE
-#define WQUEUESIZE	(WQUEUEMASK + 1)


 /************************************************************************
@@ -223,10 +221,6 @@ struct jsm_channel {
 	u16		ch_e_head;	/* Head location of the error queue */
 	u16		ch_e_tail;	/* Tail location of the error queue */

-	u8		*ch_wqueue;	/* Our write queue buffer - malloc'ed */
-	u16		ch_w_head;	/* Head location of the write queue */
-	u16		ch_w_tail;	/* Tail location of the write queue */
-
 	u64		ch_rxcount;	/* total of data received so far */
 	u64		ch_txcount;	/* total of data transmitted so far */

@@ -375,7 +369,6 @@ extern int	jsm_debug;
  * Prototypes for non-static functions used in more than one module
  *
  *************************************************************************/
-int jsm_tty_write(struct uart_port *port);
 int jsm_tty_init(struct jsm_board *);
 int jsm_uart_port_init(struct jsm_board *);
 int jsm_remove_uart_port(struct jsm_board *);
diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
index 1cc8cf6..648b6a3 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -194,7 +194,6 @@ static void __devexit jsm_remove_one(struct pci_dev *pdev)
 		if (brd->channels[i]) {
 			kfree(brd->channels[i]->ch_rqueue);
 			kfree(brd->channels[i]->ch_equeue);
-			kfree(brd->channels[i]->ch_wqueue);
 			kfree(brd->channels[i]);
 		}
 	}
diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
index 4538c3e..bd6e846 100644
--- a/drivers/tty/serial/jsm/jsm_neo.c
+++ b/drivers/tty/serial/jsm/jsm_neo.c
@@ -496,12 +496,15 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
 	int s;
 	int qlen;
 	u32 len_written = 0;
+	struct circ_buf *circ;

 	if (!ch)
 		return;

+	circ = &ch->uart_port.state->xmit;
+
 	/* No data to write to the UART */
-	if (ch->ch_w_tail == ch->ch_w_head)
+	if (uart_circ_empty(circ))
 		return;

 	/* If port is "stopped", don't send any data to the UART */
@@ -517,11 +520,10 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
 		if (ch->ch_cached_lsr & UART_LSR_THRE) {
 			ch->ch_cached_lsr &= ~(UART_LSR_THRE);

-			writeb(ch->ch_wqueue[ch->ch_w_tail], &ch->ch_neo_uart->txrx);
+			writeb(circ->buf[circ->tail], &ch->ch_neo_uart->txrx);
 			jsm_printk(WRITE, INFO, &ch->ch_bd->pci_dev,
-					"Tx data: %x\n", ch->ch_wqueue[ch->ch_w_head]);
-			ch->ch_w_tail++;
-			ch->ch_w_tail &= WQUEUEMASK;
+					"Tx data: %x\n", circ->buf[circ->head]);
+			circ->tail = (circ->tail + 1) & (UART_XMIT_SIZE - 1);
 			ch->ch_txcount++;
 		}
 		return;
@@ -536,36 +538,36 @@ static void neo_copy_data_from_queue_to_uart(struct jsm_channel *ch)
 	n = UART_17158_TX_FIFOSIZE - ch->ch_t_tlevel;

 	/* cache head and tail of queue */
-	head = ch->ch_w_head & WQUEUEMASK;
-	tail = ch->ch_w_tail & WQUEUEMASK;
-	qlen = (head - tail) & WQUEUEMASK;
+	head = circ->head & (UART_XMIT_SIZE - 1);
+	tail = circ->tail & (UART_XMIT_SIZE - 1);
+	qlen = uart_circ_chars_pending(circ);

 	/* Find minimum of the FIFO space, versus queue length */
 	n = min(n, qlen);

 	while (n > 0) {

-		s = ((head >= tail) ? head : WQUEUESIZE) - tail;
+		s = ((head >= tail) ? head : UART_XMIT_SIZE) - tail;
 		s = min(s, n);

 		if (s <= 0)
 			break;

-		memcpy_toio(&ch->ch_neo_uart->txrxburst, ch->ch_wqueue + tail, s);
+		memcpy_toio(&ch->ch_neo_uart->txrxburst, circ->buf + tail, s);
 		/* Add and flip queue if needed */
-		tail = (tail + s) & WQUEUEMASK;
+		tail = (tail + s) & (UART_XMIT_SIZE - 1);
 		n -= s;
 		ch->ch_txcount += s;
 		len_written += s;
 	}

 	/* Update the final tail */
-	ch->ch_w_tail = tail & WQUEUEMASK;
+	circ->tail = tail & (UART_XMIT_SIZE - 1);

 	if (len_written >= ch->ch_t_tlevel)
 		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);

-	if (!jsm_tty_write(&ch->uart_port))
+	if (uart_circ_empty(circ))
 		uart_write_wakeup(&ch->uart_port);
 }

@@ -946,7 +948,6 @@ static void neo_param(struct jsm_channel *ch)
 	if ((ch->ch_c_cflag & (CBAUD)) == 0) {
 		ch->ch_r_head = ch->ch_r_tail = 0;
 		ch->ch_e_head = ch->ch_e_tail = 0;
-		ch->ch_w_head = ch->ch_w_tail = 0;

 		neo_flush_uart_write(ch);
 		neo_flush_uart_read(ch);
diff --git a/drivers/tty/serial/jsm/jsm_tty.c b/drivers/tty/serial/jsm/jsm_tty.c
index 7a4a914..434bd88 100644
--- a/drivers/tty/serial/jsm/jsm_tty.c
+++ b/drivers/tty/serial/jsm/jsm_tty.c
@@ -118,6 +118,19 @@ static void jsm_tty_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	udelay(10);
 }

+/*
+ * jsm_tty_write()
+ *
+ * Take data from the user or kernel and send it out to the FEP.
+ * In here exists all the Transparent Print magic as well.
+ */
+static void jsm_tty_write(struct uart_port *port)
+{
+	struct jsm_channel *channel;
+	channel = container_of(port, struct jsm_channel, uart_port);
+	channel->ch_bd->bd_ops->copy_data_from_queue_to_uart(channel);
+}
+
 static void jsm_tty_start_tx(struct uart_port *port)
 {
 	struct jsm_channel *channel = (struct jsm_channel *)port;
@@ -216,14 +229,6 @@ static int jsm_tty_open(struct uart_port *port)
 			return -ENOMEM;
 		}
 	}
-	if (!channel->ch_wqueue) {
-		channel->ch_wqueue = kzalloc(WQUEUESIZE, GFP_KERNEL);
-		if (!channel->ch_wqueue) {
-			jsm_printk(INIT, ERR, &channel->ch_bd->pci_dev,
-				"unable to allocate write queue buf");
-			return -ENOMEM;
-		}
-	}

 	channel->ch_flags &= ~(CH_OPENING);
 	/*
@@ -237,7 +242,6 @@ static int jsm_tty_open(struct uart_port *port)
 	 */
 	channel->ch_r_head = channel->ch_r_tail = 0;
 	channel->ch_e_head = channel->ch_e_tail = 0;
-	channel->ch_w_head = channel->ch_w_tail = 0;

 	brd->bd_ops->flush_uart_write(channel);
 	brd->bd_ops->flush_uart_read(channel);
@@ -836,75 +840,3 @@ void jsm_check_queue_flow_control(struct jsm_channel *ch)
 		}
 	}
 }
-
-/*
- * jsm_tty_write()
- *
- * Take data from the user or kernel and send it out to the FEP.
- * In here exists all the Transparent Print magic as well.
- */
-int jsm_tty_write(struct uart_port *port)
-{
-	int bufcount;
-	int data_count = 0,data_count1 =0;
-	u16 head;
-	u16 tail;
-	u16 tmask;
-	u32 remain;
-	int temp_tail = port->state->xmit.tail;
-	struct jsm_channel *channel = (struct jsm_channel *)port;
-
-	tmask = WQUEUEMASK;
-	head = (channel->ch_w_head) & tmask;
-	tail = (channel->ch_w_tail) & tmask;
-
-	if ((bufcount = tail - head - 1) < 0)
-		bufcount += WQUEUESIZE;
-
-	bufcount = min(bufcount, 56);
-	remain = WQUEUESIZE - head;
-
-	data_count = 0;
-	if (bufcount >= remain) {
-		bufcount -= remain;
-		while ((port->state->xmit.head != temp_tail) &&
-		(data_count < remain)) {
-			channel->ch_wqueue[head++] =
-			port->state->xmit.buf[temp_tail];
-
-			temp_tail++;
-			temp_tail &= (UART_XMIT_SIZE - 1);
-			data_count++;
-		}
-		if (data_count == remain) head = 0;
-	}
-
-	data_count1 = 0;
-	if (bufcount > 0) {
-		remain = bufcount;
-		while ((port->state->xmit.head != temp_tail) &&
-			(data_count1 < remain)) {
-			channel->ch_wqueue[head++] =
-				port->state->xmit.buf[temp_tail];
-
-			temp_tail++;
-			temp_tail &= (UART_XMIT_SIZE - 1);
-			data_count1++;
-
-		}
-	}
-
-	port->state->xmit.tail = temp_tail;
-
-	data_count += data_count1;
-	if (data_count) {
-		head &= tmask;
-		channel->ch_w_head = head;
-	}
-
-	if (data_count) {
-		channel->ch_bd->bd_ops->copy_data_from_queue_to_uart(channel);
-	}
-
-	return data_count;
-}
-- 
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: jsm: remove buggy write queue
  2011-09-13 17:55 ` jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
@ 2011-09-22 22:38   ` Greg KH
  0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2011-09-22 22:38 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: stable, Breno Leitao, lucaskt, Alan Cox, linux-serial, linux-kernel

On Tue, Sep 13, 2011 at 02:55:23PM -0300, Thadeu Lima de Souza Cascardo wrote:
> jsm uses a write queue that copies from uart_core circular buffer. This
> copying however has some bugs, like not wrapping the head counter. Since
> this write queue is also a circular buffer, the consumer function is
> ready to use the uart_core circular buffer directly.
> 
> This buggy copying function was making some bytes be dropped when
> transmitting to a raw tty, doing something like this.
> 
> [root@hostname ~]$ cat /dev/ttyn1 > cascardo/dump &
> [1] 2658
> [root@hostname ~]$ cat /proc/tty/drivers > /dev/ttyn0
> [root@hostname ~]$ cat /proc/tty/drivers
> /dev/tty             /dev/tty        5       0 system:/dev/tty
> /dev/console         /dev/console    5       1 system:console
> /dev/ptmx            /dev/ptmx       5       2 system
> /dev/vc/0            /dev/vc/0       4       0 system:vtmaster
> jsm                  /dev/ttyn     250 0-31 serial
> serial               /dev/ttyS       4 64-95 serial
> hvc                  /dev/hvc      229 0-7 system
> pty_slave            /dev/pts      136 0-1048575 pty:slave
> pty_master           /dev/ptm      128 0-1048575 pty:master
> unknown              /dev/tty        4 1-63 console
> [root@hostname ~]$ cat cascardo/dump
> /dev/tty             /dev/tty        5       0 system:/dev/tty
> /dev/console         /dev/console    5       1 system:console
> /dev/ptmx            /dev/ptmx       5       2 system
> /dev/vc/0            /dev/vc/0       4       0 system:vtmaste[root@hostname ~]$
> 
> This patch drops the driver write queue entirely, using the circular
> buffer from uart_core only.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> ---
> 
> Hello, Greg.
> 
> This should probably go to 3.1 and stable releases, since it fixes a
> bug.

I already have this in my tree queued up for 3.2, so please remind me of
this when it hits Linus's tree during the 3.2 merge window.

thanks,

greg k-h

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

end of thread, other threads:[~2011-09-22 23:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-24 16:14 [PATCH 1/3] jsm: remove remaining flip buffer code Thadeu Lima de Souza Cascardo
2011-08-24 16:14 ` [PATCH 2/3] jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
2011-08-24 16:14 ` [PATCH 3/3] jsm: print byte we are dequeing Thadeu Lima de Souza Cascardo
2011-09-13 17:55 ` jsm: remove buggy write queue Thadeu Lima de Souza Cascardo
2011-09-22 22:38   ` Greg KH

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