Page MenuHomeFreeBSD

Introduce UART driver module for Armada 3700
ClosedPublic

Authored by mw on Sep 6 2017, 7:20 PM.
Tags
Referenced Files
F82196296: D12250.id.diff
Fri, Apr 26, 9:02 AM
F82196217: D12250.id32836.diff
Fri, Apr 26, 9:02 AM
F82196154: D12250.id32729.diff
Fri, Apr 26, 9:01 AM
F82161448: D12250.diff
Fri, Apr 26, 1:59 AM
Unknown Object (File)
Wed, Apr 24, 6:49 AM
Unknown Object (File)
Sat, Apr 20, 6:17 PM
Unknown Object (File)
Dec 31 2023, 2:36 AM
Unknown Object (File)
Dec 13 2023, 6:28 AM

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 - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/dev/uart/uart_dev_mvebu.c
121

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

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

473

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 inline comments.
sys/dev/uart/uart_dev_mvebu.c
411

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

417–444

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

542–545

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

sys/dev/uart/uart_dev_mvebu.c
411

OK

417–444

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

542–545

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
473

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.

  • 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.
head/sys/modules/uart/Makefile
33 ↗(On Diff #32836)

new entry should be added in alpha order

head/sys/modules/uart/Makefile
33 ↗(On Diff #32836)

Ok, will change it, thanks.

sys/dev/uart/uart_dev_mvebu.c
473

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
473

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.