* [PATCH -rt] Buggy uart (for 2.6.16)
@ 2006-04-10 13:37 Steven Rostedt
2006-04-10 16:53 ` Gunther Mayer
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Steven Rostedt @ 2006-04-10 13:37 UTC (permalink / raw)
To: Ingo Molnar; +Cc: LKML
[-- Attachment #1: Type: text/plain, Size: 2076 bytes --]
Ingo,
I've noticed that you dropped my "buggy uart" patch. Probably because
the 2.6.14 version would cause a deadlock on 2.6.16. I've sent you a
new update, but it must have been lost in all the noise. Here's the
patch again. If you don't think this is a bug, try running the attached
program on the machine that deadlocked (it is the one with the buggy
uart). Without the patch, the serial_test will miss a wake up, and then
be stuck in the sleeping TASK_INTERRUPTIBLE state (at least you can
still kill it). With the patch, it runs fine.
I ran the program with the following parameters:
# ./serial_test /dev/ttyS0 115200 8 0 0 4
(Disclaimer: I did not write this serial_test. It was hacked up by my
customer to show me that this bug exists).
This may also be a bug with the vanilla kernel, since I don't see why it
is not. I'll run more tests on the vanilla kernel, and if it too misses
a wake up, I'll submit this to vanilla as well.
Some 8250 uarts don't zero out the NO_INTERRUPT bit of the IIR register
on transmit empty interrupts. If this happens, then the interrupt handler
won't process any transmits that are waiting, and we can have processes
stuck waiting to transmit over the serial.
This patch has the interrupt process the transmits regardless if
the interrupt handler didn't already handle the transmits, and
the uart was previously (on setup) detected to be buggy.
-- Steve
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Index: linux-2.6.16-rt14/drivers/serial/8250.c
===================================================================
--- linux-2.6.16-rt14.orig/drivers/serial/8250.c 2006-03-20 00:53:29.000000000 -0500
+++ linux-2.6.16-rt14/drivers/serial/8250.c 2006-04-10 09:14:38.000000000 -0400
@@ -1336,6 +1336,14 @@
"irq%d\n", irq);
break;
}
+ /*
+ * If we have a buggy TX line, that doesn't
+ * notify us via iir that we need to transmit
+ * then force the call.
+ */
+ if (!handled && (up->bugs & UART_BUG_TXEN))
+ serial8250_handle_port(up, regs);
+
} while (l != end);
spin_unlock(&i->lock);
[-- Attachment #2: serial_test.c --]
[-- Type: text/x-csrc, Size: 11678 bytes --]
#include <termios.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <sys/signal.h>
#include <sys/types.h>
#define BAUDRATE B38400
#define MODEMDEVICE "/dev/ttyS1"
#define _POSIX_SOURCE 1 //POSIX compliant source
#define FALSE 0
#define TRUE 1
volatile int STOP=FALSE;
void signal_handler_IO (int status); //definition of signal handler
int wait_flag=TRUE; //TRUE while no signal received
char devicename[80];
long Baud_Rate = 38400; // default Baud Rate (110 through 38400)
long BAUD; // derived baud rate from command line
long DATABITS;
long STOPBITS;
long PARITYON;
long PARITY;
int Data_Bits = 8; // Number of data bits
int Stop_Bits = 1; // Number of stop bits
int Parity = 0; // Parity as follows:
// 00 = NONE, 01 = Odd, 02 = Even, 03 = Mark, 04 = Space
int Format = 4;
FILE *input;
FILE *output;
int status;
int main(int Parm_Count, char *Parms[])
{
char version[80] = " POSIX compliant Communications test program version 1.00 4-25-1999\r\n";
char version1[80] = " Copyright(C) Mark Zehner/Peter Baumann 1999\r\n";
char version2[80] = " This code is based on a DOS based test program by Mark Zehner and a Serial\r\n";
char version3[80] = " Programming POSIX howto by Peter Baumann, integrated by Mark Zehner\r\n";
char version4[80] = " This program allows you to send characters out the specified port by typing\r\n";
char version5[80] = " on the keyboard. Characters typed will be echoed to the console, and \r\n";
char version6[80] = " characters received will be echoed to the console.\r\n";
char version7[80] = " The setup parameters for the device name, receive data format, baud rate\r\n";
char version8[80] = " and other serial port parameters must be entered on the command line \r\n";
char version9[80] = " To see how to do this, just type the name of this program. \r\n";
char version10[80] = " This program is free software; you can redistribute it and/or modify it\r\n";
char version11[80] = " under the terms of the GNU General Public License as published by the \r\n";
char version12[80] = " Free Software Foundation, version 2.\r\n";
char version13[80] = " This program comes with ABSOLUTELY NO WARRANTY.\r\n";
char instr[100] ="\r\nOn the command you must include six items in the following order, they are:\r\n";
char instr1[80] =" 1. The device name Ex: ttyS0 for com1, ttyS1 for com2, etc\r\n";
char instr2[80] =" 2. Baud Rate Ex: 38400 \r\n";
char instr3[80] =" 3. Number of Data Bits Ex: 8 \r\n";
char instr4[80] =" 4. Number of Stop Bits Ex: 0 or 1\r\n";
char instr5[80] =" 5. Parity Ex: 0=none, 1=odd, 2=even\r\n";
char instr6[80] =" 6. Format of data received: 1=hex, 2=dec, 3=hex/asc, 4=dec/asc, 5=asc\r\n";
char instr7[80] =" Example command line: com ttyS0 38400 8 0 0 4 \r\n";
char Param_strings[7][80];
char message[90];
char cntmessage[90];
int fd, tty, res, i, error, cnt = 1;
char In1;
char Keys[] = "jfdslgjdlskfgklsdfjgkdlfsklsjgfdgsdfjglkdsf";
struct termios oldtio, newtio; //place for old and new port settings for serial port
struct termios oldkey, newkey; //place tor old and new port settings for keyboard teletype
struct sigaction saio; //definition of signal action
char buf[255]; //buffer for where data is put
input = fopen("/dev/tty", "r"); //open the terminal keyboard
output = fopen("/dev/tty", "w"); //open the terminal screen
if (!input || !output){
fprintf(stderr, "Unable to open /dev/tty\n");
exit(1);
}
error=0;
fputs(version,output); //display the program introduction
fputs(version1,output);
fputs(version2,output);
fputs(version3,output);
fputs(version4,output);
fputs(version5,output);
fputs(version6,output);
fputs(version7,output);
fputs(version8,output);
fputs(version9,output);
fputs(version10,output);
fputs(version11,output);
fputs(version12,output);
fputs(version13,output);
//read the parameters from the command line
if (Parm_Count==7){ //if there are the right number of parameters on the command line
for (i=1; i < Parm_Count; i++) // for all wild search parameters
strcpy(Param_strings[i-1],Parms[i]);
i=sscanf(Param_strings[0],"%s",devicename);
if (i != 1)
error=1;
i=sscanf(Param_strings[1],"%li",&Baud_Rate);
if (i != 1)
error=1;
i=sscanf(Param_strings[2],"%i",&Data_Bits);
if (i != 1)
error=1;
i=sscanf(Param_strings[3],"%i",&Stop_Bits);
if (i != 1)
error=1;
i=sscanf(Param_strings[4],"%i",&Parity);
if (i != 1)
error=1;
i=sscanf(Param_strings[5],"%i",&Format);
if (i != 1)
error=1;
sprintf(message,"Device=%s, Baud=%li\r\n",devicename, Baud_Rate); //output the received setup parameters
fputs(message,output);
sprintf(message,"Data Bits=%i Stop Bits=%i Parity=%i Format=%i\r\n",Data_Bits, Stop_Bits, Parity, Format);
fputs(message,output);
} //end of if param_count==7
if ((Parm_Count==7) && (error==0)){ //if the command line entries were correct
//run the program
tty = open("/dev/tty", O_RDWR | O_NOCTTY | O_NONBLOCK); //set the user console port up
tcgetattr(tty,&oldkey); // save current port settings //so commands are interpreted right for this program
// set new port settings for non-canonical input processing //must be NOCTTY
newkey.c_cflag = BAUDRATE | CRTSCTS | CS8 | CLOCAL | CREAD;
newkey.c_iflag = IGNPAR;
newkey.c_oflag = 0;
newkey.c_lflag = 0; //ICANON;
newkey.c_cc[VMIN]=1;
newkey.c_cc[VTIME]=0;
tcflush(tty, TCIFLUSH);
tcsetattr(tty,TCSANOW,&newkey);
switch (Baud_Rate){
case 38400:
default:
BAUD = B38400;
break;
case 19200:
BAUD = B19200;
break;
case 9600:
BAUD = B9600;
break;
case 4800:
BAUD = B4800;
break;
case 2400:
BAUD = B2400;
break;
case 1800:
BAUD = B1800;
break;
case 1200:
BAUD = B1200;
break;
case 600:
BAUD = B600;
break;
case 300:
BAUD = B300;
break;
case 200:
BAUD = B200;
break;
case 150:
BAUD = B150;
break;
case 134:
BAUD = B134;
break;
case 110:
BAUD = B110;
break;
case 75:
BAUD = B75;
break;
case 50:
BAUD = B50;
break;
} //end of switch baud_rate
switch (Data_Bits){
case 8:
default:
DATABITS = CS8;
break;
case 7:
DATABITS = CS7;
break;
case 6:
DATABITS = CS6;
break;
case 5:
DATABITS = CS5;
break;
} //end of switch data_bits
switch (Stop_Bits){
case 1:
default:
STOPBITS = 0;
break;
case 2:
STOPBITS = CSTOPB;
break;
} //end of switch stop bits
switch (Parity){
case 0:
default: //none
PARITYON = 0;
PARITY = 0;
break;
case 1: //odd
PARITYON = PARENB;
PARITY = PARODD;
break;
case 2: //even
PARITYON = PARENB;
PARITY = 0;
break;
} //end of switch parity
//install the serial handler before making the device asynchronous
saio.sa_handler = signal_handler_IO;
sigemptyset(&saio.sa_mask); //saio.sa_mask = 0;
saio.sa_flags = 0;
saio.sa_restorer = NULL;
sigaction(SIGIO,&saio,NULL);
// loop while waiting for input. normally we would do something useful here
while (STOP==FALSE){
//open the device(com port) to be non-blocking (read will return immediately)
sprintf(cntmessage,"============== %d =============\r\n", cnt);
fputs(cntmessage, output);
fputs("Trying to open device\r\n", output);
fd = open(devicename, O_RDWR | O_NOCTTY | O_NONBLOCK);
if (fd < 0){
perror(devicename);
exit(-1);
}
fputs("Opened device\r\n", output);
// allow the process to receive SIGIO
fcntl(fd, F_SETOWN, getpid());
// Make the file descriptor asynchronous (the manual page says only
// O_APPEND and O_NONBLOCK, will work with F_SETFL...)
fcntl(fd, F_SETFL, FASYNC);
tcgetattr(fd,&oldtio); // save current port settings
// set new port settings for canonical input processing
newtio.c_cflag = BAUD | /* CRTSCTS | */ DATABITS | STOPBITS | PARITYON | PARITY | CLOCAL | CREAD;
newtio.c_iflag = IGNPAR;
newtio.c_oflag = 0;
newtio.c_lflag = 0; //ICANON;
newtio.c_cc[VMIN]=1;
newtio.c_cc[VTIME]=0;
tcflush(fd, TCIFLUSH);
fputs("Trying to set attributes\r\n", output);
tcsetattr(fd,TCSANOW,&newtio);
fputs("Set attributes\r\n", output);
// status = fread(&Key,1,1,input);
// if (status==1){ //if a key was hit
// switch (Key){ /* branch to appropiate key handler */
// case 0x1b: /* Esc */
// STOP=TRUE;
// break;
// default:
// fputc((int) Key,output);
//sprintf(message,"%x ",Key); //debug
//fputs(message,output);
write(fd,Keys,sizeof(Keys)); //write 1 byte to the port
ioctl(fd, TCSBRK, NULL);
// break;
// } //end of switch key
// } //end if a key was hit
// after receiving SIGIO, wait_flag = FALSE, input is available and can be read
if (wait_flag==FALSE){ //if input is available
res = read(fd,buf,255);
if (res){
for (i=0; i<res; i++){ //for all chars in string
In1 = buf[i];
switch (Format){
case 1: //hex
sprintf(message,"%x ",In1);
fputs(message,output);
break;
case 2: //decimal
sprintf(message,"%d ",In1);
fputs(message,output);
break;
case 3: //hex and asc
if ((In1) || (In1)){
sprintf(message,"%x",In1);
fputs(message,output);
} else
fputc ((int) In1, output);
break;
case 4: //decimal and asc
default:
if ((In1) || (In1)){
sprintf(message,"%d",In1);
fputs(message,output);
} else
fputc ((int) In1, output);
break;
case 5: //asc
fputc ((int) In1, output);
break;
} //end of switch format
} //end of for all chars in string
} //end if res?
//buf[res]=0;
//printf(":%s:%d\n", buf, res);
//if (res==1) STOP=TRUE; /* stop loop if only a CR was input */
wait_flag = TRUE; /* wait for new input */
} //end if wait flag == FALSE
fputs("Trying to reset attributes\r\n", output);
tcsetattr(fd,TCSANOW,&oldtio);
fputs("Reset attributes\r\n", output);
fputs("Trying to close device\r\n", output);
close(fd); //close the com port
fputs("Closed device\r\n", output);
cnt++;
usleep(500000);
} //while stop==FALSE
// restore old port settings
tcsetattr(tty,TCSANOW,&oldkey);
close(tty);
} else { //end if command line entrys were correct
//give instructions on how to use the command line
fputs(instr,output);
fputs(instr1,output);
fputs(instr2,output);
fputs(instr3,output);
fputs(instr4,output);
fputs(instr5,output);
fputs(instr6,output);
fputs(instr7,output);
}
fclose(input);
fclose(output);
return 0;
} //end of main
/***************************************************************************
* signal handler. sets wait_flag to FALSE, to indicate above loop that *
* characters have been received. *
***************************************************************************/
void signal_handler_IO (int status){
//printf("received SIGIO signal.\n");
wait_flag = FALSE;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -rt] Buggy uart (for 2.6.16)
2006-04-10 13:37 Steven Rostedt
@ 2006-04-10 16:53 ` Gunther Mayer
2006-04-10 17:51 ` Steven Rostedt
` (2 more replies)
2006-04-12 6:27 ` Ingo Molnar
2006-05-07 8:30 ` Russell King
2 siblings, 3 replies; 8+ messages in thread
From: Gunther Mayer @ 2006-04-10 16:53 UTC (permalink / raw)
To: Steven Rostedt; +Cc: LKML
Steven Rostedt wrote:
>Ingo,
>
>I've noticed that you dropped my "buggy uart" patch. Probably because
>the 2.6.14 version would cause a deadlock on 2.6.16. I've sent you a
>new update, but it must have been lost in all the noise. Here's the
>patch again. If you don't think this is a bug, try running the attached
>program on the machine that deadlocked (it is the one with the buggy
>uart). Without the patch, the serial_test will miss a wake up, and then
>be stuck in the sleeping TASK_INTERRUPTIBLE state (at least you can
>still kill it). With the patch, it runs fine.
>
>I ran the program with the following parameters:
>
># ./serial_test /dev/ttyS0 115200 8 0 0 4
>
>(Disclaimer: I did not write this serial_test. It was hacked up by my
>customer to show me that this bug exists).
>
>This may also be a bug with the vanilla kernel, since I don't see why it
>is not. I'll run more tests on the vanilla kernel, and if it too misses
>a wake up, I'll submit this to vanilla as well.
>
>
>Some 8250 uarts don't zero out the NO_INTERRUPT bit of the IIR register
>
>
Can you name the exact 8250 model which is buggy ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -rt] Buggy uart (for 2.6.16)
2006-04-10 16:53 ` Gunther Mayer
@ 2006-04-10 17:51 ` Steven Rostedt
2006-04-10 20:22 ` Steven Rostedt
2006-04-11 20:57 ` Steven Rostedt
2 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2006-04-10 17:51 UTC (permalink / raw)
To: Gunther Mayer; +Cc: LKML
On Mon, 2006-04-10 at 18:53 +0200, Gunther Mayer wrote:
> >
> >
> Can you name the exact 8250 model which is buggy ?
>
It's a custom board and the serial is not attached to the PCI. So you
will have to wait till tomorrow before I can get the specs. The ones
that have the specs are in Europe.
-- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -rt] Buggy uart (for 2.6.16)
2006-04-10 16:53 ` Gunther Mayer
2006-04-10 17:51 ` Steven Rostedt
@ 2006-04-10 20:22 ` Steven Rostedt
2006-04-11 20:57 ` Steven Rostedt
2 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2006-04-10 20:22 UTC (permalink / raw)
To: Gunther Mayer; +Cc: LKML
On Mon, 2006-04-10 at 18:53 +0200, Gunther Mayer wrote:
> >
> >
> Can you name the exact 8250 model which is buggy ?
>
I just found some more information. The board's chipset is an Intel
440BX. I think this is responsible for the uarts.
-- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -rt] Buggy uart (for 2.6.16)
[not found] ` <60bbB-6XU-17@gated-at.bofh.it>
@ 2006-04-10 23:25 ` Robert Hancock
0 siblings, 0 replies; 8+ messages in thread
From: Robert Hancock @ 2006-04-10 23:25 UTC (permalink / raw)
To: linux-kernel; +Cc: rostedt
Steven Rostedt wrote:
> I just found some more information. The board's chipset is an Intel
> 440BX. I think this is responsible for the uarts.
No, it would be whatever Super I/O chip is present on the board.
--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -rt] Buggy uart (for 2.6.16)
2006-04-10 16:53 ` Gunther Mayer
2006-04-10 17:51 ` Steven Rostedt
2006-04-10 20:22 ` Steven Rostedt
@ 2006-04-11 20:57 ` Steven Rostedt
2 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2006-04-11 20:57 UTC (permalink / raw)
To: Gunther Mayer; +Cc: LKML, Ingo Molnar
On Mon, 2006-04-10 at 18:53 +0200, Gunther Mayer wrote:
> >
> Can you name the exact 8250 model which is buggy ?
>
>From the spec:
COM1 und COM2 werden mittels der Ultra IO FDC37C675 implementiert. COM3
und COM4 sind 16C550 oder kompatible.
I'm using COM1 and COM2 so it's the Ultra IO FDC37C675.
-- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -rt] Buggy uart (for 2.6.16)
2006-04-10 13:37 Steven Rostedt
2006-04-10 16:53 ` Gunther Mayer
@ 2006-04-12 6:27 ` Ingo Molnar
2006-05-07 8:30 ` Russell King
2 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2006-04-12 6:27 UTC (permalink / raw)
To: Steven Rostedt; +Cc: LKML
* Steven Rostedt <rostedt@goodmis.org> wrote:
> Ingo,
>
> I've noticed that you dropped my "buggy uart" patch. Probably because
> the 2.6.14 version would cause a deadlock on 2.6.16. I've sent you a
> new update, but it must have been lost in all the noise. Here's the
> patch again. [...]
thanks - indeed i missed your updated patch. Applied.
Ingo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -rt] Buggy uart (for 2.6.16)
2006-04-10 13:37 Steven Rostedt
2006-04-10 16:53 ` Gunther Mayer
2006-04-12 6:27 ` Ingo Molnar
@ 2006-05-07 8:30 ` Russell King
2 siblings, 0 replies; 8+ messages in thread
From: Russell King @ 2006-05-07 8:30 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Ingo Molnar, LKML
On Mon, Apr 10, 2006 at 09:37:05AM -0400, Steven Rostedt wrote:
> I've noticed that you dropped my "buggy uart" patch.
Note that this isn't going to be merged into mainline because the
"buggy uart" test triggers inappropriately on some ports, so we
need to ensure that this patch doesn't rely upon that wrong behaviour.
Unfortunately, it's taking longer than I expected to get my serial
git tree into a state where it can be merged, so this will have to
wait. Nevertheless, please test with the following patch applied.
# Base git commit: e0a515bc6a2188f02916e976f419a8640312e32a
# (Merge branch 'merge' of git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc)
#
# Author: Russell King (Tue May 2 16:04:29 BST 2006)
# Committer: Russell King (Tue May 2 16:04:29 BST 2006)
#
# [SERIAL] 8250: add locking to console write function
#
# x86 SMP breaks as a result of the previous change, we have no real
# option other than to add locking to the 8250 console write function.
# If an oops is in progress, try to acquire the lock. If we fail to
# do so, continue anyway.
#
# Signed-off-by: Russell King
#
# drivers/serial/8250.c | 10 ++++++++++
# 1 files changed, 10 insertions(+), 0 deletions(-)
#
# Author: Russell King (Sun Apr 30 11:30:15 BST 2006)
# Committer: Russell King (Sun Apr 30 11:30:15 BST 2006)
#
# [SERIAL] Remove unconditional enable of TX irq for console
#
# A bug report from Gerd Hoffmann has highlighted that unconditionally
# enabling the transmit interrupt at the end of console writes is very
# bad.
#
# In Gerd's case, it causes the test for buggy UARTs to give false
# positives, incorrectly identifying ports as buggy when they are not.
#
# Moreover, if we unconditionally enable the interrupt, and the port
# is sharing it's interrupt with other ports, there is the very real
# possibility that we'll cause an interrupt storm. (Not all ports use
# OUT2 as an interrupt mask.)
#
# Hence, revert part of f91a3715db2bb44fcf08cec642e68f919b70f7f4 and
# all of f5968b37b3ad35b682b574b578843a0361218aff until a better solution
# can be found.
#
# Signed-off-by: Russell King
#
# drivers/serial/8250.c | 3 +--
# 1 files changed, 1 insertions(+), 2 deletions(-)
#
# Author: Jon Anders Haugum (Sun Apr 30 11:20:56 BST 2006)
# Committer: Russell King (Sun Apr 30 11:20:56 BST 2006)
#
# [SERIAL] 8250: set divisor register correctly for AMD Alchemy SoC uart
#
# Alchemy SoC uart have got a non-standard divisor register that needs some
# special handling.
#
# This patch adds divisor read/write functions with test and special
# handling for Alchemy internal uart.
#
# Signed-off-by: Jon Anders Haugum
# Signed-off-by: Russell King
#
# drivers/serial/8250.c | 55 +++++++++++++++++++++++++++++++++++++------------
# 1 files changed, 42 insertions(+), 13 deletions(-)
#
# Author: Sergei Shtylyov (Sun Apr 30 11:15:58 BST 2006)
# Committer: Russell King (Sun Apr 30 11:15:58 BST 2006)
#
# [SERIAL] AMD Alchemy UART: claim memory range
#
# I've noticed that the 8250/Au1x00 driver (drivers/serial/8250_au1x00.c)
# doesn't claim UART memory ranges and uses wrong (KSEG1-based) UART
# addresses instead of the physical ones.
#
# Signed-off-by: Sergei Shtylyov
# Signed-off-by: Russell King
#
# drivers/serial/8250.c | 6 ++++++
# drivers/serial/8250_au1x00.c | 5 ++---
# 2 files changed, 8 insertions(+), 3 deletions(-)
#
# Author: Russell King (Sun Apr 30 11:13:50 BST 2006)
# Committer: Russell King (Sun Apr 30 11:13:50 BST 2006)
#
# [SERIAL] Clean up serial locking when obtaining a reference to a port
#
# The locking for the uart_port is over complicated, and can be
# simplified if we introduce a flag to indicate that a port is "dead"
# and will be removed.
#
# This also helps the validator because it removes a case of non-nested
# unlock ordering.
#
# Signed-off-by: Russell King
# Signed-off-by: Ingo Molnar
# Signed-off-by: Andrew Morton
#
# drivers/serial/serial_core.c | 114 ++++++++++++++++++++++--------------------
# include/linux/serial_core.h | 1
# 2 files changed, 61 insertions(+), 54 deletions(-)
#
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -362,6 +362,40 @@ serial_out(struct uart_8250_port *up, in
#define serial_inp(up, offset) serial_in(up, offset)
#define serial_outp(up, offset, value) serial_out(up, offset, value)
+/* Uart divisor latch read */
+static inline int _serial_dl_read(struct uart_8250_port *up)
+{
+ return serial_inp(up, UART_DLL) | serial_inp(up, UART_DLM) << 8;
+}
+
+/* Uart divisor latch write */
+static inline void _serial_dl_write(struct uart_8250_port *up, int value)
+{
+ serial_outp(up, UART_DLL, value & 0xff);
+ serial_outp(up, UART_DLM, value >> 8 & 0xff);
+}
+
+#ifdef CONFIG_SERIAL_8250_AU1X00
+/* Au1x00 haven't got a standard divisor latch */
+static int serial_dl_read(struct uart_8250_port *up)
+{
+ if (up->port.iotype == UPIO_AU)
+ return __raw_readl(up->port.membase + 0x28);
+ else
+ return _serial_dl_read(up);
+}
+
+static void serial_dl_write(struct uart_8250_port *up, int value)
+{
+ if (up->port.iotype == UPIO_AU)
+ __raw_writel(value, up->port.membase + 0x28);
+ else
+ _serial_dl_write(up, value);
+}
+#else
+#define serial_dl_read(up) _serial_dl_read(up)
+#define serial_dl_write(up, value) _serial_dl_write(up, value)
+#endif
/*
* For the 16C950
@@ -494,7 +528,8 @@ static void disable_rsa(struct uart_8250
*/
static int size_fifo(struct uart_8250_port *up)
{
- unsigned char old_fcr, old_mcr, old_dll, old_dlm, old_lcr;
+ unsigned char old_fcr, old_mcr, old_lcr;
+ unsigned short old_dl;
int count;
old_lcr = serial_inp(up, UART_LCR);
@@ -505,10 +540,8 @@ static int size_fifo(struct uart_8250_po
UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
serial_outp(up, UART_MCR, UART_MCR_LOOP);
serial_outp(up, UART_LCR, UART_LCR_DLAB);
- old_dll = serial_inp(up, UART_DLL);
- old_dlm = serial_inp(up, UART_DLM);
- serial_outp(up, UART_DLL, 0x01);
- serial_outp(up, UART_DLM, 0x00);
+ old_dl = serial_dl_read(up);
+ serial_dl_write(up, 0x0001);
serial_outp(up, UART_LCR, 0x03);
for (count = 0; count < 256; count++)
serial_outp(up, UART_TX, count);
@@ -519,8 +552,7 @@ static int size_fifo(struct uart_8250_po
serial_outp(up, UART_FCR, old_fcr);
serial_outp(up, UART_MCR, old_mcr);
serial_outp(up, UART_LCR, UART_LCR_DLAB);
- serial_outp(up, UART_DLL, old_dll);
- serial_outp(up, UART_DLM, old_dlm);
+ serial_dl_write(up, old_dl);
serial_outp(up, UART_LCR, old_lcr);
return count;
@@ -750,8 +782,7 @@ static void autoconfig_16550a(struct uar
serial_outp(up, UART_LCR, 0xE0);
- quot = serial_inp(up, UART_DLM) << 8;
- quot += serial_inp(up, UART_DLL);
+ quot = serial_dl_read(up);
quot <<= 3;
status1 = serial_in(up, 0x04); /* EXCR1 */
@@ -759,8 +790,7 @@ static void autoconfig_16550a(struct uar
status1 |= 0x10; /* 1.625 divisor for baud_base --> 921600 */
serial_outp(up, 0x04, status1);
- serial_outp(up, UART_DLL, quot & 0xff);
- serial_outp(up, UART_DLM, quot >> 8);
+ serial_dl_write(up, quot);
serial_outp(up, UART_LCR, 0);
@@ -1862,8 +1892,7 @@ serial8250_set_termios(struct uart_port
serial_outp(up, UART_LCR, cval | UART_LCR_DLAB);/* set DLAB */
}
- serial_outp(up, UART_DLL, quot & 0xff); /* LS of divisor */
- serial_outp(up, UART_DLM, quot >> 8); /* MS of divisor */
+ serial_dl_write(up, quot);
/*
* LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR
@@ -1906,6 +1935,9 @@ static int serial8250_request_std_resour
int ret = 0;
switch (up->port.iotype) {
+ case UPIO_AU:
+ size = 0x100000;
+ /* fall thru */
case UPIO_MEM:
if (!up->port.mapbase)
break;
@@ -1938,6 +1970,9 @@ static void serial8250_release_std_resou
unsigned int size = 8 << up->port.regshift;
switch (up->port.iotype) {
+ case UPIO_AU:
+ size = 0x100000;
+ /* fall thru */
case UPIO_MEM:
if (!up->port.mapbase)
break;
@@ -2200,10 +2235,17 @@ static void
serial8250_console_write(struct console *co, const char *s, unsigned int count)
{
struct uart_8250_port *up = &serial8250_ports[co->index];
+ unsigned long flags;
unsigned int ier;
+ int locked = 1;
touch_nmi_watchdog();
+ if (oops_in_progress) {
+ locked = spin_trylock_irqsave(&up->port.lock, flags);
+ } else
+ spin_lock_irqsave(&up->port.lock, flags);
+
/*
* First save the IER then disable the interrupts
*/
@@ -2221,8 +2263,10 @@ serial8250_console_write(struct console
* and restore the IER
*/
wait_for_xmitr(up, BOTH_EMPTY);
- up->ier |= UART_IER_THRI;
- serial_out(up, UART_IER, ier | UART_IER_THRI);
+ serial_out(up, UART_IER, ier);
+
+ if (locked)
+ spin_unlock_irqrestore(&up->port.lock, flags);
}
static int serial8250_console_setup(struct console *co, char *options)
diff --git a/drivers/serial/8250_au1x00.c b/drivers/serial/8250_au1x00.c
--- a/drivers/serial/8250_au1x00.c
+++ b/drivers/serial/8250_au1x00.c
@@ -30,13 +30,12 @@
{ \
.iobase = _base, \
.membase = (void __iomem *)_base,\
- .mapbase = _base, \
+ .mapbase = CPHYSADDR(_base), \
.irq = _irq, \
.uartclk = 0, /* filled */ \
.regshift = 2, \
.iotype = UPIO_AU, \
- .flags = UPF_SKIP_TEST | \
- UPF_IOREMAP, \
+ .flags = UPF_SKIP_TEST \
}
static struct plat_serial8250_port au1x00_data[] = {
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1500,20 +1500,18 @@ uart_block_til_ready(struct file *filp,
static struct uart_state *uart_get(struct uart_driver *drv, int line)
{
struct uart_state *state;
+ int ret = 0;
- mutex_lock(&port_mutex);
state = drv->state + line;
if (mutex_lock_interruptible(&state->mutex)) {
- state = ERR_PTR(-ERESTARTSYS);
- goto out;
+ ret = -ERESTARTSYS;
+ goto err;
}
state->count++;
- if (!state->port) {
- state->count--;
- mutex_unlock(&state->mutex);
- state = ERR_PTR(-ENXIO);
- goto out;
+ if (!state->port || state->port->flags & UPF_DEAD) {
+ ret = -ENXIO;
+ goto err_unlock;
}
if (!state->info) {
@@ -1531,15 +1529,17 @@ static struct uart_state *uart_get(struc
tasklet_init(&state->info->tlet, uart_tasklet_action,
(unsigned long)state);
} else {
- state->count--;
- mutex_unlock(&state->mutex);
- state = ERR_PTR(-ENOMEM);
+ ret = -ENOMEM;
+ goto err_unlock;
}
}
-
- out:
- mutex_unlock(&port_mutex);
return state;
+
+ err_unlock:
+ state->count--;
+ mutex_unlock(&state->mutex);
+ err:
+ return ERR_PTR(ret);
}
/*
@@ -2085,45 +2085,6 @@ uart_configure_port(struct uart_driver *
}
}
-/*
- * This reverses the effects of uart_configure_port, hanging up the
- * port before removal.
- */
-static void
-uart_unconfigure_port(struct uart_driver *drv, struct uart_state *state)
-{
- struct uart_port *port = state->port;
- struct uart_info *info = state->info;
-
- if (info && info->tty)
- tty_vhangup(info->tty);
-
- mutex_lock(&state->mutex);
-
- state->info = NULL;
-
- /*
- * Free the port IO and memory resources, if any.
- */
- if (port->type != PORT_UNKNOWN)
- port->ops->release_port(port);
-
- /*
- * Indicate that there isn't a port here anymore.
- */
- port->type = PORT_UNKNOWN;
-
- /*
- * Kill the tasklet, and free resources.
- */
- if (info) {
- tasklet_kill(&info->tlet);
- kfree(info);
- }
-
- mutex_unlock(&state->mutex);
-}
-
static struct tty_operations uart_ops = {
.open = uart_open,
.close = uart_close,
@@ -2270,6 +2231,7 @@ int uart_add_one_port(struct uart_driver
state = drv->state + port->line;
mutex_lock(&port_mutex);
+ mutex_lock(&state->mutex);
if (state->port) {
ret = -EINVAL;
goto out;
@@ -2304,7 +2266,13 @@ int uart_add_one_port(struct uart_driver
port->cons && !(port->cons->flags & CON_ENABLED))
register_console(port->cons);
+ /*
+ * Ensure UPF_DEAD is not set.
+ */
+ port->flags &= ~UPF_DEAD;
+
out:
+ mutex_unlock(&state->mutex);
mutex_unlock(&port_mutex);
return ret;
@@ -2322,6 +2290,7 @@ int uart_add_one_port(struct uart_driver
int uart_remove_one_port(struct uart_driver *drv, struct uart_port *port)
{
struct uart_state *state = drv->state + port->line;
+ struct uart_info *info;
BUG_ON(in_interrupt());
@@ -2332,11 +2301,48 @@ int uart_remove_one_port(struct uart_dri
mutex_lock(&port_mutex);
/*
+ * Mark the port "dead" - this prevents any opens from
+ * succeeding while we shut down the port.
+ */
+ mutex_lock(&state->mutex);
+ port->flags |= UPF_DEAD;
+ mutex_unlock(&state->mutex);
+
+ /*
* Remove the devices from devfs
*/
tty_unregister_device(drv->tty_driver, port->line);
- uart_unconfigure_port(drv, state);
+ info = state->info;
+ if (info && info->tty)
+ tty_vhangup(info->tty);
+
+ /*
+ * All users of this port should now be disconnected from
+ * this driver, and the port shut down. We should be the
+ * only thread fiddling with this port from now on.
+ */
+ state->info = NULL;
+
+ /*
+ * Free the port IO and memory resources, if any.
+ */
+ if (port->type != PORT_UNKNOWN)
+ port->ops->release_port(port);
+
+ /*
+ * Indicate that there isn't a port here anymore.
+ */
+ port->type = PORT_UNKNOWN;
+
+ /*
+ * Kill the tasklet, and free resources.
+ */
+ if (info) {
+ tasklet_kill(&info->tlet);
+ kfree(info);
+ }
+
state->port = NULL;
mutex_unlock(&port_mutex);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -254,6 +254,7 @@ struct uart_port {
#define UPF_CONS_FLOW ((__force upf_t) (1 << 23))
#define UPF_SHARE_IRQ ((__force upf_t) (1 << 24))
#define UPF_BOOT_AUTOCONF ((__force upf_t) (1 << 28))
+#define UPF_DEAD ((__force upf_t) (1 << 30))
#define UPF_IOREMAP ((__force upf_t) (1 << 31))
#define UPF_CHANGE_MASK ((__force upf_t) (0x17fff))
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2006-05-07 8:30 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <604MF-5Tc-7@gated-at.bofh.it>
[not found] ` <6083V-2pi-9@gated-at.bofh.it>
[not found] ` <60bbB-6XU-17@gated-at.bofh.it>
2006-04-10 23:25 ` [PATCH -rt] Buggy uart (for 2.6.16) Robert Hancock
2006-04-10 13:37 Steven Rostedt
2006-04-10 16:53 ` Gunther Mayer
2006-04-10 17:51 ` Steven Rostedt
2006-04-10 20:22 ` Steven Rostedt
2006-04-11 20:57 ` Steven Rostedt
2006-04-12 6:27 ` Ingo Molnar
2006-05-07 8:30 ` Russell King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox