From 72a1534c77cde20908a60b04fb31ffb93d6b7494 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 15 Aug 2026 21:23:18 +0200 Subject: [PATCH 1/5] config: disable interrupts by default Experience has shown that enabling interrupts by default can lead to surprising behavior during initialization, unless initialization is performed with CPU interrupts disabled [0]. Let's use a safer and simpler default. [0] https://github.com/rust-osdev/bootloader/pull/565#discussion_r3430557307 --- CHANGELOG.md | 3 +++ src/config.rs | 5 ++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffad347..f11fcd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- **Breaking:** `Config::default()` now disables **all** interrupts by default + (was `IER::DATA_READY`). + ## 0.7.0 - 2026-08-15 - **Breaking:** Changed the return type of `Uart16550::config(&self)` from diff --git a/src/config.rs b/src/config.rs index b0bf0a7..79e9b99 100644 --- a/src/config.rs +++ b/src/config.rs @@ -166,13 +166,12 @@ impl Config { /// configuration. /// /// More precisely, the default configuration uses a [8-N-1] transmission - /// with a baud rate of [`BaudRate::Baud9600`]. It also activates the FIFO - /// and the [`IER::DATA_READY`] interrupt. + /// with a baud rate of [`BaudRate::Baud9600`]. It also activates the FIFO. /// /// [8-N-1]: https://en.wikipedia.org/wiki/Serial_port#Conventional_notation pub const DEFAULT: Self = Self { // Properties and behavior of the UART - interrupts: IER::DATA_READY, + interrupts: IER::empty(), frequency: CLK_FREQUENCY_HZ, prescaler_division_factor: None, fifo_trigger_level: Some(FifoTriggerLevel::Fourteen), From 82ad27ab53c1b8e88fad9ca5acf2d427251a254e Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 16 Aug 2026 12:09:45 +0200 Subject: [PATCH 2/5] doc: document more side effects --- src/lib.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index e30a207..10ea330 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -826,6 +826,12 @@ impl Uart16550 { } /// Fetches the current value from the [`ISR`]. + /// + /// # Side Effects + /// + /// Reading the [`ISR`] clears a pending transmitter holding register empty + /// interrupt if it is the interrupt currently indicated by the [`ISR`]. + /// This does not clear the corresponding [`LSR::THR_EMPTY`] status flag. pub fn isr(&mut self) -> ISR { // SAFETY: We operate on valid register addresses. let val = unsafe { self.backend.read(offsets::ISR as u8) }; @@ -847,6 +853,12 @@ impl Uart16550 { } /// Fetches the current value from the [`LSR`]. + /// + /// # Side Effects + /// + /// Reading the [`LSR`] clears a pending receiver line status interrupt. + /// It also clears the [`LSR::OVERRUN_ERROR`], [`LSR::PARITY_ERROR`], + /// [`LSR::FRAMING_ERROR`], and [`LSR::BREAK_INTERRUPT`] status flags. pub fn lsr(&mut self) -> LSR { // SAFETY: We operate on valid register addresses. let val = unsafe { self.backend.read(offsets::LSR as u8) }; @@ -854,6 +866,11 @@ impl Uart16550 { } /// Fetches the current value from the [`MSR`]. + /// + /// # Side Effects + /// + /// Reading the [`MSR`] clears a pending modem status interrupt and clears + /// the modem status change indicators in bits 0 through 3. pub fn msr(&mut self) -> MSR { // SAFETY: We operate on valid register addresses. let val = unsafe { self.backend.read(offsets::MSR as u8) }; From e73f64c334a31d7f0ce9da1d4c27ec1cca45545e Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 16 Aug 2026 12:13:03 +0200 Subject: [PATCH 3/5] uart: enable interrupts last in Uart16550::init() - prevent that the function is raising interrupts - document interrupt behavior and recommendations --- CHANGELOG.md | 2 ++ src/lib.rs | 43 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f11fcd2..3a75042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ - **Breaking:** `Config::default()` now disables **all** interrupts by default (was `IER::DATA_READY`). +- Made `Uart16550::init()` enable configured interrupts only at the end. Also + documented when interrupts can still become pending. ## 0.7.0 - 2026-08-15 diff --git a/src/lib.rs b/src/lib.rs index 10ea330..dd36950 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -397,14 +397,30 @@ impl Uart16550 { /// the device works. Further, a call to [`Self::check_connected`] helps to /// detect if a remote is connected. /// + /// # Interrupts + /// + /// As one of its first steps, this function disables all device interrupts. + /// The interrupts selected in the [`Config`] are enabled again only at the + /// very end of the initialization sequence. + /// + /// An interrupt may still become pending after the configured interrupts + /// have been enabled but before this function returns, for example because + /// data arrives asynchronously. + /// + /// [`IER::THR_EMPTY`] is a special case. If enabled, it will immediately + /// trigger an interrupt because the transmitter is guaranteed to be empty + /// when the configured interrupts are enabled at the end of initialization. + /// + /// **Recommendation:** Therefore, when using the device in an + /// interrupt-driven setup, it is safest to call this function from an + /// interrupt-free section. + /// /// # Caution /// /// Callers must ensure that using this type with the underlying hardware /// is done only in a context where such operations are valid and safe /// (e.g., you have exclusive device access). /// - /// It is recommended to disable interrupts before calling this function. - /// /// Further, the serial config must match the expectations of the receiver /// on the other side. Otherwise, garbage will be received. pub fn init(&mut self, config: Config) -> Result<(), InitError> { @@ -496,13 +512,6 @@ impl Uart16550 { self.backend.write(offsets::MCR as u8, mcr.bits()); } - // Set interrupts. - // SAFETY: We operate on valid register addresses. - unsafe { - self.backend - .write(offsets::IER as u8, self.config.interrupts.bits()); - } - // In case there is anything in THR, THR's FIFO or TSR (for // example because the device was already initialized by another // driver), we wait for the data to be drained. This way, we can ensure @@ -523,6 +532,22 @@ impl Uart16550 { hint::spin_loop() } } + + // Bring status bits into a clean state. + { + // Clear receiver line-status deltas / interrupt indicators. + let _ = self.lsr(); + + // Clear modem-status deltas. + let _ = self.msr(); + } + + // Set interrupts as the last step. + // SAFETY: We operate on valid register addresses. + unsafe { + self.backend + .write(offsets::IER as u8, self.config.interrupts.bits()); + } Ok(()) } From 37b3f3ebccc57a304c0a634e31e5ce3f17b51af9 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 16 Aug 2026 12:13:48 +0200 Subject: [PATCH 4/5] uart: make init() more robust against invalid device state In case a previous (incomplete) init sequence took place, this helps us to cleanly init the device. --- src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index dd36950..c91167f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -448,6 +448,12 @@ impl Uart16550 { check_fn(0x73)?; } + // Clear DLAB. + // SAFETY: We operate on valid register addresses. + unsafe { + self.backend.write(offsets::LCR as u8, 0); + } + // Disable all interrupts (for now). // SAFETY: We operate on valid register addresses. unsafe { From 6b314f3b143937e45454eaa1a605771972130cc8 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sun, 16 Aug 2026 10:55:39 +0200 Subject: [PATCH 5/5] chore: v0.8.0 --- CHANGELOG.md | 2 ++ Cargo.lock | 2 +- Cargo.toml | 2 +- test/Cargo.lock | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a75042..30a8bae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +## 0.8.0 - 2026-08-16 + - **Breaking:** `Config::default()` now disables **all** interrupts by default (was `IER::DATA_READY`). - Made `Uart16550::init()` enable configured interrupts only at the end. Also diff --git a/Cargo.lock b/Cargo.lock index e81111a..02762e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -136,7 +136,7 @@ dependencies = [ [[package]] name = "uart_16550" -version = "0.7.0" +version = "0.8.0" dependencies = [ "assert2", "bitflags", diff --git a/Cargo.toml b/Cargo.toml index 3f639e7..b6f0d83 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "uart_16550" -version = "0.7.0" +version = "0.8.0" description = """ Simple yet highly configurable low-level driver for 16550 UART devices, typically known and used as serial ports or COM ports. Easy integration into diff --git a/test/Cargo.lock b/test/Cargo.lock index 3ab4a5e..e5a6967 100644 --- a/test/Cargo.lock +++ b/test/Cargo.lock @@ -38,7 +38,7 @@ checksum = "8189cbf0422ac15d7d544ed5f331fbe4e5b9da88133568fd359083b186a547f3" [[package]] name = "uart_16550" -version = "0.7.0" +version = "0.8.0" dependencies = [ "bitflags", ]