drivers,uart: small improvements and cleanup #258
Conversation
wipawel
added
improvement
This is merely a cleanup. No functional change. Signed-off-by: Pawel Wieczorkiewicz <[email protected]>
It might be used for interrupts on some devices. Signed-off-by: Pawel Wieczorkiewicz <[email protected]>
Set interrupt level to 1 byte. Signed-off-by: Pawel Wieczorkiewicz <[email protected]>
Enable interrupts for RX data and MSR changes. Signed-off-by: Pawel Wieczorkiewicz <[email protected]>
Implement a corresponding iir_t type for the IIR register. Perform RX data reads upon RX data available event and character timeout event (triggered by FIFO). Signed-off-by: Pawel Wieczorkiewicz <[email protected]>
When no com_ports have been detected, set a default one according to the com1 boot-time configuration. Ignore unset com_ports in uart_interrupt_handler(). Signed-off-by: Pawel Wieczorkiewicz <[email protected]>
| #define UART_IIR_EVENT_CHAR_TIMEOUT 0x6 | ||
|
|
||
| #define UART_IIR_FIFO_NO_FIFO 0x0 | ||
| #define UART_IIR_FIFO_UNUSABLE_FIFO 0x1 |
There was a problem hiding this comment.
I'm reading this: https://en.wikibooks.org/wiki/Serial_Programming/8250_UART_Programming#Interrupt_Identification_Register
Shouldn't this be 0x2?
| Bit 7 | Bit 6 | Notes |
|---|---|---|
| 0 | 1 | Reserved condition |
| 1 | 0 | FIFO enabled, but not functioning |
There was a problem hiding this comment.
Hmm other sources seem to disagree:
http://www.larvierinehart.com/serial/serialadc/serial.htm#16
There was a problem hiding this comment.
Have you tested this or do we just trust either?
There was a problem hiding this comment.
Yes, I tested my variant on many different devices and it works.
| struct __packed { | ||
| uint8_t no_int_pend : 1, event : 3, rsvd : 1, fifo_64b : 1, fifo_status : 2; | ||
| }; |
There was a problem hiding this comment.
The order of bit field members in C is implementation defined and for gcc it's defined by the ABI.
I'm fine with that, but we probably want a block like this?:
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
// order as above
#else
// reverse order
#endif
Not that it matters now, but in case someone wants to add Aarch64 support or the like.
There was a problem hiding this comment.
Right, this is a very good point and IMO deserves a dedicated issue not to forget about it. We use similar bit-fields (where order would matter) in several other places too.
Improvements:
init_uart()