Re: DM3xx: UART transmit delays
Brian Murphy br...@murphy.dk writes: On Mon, Feb 15, 2010 at 10:07 PM, Bansal, Prateek pban...@ti.com wrote: Hi Brian, Thanks for sharing the patch. We tested this on 2.6.32 kernel on DM355 device and it worked fine however we are yet to get this working with 2.6.30 kernel. One of the things that we are trying to investigate is why the transmit FIFO empty interrupts are not generated. If you have more visibility into this, I would appreciate any details. Considering we see this issue across number of devices OMAP-L137, DM6446 and DM355, it points to a software issue rather than hardware one but needs to be confirmed. I have it working with several kernels, It needed a little massaging to work with the latest as someone else was playing in the same place as me :). The issue the patch fixes is a hardware issue. The UART in the TI SOC devices does not have the same behaviour as a 16550 uart and the linux kernel driver requires that behaviour to function efficiently. The workaround in the current kernel driver is not good and hides the problem in a very bad way. I will try again next week to get a discusion going on the linux-serial list. It seems like Alan Cox is still active despite threats to the contary so I may send directly to him to see if that helps. [sorry to jump in late on this thread...] Unfortunately, last time I checked, the 8250 driver was orphaned (no maintainer.) And since there are 8250 UARTs on *tons* of devices out there, folks are reluctant to do significant changes without lots of testing. I suggest in your repost of this patch to be also list the devices that you (or others) have validated it on. That always helps. You might also request others do do some testing on their platforms as well. Kevin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM3xx: UART transmit delays
On 17/02/10 16:54, Brian Murphy wrote: I agree that it wil take some work to convince the maintainers of the 8250 driver (Alan Cox??) that these changes should be accepted. Even the real 16550 will benefit from the change as an interrupt does not have to be handled before transmission can start. Also there is less code to test and maintain. That's got to be good, right? Good luck ;) Nick. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM3xx: UART transmit delays
On Mon, Feb 15, 2010 at 10:07 PM, Bansal, Prateek pban...@ti.com wrote: Hi Brian, Thanks for sharing the patch. We tested this on 2.6.32 kernel on DM355 device and it worked fine however we are yet to get this working with 2.6.30 kernel. One of the things that we are trying to investigate is why the transmit FIFO empty interrupts are not generated. If you have more visibility into this, I would appreciate any details. Considering we see this issue across number of devices OMAP-L137, DM6446 and DM355, it points to a software issue rather than hardware one but needs to be confirmed. I have it working with several kernels, It needed a little massaging to work with the latest as someone else was playing in the same place as me :). The issue the patch fixes is a hardware issue. The UART in the TI SOC devices does not have the same behaviour as a 16550 uart and the linux kernel driver requires that behaviour to function efficiently. The workaround in the current kernel driver is not good and hides the problem in a very bad way. I will try again next week to get a discusion going on the linux-serial list. It seems like Alan Cox is still active despite threats to the contary so I may send directly to him to see if that helps. /Brian ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM3xx: UART transmit delays
My understanding of the issue is this: The UART device only generates a FIFO empty interrupt if interrupts are enabled and the FIFO goes from not empty to empty. If you then disable the interrupt and later reenable it, the interrupt is not reasserted to indicate that the FIFO is still empty. This seems like reasonable behaviour, but is not how a true 16550A device should behave. The interrupt should be reasseted if the FIFO is still empty and the interrupt is reenabled. The 8250 driver in the Linux kernel is written to depend upon this reassertion behaviour. It has tests to detect the failure of this behaviour and enables BUG workarounds (using a 200ms timer), if required. Brian's patch changes the driver to not depend on this behaviour and so doesn't need to detect its absence or invoke workarounds. It also avoids 200ms timer delays - which is the big bonus. I don't know what you did for your 2.6.30 backport, but I have it working in a 2.6.18 backport with no real changes. BTW it is not just a Davinci/OMAP problem I think... Correct. You would think that TI should be able to make a 16550 compatible UART. I have seen a few SOCs with this feature, I seem to remember a designware UART that behaves this way too. I agree that it wil take some work to convince the maintainers of the 8250 driver (Alan Cox??) that these changes should be accepted. Even the real 16550 will benefit from the change as an interrupt does not have to be handled before transmission can start. /Brian ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM3xx: UART transmit delays
On 15/02/10 21:07, Bansal, Prateek wrote: Hi Brian, Thanks for sharing the patch. We tested this on 2.6.32 kernel on DM355 device and it worked fine however we are yet to get this working with 2.6.30 kernel. One of the things that we are trying to investigate is why the transmit FIFO empty interrupts are not generated. If you have more visibility into this, I would appreciate any details. Considering we see this issue across number of devices OMAP-L137, DM6446 and DM355, it points to a software issue rather than hardware one but needs to be confirmed. Prateek My understanding of the issue is this: The UART device only generates a FIFO empty interrupt if interrupts are enabled and the FIFO goes from not empty to empty. If you then disable the interrupt and later reenable it, the interrupt is not reasserted to indicate that the FIFO is still empty. This seems like reasonable behaviour, but is not how a true 16550A device should behave. The interrupt should be reasseted if the FIFO is still empty and the interrupt is reenabled. The 8250 driver in the Linux kernel is written to depend upon this reassertion behaviour. It has tests to detect the failure of this behaviour and enables BUG workarounds (using a 200ms timer), if required. Brian's patch changes the driver to not depend on this behaviour and so doesn't need to detect its absence or invoke workarounds. It also avoids 200ms timer delays - which is the big bonus. I don't know what you did for your 2.6.30 backport, but I have it working in a 2.6.18 backport with no real changes. BTW it is not just a Davinci/OMAP problem I think... Nick. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM3xx: UART transmit delays
On 11/02/10 09:58, Brian Murphy wrote: Hi Folks, We have been working with L137 and there is an issue where the serial driver handles the UART in a very bad way. This may be the same issue as described in the other two mails on this thread. The issue is that the serial driver 8250.c expects the uart to generate a fifo empty interrupt if the FIFO is empty and interrupts are enabled. The UART in the L137 only generates a FIFO empty interrupt the first time. The serial driver handles this situation by starting a timer which times out after 200ms when interrupts are enabled and only then begins to transmit. I have rewritten the interrupt handling so the driver maintains a flag which tells whether the UART is currently transmitting or not. If not then we begin filling the TX fifo immediately (we do not wait for an interrupt which never comes). This fixes the transmit latency issue for us. I have attached the patch. It is against the linux-davinci git at kernel.org as of friday last week. I have sent the patch to the linux-serial list with no response. Could you who have problems with serial performance test the patch? I can try to send it again to linux-serial if I get good feedback from you. /Brian Hi Brian, I know you developed this on the OMAP-L137 and tested it yourself, but I can confirm I get the same results here on the same device. (I had to back port it to MVL5 first.) I spent quite a bit of time studying the patch and I would be happy to endorse it (though I'm sure that won't count for anything). It has a few minor style problems, but the approach looks good to me. I think it will need testing on many more platforms before it is accepted though :( Thanks for sharing. The product I am working on requires efficient serial ports and this patch has saved me a bunch of time. Nick. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: DM3xx: UART transmit delays
Hi Brian, Thanks for sharing the patch. We tested this on 2.6.32 kernel on DM355 device and it worked fine however we are yet to get this working with 2.6.30 kernel. One of the things that we are trying to investigate is why the transmit FIFO empty interrupts are not generated. If you have more visibility into this, I would appreciate any details. Considering we see this issue across number of devices OMAP-L137, DM6446 and DM355, it points to a software issue rather than hardware one but needs to be confirmed. Prateek -Original Message- From: davinci-linux-open-source-boun...@linux.davincidsp.com [mailto:davinci-linux-open-source-boun...@linux.davincidsp.com] On Behalf Of Nick Thompson Sent: Monday, February 15, 2010 5:59 AM To: Brian Murphy Cc: davinci-linux-open-source@linux.davincidsp.com Subject: Re: DM3xx: UART transmit delays On 11/02/10 09:58, Brian Murphy wrote: Hi Folks, We have been working with L137 and there is an issue where the serial driver handles the UART in a very bad way. This may be the same issue as described in the other two mails on this thread. The issue is that the serial driver 8250.c expects the uart to generate a fifo empty interrupt if the FIFO is empty and interrupts are enabled. The UART in the L137 only generates a FIFO empty interrupt the first time. The serial driver handles this situation by starting a timer which times out after 200ms when interrupts are enabled and only then begins to transmit. I have rewritten the interrupt handling so the driver maintains a flag which tells whether the UART is currently transmitting or not. If not then we begin filling the TX fifo immediately (we do not wait for an interrupt which never comes). This fixes the transmit latency issue for us. I have attached the patch. It is against the linux-davinci git at kernel.org as of friday last week. I have sent the patch to the linux-serial list with no response. Could you who have problems with serial performance test the patch? I can try to send it again to linux-serial if I get good feedback from you. /Brian Hi Brian, I know you developed this on the OMAP-L137 and tested it yourself, but I can confirm I get the same results here on the same device. (I had to back port it to MVL5 first.) I spent quite a bit of time studying the patch and I would be happy to endorse it (though I'm sure that won't count for anything). It has a few minor style problems, but the approach looks good to me. I think it will need testing on many more platforms before it is accepted though :( Thanks for sharing. The product I am working on requires efficient serial ports and this patch has saved me a bunch of time. Nick. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: DM3xx: UART transmit delays
Hi Folks,
We have been working with L137 and there is an issue where the serial
driver handles
the UART in a very bad way. This may be the same issue as described
in the other
two mails on this thread.
The issue is that the serial driver 8250.c expects the uart to
generate a fifo empty interrupt
if the FIFO is empty and interrupts are enabled. The UART in the L137
only generates a FIFO
empty interrupt the first time. The serial driver handles this
situation by starting a timer which
times out after 200ms when interrupts are enabled and only then begins
to transmit.
I have rewritten the interrupt handling so the driver maintains a flag
which tells whether the UART
is currently transmitting or not. If not then we begin filling the TX
fifo immediately (we do not
wait for an interrupt which never comes). This fixes the transmit
latency issue for us.
I have attached the patch. It is against the linux-davinci git at
kernel.org as of friday last week.
I have sent the patch to the linux-serial list with no response.
Could you who have problems with serial performance test the patch? I
can try to send it
again to linux-serial if I get good feedback from you.
/Brian
From 73b3fdf65baef4d532787677dc709b6390b24d8a Mon Sep 17 00:00:00 2001
From: Brian Murphy b...@brm-laptop.ttnet
Date: Fri, 5 Feb 2010 15:59:02 +0100
Subject: [PATCH] performance fix
---
drivers/serial/8250.c | 204 +
drivers/serial/8250.h |4 +-
2 files changed, 38 insertions(+), 170 deletions(-)
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index c3e37c8..646e8ce 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -150,6 +150,7 @@ struct uart_8250_port {
unsigned char lsr_saved_flags;
#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
unsigned char msr_saved_flags;
+ unsigned char tx_active;
/*
* We provide a per-port pm hook.
@@ -524,25 +525,6 @@ static void set_io_from_upio(struct uart_port *p)
up-cur_iotype = p-iotype;
}
-static void
-serial_out_sync(struct uart_8250_port *up, int offset, int value)
-{
- struct uart_port *p = up-port;
- switch (p-iotype) {
- case UPIO_MEM:
- case UPIO_MEM32:
-#ifdef CONFIG_SERIAL_8250_AU1X00
- case UPIO_AU:
-#endif
- case UPIO_DWAPB:
- p-serial_out(p, offset, value);
- p-serial_in(p, UART_LCR); /* safe, no side-effects */
- break;
- default:
- p-serial_out(p, offset, value);
- }
-}
-
#define serial_in(up, offset) \
(up-port.serial_in((up)-port, (offset)))
#define serial_out(up, offset, value) \
@@ -1311,14 +1293,20 @@ static inline void __stop_tx(struct uart_8250_port *p)
p-ier = ~UART_IER_THRI;
serial_out(p, UART_IER, p-ier);
}
+ p-tx_active = 0;
}
static void serial8250_stop_tx(struct uart_port *port)
{
struct uart_8250_port *up = (struct uart_8250_port *)port;
+ unsigned long flags;
+
+ spin_lock_irqsave(up-port.lock, flags);
__stop_tx(up);
+ spin_unlock_irqrestore(up-port.lock, flags);
+
/*
* We really want to stop the transmitter from sending.
*/
@@ -1333,22 +1321,23 @@ static void transmit_chars(struct uart_8250_port *up);
static void serial8250_start_tx(struct uart_port *port)
{
struct uart_8250_port *up = (struct uart_8250_port *)port;
+ unsigned long flags;
+
+ spin_lock_irqsave(up-port.lock, flags);
if (!(up-ier UART_IER_THRI)) {
up-ier |= UART_IER_THRI;
serial_out(up, UART_IER, up-ier);
+ }
- if (up-bugs UART_BUG_TXEN) {
- unsigned char lsr;
- lsr = serial_in(up, UART_LSR);
- up-lsr_saved_flags |= lsr LSR_SAVE_FLAGS;
- if ((up-port.type == PORT_RM9000) ?
-(lsr UART_LSR_THRE) :
-(lsr UART_LSR_TEMT))
-transmit_chars(up);
- }
+ if (!up-tx_active)
+ {
+ up-tx_active = 1;
+ transmit_chars(up);
}
+ spin_unlock_irqrestore(up-port.lock, flags);
+
/*
* Re-enable the transmitter if we disabled it.
*/
@@ -1470,7 +1459,8 @@ static void transmit_chars(struct uart_8250_port *up)
serial8250_stop_tx(up-port);
return;
}
- if (uart_circ_empty(xmit)) {
+ if (uart_circ_empty(xmit))
+ {
__stop_tx(up);
return;
}
@@ -1488,9 +1478,6 @@ static void transmit_chars(struct uart_8250_port *up)
uart_write_wakeup(up-port);
DEBUG_INTR(THRE...);
-
- if (uart_circ_empty(xmit))
- __stop_tx(up);
}
static unsigned int check_modem_status(struct uart_8250_port *up)
@@ -1519,12 +1506,9 @@ static unsigned int check_modem_status(struct uart_8250_port *up)
/*
* This handles the interrupt from one port.
*/
-static void serial8250_handle_port(struct uart_8250_port *up)
+static void __serial8250_handle_port(struct uart_8250_port *up)
{
unsigned int status;
- unsigned long flags;
-
- spin_lock_irqsave(up-port.lock, flags);
status = serial_inp(up, UART_LSR);
@@ -1534,8 +1518,19 @@ static void serial8250_handle_port(struct uart_8250_port *up)
receive_chars(up, status);
check_modem_status(up);
if (status UART_LSR_THRE)
+ {
+ up-tx_active = 1;
transmit_chars(up);
+ }
+
+}
+static void
Re: DM3xx: UART transmit delays
On Thursday 11 February 2010 11:58:47 am Brian Murphy wrote: Hi Folks, We have been working with L137 and there is an issue where the serial driver handles the UART in a very bad way. This may be the same issue as described in the other two mails on this thread. The issue is that the serial driver 8250.c expects the uart to generate a fifo empty interrupt if the FIFO is empty and interrupts are enabled. The UART in the L137 only generates a FIFO empty interrupt the first time. The serial driver handles this situation by starting a timer which times out after 200ms when interrupts are enabled and only then begins to transmit. I have rewritten the interrupt handling so the driver maintains a flag which tells whether the UART is currently transmitting or not. If not then we begin filling the TX fifo immediately (we do not wait for an interrupt which never comes). This fixes the transmit latency issue for us. I have attached the patch. It is against the linux-davinci git at kernel.org as of friday last week. I have sent the patch to the linux-serial list with no response. Could you who have problems with serial performance test the patch? I can try to send it again to linux-serial if I get good feedback from you. This patch solves the problem for DM6446. Thanks for sharing your work. Best Regards, Caglar /Brian ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
DM3xx: UART transmit delays
Hi,
When working with DM355 UART1, we are experiencing a strange behavior. When
calling the write function for this UART, sometimes it takes 200 ms for the
write function to complete. This happens when a write function is called right
after another write function. There is not a lot of data sent to the UART at
one time, at most 32 bytes. The UART is running at 100,000 baud and would take
at most 3.2 ms to send this data, not 200 ms.
We have tested this in non-Linux environment and found there were no delays.
Has anyone came across this issue on UART transmits on DM355 devices?
Below is simple test code that re-creates this issue.
static int halUartWrite(int fd, uint8_t* buffer, size_t s)
{
int num = 0;
if( fd != invalidHandle )
{
num = write(fd, buffer, s);
tcdrain(fd);
}
return num;
}
int main(int argc, char** argv)
{
char testString[] = A really long test to see what happens.;
char buffer[100];
time_t currentTime;
int cartridgeSerialPortFileDevice = halUartOpen(/dev/ttyS1, 10);
if( cartridgeSerialPortFileDevice == invalidHandle )
{
printf(ttyS1 failed\n);
exit(-1);
}
printf(Length of test string (%s) %d\n, testString, sizeof testString);
currentTime = currentTime_ms();
halUartWrite(cartridgeSerialPortFileDevice, testString, sizeof testString);
printf(Elapsed time (ms) after 1st write: %d\n, currentTime_ms() -
currentTime);
halUartWrite(cartridgeSerialPortFileDevice, testString, sizeof testString);
// 200 ms delay until this is seen
printf(Elapsed time (ms) after 2nd write: %d\n, currentTime_ms() -
currentTime);
halUartRead(cartridgeSerialPortFileDevice, buffer, sizeof buffer);
printf(Finished\n);
}
Thanks,
Prateek Bansal
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM3xx: UART transmit delays
On Thursday 11 February 2010 12:42:50 am Bansal, Prateek wrote:
Hi,
Hi,
When working with DM355 UART1, we are experiencing a strange behavior. When
calling the write function for this UART, sometimes it takes 200 ms for
the write function to complete. This happens when a write function is
called right after another write function. There is not a lot of data sent
to the UART at one time, at most 32 bytes. The UART is running at 100,000
baud and would take at most 3.2 ms to send this data, not 200 ms.
The exact same problem exists for DM6446, too. Out 1 of 2 write requests to
the UART is delayed by 200 ms. Haven't investigated the reason though.
Besr regards,
Caglar
We have tested this in non-Linux environment and found there were no
delays. Has anyone came across this issue on UART transmits on DM355
devices?
Below is simple test code that re-creates this issue.
static int halUartWrite(int fd, uint8_t* buffer, size_t s)
{
int num = 0;
if( fd != invalidHandle )
{
num = write(fd, buffer, s);
tcdrain(fd);
}
return num;
}
int main(int argc, char** argv)
{
char testString[] = A really long test to see what happens.;
char buffer[100];
time_t currentTime;
int cartridgeSerialPortFileDevice = halUartOpen(/dev/ttyS1, 10);
if( cartridgeSerialPortFileDevice == invalidHandle )
{
printf(ttyS1 failed\n);
exit(-1);
}
printf(Length of test string (%s) %d\n, testString, sizeof
testString);
currentTime = currentTime_ms();
halUartWrite(cartridgeSerialPortFileDevice, testString, sizeof
testString); printf(Elapsed time (ms) after 1st write: %d\n,
currentTime_ms() - currentTime);
halUartWrite(cartridgeSerialPortFileDevice, testString, sizeof
testString); // 200 ms delay until this is seen printf(Elapsed time (ms)
after 2nd write: %d\n, currentTime_ms() - currentTime);
halUartRead(cartridgeSerialPortFileDevice, buffer, sizeof buffer);
printf(Finished\n);
}
Thanks,
Prateek Bansal
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
