Skip to contentSkip to conversation

Serial: DaVinci: Revert double IIR test

PATCH Mark Lokowich 2 participants 2 messages Aug 11, 2008 4 messages not in the archive TTY layer and serial drivers

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 6 lines · Philip Balister
Quoted 5 lines · Kevin Hilman
Quoted 2 lines · Mark Lokowich

Revert double IIR test in 8250 driver

Quoted 9 lines
Quoted 2 lines

Can you give a more detailed description of what the problem is that is being fixed?

Without this patch, serial console behavior is strange. You type and nothing is echoed and you hit return and things happen. It feels like two processes are grabbing input.

I think someone posted a more technical description, but I couldn't find it quickly.

Philip

This section of the driver is a test for faulty 16650 implementations that require a backup timer. The patch reverts the change that compares two states (instead of one, as in 2.6.25.2) of the IIR to determine if the workaround is required. The DaVinci UART does not need the backup timer, but the double conditional passed, enabling the timer and causing interrupt handling errors on the console and other UARTS.

Mark

Quoted 55 lines
Quoted 55 lines

This is in the generic 8250 driver so will affect all platforms. For this to be accepted, it should be discussed on linux-serial[1]

Kevin

Quoted 32 lines

---

drivers/serial/8250.c +2 -3
@@ -1867,7 +1867,6 @@
    }
 
    if (is_real_interrupt(up->port.irq)) {
-        unsigned char iir1;
        /*
         * Test for UARTs that do not reassert THRE when the
         * transmitter is idle and the interrupt has already
@@ -1881,7 +1880,7 @@
        wait_for_xmitr(up, UART_LSR_THRE);
        serial_out_sync(up, UART_IER, UART_IER_THRI);
        udelay(1); /* allow THRE to set */
-        iir1 = serial_in(up, UART_IIR);
+        serial_in(up, UART_IIR);
        serial_out(up, UART_IER, 0);
        serial_out_sync(up, UART_IER, UART_IER_THRI);
        udelay(1); /* allow a working UART time to re-assert THRE */
@@ -1894,7 +1893,7 @@
         * If the interrupt is not reasserted, setup a timer to
         * kick the UART on a regular basis.
         */
-        if (!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) {
+        if (iir & UART_IIR_NO_INT) {
            pr_debug("ttyS%d - using backup timer\n", port->line);
            up->timer.function = serial8250_backup_timeout;
            up->timer.data = (unsigned long)up;
Quoted 4 lines

_______________________________________________
Davinci-linux-open-source mailing list Davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

[1] From MAINTAINERS:

8250/16?50 (AND CLONE UARTS) SERIAL DRIVER

L:    [email protected]
W:    http://serial.sourceforge.net
S:    Orphan

_______________________________________________
Davinci-linux-open-source mailing list Davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

#5 lore raw
Quoted 19 lines · Mark Lokowich
Quoted 9 lines · Philip Balister
Quoted 7 lines · Kevin Hilman
Quoted 2 lines · Mark Lokowich

Revert double IIR test in 8250 driver

Can you give a more detailed description of what the problem is that is being fixed?

Quoted 9 lines

Without this patch, serial console behavior is strange. You type and nothing is echoed and you hit return and things happen. It feels like two processes are grabbing input.

I think someone posted a more technical description, but I couldn't find it quickly.

Philip

Quoted 8 lines

This section of the driver is a test for faulty 16650 implementations that require a backup timer. The patch reverts the change that compares two states (instead of one, as in 2.6.25.2) of the IIR to determine if the workaround is required. The DaVinci UART does not need the backup timer, but the double conditional passed, enabling the timer and causing interrupt handling errors on the console and other UARTS.

Thanks Mark,

Do you have any recommendations for ways to conditionalize this code so the test still works on other platforms. Maybe via a new flag in platform data?

I want to be sure there is a mainline-acceptable fix here, so we are not divirging from mainline in generic code.

Kevin

Quoted 56 lines
Quoted 56 lines
Quoted 56 lines

This is in the generic 8250 driver so will affect all platforms. For this to be accepted, it should be discussed on linux-serial[1]

Kevin

Quoted 32 lines

---

drivers/serial/8250.c +2 -3
@@ -1867,7 +1867,6 @@
    }
 
    if (is_real_interrupt(up->port.irq)) {
-        unsigned char iir1;
        /*
         * Test for UARTs that do not reassert THRE when the
         * transmitter is idle and the interrupt has already
@@ -1881,7 +1880,7 @@
        wait_for_xmitr(up, UART_LSR_THRE);
        serial_out_sync(up, UART_IER, UART_IER_THRI);
        udelay(1); /* allow THRE to set */
-        iir1 = serial_in(up, UART_IIR);
+        serial_in(up, UART_IIR);
        serial_out(up, UART_IER, 0);
        serial_out_sync(up, UART_IER, UART_IER_THRI);
        udelay(1); /* allow a working UART time to re-assert THRE */
@@ -1894,7 +1893,7 @@
         * If the interrupt is not reasserted, setup a timer to
         * kick the UART on a regular basis.
         */
-        if (!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) {
+        if (iir & UART_IIR_NO_INT) {
            pr_debug("ttyS%d - using backup timer\n", port->line);
            up->timer.function = serial8250_backup_timeout;
            up->timer.data = (unsigned long)up;
Quoted 4 lines

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

[1] From MAINTAINERS:

8250/16?50 (AND CLONE UARTS) SERIAL DRIVER

L:    [email protected]
W:    http://serial.sourceforge.net
S:    Orphan

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source