Introduce UART driver module for Armada 3700
ClosedPublic

Authored by mw on Wed, Sep 6, 7:20 PM.

Details

Summary

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

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mw created this revision.Wed, Sep 6, 7:20 PM
ian added inline comments.Wed, Sep 6, 7:56 PM
sys/dev/uart/uart_dev_mvebu.c
120 ↗(On Diff #32729)

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

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

472 ↗(On Diff #32729)

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 added a subscriber: andrew.Wed, Sep 6, 10:33 PM
andrew added inline comments.
sys/dev/uart/uart_dev_mvebu.c
410 ↗(On Diff #32729)

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

416–443 ↗(On Diff #32729)

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

541–544 ↗(On Diff #32729)

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

sys/dev/uart/uart_dev_mvebu.c
410 ↗(On Diff #32729)

OK

416–443 ↗(On Diff #32729)

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

541–544 ↗(On Diff #32729)

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

sys/dev/uart/uart_dev_mvebu.c
472 ↗(On Diff #32729)

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 updated this revision to Diff 32767.Thu, Sep 7, 4:25 PM
  • 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
This revision was automatically updated to reflect the committed changes.
emaste added inline comments.Sat, Sep 9, 1:26 PM
head/sys/modules/uart/Makefile
33

new entry should be added in alpha order

mw added inline comments.Sat, Sep 9, 3:47 PM
head/sys/modules/uart/Makefile
33

Ok, will change it, thanks.

ian added inline comments.Sat, Sep 9, 4:04 PM
sys/dev/uart/uart_dev_mvebu.c
472 ↗(On Diff #32729)

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?

sys/dev/uart/uart_dev_mvebu.c
472 ↗(On Diff #32729)

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.