com_fdt.c: enable 16550 FIFO

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

com_fdt.c: enable 16550 FIFO

SASANO Takayoshi
Hello,

Allwinner A10/20' UART has FIFO, but dmesg says no working FIFO.
This is caused by com_fdt.c tells 16550 not 16550A.
So here is the diff to fix this.

----

Index: com_fdt.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/com_fdt.c,v
retrieving revision 1.3
diff -u -p -r1.3 com_fdt.c
--- com_fdt.c 6 Aug 2018 10:52:30 -0000 1.3
+++ com_fdt.c 31 Dec 2019 00:51:07 -0000
@@ -117,7 +117,7 @@ com_fdt_attach(struct device *parent, st
 
  sc->sc_iot = faa->fa_iot;
  sc->sc_iobase = faa->fa_reg[0].addr;
- sc->sc_uarttype = COM_UART_16550;
+ sc->sc_uarttype = COM_UART_16550A;
  sc->sc_frequency = freq ? freq : COM_FREQ;
 
  sc->sc_reg_width = OF_getpropint(faa->fa_node, "reg-io-width", 4);

----

Diff is tested on Allwinner A20, Banana Pi BPI-M1 board.

(before)
sxits0 at simplebus0
com0 at simplebus0: ns16550, no working fifo
com0: console
com1 at simplebus0: ns16550, no working fifo
com2 at simplebus0: ns16550, no working fifo
sxitwi0 at simplebus0

(after)
sxits0 at simplebus0
com0 at simplebus0: ns16550a, 16 byte fifo
com0: console
com1 at simplebus0: ns16550a, 16 byte fifo
com2 at simplebus0: ns16550a, 16 byte fifo
sxitwi0 at simplebus0

Allwinner's UART has 64byte FIFO and com_fifo_probe() detects exact size.
But com_fifo_probe() adopts smaller FIFO size so the result is 16byte,
comes from sc->sc_fifolen.

Is there any reason to disable FIFO on FDT based UART devices?
If there is no problem, I want to commit this.

Regards,

--
SASANO Takayoshi (JG1UAA) <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Re: com_fdt.c: enable 16550 FIFO

Mark Kettenis
> Date: Thu, 02 Jan 2020 05:43:49 +0900
> From: SASANO Takayoshi <[hidden email]>
>
> Hello,
>
> Allwinner A10/20' UART has FIFO, but dmesg says no working FIFO.
> This is caused by com_fdt.c tells 16550 not 16550A.
> So here is the diff to fix this.
>
> ----
>
> Index: com_fdt.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/com_fdt.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 com_fdt.c
> --- com_fdt.c 6 Aug 2018 10:52:30 -0000 1.3
> +++ com_fdt.c 31 Dec 2019 00:51:07 -0000
> @@ -117,7 +117,7 @@ com_fdt_attach(struct device *parent, st
>  
>   sc->sc_iot = faa->fa_iot;
>   sc->sc_iobase = faa->fa_reg[0].addr;
> - sc->sc_uarttype = COM_UART_16550;
> + sc->sc_uarttype = COM_UART_16550A;
>   sc->sc_frequency = freq ? freq : COM_FREQ;
>  
>   sc->sc_reg_width = OF_getpropint(faa->fa_node, "reg-io-width", 4);
>
> ----
>
> Diff is tested on Allwinner A20, Banana Pi BPI-M1 board.
>
> (before)
> sxits0 at simplebus0
> com0 at simplebus0: ns16550, no working fifo
> com0: console
> com1 at simplebus0: ns16550, no working fifo
> com2 at simplebus0: ns16550, no working fifo
> sxitwi0 at simplebus0
>
> (after)
> sxits0 at simplebus0
> com0 at simplebus0: ns16550a, 16 byte fifo
> com0: console
> com1 at simplebus0: ns16550a, 16 byte fifo
> com2 at simplebus0: ns16550a, 16 byte fifo
> sxitwi0 at simplebus0
>
> Allwinner's UART has 64byte FIFO and com_fifo_probe() detects exact size.
> But com_fifo_probe() adopts smaller FIFO size so the result is 16byte,
> comes from sc->sc_fifolen.
>
> Is there any reason to disable FIFO on FDT based UART devices?
> If there is no problem, I want to commit this.

Not enabling the FIFO was a deliberate decision.  com@fdt supports a
wide range of hardware that is more-or-less compatible with the
ns16550 UART.  But there are quirks, and on some variants this affects
the FIFOs.  So disabling the FIFO seemed a safe choice.  Especially
because the FIFO depth probing code sometimes causes artifacts such as
loss of output characters.

The Allwinner A20 has a Synposys Designware UART.  That UART may be
integrated on SoC with various options, i.e. various FIFO sizes or no
FIFO at all.  The particular configuration can be read from a
parameter register.  Maybe changing the type of the UART and setting
the FIFO size based on that register avoids problems.

Official documentation for the Designware UART is only available under
NDA, but some documentation is included in various hardware manuals
such as the Altera "Cyclone V Hard Processor System Technical
Reference Manual".

Reply | Threaded
Open this post in threaded view
|

Re: com_fdt.c: enable 16550 FIFO

SASANO Takayoshi
Hi,

> Not enabling the FIFO was a deliberate decision.

Thanks, that is I want to hear.
I cannot jude whether the code is something mistaken or intentionally
disabled FIFO.

I want to use JumboSPOT (RF modem for ham radio via UART) with Banana Pi
BPI-M1, so enabling 16550 FIFO is important for such a system.

But, I agree writing safe code to support many variant 16550.
This is more important thing.

Regards,
--
SASANO Takayoshi (JG1UAA) <[hidden email]>