Introduce UART driver module for Armada 3700

[mw]

Summary

This patch adds support for UART in Armada 3700 family.
It exposes both low-level UART interface, as well as
standard driver methods.

[• ian]

sys/dev/uart/uart_dev_mvebu.c

120

move the semicolon to its own line below the while() to avoid compiler warnings.

same thing on similar while (cond); lines below.

472

SER_INT_TXIDLE does not mean the tx fifo is empty. It means the fifo is able to accept sc->sc_txfifosz more bytes of data without overflowing.

If the hardware can be configured to provide an interrupt at different levels of fifo full/emptyness, it's best to program it to interrupt when 3/4 of the buffer is empty, and set txfifosz to that value. The important point is that txfifosz should exactly match the number of bytes free in the fifo at the time SER_INT_TXIDLE is signalled back to the core routines.

The sc->rxfifosz should be the actual hardware size, regardless of any configurable level for interrupt signaling. The only thing the upper layers do with rxfifosz is set the size of some intermediate buffers such that it is always safe for the low level code to push a full rx fifo amount of data up in one bus_receive() call.

[andrew]

sys/dev/uart/uart_dev_mvebu.c

410

This is a bit difficult to read. Can you split it out to an if statement?

416–443

Is this correct? The other drivers don't set the baudrate here.

541–544

How does this work? All of those bits will have been masked out above.

[pdk_semihalf.com]

sys/dev/uart/uart_dev_mvebu.c

410

OK

416–443

I've checked it (commit e8dfdbc0d9271) and this is not correct. Thanks for your comment.

541–544

At line 536 I'm resetting all bits except 4 bits providing additional information. Unfortunately, bits positions in register don't match ones in FreeBSD macros, so this code 'moves' bits to correct position.

Example:
Assume that we received 'A' char and break was detected

xc = 0x8041
rx = 0x41 ('A')
er = 0x8000 (break detected)
er & RBR_BRK_DET = 0x8000
0x8000 >> 7 = 0x100 (UART_STAT_BREAK)
rx | er = 0x141

I've just realized that it has a flaw. What if someone change UART_STAT_BRAKE to another value, even worse if new value will be higher than RBR_BRK_DET. Due to this I will replace it with ifs, if you find this necesseary

[pdk_semihalf.com]

sys/dev/uart/uart_dev_mvebu.c

472

Unfortunately, things are more complicated. Hardware provides such interrupt (let's say half-full), which is configurable, but this interrupt triggers when FIFO has more or equal bytes than configured. I've tested it, and it doesn't work properly, so I don't find it useful. Maybe do you have some hints how to use that interrupt, I would be grateful.

[mw]

• apply style improvements in while loops
• fix baudrate code in uart_mvebu_bus_ioctl
• replace ternary with if in uart_mvebu_bus_ioctl
• correct comment around interrupt generation in uart_mvebu_bus_transmit

[emaste]

head/sys/modules/uart/Makefile

33 ↗

(On Diff #32836)

new entry should be added in alpha order

[mw]

head/sys/modules/uart/Makefile

33 ↗

(On Diff #32836)

Ok, will change it, thanks.

[• ian]

sys/dev/uart/uart_dev_mvebu.c

472

Sorry for the delay in responding to this.

I would expect STAT_RX_FIFO_HALF to be asserted when there are >= 32 bytes in the rx, and STAT_TX_FIFO_HALF to be asserted when there are <= 32 bytes in the tx. Are you saying TX behaves just like RX and asserts when there is less than half?

[pdk_semihalf.com]

sys/dev/uart/uart_dev_mvebu.c

472

TX behaves like RX and asserts when there is more (not less) than half. Tested empirically. Good candidate was STAT_TX_FIFO_EMPT, but there is no interrupt connected with this.
STAT_TX_RDY is almost always asserted (UART_TSH clean).
STAT_TX_EMPT (TSH and UART shift register clean) is never asserted and I spent 3 days figuring out why. It seems to be a bug in hardware.