linux-kernel - Re: [PATCH] Clear previous interrupts after fifo is disabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOAMb1BjC2bF=rpJAhRUrB_U2mp8HGWxfcp+=TZ1etWzjUEwig@mail.gmail.com>
Date:	Tue, 28 Feb 2012 10:35:30 +0900
From:	Chanho Min <chanho0207@...il.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Alan Cox <alan@...ux.intel.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Shreshtha Kumar Sahu <shreshthakumar.sahu@...ricsson.com>,
	"Kim, Jong-Sung" <neidhard.kim@....com>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH] Clear previous interrupts after fifo is disabled

On Mon, Feb 27, 2012 at 8:02 PM, Russell King - ARM Linux
<linux@....linux.org.uk> wrote:
> On Mon, Feb 27, 2012 at 10:48:58AM +0000, Russell King - ARM Linux wrote:
>> I'd much prefer to only clear those interrupts which actually need to be
>> cleared at this point.  So, I'd suggest this approach instead:
>
> Thinking about this a little more, we definitely want to mask and clear
> interrupts at probe time as well:
>
I'm not satisfied with this completely. RIS has some pending
interrupts even if interrupts are masked/disabled in IMSC. If your
patch is applied, interrupt can be pended as bellows and RXFE of the
flag register is set as well.

writew(UART011_OEIS | UART011_BEIS | UART011_PEIS | UART011_FEIS |
       UART011_RTIS | UART011_RXIS, uap->port.membase + UART011_ICR);
...
Interrupt is occured and pended in RIS
..
writew(uap->im, uap->port.membase + UART011_IMSC);

Root cause is that Rx interrupt set but Rx fifo is empty. If we just
remove the sentence for clearing LCRH, nothing happens and interrupt
handler don't this misbehave. Also I don't fully understand why we
need to clear interrupts at probe time. If we prefer to only clear
those interrupts which actually need to be cleared, this is the
improved patch.

Thanks,

Signed-off-by: Chanho Min <chanho.min@....com>
---
 drivers/tty/serial/amba-pl011.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 6800f5f..96d1828 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1397,7 +1397,12 @@ static int pl011_startup(struct uart_port *port)
 	writew(cr, uap->port.membase + UART011_CR);
 	writew(0, uap->port.membase + UART011_FBRD);
 	writew(1, uap->port.membase + UART011_IBRD);
+	spin_lock_irq(&uap->port.lock);
 	writew(0, uap->port.membase + uap->lcrh_rx);
+	/* Clear pending error and receive interrupts */
+	writew(UART011_OEIS | UART011_BEIS | UART011_PEIS | UART011_FEIS |
+		UART011_RTIS | UART011_RXIS, uap->port.membase + UART011_ICR);
+	spin_unlock_irq(&uap->port.lock);
 	if (uap->lcrh_tx != uap->lcrh_rx) {
 		int i;
 		/*
@@ -1417,10 +1422,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
 	 */
-- 
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/