Hello.

Marc St-Jean wrote:

quoted

quoted

Fourth attempt at the serial driver patch for the PMC-Sierra MSP71xx 

device.

quoted

quoted

There are three different fixes:
1. Fix for DesignWare THRE errata
- Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
tree also fixes it. This patch needs to be applied on top of "mm" patch.

    I think you need to submit your patch to Andrew Morton since it requires a 
patch from his tree.

quoted

quoted

@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
                      handled = 1;

                      end = NULL;
+             } else if (up->port.iotype == UPIO_DWAPB &&
+                             (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {

quoted

    Worth aligning this line with the opening paren of if... but's that's
nitpicking. :-)

No problem I'll change it. I just usually align to the closest tab stop to
the opening parenthesis.

    It haven't really changed in the last patch. :-)

quoted

quoted

+                     /* The DesignWare APB UART has an Busy Detect 

(0x07)

quoted

+                      * interrupt meaning an LCR write attempt 

occured while the

quoted

+                      * UART was busy. The interrupt must be cleared 

by reading

quoted

+                      * the UART status register (USR) and the LCR 

re-written. */

quoted

+                     unsigned int status;
+                     status = *(volatile u32 *)up->port.private_data;
+                     serial_out(up, UART_LCR, up->lcr);
+
+                     handled = 1;
+
+                     end = NULL;
              } else if (end == NULL)
                      end = l;

      return 0;

quoted

   Still, shouldn't you be doing this in serial8250_timeout()

No, the serial8250_timeout is for issue 1 at the top, this is for
issue 2.

    It's for lost interrupts, IIUC. They use anothe timeout handler for the 
workaround...

quoted

also?
What IRQ numbers this UART is using, BTW?

For the ports on the device they are 27 and 42. Is there any significance
that I'm not aware of?

    Yeah, IRQ0 is treated as no IRQ by 8250, and in this case it falls back to 
using serial8250_timeout() to handle "interrupts".

quoted

quoted

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..bd9711a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h

@@ -276,6 +277,7 @@ #define UPF_USR_MASK              ((__force upf_t) (
      struct device           *dev;                   /* parent 

device */

quoted

      unsigned char           hub6;                   /* this should 

be in the 8250 driver */

quoted

      unsigned char           unused[3];
+     void                            *private_data;          /* 

generic platform data pointer */

quoted

   One tab to many before *private_data...

In my editor using 4 columns per tab it lines up with everything. Is there
some convention that patches should be made assuming 8 columns?

    Documentation/CodingStyle, chapter 1. :-)

quoted

   Oops, your mailer went and did it again. :-)

I'm completely giving up on Thunderbird,unless someone can point out

    Ypu should have long ago. :-)

the specific internal configuration items which needs a kick!

    Only the attachments will work in the Mozilla kind mailer, AFAIK.
The last patch looked OK at last. :-)

Thanks,
Marc

WBR, Sergei