linux-mips
[Top] [All Lists]

Re: [PATCH 6/12] drivers: PMC MSP71xx serial driver

To: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>
Subject: Re: [PATCH 6/12] drivers: PMC MSP71xx serial driver
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date: Tue, 27 Mar 2007 22:48:05 +0400
Cc: linux-mips@linux-mips.org
In-reply-to: <460963FB.9090101@pmc-sierra.com>
Organization: MontaVista Software Inc.
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <460963FB.9090101@pmc-sierra.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; U; Linux i686; rv:1.7.2) Gecko/20040803
Hello.

Marc St-Jean wrote:

> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_serial.c b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
> new file mode 100644
> index 0000000..3b956e9
> --- /dev/null
> +++ b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
> @@ -0,0 +1,185 @@
[...]
> +#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 */
> +     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.

Hmmm, this has been working for several months. I'll remove, retest and
resubmit.

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.

Are you aware if this is the case for the "putchar" used by early_printk
as well?

   No idea, ask Ralf. ;-)

Thanks,
Marc

WBR, Sergei

<Prev in Thread] Current Thread [Next in Thread>