* Re: [PATCH v3] serial: PL011: clear pending interrupts
From: Chanho Min @ 2012-03-14 9:21 UTC (permalink / raw)
To: Linus Walleij
Cc: Linus Walleij, Greg Kroah-Hartman, linux-serial, linux-arm-kernel,
Russell King, Jong-Sung Kim, stable, Shreshtha Kumar Sahu
In-Reply-To: <CACRpkdZXrMW653W=tysY8ncqBKDun3TgPqnEzrdZPqwpW8shYA@mail.gmail.com>
On Wed, Mar 14, 2012 at 6:02 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> Thanks Chanho, Greg will you add the Reported/Reviewed-by
> tags or do you want me to send a v4 of this?
>
> Yours,
> Linus Walleij
No, you don't need to send it again if greg will do it.
Thanks,
Chanho Min
^ permalink raw reply
* Re: [PATCH v3] serial: PL011: clear pending interrupts
From: Linus Walleij @ 2012-03-14 9:02 UTC (permalink / raw)
To: Chanho Min
Cc: Linus Walleij, Greg Kroah-Hartman, linux-serial, linux-arm-kernel,
Russell King, Jong-Sung Kim, stable, Shreshtha Kumar Sahu
In-Reply-To: <CAOAMb1AKmsNQbmVAaYq5G8cdafCVpRyA=5xgocqYfLsQ6BLXMw@mail.gmail.com>
On Wed, Mar 14, 2012 at 2:47 AM, Chanho Min <chanho0207@gmail.com> wrote:
>
> I've tested this patch, and have confirmed that the issue is resolved
> on our CA9 platform.
> In addition, May I ask to change the tag as follows.
>
> Reported-by: Chanho Min <chanho.min@lge.com>
> Reviewed-by: Jong-Sung Kim <neidhard.kim@lge.com>
Thanks Chanho, Greg will you add the Reported/Reviewed-by
tags or do you want me to send a v4 of this?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3] serial: PL011: clear pending interrupts
From: Chanho Min @ 2012-03-14 1:47 UTC (permalink / raw)
To: Linus Walleij
Cc: Greg Kroah-Hartman, linux-serial, linux-arm-kernel, Russell King,
Jong-Sung Kim, Linus Walleij, stable, Shreshtha Kumar Sahu
In-Reply-To: <1331641643-25929-1-git-send-email-linus.walleij@stericsson.com>
On Tue, Mar 13, 2012 at 9:27 PM, Linus Walleij
<linus.walleij@stericsson.com> wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
>
> Chanho Min reported that when the boot loader transfers
> control to the kernel, there may be pending interrupts
> causing the UART to lock up in an eternal loop trying to
> pick tokens from the FIFO (since the RX interrupt flag
> indicates there are tokens) while in practice there are
> no tokens - in fact there is only a pending IRQ flag.
>
> This patch address the issue with a combination of two
> patches suggested by Russell King that clears and mask
> all interrupts at probe() and clears any pending error
> and RX interrupts at port startup time.
>
> We suspect the spurious interrupts are a side-effect of
> switching the UART from FIFO to non-FIFO mode.
I've tested this patch, and have confirmed that the issue is resolved
on our CA9 platform.
In addition, May I ask to change the tag as follows.
Reported-by: Chanho Min <chanho.min@lge.com>
Reviewed-by: Jong-Sung Kim <neidhard.kim@lge.com>
Thanks a lot !
Chanho Min
^ permalink raw reply
* [PATCH v3] serial: PL011: clear pending interrupts
From: Linus Walleij @ 2012-03-13 12:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, linux-serial, Chanho Min
Cc: linux-arm-kernel, Russell King, Jong-Sung Kim, Linus Walleij,
stable, Shreshtha Kumar Sahu
From: Linus Walleij <linus.walleij@linaro.org>
Chanho Min reported that when the boot loader transfers
control to the kernel, there may be pending interrupts
causing the UART to lock up in an eternal loop trying to
pick tokens from the FIFO (since the RX interrupt flag
indicates there are tokens) while in practice there are
no tokens - in fact there is only a pending IRQ flag.
This patch address the issue with a combination of two
patches suggested by Russell King that clears and mask
all interrupts at probe() and clears any pending error
and RX interrupts at port startup time.
We suspect the spurious interrupts are a side-effect of
switching the UART from FIFO to non-FIFO mode.
Cc: stable@kernel.org
Cc: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>
Cc: Jong-Sung Kim <neidhard.kim@lge.com>
Reported-by: Chanho Min <chanho0207@gmail.com>
Suggested-by: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Instead of making the IRQ fastpath account for spurious
IRQs with zero characters, make sure to clear out any
pending IRQ flags in startup().
ChangeLog v2->v3:
- Attempt to clear out any spuriously appearing RX interrups
in startup() instead of in the interrupt handler. Let's
see if this works.
---
drivers/tty/serial/amba-pl011.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 6800f5f..7e01399 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1381,6 +1381,10 @@ static int pl011_startup(struct uart_port *port)
uap->port.uartclk = clk_get_rate(uap->clk);
+ /* Clear pending error and receive interrupts */
+ writew(UART011_OEIS | UART011_BEIS | UART011_PEIS | UART011_FEIS |
+ UART011_RTIS | UART011_RXIS, uap->port.membase + UART011_ICR);
+
/*
* Allocate the IRQ
*/
@@ -1417,10 +1421,6 @@ static int pl011_startup(struct uart_port *port)
cr |= UART01x_CR_UARTEN | UART011_CR_RXE | UART011_CR_TXE;
writew(cr, uap->port.membase + UART011_CR);
- /* Clear pending error interrupts */
- writew(UART011_OEIS | UART011_BEIS | UART011_PEIS | UART011_FEIS,
- uap->port.membase + UART011_ICR);
-
/*
* initialise the old status of the modem signals
*/
@@ -1435,6 +1435,9 @@ static int pl011_startup(struct uart_port *port)
* as well.
*/
spin_lock_irq(&uap->port.lock);
+ /* Clear out any spuriously appearing RX interrupts */
+ writew(UART011_RTIS | UART011_RXIS,
+ uap->port.membase + UART011_ICR);
uap->im = UART011_RTIM;
if (!pl011_dma_rx_running(uap))
uap->im |= UART011_RXIM;
@@ -1927,6 +1930,10 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
goto unmap;
}
+ /* Ensure interrupts from this UART are masked and cleared */
+ writew(0, uap->port.membase + UART011_IMSC);
+ writew(0xffff, uap->port.membase + UART011_ICR);
+
uap->vendor = vendor;
uap->lcrh_rx = vendor->lcrh_rx;
uap->lcrh_tx = vendor->lcrh_tx;
--
1.7.8
^ permalink raw reply related
* [PATCH] serial: bfin-uart: Don't access tty circular buffer in TX DMA interrupt after it is reset.
From: Sonic Zhang @ 2012-03-13 7:51 UTC (permalink / raw)
To: linux-serial, Greg Kroah-Hartman; +Cc: LKML, Sonic Zhang
From: Sonic Zhang <sonic.zhang@analog.com>
When kernel reboot, tty circular buffer is reset before last TX DMA interrupt is called,
while the buffer tail is updated in TX DMA interrupt handler. So, don't update the buffer
tail if it is reset.
Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
---
drivers/tty/serial/bfin_uart.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/bfin_uart.c b/drivers/tty/serial/bfin_uart.c
index 5856407..a92626a 100644
--- a/drivers/tty/serial/bfin_uart.c
+++ b/drivers/tty/serial/bfin_uart.c
@@ -537,11 +537,13 @@ static irqreturn_t bfin_serial_dma_tx_int(int irq, void *dev_id)
* when start a new tx.
*/
UART_CLEAR_IER(uart, ETBEI);
- xmit->tail = (xmit->tail + uart->tx_count) & (UART_XMIT_SIZE - 1);
uart->port.icount.tx += uart->tx_count;
+ if (!uart_circ_empty(xmit)) {
+ xmit->tail = (xmit->tail + uart->tx_count) & (UART_XMIT_SIZE - 1);
- if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
- uart_write_wakeup(&uart->port);
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(&uart->port);
+ }
bfin_serial_dma_tx_chars(uart);
}
--
1.7.0.4
^ permalink raw reply related
* TTY: speakup, do not use serialP
From: covici @ 2012-03-12 22:59 UTC (permalink / raw)
To: linux-serial
Hi. I read the patch with the above name, and although it may be
necessary because the structures are going away, it does not solve the
problem of getting a -16 error code when releasing a port held by the
serial drivers. If I am correct about this, how can I fix this problem
-- if not please enlighten me as to how to release the region?
Thanks in advance for any suggestions.
--
Your life is like a penny. You're going to lose it. The question is:
How do
you spend it?
John Covici
covici@ccs.covici.com
^ permalink raw reply
* RE: [PATCH] serial: PL011: clear pending interrupts
From: Kim, Jong-Sung @ 2012-03-12 11:28 UTC (permalink / raw)
To: 'Russell King - ARM Linux', 'Linus Walleij'
Cc: linux-serial, 'Chanho Min', linux-arm-kernel,
'Linus Walleij', stable, 'Shreshtha Kumar Sahu'
In-Reply-To: <20120312101100.GA8649@n2100.arm.linux.org.uk>
> On Mon, Mar 12, 2012 at 08:32:21AM +0000, Russell King - ARM Linux wrote:
> > On Mon, Mar 12, 2012 at 09:25:50AM +0100, Linus Walleij wrote:
> > > diff --git a/drivers/tty/serial/amba-pl011.c
> > > b/drivers/tty/serial/amba-pl011.c index 6800f5f..ff3fed0 100644
> > > --- a/drivers/tty/serial/amba-pl011.c
> > > +++ b/drivers/tty/serial/amba-pl011.c
> > > @@ -224,6 +224,11 @@ static int pl011_fifo_to_tty(struct
uart_amba_port
> *uap)
> > > uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
> > > }
> > >
> >
> > What if we really do end up receiving some characters here?
> >
> > > + /* RXIS but RXFE? Just clear the interrupt */
> > > + if(unlikely(fifotaken == 0))
> > > + writew(UART011_RTIS | UART011_RXIS,
> > > + uap->port.membase + UART011_ICR);
>
> BTW, I don't see why we need any of this stuff.
Actually, I meant my patch applied alone, not with your patch. My thought
was the problem we're trying to handle is undocumented and is not guaranteed
not to happen again.
^ permalink raw reply
* Re: [PATCH] serial: PL011: clear pending interrupts
From: Russell King - ARM Linux @ 2012-03-12 11:28 UTC (permalink / raw)
To: Kim, Jong-Sung
Cc: 'Linus Walleij', linux-serial, 'Chanho Min',
linux-arm-kernel, 'Linus Walleij', stable,
'Shreshtha Kumar Sahu'
In-Reply-To: <01e101cd0042$5bf692a0$13e3b7e0$@lge.com>
On Mon, Mar 12, 2012 at 08:22:07PM +0900, Kim, Jong-Sung wrote:
> Maybe the character is handled on next upcoming RTIS or RXIS in normal.
> However, I agree with you there is possibility of already asserted RXIS at
> this point. (with debugger?) In such a case, we may lose Rx interrupts
> forever because it's not level-triggered, and the ISR lacks error interrupt
> handling. Correct me if something wrong.
The handler does not lack error handling - error handling is associated
with a character, and we deal with errors when we read the associated
character from the receive register.
So, we have no need for the error interrupts.
The PL011, as with all serial drivers I write, has full error handling
incorporated.
^ permalink raw reply
* RE: [PATCH] serial: PL011: clear pending interrupts
From: Kim, Jong-Sung @ 2012-03-12 11:22 UTC (permalink / raw)
To: 'Russell King - ARM Linux', 'Linus Walleij'
Cc: 'Greg Kroah-Hartman', linux-serial, 'Chanho Min',
linux-arm-kernel, 'Linus Walleij', stable,
'Shreshtha Kumar Sahu'
In-Reply-To: <20120312083221.GW17370@n2100.arm.linux.org.uk>
> > diff --git a/drivers/tty/serial/amba-pl011.c
> > b/drivers/tty/serial/amba-pl011.c index 6800f5f..ff3fed0 100644
> > --- a/drivers/tty/serial/amba-pl011.c
> > +++ b/drivers/tty/serial/amba-pl011.c
> > @@ -224,6 +224,11 @@ static int pl011_fifo_to_tty(struct uart_amba_port
> *uap)
> > uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
> > }
> >
>
> What if we really do end up receiving some characters here?
Maybe the character is handled on next upcoming RTIS or RXIS in normal.
However, I agree with you there is possibility of already asserted RXIS at
this point. (with debugger?) In such a case, we may lose Rx interrupts
forever because it's not level-triggered, and the ISR lacks error interrupt
handling. Correct me if something wrong.
Thank you for your comment, Russell.
>
> > + /* RXIS but RXFE? Just clear the interrupt */
> > + if(unlikely(fifotaken == 0))
> > + writew(UART011_RTIS | UART011_RXIS,
> > + uap->port.membase + UART011_ICR);
^ permalink raw reply
* Re: [PATCH] serial: PL011: clear pending interrupts
From: Linus Walleij @ 2012-03-12 10:48 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Linus Walleij, linux-serial, Chanho Min, linux-arm-kernel,
Jong-Sung Kim, stable, Shreshtha Kumar Sahu
In-Reply-To: <20120312101100.GA8649@n2100.arm.linux.org.uk>
On Mon, Mar 12, 2012 at 11:11 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>> > + /* RXIS but RXFE? Just clear the interrupt */
>> > + if(unlikely(fifotaken == 0))
>> > + writew(UART011_RTIS | UART011_RXIS,
>> > + uap->port.membase + UART011_ICR);
>
> BTW, I don't see why we need any of this stuff.
>
> The problem as described is that the interrupt handler is called with one
> of the receive flags set (UART011_RTIS or UART011_RXIS) but with
> UART01x_FR_RXFE clear.
>
> The problem report indicates that this is caused when we switch from FIFO
> mode to non-FIFO mode, and the FIFO contained some unread characters.
>
> That can only happen once we've enabled interrupts in the IMSC register,
> which doesn't happen until later in the startup function.
>
> So:
>
> spin_lock_irq(&uap->port.lock);
> uap->im = UART011_RTIM;
> if (!pl011_dma_rx_running(uap))
> uap->im |= UART011_RXIM;
> writew(uap->im, uap->port.membase + UART011_IMSC);
> spin_unlock_irq(&uap->port.lock);
>
> clearing the transmit/receive interrupts here _before_ we unmask them
> should be sufficient.
Right! OK I'll cook a v2 patch according to this suggestion and
request Chanho to test it.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] serial: PL011: clear pending interrupts
From: Russell King - ARM Linux @ 2012-03-12 10:11 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-serial, Chanho Min, linux-arm-kernel, Jong-Sung Kim,
Linus Walleij, stable, Shreshtha Kumar Sahu
In-Reply-To: <20120312083221.GW17370@n2100.arm.linux.org.uk>
On Mon, Mar 12, 2012 at 08:32:21AM +0000, Russell King - ARM Linux wrote:
> On Mon, Mar 12, 2012 at 09:25:50AM +0100, Linus Walleij wrote:
> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> > index 6800f5f..ff3fed0 100644
> > --- a/drivers/tty/serial/amba-pl011.c
> > +++ b/drivers/tty/serial/amba-pl011.c
> > @@ -224,6 +224,11 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)
> > uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
> > }
> >
>
> What if we really do end up receiving some characters here?
>
> > + /* RXIS but RXFE? Just clear the interrupt */
> > + if(unlikely(fifotaken == 0))
> > + writew(UART011_RTIS | UART011_RXIS,
> > + uap->port.membase + UART011_ICR);
BTW, I don't see why we need any of this stuff.
The problem as described is that the interrupt handler is called with one
of the receive flags set (UART011_RTIS or UART011_RXIS) but with
UART01x_FR_RXFE clear.
The problem report indicates that this is caused when we switch from FIFO
mode to non-FIFO mode, and the FIFO contained some unread characters.
That can only happen once we've enabled interrupts in the IMSC register,
which doesn't happen until later in the startup function.
So:
spin_lock_irq(&uap->port.lock);
uap->im = UART011_RTIM;
if (!pl011_dma_rx_running(uap))
uap->im |= UART011_RXIM;
writew(uap->im, uap->port.membase + UART011_IMSC);
spin_unlock_irq(&uap->port.lock);
clearing the transmit/receive interrupts here _before_ we unmask them
should be sufficient.
As we never turn the FIFO off after that point, we can't then cause the
condition. So I don't see any point in adding stuff to the interrupt
handling for this.
^ permalink raw reply
* Re: [PATCH] serial: PL011: clear pending interrupts
From: Russell King - ARM Linux @ 2012-03-12 10:03 UTC (permalink / raw)
To: James Courtier-Dutton
Cc: Linus Walleij, Greg Kroah-Hartman, linux-serial, Chanho Min,
linux-arm-kernel, Jong-Sung Kim, Linus Walleij, stable,
Shreshtha Kumar Sahu
In-Reply-To: <CAAMvbhF7-5FyzjgtK7EjFZSJg4KBxdMdd1jnTRTLA-LnsVhKyw@mail.gmail.com>
On Mon, Mar 12, 2012 at 09:27:28AM +0000, James Courtier-Dutton wrote:
> On 12 March 2012 08:25, Linus Walleij <linus.walleij@stericsson.com> wrote:
> > From: Linus Walleij <linus.walleij@linaro.org>
> >
> > Chanho Min reported that when the boot loader transfers
> > control to the kernel, there may be pending interrupts
> > causing the UART to lock up in an eternal loop trying to
> > pick tokens from the FIFO (since the RX interrupt flag
> > indicates there are tokens) while in practice there are
> > no tokens - in fact there is only a pending IRQ flag.
> >
>
> I might be off base here, because I don't know this particular UART,
> but if an IRQ routine is called because the RX interrupt flag is set,
> shouldn't the IRQ routine be written in such a way that it can never
> lock up.
The problem is that there are two flags:
1. The receiver interrupt status flags
2. The fifo status flags
It appears that there's a condition where the receiver interrupt flags can
indicate that characters are pending, but the status flags say that there
are no characters pending in the receive buffer.
This occurs because it seems there's a hardware bug when switching from
FIFO mode to non-FIFO mode - this causes previously received characters
to be hidden from the FIFO status flags.
^ permalink raw reply
* Re: [PATCH] serial: PL011: clear pending interrupts
From: James Courtier-Dutton @ 2012-03-12 9:27 UTC (permalink / raw)
To: Linus Walleij
Cc: Greg Kroah-Hartman, linux-serial, Chanho Min, linux-arm-kernel,
Russell King, Jong-Sung Kim, Linus Walleij, stable,
Shreshtha Kumar Sahu
In-Reply-To: <1331540750-25697-1-git-send-email-linus.walleij@stericsson.com>
On 12 March 2012 08:25, Linus Walleij <linus.walleij@stericsson.com> wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
>
> Chanho Min reported that when the boot loader transfers
> control to the kernel, there may be pending interrupts
> causing the UART to lock up in an eternal loop trying to
> pick tokens from the FIFO (since the RX interrupt flag
> indicates there are tokens) while in practice there are
> no tokens - in fact there is only a pending IRQ flag.
>
I might be off base here, because I don't know this particular UART,
but if an IRQ routine is called because the RX interrupt flag is set,
shouldn't the IRQ routine be written in such a way that it can never
lock up.
If the IRQ routing has a loop that is used to read each byte from the
port, the loop should have a counter limit in it, so in case of some
unexpected situation, it can always make an exit, and not lock up.
So, in your case, if the loop counter limit is hit, it would mean that
although we had an RX interrupt flag set, there are no tokens, so act
according with a suitable error path.
I had to implement this in one Linux kernel driver due to an error
condition that happened on PCMCIA card insertion.
On the first insertion, there were unwanted IRQ calls, but after the
second false IRQ call, it calmed down. I used a counter limit in the
IRQ driver in order to handle the false IRQ calls situation.
Kind Regards
James
^ permalink raw reply
* Re: [PATCH] serial: PL011: clear pending interrupts
From: Russell King - ARM Linux @ 2012-03-12 8:32 UTC (permalink / raw)
To: Linus Walleij
Cc: Greg Kroah-Hartman, linux-serial, Chanho Min, linux-arm-kernel,
Jong-Sung Kim, Linus Walleij, stable, Shreshtha Kumar Sahu
In-Reply-To: <1331540750-25697-1-git-send-email-linus.walleij@stericsson.com>
On Mon, Mar 12, 2012 at 09:25:50AM +0100, Linus Walleij wrote:
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 6800f5f..ff3fed0 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -224,6 +224,11 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)
> uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
> }
>
What if we really do end up receiving some characters here?
> + /* RXIS but RXFE? Just clear the interrupt */
> + if(unlikely(fifotaken == 0))
> + writew(UART011_RTIS | UART011_RXIS,
> + uap->port.membase + UART011_ICR);
^ permalink raw reply
* Re: [PATCH] Clear previous interrupts after fifo is disabled
From: Linus Walleij @ 2012-03-12 8:29 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Chanho Min, Russell King, Alan Cox, Shreshtha Kumar Sahu,
Kim, Jong-Sung, linux-kernel, linux-serial
In-Reply-To: <20120308184954.GA1275@kroah.com>
On Thu, Mar 8, 2012 at 7:49 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> Can someone, if this is still needed, and everyone agrees on how to
> solve it, please send me the needed fix?
I've send a patch combining the probe() and startup() fixes from Russell
with the latest patch from Jong-Sung Kim into what may very well
be the silver bullet.
Subject "serial: PL011: clear pending interrupts"
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] serial: PL011: clear pending interrupts
From: Linus Walleij @ 2012-03-12 8:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, linux-serial, Chanho Min
Cc: linux-arm-kernel, Russell King, Jong-Sung Kim, Linus Walleij,
stable, Shreshtha Kumar Sahu
From: Linus Walleij <linus.walleij@linaro.org>
Chanho Min reported that when the boot loader transfers
control to the kernel, there may be pending interrupts
causing the UART to lock up in an eternal loop trying to
pick tokens from the FIFO (since the RX interrupt flag
indicates there are tokens) while in practice there are
no tokens - in fact there is only a pending IRQ flag.
This patch address the issue with a combination of a patch
from Russell King that clears and mask all interrupts at
probe() and clears any pending error and RX interrupts at
port startup time, and a patch from Jong-Sung Kim that
clears any RX interrupts (including timeouts) even if if
there are zero tokens in the FIFO.
This way these pending interrupts should be addressed in
two ways and solidify the driver in both probe() and
IRQ paths.
Cc: stable@kernel.org
Cc: Shreshtha Kumar Sahu <shreshthakumar.sahu@stericsson.com>
Reported-by: Chanho Min <chanho0207@gmail.com>
Suggested-by: Russell King <linux@arm.linux.org.uk>
Suggested-by: Jong-Sung Kim <neidhard.kim@lge.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
OK Greg requested that we send out this combined approach, can
addressees (Chanho especially) please confirm that the patch
solves the problem? Tested on U300 and U8500.
---
drivers/tty/serial/amba-pl011.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 6800f5f..ff3fed0 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -224,6 +224,11 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)
uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
}
+ /* RXIS but RXFE? Just clear the interrupt */
+ if(unlikely(fifotaken == 0))
+ writew(UART011_RTIS | UART011_RXIS,
+ uap->port.membase + UART011_ICR);
+
return fifotaken;
}
@@ -1381,6 +1386,10 @@ static int pl011_startup(struct uart_port *port)
uap->port.uartclk = clk_get_rate(uap->clk);
+ /* Clear pending error and receive interrupts */
+ writew(UART011_OEIS | UART011_BEIS | UART011_PEIS | UART011_FEIS |
+ UART011_RTIS | UART011_RXIS, uap->port.membase + UART011_ICR);
+
/*
* Allocate the IRQ
*/
@@ -1417,10 +1426,6 @@ static int pl011_startup(struct uart_port *port)
cr |= UART01x_CR_UARTEN | UART011_CR_RXE | UART011_CR_TXE;
writew(cr, uap->port.membase + UART011_CR);
- /* Clear pending error interrupts */
- writew(UART011_OEIS | UART011_BEIS | UART011_PEIS | UART011_FEIS,
- uap->port.membase + UART011_ICR);
-
/*
* initialise the old status of the modem signals
*/
@@ -1927,6 +1932,10 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
goto unmap;
}
+ /* Ensure interrupts from this UART are masked and cleared */
+ writew(0, uap->port.membase + UART011_IMSC);
+ writew(0xffff, uap->port.membase + UART011_ICR);
+
uap->vendor = vendor;
uap->lcrh_rx = vendor->lcrh_rx;
uap->lcrh_tx = vendor->lcrh_tx;
--
1.7.8
^ permalink raw reply related
* RE: [PATCH] Clear previous interrupts after fifo is disabled
From: Kim, Jong-Sung @ 2012-03-12 1:24 UTC (permalink / raw)
To: 'Chanho Min', 'Greg Kroah-Hartman'
Cc: 'Linus Walleij', 'Russell King',
'Alan Cox', 'Shreshtha Kumar Sahu', linux-kernel,
linux-serial
In-Reply-To: <CAOAMb1Bpx4ucHgKy+oT2rvt16cubFR3Vi8o3fjwrT65DdJko_Q@mail.gmail.com>
> -----Original Message-----
> From: Chanho Min [mailto:chanho0207@gmail.com]
> Sent: Saturday, March 10, 2012 11:15 AM
> To: Greg Kroah-Hartman
> Cc: Linus Walleij; Russell King; Alan Cox; Shreshtha Kumar Sahu; Kim,
Jong-
> Sung; linux-kernel@vger.kernel.org; linux-serial@vger.kernel.org
> Subject: Re: [PATCH] Clear previous interrupts after fifo is disabled
>
> >> To me it seems Russell's patch solves part of the problem, and
> >> Jong-Sung Kim's patch on top of that solves the entire problem, but
> >> Chanho need to come back and tell whether this is the case in
> >> practice.
> >
> > Ok, then, once it is tested, can someone resend them to me?
> >
> > thanks,
> >
> > greg k-h
>
> I checked that Jong-Sung Kim's patch solved this hang-up issue and agree
on
> it. But, RTIS seems to be cleared as well.
You're right. RTIS should be cleared as well as RXIS. Revised patch:
diff --git a/drivers/tty/serial/amba-pl011.c
b/drivers/tty/serial/amba-pl011.c
index 6800f5f..39520db 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -224,6 +224,11 @@ static int pl011_fifo_to_tty(struct uart_amba_port
*uap)
uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
}
+ /* RTIS and/or RXIS, but RXFE? Just clear the interrupt(s) */
+ if(unlikely(fifotaken == 0))
+ writew(UART011_RTIS | UART011_RXIS, uap->port.membase +
+ UART011_ICR);
+
return fifotaken;
}
^ permalink raw reply related
* [PATCH] tty: serial: vt8500: fix annotations for probe/remove
From: Wolfram Sang @ 2012-03-11 14:02 UTC (permalink / raw)
To: linux-serial; +Cc: Greg Kroah-Hartman, Alexey Charkov, Wolfram Sang
Fixes:
WARNING: drivers/tty/serial/built-in.o(.data+0x30): Section mismatch in reference from the variable vt8500_platform_driver to the function .init.text:vt8500_serial_probe()
The variable vt8500_platform_driver references
the function __init vt8500_serial_probe()
And mark the remove pointer while we are here.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
drivers/tty/serial/vt8500_serial.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 026cb9e..2be006f 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -544,7 +544,7 @@ static struct uart_driver vt8500_uart_driver = {
.cons = VT8500_CONSOLE,
};
-static int __init vt8500_serial_probe(struct platform_device *pdev)
+static int __devinit vt8500_serial_probe(struct platform_device *pdev)
{
struct vt8500_port *vt8500_port;
struct resource *mmres, *irqres;
@@ -605,7 +605,7 @@ static int __devexit vt8500_serial_remove(struct platform_device *pdev)
static struct platform_driver vt8500_platform_driver = {
.probe = vt8500_serial_probe,
- .remove = vt8500_serial_remove,
+ .remove = __devexit_p(vt8500_serial_remove),
.driver = {
.name = "vt8500_serial",
.owner = THIS_MODULE,
--
1.7.2.5
^ permalink raw reply related
* MAIL 10.03.2012
From: Mr. Richard Tang @ 2012-03-11 10:05 UTC (permalink / raw)
To: Recipients
Mail From Tang.
I work with the hang seng Bank Hong Kong.I have a business proposition for you involving a trade in my bank which I know we will be of mutual benefit to both of us, If interested mail me at:tangricad@yahoo.co.jp
Regards,
Richard Tang.
^ permalink raw reply
* Re: [PATCH] Clear previous interrupts after fifo is disabled
From: Chanho Min @ 2012-03-10 2:14 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linus Walleij, Russell King, Alan Cox, Shreshtha Kumar Sahu,
Kim, Jong-Sung, linux-kernel, linux-serial
In-Reply-To: <20120309163739.GA9545@kroah.com>
>> To me it seems Russell's patch solves part of the problem,
>> and Jong-Sung Kim's patch on top of that solves the entire
>> problem, but Chanho need to come back and tell whether
>> this is the case in practice.
>
> Ok, then, once it is tested, can someone resend them to me?
>
> thanks,
>
> greg k-h
I checked that Jong-Sung Kim's patch solved this hang-up issue
and agree on it. But, RTIS seems to be cleared as well.
^ permalink raw reply
* Re: [PATCH 3/7] serial: sh-sci: restore big-endian operation.
From: Thomas Schwinge @ 2012-03-09 20:23 UTC (permalink / raw)
To: Greg KH; +Cc: Paul Mundt, linux-sh, linux-serial
In-Reply-To: <20120309171007.GA25965@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 2160 bytes --]
Hi Greg!
First -- as you probably have guessed already -- I'm not (not yet?) :-)
too familiar with the Linux kernel patch submission process. I have
spent quite some time reading the included documentation, but apparently
missed some details... :-)
On Fri, 9 Mar 2012 09:10:07 -0800, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Mar 09, 2012 at 05:38:49PM +0100, Thomas Schwinge wrote:
> > As of 37b7a97884ba64bf7d403351ac2a9476ab4f1bba we have to use the
> > endianess-agnostic I/O accessor functions.
>
> Please always put a human readable description of what the git id is, so
> that we have a chance to understand what is going on.
Sorry. ``sh: machvec IO death.'' -- well, the essential part what I'm
fixing here is that before this commit readw (and friends, such as
ioread16) didn't do any endianess modification of the data, but directly
invoked __raw_readw, whereas after 37b7a978 they do such modification by
means of filtering through le16_to_cpu (in the readw/ioread16 case).
This is where things break for big endian (but not little endian, where
this is a no-op).
> Are these patches supposed to be going through the sh tree, or the
> serial tree to Linus?
I'm assuming that Paul Mundt will handle the whole lot in an SH tree.
Paul, by the way, if that's more convenient I can also publish my Git
tree.
> And watch your To: line, it seemed to be munged :(
Yes. By means of scripts/get_maintainer.pl (and some manual adjustments)
I created the Cc: lists that are part of my commits' log messages. Then,
I just had git send-email send that out, without explicitly specifying a
primary recipient (To: line). I assume I should have put Paul there?
On this patch, the linux-serial was Cced (as suggested by
scripts/get_maintainer.pl), as it touches a TTY/serial driver, and I
wanted to make sure to give you an opportunity to review that the patch
doesn't introduce any breakage on non-SH architectures, for example.
(Despite its indicative name, sh-sci also seems to be used on some ARM
boards.)
> confused,
Sorry; will hopefully improve. :-)
Grüße,
Thomas
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply
* [PATCH 4/5] pch_uart: Use existing default_baud in setup_console
From: Darren Hart @ 2012-03-09 17:51 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Darren Hart, Tomoya MORINAGA, Feng Tang, Greg Kroah-Hartman,
Alan Cox, linux-serial
In-Reply-To: <cover.1331315372.git.dvhart@linux.intel.com>
Rather than hardcode 9600, use the existing default_baud parameter (which
also defaults to 9600).
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
drivers/tty/serial/pch_uart.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 46f6fbf..cca742b 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1566,7 +1566,7 @@ pch_console_write(struct console *co, const char *s, unsigned int count)
static int __init pch_console_setup(struct console *co, char *options)
{
struct uart_port *port;
- int baud = 9600;
+ int baud = default_baud;
int bits = 8;
int parity = 'n';
int flow = 'n';
--
1.7.6.5
^ permalink raw reply related
* [PATCH 0/5 V3] pch_uart: Cleanups, board quirks, and user uartclk parameter
From: Darren Hart @ 2012-03-09 17:51 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Tomoya MORINAGA, Feng Tang, Greg Kroah-Hartman, Alan Cox,
linux-serial, Darren Hart
This series does some minor clean-up to the pch_uart driver, adds support
for the Fish River Island II UART clock quirks, and introduces a user_uartclk
parameter to aid in developing for early and changing hardware.
In order to support an early serial console, we cannot rely on the pch_phub
probe function to setup the CFGCLK register. This series relies on the board
quirks and does not force the setting of the CLKREG in the pch_phub code.
Instead, it aligns with what is the default configuration (defined by firmware)
for a given board. The user_uartclk parameter provides a mechanism to force a
specific uartclk if necessary.
V2: Incorporates Alan Cox's feedback into 2/4: Add Fish River Island II uart
clock quirks, refactoring the clock quirks into a new function and using
a more appropriate name for a reused string variable.
Add support for the two firmware variants for the FRI2.
V3: Rebase on linux-next.git/master rather than linux-2.6.git.
Add module parameter descriptions.
The following changes since commit d5ba8c14a256799efb1048151fe3490f9cb989b9:
Add linux-next specific files for 20120309 (2012-03-09 19:04:19 +1100)
are available in the git repository at:
git://git.infradead.org/users/dvhart/linux-next.git pch_uart_v3
http://git.infradead.org/users/dvhart/linux-next.git/shortlog/refs/heads/pch_uart_v3
Darren Hart (5):
pch_uart: Use uartclk instead of base_baud
pch_uart: Add Fish River Island II uart clock quirks
pch_uart: Add user_uartclk parameter
pch_uart: Use existing default_baud in setup_console
pch_uart: Add module parameter descriptions
drivers/tty/serial/pch_uart.c | 59 ++++++++++++++++++++++++++++------------
1 files changed, 41 insertions(+), 18 deletions(-)
--
1.7.6.5
^ permalink raw reply
* [PATCH 3/5] pch_uart: Add user_uartclk parameter
From: Darren Hart @ 2012-03-09 17:51 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Darren Hart, Tomoya MORINAGA, Feng Tang, Greg Kroah-Hartman,
Alan Cox, linux-serial
In-Reply-To: <cover.1331315372.git.dvhart@linux.intel.com>
For cases where boards with non-default clocks are not yet added to the kernel
or when the clock varies across hardware revisions, it is useful to be
able to specify the UART clock on the kernel command line.
Add the user_uartclk parameter and prefer it, if set, to the default and
board specific UART clock settings. Specify user_uartclock on the command-line
with "pch_uart.user_uartclk=48000000".
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
drivers/tty/serial/pch_uart.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 88a1be0..46f6fbf 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -295,6 +295,7 @@ static struct pch_uart_driver_data drv_dat[] = {
static struct eg20t_port *pch_uart_ports[PCH_UART_NR];
#endif
static unsigned int default_baud = 9600;
+static unsigned int user_uartclk = 0;
static const int trigger_level_256[4] = { 1, 64, 128, 224 };
static const int trigger_level_64[4] = { 1, 16, 32, 56 };
static const int trigger_level_16[4] = { 1, 4, 8, 14 };
@@ -372,6 +373,9 @@ static int pch_uart_get_uartclk(void)
{
const char *cmp;
+ if (user_uartclk)
+ return user_uartclk;
+
cmp = dmi_get_system_info(DMI_BOARD_NAME);
if (cmp && strstr(cmp, "CM-iTC"))
return CMITC_UARTCLK;
@@ -1860,3 +1864,4 @@ module_exit(pch_uart_module_exit);
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("Intel EG20T PCH UART PCI Driver");
module_param(default_baud, uint, S_IRUGO);
+module_param(user_uartclk, uint, S_IRUGO);
--
1.7.6.5
^ permalink raw reply related
* [PATCH 5/5] pch_uart: Add module parameter descriptions
From: Darren Hart @ 2012-03-09 17:51 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Darren Hart, Tomoya MORINAGA, Feng Tang, Greg Kroah-Hartman,
Alan Cox, linux-serial
In-Reply-To: <cover.1331315372.git.dvhart@linux.intel.com>
Document default_baud and user_uartclk module parameters.
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
drivers/tty/serial/pch_uart.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index cca742b..332f2eb 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1864,4 +1864,8 @@ module_exit(pch_uart_module_exit);
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("Intel EG20T PCH UART PCI Driver");
module_param(default_baud, uint, S_IRUGO);
+MODULE_PARM_DESC(default_baud,
+ "Default BAUD for initial driver state and console (default 9600)");
module_param(user_uartclk, uint, S_IRUGO);
+MODULE_PARM_DESC(user_uartclk,
+ "Override UART default or board specific UART clock");
--
1.7.6.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox