drivers: PMC MSP71xx serial driver

PATCH Marc St-Jean 1 participant 1 message Mar 27, 2007 MIPS

Share & tools

Marc St-Jean

author

2007-03-27 20:57 UTC #0 lore raw

Quoted 39 lines · Sergei Shtylyov

Hello.
Quoted 32 lines · Marc St-Jean
Quoted 23 lines
Review
Quoted 1 line
(quoted diff) +0 -0
b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
Quoted 5 lines
new file mode 100644
index 0000000..3b956e9
arch/mips/pmc-sierra/msp71xx/msp_serial.c new file +0 -0
@@ -0,0 +1,185 @@
[...]
Quoted 15 lines
arch/mips/pmc-sierra/msp71xx/msp_serial.c new file +15 -0
+#ifdef CONFIG_KGDB
+/*
+ * kgdb uses serial port 1 so the console can remain on port 0.
+ * To use port 0 change the definition to read as follows:
+ * #define DEBUG_PORT_BASE KSEG1ADDR(MSP_UART0_BASE)
+ */
+#define DEBUG_PORT_BASE KSEG1ADDR(MSP_UART1_BASE)
+
+int putDebugChar(char c)
+{
+     volatile uint32_t *uart = (volatile uint32_t *)DEBUG_PORT_BASE;
+     uint32_t val = (uint32_t)c;
+
+     local_irq_disable();
+     while (!(uart[5] & 0x20)); /* Wait for TXRDY */
Quoted 8 lines
arch/mips/pmc-sierra/msp71xx/msp_serial.c
Quoted 3 lines
arch/mips/pmc-sierra/msp71xx/msp_serial.c new file +3 -0
+     uart[0] = val;
+     while (!(uart[5] & 0x20)); /* Wait for TXRDY */
+     local_irq_enable();
Gah, why you decided to put local_irq_enable() there?! KGDB expects
interrupts to be *disabled* while it has control, else some subtle state corruptions will ensue, and it will eventually lock up. Please remove these 2 calls completely.
Quoted 2 lines
Hmmm, this has been working for several months. I'll remove, retest and resubmit.

Quoted 6 lines

I should probably have said "may". From my experience with KGDBoE
though
(well, it's somewhat different KGDB implementation :-) it locks up pretty quickly because of local_irq_enable() or spin_unlock_irq(). Nevertheless, enabling interrupts while KGDB has control is undesirable. And look at the other KGDB serial code inarch/mips/ -- nobody else does this.

A search showed that some other architectures do use local_irq_disable()/enable() and our code may have been derived from it. None of the MIPS platforms do and the kgdb implementation seams to be arch specific so I'll drop them.

Quoted 4 lines

Quoted 2 lines
Are you aware if this is the case for the "putchar" used by early_printk as well?
No idea, ask Ralf. ;-)

I searched putchar implementations and there only seems to be one more for the cobalt. It also doesn't twiddle the irg state so I'll drop it there as well.

I tested and seems well without the local_irq_disable()/enable(), I'll resubmit the path.

Thanks,
Marc