LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: akpm@osdl.org, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org
Subject: [PATCH] genericserial: Remove bogus optimisation check and dead code paths
Date: Tue, 3 Jul 2007 15:20:21 +0100	[thread overview]
Message-ID: <20070703152021.6eb8013d@the-village.bc.nu> (raw)

We've been using the 'new locking' for a long time now so it seems
pointless keeping the old one around. Remove it and undo the macros it
uses back into real code for readability. Remove the bogus 'no termios
change' checks.

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --exclude-from /usr/src/exclude --recursive linux.vanilla-2.6.22-rc6-mm1/drivers/char/generic_serial.c linux-2.6.22-rc6-mm1/drivers/char/generic_serial.c
--- linux.vanilla-2.6.22-rc6-mm1/drivers/char/generic_serial.c	2007-07-02 20:47:23.000000000 +0100
+++ linux-2.6.22-rc6-mm1/drivers/char/generic_serial.c	2007-07-02 21:05:53.000000000 +0100
@@ -43,16 +43,6 @@
 
 #define func_enter() gs_dprintk (GS_DEBUG_FLOW, "gs: enter %s\n", __FUNCTION__)
 #define func_exit()  gs_dprintk (GS_DEBUG_FLOW, "gs: exit  %s\n", __FUNCTION__)
-#define NEW_WRITE_LOCKING 1
-#if NEW_WRITE_LOCKING
-#define DECL      /* Nothing */
-#define LOCKIT    mutex_lock(& port->port_write_mutex);
-#define RELEASEIT mutex_unlock(&port->port_write_mutex);
-#else
-#define DECL      unsigned long flags;
-#define LOCKIT    save_flags (flags);cli ()
-#define RELEASEIT restore_flags (flags)
-#endif
 
 #define RS_EVENT_WRITE_WAKEUP	1
 
@@ -62,7 +52,6 @@
 void gs_put_char(struct tty_struct * tty, unsigned char ch)
 {
 	struct gs_port *port;
-	DECL
 
 	func_enter (); 
 
@@ -75,11 +64,11 @@
 	if (! (port->flags & ASYNC_INITIALIZED)) return;
 
 	/* Take a lock on the serial tranmit buffer! */
-	LOCKIT;
+	mutex_lock(& port->port_write_mutex);
 
 	if (port->xmit_cnt >= SERIAL_XMIT_SIZE - 1) {
 		/* Sorry, buffer is full, drop character. Update statistics???? -- REW */
-		RELEASEIT;
+		mutex_unlock(&port->port_write_mutex);
 		return;
 	}
 
@@ -87,13 +76,11 @@
 	port->xmit_head &= SERIAL_XMIT_SIZE - 1;
 	port->xmit_cnt++;  /* Characters in buffer */
 
-	RELEASEIT;
+	mutex_unlock(&port->port_write_mutex);
 	func_exit ();
 }
 
 
-#ifdef NEW_WRITE_LOCKING
-
 /*
 > Problems to take into account are:
 >       -1- Interrupts that empty part of the buffer.
@@ -166,90 +153,6 @@
 	func_exit ();
 	return total;
 }
-#else
-/*
-> Problems to take into account are:
->       -1- Interrupts that empty part of the buffer.
->       -2- page faults on the access to userspace. 
->       -3- Other processes that are also trying to do a "write". 
-*/
-
-int gs_write(struct tty_struct * tty,
-                    const unsigned char *buf, int count)
-{
-	struct gs_port *port;
-	int c, total = 0;
-	int t;
-	unsigned long flags;
-
-	func_enter ();
-
-	/* The standard serial driver returns 0 in this case. 
-	   That sounds to me as "No error, I just didn't get to writing any
-	   bytes. Feel free to try again." 
-	   The "official" way to write n bytes from buf is:
-
-		 for (nwritten = 0;nwritten < n;nwritten += rv) {
-			 rv = write (fd, buf+nwritten, n-nwritten);
-			 if (rv < 0) break; // Error: bail out. //
-		 } 
-
-	   which will loop endlessly in this case. The manual page for write
-	   agrees with me. In practise almost everybody writes 
-	   "write (fd, buf,n);" but some people might have had to deal with 
-	   incomplete writes in the past and correctly implemented it by now... 
-	 */
-
-	if (!tty) return -EIO;
-
-	port = tty->driver_data;
-	if (!port || !port->xmit_buf)
-		return -EIO;
-
-	local_save_flags(flags);
-	while (1) {
-		cli();
-		c = count;
-
-		/* This is safe because we "OWN" the "head". Noone else can 
-		   change the "head": we own the port_write_mutex. */
-		/* Don't overrun the end of the buffer */
-		t = SERIAL_XMIT_SIZE - port->xmit_head;
-		if (t < c) c = t;
-
-		/* This is safe because the xmit_cnt can only decrease. This 
-		   would increase "t", so we might copy too little chars. */
-		/* Don't copy past the "head" of the buffer */
-		t = SERIAL_XMIT_SIZE - 1 - port->xmit_cnt;
-		if (t < c) c = t;
-
-		/* Can't copy more? break out! */
-		if (c <= 0) {
-			local_restore_flags(flags);
-			break;
-		}
-		memcpy(port->xmit_buf + port->xmit_head, buf, c);
-		port->xmit_head = ((port->xmit_head + c) &
-		                   (SERIAL_XMIT_SIZE-1));
-		port->xmit_cnt += c;
-		local_restore_flags(flags);
-		buf += c;
-		count -= c;
-		total += c;
-	}
-
-	if (port->xmit_cnt && 
-	    !tty->stopped && 
-	    !tty->hw_stopped &&
-	    !(port->flags & GS_TX_INTEN)) {
-		port->flags |= GS_TX_INTEN;
-		port->rd->enable_tx_interrupts (port);
-	}
-	func_exit ();
-	return total;
-}
-
-#endif
 
 
 
@@ -737,23 +640,6 @@
 		gs_dprintk (GS_DEBUG_TERMIOS, "termios structure (%p):\n", tiosp);
 	}
 
-	/* This is an optimization that is only allowed for dumb cards */
-	/* Smart cards require knowledge of iflags and oflags too: that 
-	   might change hardware cooking mode.... */
-	if (old_termios) {
-		if(   (tiosp->c_iflag == old_termios->c_iflag)
-		   && (tiosp->c_oflag == old_termios->c_oflag)
-		   && (tiosp->c_cflag == old_termios->c_cflag)
-		   && (tiosp->c_lflag == old_termios->c_lflag)
-		   && (tiosp->c_line  == old_termios->c_line)
-		   && (memcmp(tiosp->c_cc, old_termios->c_cc, NCC) == 0)) {
-			gs_dprintk(GS_DEBUG_TERMIOS, "gs_set_termios: optimized away\n");
-			return /* 0 */;
-		}
-	} else 
-		gs_dprintk(GS_DEBUG_TERMIOS, "gs_set_termios: no old_termios: "
-		           "no optimization\n");
-
 	if(old_termios && (gs_debug & GS_DEBUG_TERMIOS)) {
 		if(tiosp->c_iflag != old_termios->c_iflag)  printk("c_iflag changed\n");
 		if(tiosp->c_oflag != old_termios->c_oflag)  printk("c_oflag changed\n");

             reply	other threads:[~2007-07-03 14:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-03 14:20 Alan Cox [this message]
2007-07-03 14:39 ` Morten Helgesen
2007-07-03 15:26   ` Alan Cox
2007-07-03 21:33     ` Morten Helgesen
2007-07-03 21:36     ` Morten Helgesen
2007-07-03 22:31       ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070703152021.6eb8013d@the-village.bc.nu \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --subject='Re: [PATCH] genericserial: Remove bogus optimisation check and dead code paths' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).