Skip to content

Small fixes found by testing against a 16550A model - #84

Merged
phip1611 merged 8 commits into
rust-osdev:mainfrom
phip1611:bugfixes
Oct 10, 2026
Merged

phip1611 merged 8 commits into
rust-osdev:mainfrom
phip1611:bugfixes

Conversation

@phip1611

@phip1611 phip1611 commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Small, non-breaking fixes for bugs found by testing the driver against a 16550A model that follows the Linux 8250 driver (#83):

  • init() no longer leaves DLAB set on an invalid baud rate, and discards stale data also without FIFO.
  • test_loopback() leaves loopback mode also when it fails.
  • embedded_io::Write::flush() waits for the transmitter.
  • calc_baud_rate() no longer overflows; check_present() restores the scratch register; a doc fix.

More fixes follow in separate PRs.

🤖 Generated with Claude Code

The constructors initialize the device fields in several places. Route
them through new_with_backend(), so new fields only need to be set in
one place.
An invalid baud rate made init() return with DLAB set. Offsets 0 and 1
then address the divisor latch instead of the data registers and IER,
so the device is unusable until the next successful init().
The denominator 16 * (psd + 1) * divisor overflows u32 for large inputs,
e.g., when calc_divisor() gets a very high baud rate. Calculate in u64.
Without FIFO, the FCR reset bits have no effect, so a byte received
before init() stayed in RBR and was returned as new data. Read it out
right after configuring the FIFOs, before signaling readiness via DTR
and RTS, and also read IIR at the end, as Linux does.
On errors, test_loopback() returned early and left the device in
loopback mode, which disconnects it from the outside. Restore MCR on all
paths.
flush() returned immediately, although up to a FIFO of data was still
being sent. The embedded-io contract requires that all buffered data
reaches its destination, e.g., before a reset or a baud rate change.
The presence check overwrote the scratch register, which firmware or
other software may use. Linux restores it in its scratch test, too.
It described the meaning of ISR bit 0 inverted.
@phip1611 phip1611 self-assigned this Oct 10, 2026
@phip1611 phip1611 changed the title Fix bugs found by testing against a 16550A model Small fixes found by testing against a 16550A model Oct 10, 2026
Comment thread src/embedded_io.rs
/// This blocks as long as the transmitter is stalled, e.g., by hardware
/// flow control.
fn flush(&mut self) -> Result<(), Self::Error> {
// Wait until the transmitter sent everything, like Linux' tcdrain().

@phip1611 phip1611 Oct 10, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @mkroening - there is an upcoming bugfix (relevant on real hardware)

@phip1611
phip1611 marked this pull request as ready for review October 10, 2026 18:07
@phip1611
phip1611 added this pull request to the merge queue Oct 10, 2026
Merged via the queue into rust-osdev:main with commit a69e61d Oct 10, 2026
18 checks passed
@phip1611
phip1611 deleted the bugfixes branch October 10, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant