Re: [linux-sunxi] Designware UART bug

From: Tim Kryger <tim.kryger@gmail.com>
To: Olliver Schinagl <oliver@schinagl.nl>
Cc: Chen-Yu Tsai <wens@csie.org>, Jamie Iles <jamie@jamieiles.com>,
	Tim Kryger <tim.kryger@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	dev <dev@linux-sunxi.org>,
	Maxime Ripard <maxime.ripard@free-electrons.com>
Subject: Re: [linux-sunxi] Designware UART bug
Date: Wed, 3 May 2017 20:51:31 -0700	[thread overview]
Message-ID: <CAD7vxxK-LmcwAeT5UJg1cJiiVc=wKn9SRNb5QzbxuEi8TyOO3g@mail.gmail.com> (raw)
In-Reply-To: <0ea81628-3df9-3420-235d-da7165423308@schinagl.nl>

On Wed, May 3, 2017 at 8:40 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Hey Tim,

>
> Ok, so as far as I understand (from the datasheet) the intended way to do
> this would be to check for the BUSY IRQ & USR[0] IRQ and if it is busy,
> (re-write) the LCR. We no longer do this because it did not work due to the
> delayed interrupt.
>
> So one question is, why are we not checking the USR[0] reg whilst doing the
> loop? Is that register not there for exactly that purpose? But I guess
> brute-forcing it works more reliable I suppose then.

That status bit isn't particularly helpful since even if it reports
the UART is idle, there is no guarantee it won't become busy before
the attempted write of the LCR happens.

> But secondly, when is the UART_IIR_BUSY interrupt handled? And it not being
> handled, could that be the actual reason things where failing? (Or as I read
> somewhere a silicon bug?)
>
> Right now, we have serial8250_handle_irq() and if that returns 0, we check
> if we (still) have an unhandled UART_IIR_BUSY interrupt.
> But the only way for serial8250_handle_irq() to return 0, is if it has set
> UART_NO_INT.
>
> If we do not have an IRQ, i'm pretty sure, UART_IIR_BUSY can't be triggered
> right? And if it IS the interrupt that caused us to go into the interrupt
> handler, well, handle_irq does its thing and then returns 1 for finishing
> it, which in turn causes the UART_IIR_BUSY check to be skipped.
>
> So we never clear the UART_IIR_BUSY interrupt if we manage to trigger that.
> (Please do correct me if I'm wrong.

Note that the LSB is set in each of the following defines.

#define UART_IIR_NO_INT         0x01 /* No interrupts pending */
#define UART_IIR_BUSY           0x07 /* DesignWare APB Busy Detect */

Thus, when iir is UART_IIR_BUSY, serial8250_handle_irq bails out and
returns zero such that the interrupt gets cleared by the read of USR
in dw8250_handle_irq.

> I'm changing things in the interrupt handler a bit now to first check for
> the busy interrupt first and if that is triggered do the dummy return (clear
> it) and return (since LCR is handled alternativly.
>
> Olliver