Re: [linux-sunxi] Designware UART bug

[Chen-Yu Tsai]

On Wed, May 3, 2017 at 6:17 PM, Olliver Schinagl <oliver@xxxxxxxxxxx> wrote:
> Hey Jamie,
>
> Several years ago you wrote the glue-code [0] for the DW 8250 IP. Over the
> years various 'fixes' have been applied to resolve certain 'weird' problems
> that Tim tried to fix with [1].
>
> After going over the datasheets and code with a comb several times now, I
> think I may have found one (of a few others) reasons and would like both
> your and Tim's thoughts here.
>
> The current (and original) code [2] uses the register offset 0x1f for the
> UART_USR register.
>
> I searched far and wide, various datasheet of physical uarts (8250 - 16950)
> and some IP cores and none seem to have the USR (and specifically the USR[0]
> bit) register, so it seems to be specific to the DW_apb_uart. However
> looking at the only databook available to me [3] of the UART IP, I cannot
> seem to find anything at register offset 0x1f.
>
> The platform I'm using uses the Allwinner A20 SoC, which also features the
> DW uart IP and also here, there is nothing at offset 0x1f.
>
> The intended register IS however actually at 0x7c.
>
> My question is thus twofold.
>
> Why was 0x1f used? Is this specific to a certain (version) UART or is this a
> long uncaught typo.

The original 8250 datasheets have registers at byte offsets. Note that the
registers are only 8 bits wide.

On Allwinner, and many other platforms, the registers are still 8 bits
wide, but are 32bit-aligned, likely for aligned bus access. 0x7c = 0x1f << 2.
The 8250 core accessor functions are supposed to handle this for you.

ChenYu

> Tim, assuming it is a typo, could this the cause which made you implement
> [1]? From what I understand, you keep writing the LCR until it takes.
>
> Initially, the UART_IIR_BUSY check looked like this:
>         if (serial8250_handle_irq(p, iir)) {
>                 return 1;
>         } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
>                 /* Clear the USR and write the LCR again. */
>                 (void)p->serial_in(p, d->usr_reg);
>                 p->serial_out(p, UART_LCR, d->last_lcr);
>
>                 return 1;
>         }
>
> what I'm missing here is, that if UART_IIR_BUSY is set, we have:
> * check the d->usr_reg (UART_USR) bit 0
> * wait until it becomes cleared (do not allow new data to be pushed out,
> optionally force the data to be pushed out)
> * write LCR register (and check if it took (and no longer loop over the LCR
> to see if the values stuck, in theory).
> * if we never get un-busy, something is wrong?
>
> All of this btw is currently moot anyway, as the only way to get into the
> (else) if, is if serial8250_handle_irq returns false. And from what I can
> see, this is only if iir == UART_IIR_NO_IRQ, in which case we never ever
> clear the USR and thus never ever clear the UART_IIR_BUSY flag.
>
> Olliver
>
>
> [0]
> https://github.com/torvalds/linux/commit/6b1a98d1c4851235d9b6764b3f7b7db7909fc760
>
> [1]
> https://github.com/torvalds/linux/commit/c49436b657d0a56a6ad90d14a7c3041add7cf64d
>
> [2]
> https://github.com/torvalds/linux/commit/6b1a98d1c4851235d9b6764b3f7b7db7909fc760#diff-d6e619fc4c51febf7880632fde5d0208R63
>
> [3] http://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf
>
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to linux-sunxi+unsubscribe@xxxxxxxxxxxxxxxxx
> For more options, visit https://groups.google.com/d/optout.