Claude review: rust: add basic serial device bus abstractions

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Overall Series Review

Subject: rust: add basic serial device bus abstractions
Author: Markus Probst <markus.probst@posteo.de>
Patches: 11
Reviewed: 2026-03-09T08:21:39.224101

---

This is a v2 series adding Rust abstractions for the serial device bus (serdev) subsystem, comprising a devres change, a C-side struct addition, the main Rust abstraction, and a sample driver. The overall structure and intent is good -- having a Rust serdev abstraction follows the same pattern as other bus drivers (platform, USB, auxiliary). However, there are several **critical soundness and correctness issues** that must be addressed before this can be merged, including a double-free, a use-after-null/UB race condition, and a broken timeout value.

---

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Overall Series Review

Subject: rust: add basic serial device bus abstractions
Author: Markus Probst <markus.probst@posteo.de>
Patches: 15
Reviewed: 2026-03-16T12:17:34.917979

---

This is a v3 patch series adding Rust abstractions for the Linux serial device bus (serdev) subsystem, consisting of 4 patches: a devres API change, a C-side struct modification, the main Rust abstraction, and a sample driver.

The overall design follows established kernel Rust driver patterns (similar to platform, USB, auxiliary bus abstractions) and the code is well-structured. However, there are several concerns, ranging from a **critical safety issue** in the `devres::register` lifetime change, to race condition concerns in the probe/receive synchronization, and a few minor API design questions.

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Patch Review

This is the main patch. Several observations:

**1. Timeout::Max maps to 0, which is wrong:**

```rust
+impl Timeout {
+    fn into_jiffies(self) -> isize {
+        match self {
+            ...
+            Self::Max => 0,
+        }
+    }
+}
```

`MAX_SCHEDULE_TIMEOUT` is `LONG_MAX` (i.e., `c_long::MAX`), not 0. A timeout of 0 means "don't wait at all" in `serdev_device_write()`. The `Timeout::Max` variant's doc says "Wait as long as possible" and references `MAX_SCHEDULE_TIMEOUT`, but then passes 0, which is the exact opposite. This should be:

```rust
Self::Max => isize::MAX, // or kernel::task::MAX_SCHEDULE_TIMEOUT
```

**2. Race condition in probe/receive synchronization:**

The `PrivateData` struct uses a `Completion` to synchronize `receive_buf_callback` with probe completion:

```rust
+        private_data.probe_complete.wait_for_completion();
+
+        // SAFETY: No one has exclusive access to `private_data.error`.
+        if unsafe { *private_data.error.get() } {
+            return length;
+        }
```

The `error` field uses `UnsafeCell<bool>` without any synchronization primitive. The SAFETY comment "No one has exclusive access to `private_data.error`" doesn't actually justify the safety — it's the *opposite* of what you'd want. The concern is whether the write to `error` in probe is guaranteed to be visible in `receive_buf_callback` on another thread. The `Completion` itself should provide a memory barrier (via `complete_all`/`wait_for_completion`), so the ordering is likely correct in practice, but this should be documented explicitly. Using an `AtomicBool` would be cleaner and avoid the need for `unsafe` altogether.

**3. `set_baudrate` returns `Result<(), u32>` instead of idiomatic kernel `Result`:**

```rust
+    pub fn set_baudrate(&self, speed: u32) -> Result<(), u32> {
```

Returning the actual baudrate set on failure is useful information, but the `Result<(), u32>` type is non-standard for kernel Rust code. The C function `serdev_device_set_baudrate` returns the actual speed set (which may differ from requested). The sample driver checks `is_err()` and returns `EINVAL`, losing the actual value. Consider either returning `Result<u32>` (the actual speed set) or `Result` with a standard error code.

**4. `write` returns `Result<u32>` but `write_all` returns `Result<usize>` — inconsistent types:**

```rust
+    pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
...
+    pub fn write(&self, data: &[u8]) -> Result<u32> {
```

Both should consistently use `usize` for byte counts.

**5. The `write` function wraps `serdev_device_write_buf` which returns `ssize_t`:**

```rust
+        let ret =
+            unsafe { bindings::serdev_device_write_buf(self.as_raw(), data.as_ptr(), data.len()) };
+        to_result(ret as i32).map(|()| ret.unsigned_abs())
```

Casting `ssize_t` (which is `isize`/`i64` on 64-bit) to `i32` could truncate positive values. Since this returns the number of bytes written, if `data.len()` > `i32::MAX`, the cast would produce an incorrect error. The same issue exists in `write_all`. Use `ret as i32` carefully — `to_result` expects a C `int`, but the value range should be OK in practice since write buffers won't exceed `i32::MAX`.

**6. `set_baudrate`, `set_flow_control`, `set_parity`, `write`, `write_all`, `write_flush`, `wait_until_sent` are all on `Device<Bound>`, but probe receives `Device<Core>`:**

Looking at the sample driver, `sdev.set_baudrate(...)` is called in `probe` which receives `&Device<Core>`. This compiles because of `impl_device_context_deref` which lets `Device<Core>` deref to `Device<Bound>`. This is the standard pattern, so it's fine.

**7. Typo in doc comments:**

```rust
+    /// Even partiy.
```

Should be "parity".

```rust
+    /// [ Device::wait_until_sent`]
```

Missing `[` — should be `[`Device::wait_until_sent`]`. This appears twice.

**8. The `Kconfig` entry `RUST_SERIAL_DEV_BUS_ABSTRACTIONS` should probably be inside the `if SERIAL_DEV_BUS` block:**

```diff
+config RUST_SERIAL_DEV_BUS_ABSTRACTIONS
+	bool "Rust Serial device bus abstractions"
+	depends on RUST
+	select SERIAL_DEV_BUS
```

It uses `select SERIAL_DEV_BUS` rather than `depends on`, which means enabling the Rust abstractions will force-enable `SERIAL_DEV_BUS`. This is placed *before* `if SERIAL_DEV_BUS`, which is a bit odd. Consider using `depends on SERIAL_DEV_BUS` instead.

**9. `remove_callback` doesn't call `drvdata_obtain` to free the driver data:**

```rust
+    extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
+        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
+        let data = unsafe { sdev.as_ref().drvdata_borrow::<T>() };
+        T::unbind(sdev, data);
+    }
```

It borrows but never takes ownership of the drvdata. The data appears to be freed by devres when the device is released, since `set_drvdata` stores it via `dev_set_drvdata`. But wait — `set_drvdata` stores data via `into_foreign()` and there's no corresponding `from_foreign()` call. The data should be cleaned up by the devres system if `set_drvdata` registers a devres action, but looking at the `device.rs` code, `set_drvdata` does *not* register a devres — it just calls `dev_set_drvdata`. This looks like a **memory leak** — the `Pin<KBox<T>>` stored via `set_drvdata` is never freed. Other bus drivers (platform, etc.) likely call `drvdata_obtain` in their remove callbacks. This needs to be verified.

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Overall Series Review

Subject: rust: add basic serial device bus abstractions
Author: Markus Probst via B4 Relay <devnull+markus.probst.posteo.de@kernel.org>
Patches: 5
Reviewed: 2026-04-12T08:55:15.809279

---

This is a well-structured 4-patch series adding Rust abstractions for the serial device bus (serdev) subsystem. The series is authored by Markus Probst and is at v4. The overall design follows established Rust kernel driver patterns (matching platform/PCI/USB driver abstractions), with proper device context modeling (`Core`/`Bound`/`Normal`), devres-managed resources, and a vtable-based `Driver` trait.

The probe/receive synchronization via a `Completion` + `UnsafeCell<bool>` stored in devres-managed `PrivateData` is subtle but appears correct -- the completion provides the necessary memory ordering guarantees. The devres LIFO cleanup order ensures the device is closed before PrivateData is freed.

The main concerns are: (1) a semantic bug in `Timeout::into_jiffies` overflow handling where an overflow silently becomes an infinite wait, (2) the `devres::register` lifetime change in patch 1 is sound but the safety argument could be stronger, (3) adding a Rust-specific field to the C `struct serdev_device` is somewhat invasive and should be discussed with the serdev maintainer, and (4) several documentation issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Patch Review

This is the main patch. Several issues found:

**Bug -- `Timeout::into_jiffies` overflow handling**:

```rust
+    fn into_jiffies(self) -> isize {
+        match self {
+            Self::Jiffies(value) => value.get().try_into().unwrap_or_default(),
+            Self::Milliseconds(value) => {
+                msecs_to_jiffies(value.get()).try_into().unwrap_or_default()
+            }
+            Self::Max => 0,
+        }
+    }
```

`unwrap_or_default()` produces `0isize` on overflow, which in the C code maps to `MAX_SCHEDULE_TIMEOUT` (infinite wait). A timeout that's too large to represent silently becomes an infinite wait rather than the longest representable finite timeout. This should use `unwrap_or(isize::MAX)` or saturating conversion instead.

**Typo in doc comment**:

```rust
+    /// Even partiy.
+    Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
```

"partiy" should be "parity".

**Broken intra-doc links** (two instances):

```rust
+    /// Note that any accepted data has only been buffered by the controller. Use
+    /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
```

The space after `[` breaks the rustdoc link. Should be `` [`Device::wait_until_sent`] `` (no space after `[`). This appears on both `write_all` and `write` methods.

**`write_all` cast concern**:

```rust
+        let ret = unsafe {
+            bindings::serdev_device_write(
+                self.as_raw(),
+                data.as_ptr(),
+                data.len(),
+                timeout.into_jiffies(),
+            )
+        };
+        // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
+        // which always fit into a `i32`.
+        to_result(ret as i32).map(|()| ret.unsigned_abs())
```

The CAST comment only addresses negative values, but `ret` is `ssize_t` (i64 on 64-bit). If the positive return value (bytes written) exceeds `i32::MAX` (~2GB), the `as i32` cast would produce a negative value, causing `to_result` to return a spurious error. For serial devices, writes of >2GB are effectively impossible, but the comment should acknowledge this assumption or the code should use a safe conversion. A more robust approach would be to check sign on the original `isize` value before casting.

**Probe/receive synchronization -- `UnsafeCell<bool>`**:

```rust
+#[pin_data]
+struct PrivateData {
+    #[pin]
+    probe_complete: Completion,
+    error: UnsafeCell<bool>,
+}
```

The use of `UnsafeCell<bool>` for `error` relies on the `Completion` providing memory ordering (write barrier from `complete_all`, read barrier from `wait_for_completion`). This is correct but worth a comment explaining the synchronization invariant, since `UnsafeCell` without an atomic or lock is unusual and will attract questions from reviewers.

**`receive_buf_callback` on probe error before `complete_all`**:

The callback path through the `from_result` closure is carefully ordered: `devres::register` -> `set_client_ops` -> `devm_serdev_device_open` -> `T::probe` -> `set error` -> `complete_all`. If any step before `set_client_ops` fails, no callback can fire. If `devm_serdev_device_open` fails, the device isn't open so no receives. If `T::probe` fails (via `set_drvdata`), `error` is set and `complete_all` is called before returning the error. The devres LIFO order then closes the device before freeing PrivateData. This analysis shows the design is correct but should be documented in a comment block for future maintainers.

**`set_baudrate` return type**:

```rust
+    pub fn set_baudrate(&self, speed: u32) -> Result<(), u32> {
```

Returning `Result<(), u32>` where `Err(actual_speed)` means the requested speed wasn't matched exactly is a usable API, but it's worth noting that many callers in the C world accept approximate baudrates (e.g., requesting 115200 and getting 115384). The current API forces exact-match semantics. Consider documenting this or providing a method that returns the actual rate.

**Methods on `Device<Bound>` only**: All device operation methods (`set_baudrate`, `set_flow_control`, `set_parity`, `write_all`, `write`, `write_flush`, `wait_until_sent`) are on `impl Device<device::Bound>`. This works from probe via `Core` -> `Bound` deref coercion, which is the correct kernel Rust pattern.

---
Generated by Claude Code Patch Reviewer

[PATCH v5 0/4] rust: add basic serial device bus abstractions

[Markus Probst]

This patch series adds the serdev device bus rust abstraction into the
kernel.

This abstraction will be used by a driver,
which targets the MCU devices in Synology devices.

Kari Argillander also messaged me, stating that he wants to write a
watchdog driver with this abstraction (needing initial device data).

@Rob: Are you willing to maintain these rust abstractions yourself,
as you are the expert on this subsystem, otherwise I would take care of
it with a "SERIAL DEVICE BUS [RUST]" section in the MAINTAINERS file. In
the second case, I assume you are going to pick those patches as-is into
your tree, after they have been reviewed?

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v5:
- fix typo in documentation
- Link to v4: https://lore.kernel.org/r/20260411-rust_serdev-v4-0-845e960c6627@posteo.de

Changes in v4:
- fixed not selecting rust serdev abstraction in sample
- Link to v3: https://lore.kernel.org/r/20260313-rust_serdev-v3-0-c9a3af214f7f@posteo.de

Changes in v3:
- fix vertical import style
- add Kconfig entry for the rust abstraction
- fix documentation in include/linux/serdev.h
- rename private_data to rust_private_data
- fix `complete_all` <-> `wait_for_completion` typo
- move drvdata_borrow call after the completion
- Link to v2: https://lore.kernel.org/r/20260306-rust_serdev-v2-0-e9b23b42b255@posteo.de

Changes in v2:
- fix documentation in `serdev::Driver::write` and
  `serdev::Driver::write_all`
- remove use of `dev_info` in probe from the sample
- remove `properties_parse` from the sample
- add optional `baudrate` property to the sample
- remove 1. patch
- remove `TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>`
  implementation
- fix import style
- add patch to return reference in `devres::register` to fix safety
  issue
- add patch to add private data to serdev_device, to fix
  `Device.drvdata()` from failing
- simplify abstraction by removing ability to receive the initial
  transmission. It may be added later in a separate patch series if
  needed.
- Link to v1: https://lore.kernel.org/r/20251220-rust_serdev-v1-0-e44645767621@posteo.de

---
Markus Probst (4):
      rust: devres: return reference in `devres::register`
      serdev: add rust private data to serdev_device
      rust: add basic serial device bus abstractions
      samples: rust: add Rust serial device bus sample device driver

 drivers/tty/serdev/Kconfig         |   7 +
 include/linux/serdev.h             |  15 +-
 rust/bindings/bindings_helper.h    |   1 +
 rust/helpers/helpers.c             |   1 +
 rust/helpers/serdev.c              |  22 ++
 rust/kernel/cpufreq.rs             |   3 +-
 rust/kernel/devres.rs              |  15 +-
 rust/kernel/drm/driver.rs          |   3 +-
 rust/kernel/lib.rs                 |   2 +
 rust/kernel/serdev.rs              | 536 +++++++++++++++++++++++++++++++++++++
 samples/rust/Kconfig               |  11 +
 samples/rust/Makefile              |   1 +
 samples/rust/rust_driver_serdev.rs |  86 ++++++
 13 files changed, 693 insertions(+), 10 deletions(-)
---
base-commit: 3131ff5a117498bb4b9db3a238bb311cbf8383ce
change-id: 20251217-rust_serdev-ee5481e9085c

[PATCH v5 1/4] rust: devres: return reference in `devres::register`

[Markus Probst]

Return the reference to the initialized data in the `devres::register`
function.

This is needed in a following commit (rust: add basic serial device bus
abstractions).

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/cpufreq.rs    |  3 ++-
 rust/kernel/devres.rs     | 15 +++++++++++++--
 rust/kernel/drm/driver.rs |  3 ++-
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index f5adee48d40c..31bf7e685097 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -1052,7 +1052,8 @@ pub fn new_foreign_owned(dev: &Device<Bound>) -> Result
     where
         T: 'static,
     {
-        devres::register(dev, Self::new()?, GFP_KERNEL)
+        devres::register(dev, Self::new()?, GFP_KERNEL)?;
+        Ok(())
     }
 }
 
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 6afe196be42c..f882bace8601 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -326,15 +326,26 @@ fn register_foreign<P>(dev: &Device<Bound>, data: P) -> Result
 /// }
 ///
 /// fn from_bound_context(dev: &Device<Bound>) -> Result {
-///     devres::register(dev, Registration::new(), GFP_KERNEL)
+///     devres::register(dev, Registration::new(), GFP_KERNEL)?;
+///     Ok(())
 /// }
 /// ```
-pub fn register<T, E>(dev: &Device<Bound>, data: impl PinInit<T, E>, flags: Flags) -> Result
+pub fn register<'a, T, E>(
+    dev: &'a Device<Bound>,
+    data: impl PinInit<T, E>,
+    flags: Flags,
+) -> Result<&'a T>
 where
     T: Send + 'static,
     Error: From<E>,
 {
     let data = KBox::pin_init(data, flags)?;
 
+    let data_ptr = &raw const *data;
+
     register_foreign(dev, data)
+        // SAFETY: `dev` is valid for the lifetime of 'a. As long as there is a reference to
+        // `Device<Bound>`, it is guaranteed that the device is not unbound and data has not been
+        // dropped. Thus `data_ptr` is also valid for the lifetime of 'a.
+        .map(|()| unsafe { &*data_ptr })
 }
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index e09f977b5b51..51e0c7e30cc2 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -145,7 +145,8 @@ pub fn new_foreign_owned(
 
         let reg = Registration::<T>::new(drm, flags)?;
 
-        devres::register(dev, reg, GFP_KERNEL)
+        devres::register(dev, reg, GFP_KERNEL)?;
+        Ok(())
     }
 
     /// Returns a reference to the `Device` instance for this registration.

-- 
2.52.0

[PATCH v5 2/4] serdev: add rust private data to serdev_device

[Markus Probst]

Add rust private data to `struct serdev_device`, as it is required by the
rust abstraction added in the following commit
(rust: add basic serial device bus abstractions).

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 include/linux/serdev.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 5654c58eb73c..c74c345d60ae 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -33,12 +33,14 @@ struct serdev_device_ops {
 
 /**
  * struct serdev_device - Basic representation of an serdev device
- * @dev:	Driver model representation of the device.
- * @nr:		Device number on serdev bus.
- * @ctrl:	serdev controller managing this device.
- * @ops:	Device operations.
- * @write_comp	Completion used by serdev_device_write() internally
- * @write_lock	Lock to serialize access when writing data
+ * @dev:	       Driver model representation of the device.
+ * @nr:		       Device number on serdev bus.
+ * @ctrl:	       serdev controller managing this device.
+ * @ops:	       Device operations.
+ * @write_comp:	       Completion used by serdev_device_write() internally
+ * @write_lock:	       Lock to serialize access when writing data
+ * @rust_private_data: Private data for the rust abstraction. This should
+ *		       not be used by the C drivers.
  */
 struct serdev_device {
 	struct device dev;
@@ -47,6 +49,7 @@ struct serdev_device {
 	const struct serdev_device_ops *ops;
 	struct completion write_comp;
 	struct mutex write_lock;
+	void *rust_private_data;
 };
 
 static inline struct serdev_device *to_serdev_device(struct device *d)

-- 
2.52.0

[PATCH v5 3/4] rust: add basic serial device bus abstractions

[Markus Probst]

Implement the basic serial device bus abstractions required to write a
serial device bus device driver with or without the need for initial device
data. This includes the following data structures:

The `serdev::Driver` trait represents the interface to the driver.

The `serdev::Device` abstraction represents a `struct serdev_device`.

In order to provide the Serdev specific parts to a generic
`driver::Registration` the `driver::RegistrationOps` trait is
implemented by `serdev::Adapter`.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/tty/serdev/Kconfig      |   7 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/serdev.c           |  22 ++
 rust/kernel/lib.rs              |   2 +
 rust/kernel/serdev.rs           | 536 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 569 insertions(+)

diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
index 46ae732bfc68..e6dfe949ad01 100644
--- a/drivers/tty/serdev/Kconfig
+++ b/drivers/tty/serdev/Kconfig
@@ -9,6 +9,13 @@ menuconfig SERIAL_DEV_BUS
 
 	  Note that you typically also want to enable TTY port controller support.
 
+config RUST_SERIAL_DEV_BUS_ABSTRACTIONS
+	bool "Rust Serial device bus abstractions"
+	depends on RUST
+	select SERIAL_DEV_BUS
+	help
+	  This enables the Rust abstraction for the serial device bus API.
+
 if SERIAL_DEV_BUS
 
 config SERIAL_DEV_CTRL_TTYPORT
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 083cc44aa952..ab521ba42673 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -80,6 +80,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/sched.h>
 #include <linux/security.h>
+#include <linux/serdev.h>
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
 #include <linux/task_work.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index a3c42e51f00a..9b87e9591cfd 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -53,6 +53,7 @@
 #include "regulator.c"
 #include "scatterlist.c"
 #include "security.c"
+#include "serdev.c"
 #include "signal.c"
 #include "slab.c"
 #include "spinlock.c"
diff --git a/rust/helpers/serdev.c b/rust/helpers/serdev.c
new file mode 100644
index 000000000000..c52b78ca3fc7
--- /dev/null
+++ b/rust/helpers/serdev.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/serdev.h>
+
+__rust_helper
+void rust_helper_serdev_device_driver_unregister(struct serdev_device_driver *sdrv)
+{
+	serdev_device_driver_unregister(sdrv);
+}
+
+__rust_helper
+void rust_helper_serdev_device_put(struct serdev_device *serdev)
+{
+	serdev_device_put(serdev);
+}
+
+__rust_helper
+void rust_helper_serdev_device_set_client_ops(struct serdev_device *serdev,
+					      const struct serdev_device_ops *ops)
+{
+	serdev_device_set_client_ops(serdev, ops);
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index d93292d47420..5107c9c1be07 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -144,6 +144,8 @@
 pub mod scatterlist;
 pub mod security;
 pub mod seq_file;
+#[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)]
+pub mod serdev;
 pub mod sizes;
 pub mod slice;
 #[cfg(CONFIG_SOC_BUS)]
diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
new file mode 100644
index 000000000000..f1464e6a3fd6
--- /dev/null
+++ b/rust/kernel/serdev.rs
@@ -0,0 +1,536 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the serial device bus.
+//!
+//! C header: [`include/linux/serdev.h`](srctree/include/linux/serdev.h)
+
+use crate::{
+    acpi,
+    device,
+    devres,
+    driver,
+    error::{
+        from_result,
+        to_result,
+        VTABLE_DEFAULT_ERROR, //
+    },
+    of,
+    prelude::*,
+    sync::Completion,
+    time::{
+        msecs_to_jiffies,
+        Jiffies,
+        Msecs, //
+    },
+    types::{
+        AlwaysRefCounted,
+        Opaque, //
+    }, //
+};
+
+use core::{
+    cell::UnsafeCell,
+    marker::PhantomData,
+    mem::offset_of,
+    num::NonZero,
+    ptr::NonNull, //
+};
+
+/// Parity bit to use with a serial device.
+#[repr(u32)]
+pub enum Parity {
+    /// No parity bit.
+    None = bindings::serdev_parity_SERDEV_PARITY_NONE,
+    /// Even partiy.
+    Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
+    /// Odd parity.
+    Odd = bindings::serdev_parity_SERDEV_PARITY_ODD,
+}
+
+/// Timeout in Jiffies.
+pub enum Timeout {
+    /// Wait for a specific amount of [`Jiffies`].
+    Jiffies(NonZero<Jiffies>),
+    /// Wait for a specific amount of [`Msecs`].
+    Milliseconds(NonZero<Msecs>),
+    /// Wait as long as possible.
+    ///
+    /// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
+    Max,
+}
+
+impl Timeout {
+    fn into_jiffies(self) -> isize {
+        match self {
+            Self::Jiffies(value) => value.get().try_into().unwrap_or_default(),
+            Self::Milliseconds(value) => {
+                msecs_to_jiffies(value.get()).try_into().unwrap_or_default()
+            }
+            Self::Max => 0,
+        }
+    }
+}
+
+/// An adapter for the registration of serial device bus device drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY:
+// - `bindings::serdev_device_driver` is a C type declared as `repr(C)`.
+// - `Drvdata<T>` is the type of the driver's device private data.
+// - `struct serdev_device_driver` embeds a `struct device_driver`.
+// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`.
+unsafe impl<T: Driver + 'static> driver::DriverLayout for Adapter<T> {
+    type DriverType = bindings::serdev_device_driver;
+    type DriverData = T;
+    const DEVICE_DRIVER_OFFSET: usize = core::mem::offset_of!(Self::DriverType, driver);
+}
+
+// SAFETY: A call to `unregister` for a given instance of `DriverType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    unsafe fn register(
+        sdrv: &Opaque<Self::DriverType>,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        let of_table = match T::OF_ID_TABLE {
+            Some(table) => table.as_ptr(),
+            None => core::ptr::null(),
+        };
+
+        let acpi_table = match T::ACPI_ID_TABLE {
+            Some(table) => table.as_ptr(),
+            None => core::ptr::null(),
+        };
+
+        // SAFETY: It's safe to set the fields of `struct serdev_device_driver` on initialization.
+        unsafe {
+            (*sdrv.get()).driver.name = name.as_char_ptr();
+            (*sdrv.get()).probe = Some(Self::probe_callback);
+            (*sdrv.get()).remove = Some(Self::remove_callback);
+            (*sdrv.get()).driver.of_match_table = of_table;
+            (*sdrv.get()).driver.acpi_match_table = acpi_table;
+        }
+
+        // SAFETY: `sdrv` is guaranteed to be a valid `DriverType`.
+        to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
+    }
+
+    unsafe fn unregister(sdrv: &Opaque<Self::DriverType>) {
+        // SAFETY: `sdrv` is guaranteed to be a valid `DriverType`.
+        unsafe { bindings::serdev_device_driver_unregister(sdrv.get()) };
+    }
+}
+
+#[pin_data]
+struct PrivateData {
+    #[pin]
+    probe_complete: Completion,
+    error: UnsafeCell<bool>,
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
+        receive_buf: if T::HAS_RECEIVE {
+            Some(Self::receive_buf_callback)
+        } else {
+            None
+        },
+        write_wakeup: Some(bindings::serdev_device_write_wakeup),
+    };
+
+    extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi::c_int {
+        // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer to
+        // a `struct serdev_device`.
+        //
+        // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
+        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
+        let id_info = <Self as driver::Adapter>::id_info(sdev.as_ref());
+
+        from_result(|| {
+            let private_data = devres::register(
+                sdev.as_ref(),
+                try_pin_init!(PrivateData {
+                    probe_complete <- Completion::new(),
+                    error: false.into(),
+                }),
+                GFP_KERNEL,
+            )?;
+
+            // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
+            unsafe {
+                (*sdev.as_raw()).rust_private_data =
+                    (&raw const *private_data).cast::<c_void>().cast_mut()
+            };
+
+            // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
+            unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::OPS) };
+
+            // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
+            // to a `serdev_device`.
+            to_result(unsafe {
+                bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
+            })?;
+
+            let data = T::probe(sdev, id_info);
+            let result = sdev.as_ref().set_drvdata(data);
+
+            // SAFETY: We have exclusive access to `private_data.error`.
+            unsafe { *private_data.error.get() = result.is_err() };
+
+            private_data.probe_complete.complete_all();
+
+            result.map(|()| 0)
+        })
+    }
+
+    extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
+        // SAFETY: The serial device bus only ever calls the remove callback with a valid pointer
+        // to a `struct serdev_device`.
+        //
+        // INVARIANT: `sdev` is valid for the duration of `remove_callback()`.
+        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
+
+        // SAFETY: `remove_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
+        // and stored a `Pin<KBox<T>>`.
+        let data = unsafe { sdev.as_ref().drvdata_borrow::<T>() };
+
+        T::unbind(sdev, data);
+    }
+
+    extern "C" fn receive_buf_callback(
+        sdev: *mut bindings::serdev_device,
+        buf: *const u8,
+        length: usize,
+    ) -> usize {
+        // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
+        // pointer to a `struct serdev_device`.
+        //
+        // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
+        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
+
+        // SAFETY:
+        // - The serial device bus only ever calls the receive buf callback with a valid pointer to
+        //   a `struct serdev_device`.
+        // - `receive_buf_callback` is only ever called after a successful call to
+        //   `probe_callback`, hence it's guaranteed that `sdev.private_data` is a pointer
+        //   to a valid `PrivateData`.
+        let private_data = unsafe { &*(*sdev.as_raw()).rust_private_data.cast::<PrivateData>() };
+
+        private_data.probe_complete.wait_for_completion();
+
+        // SAFETY: No one has exclusive access to `private_data.error`.
+        if unsafe { *private_data.error.get() } {
+            return length;
+        }
+
+        // SAFETY: `receive_buf_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
+        // and stored a `Pin<KBox<T>>`.
+        let data = unsafe { sdev.as_ref().drvdata_borrow::<T>() };
+
+        // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
+        let buf = unsafe { core::slice::from_raw_parts(buf, length) };
+
+        T::receive(sdev, data, buf)
+    }
+}
+
+impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
+    type IdInfo = T::IdInfo;
+
+    fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
+        T::OF_ID_TABLE
+    }
+
+    fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
+        T::ACPI_ID_TABLE
+    }
+}
+
+/// Declares a kernel module that exposes a single serial device bus device driver.
+///
+/// # Examples
+///
+/// ```ignore
+/// kernel::module_serdev_device_driver! {
+///     type: MyDriver,
+///     name: "Module name",
+///     authors: ["Author name"],
+///     description: "Description",
+///     license: "GPL v2",
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_serdev_device_driver {
+    ($($f:tt)*) => {
+        $crate::module_driver!(<T>, $crate::serdev::Adapter<T>, { $($f)* });
+    };
+}
+
+/// The serial device bus device driver trait.
+///
+/// Drivers must implement this trait in order to get a serial device bus device driver registered.
+///
+/// # Examples
+///
+///```
+/// # use kernel::{
+///     acpi,
+///     bindings,
+///     device::{
+///         Bound,
+///         Core, //
+///     },
+///     of,
+///     serdev, //
+/// };
+///
+/// struct MyDriver;
+///
+/// kernel::of_device_table!(
+///     OF_TABLE,
+///     MODULE_OF_TABLE,
+///     <MyDriver as serdev::Driver>::IdInfo,
+///     [
+///         (of::DeviceId::new(c"test,device"), ())
+///     ]
+/// );
+///
+/// kernel::acpi_device_table!(
+///     ACPI_TABLE,
+///     MODULE_ACPI_TABLE,
+///     <MyDriver as serdev::Driver>::IdInfo,
+///     [
+///         (acpi::DeviceId::new(c"LNUXBEEF"), ())
+///     ]
+/// );
+///
+/// #[vtable]
+/// impl serdev::Driver for MyDriver {
+///     type IdInfo = ();
+///     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+///     const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
+///
+///     fn probe(
+///         sdev: &serdev::Device<Core>,
+///         _id_info: Option<&Self::IdInfo>,
+///     ) -> impl PinInit<Self, Error> {
+///         sdev.set_baudrate(115200);
+///         sdev.write_all(b"Hello\n", serdev::Timeout::Max)?;
+///         Ok(MyDriver)
+///     }
+/// }
+///```
+#[vtable]
+pub trait Driver: Send {
+    /// The type holding driver private data about each device id supported by the driver.
+    // TODO: Use associated_type_defaults once stabilized:
+    //
+    // ```
+    // type IdInfo: 'static = ();
+    // ```
+    type IdInfo: 'static;
+
+    /// The table of OF device ids supported by the driver.
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+
+    /// The table of ACPI device ids supported by the driver.
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
+
+    /// Serial device bus device driver probe.
+    ///
+    /// Called when a new serial device bus device is added or discovered.
+    /// Implementers should attempt to initialize the device here.
+    fn probe(
+        sdev: &Device<device::Core>,
+        id_info: Option<&Self::IdInfo>,
+    ) -> impl PinInit<Self, Error>;
+
+    /// Serial device bus device driver unbind.
+    ///
+    /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
+    /// is optional.
+    ///
+    /// This callback serves as a place for drivers to perform teardown operations that require a
+    /// `&Device<Core>` or `&Device<Bound>` reference. For instance.
+    ///
+    /// Otherwise, release operations for driver resources should be performed in `Self::drop`.
+    fn unbind(sdev: &Device<device::Core>, this: Pin<&Self>) {
+        let _ = (sdev, this);
+    }
+
+    /// Serial device bus device data receive callback.
+    ///
+    /// Called when data got received from device.
+    ///
+    /// Returns the number of bytes accepted.
+    fn receive(sdev: &Device<device::Bound>, this: Pin<&Self>, data: &[u8]) -> usize {
+        let _ = (sdev, this, data);
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// The serial device bus device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct serdev_device`. The
+/// implementation abstracts the usage of an already existing C `struct serdev_device` within Rust
+/// code that we get passed from the C side.
+///
+/// # Invariants
+///
+/// A [`Device`] instance represents a valid `struct serdev_device` created by the C portion of
+/// the kernel.
+#[repr(transparent)]
+pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+    Opaque<bindings::serdev_device>,
+    PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> Device<Ctx> {
+    fn as_raw(&self) -> *mut bindings::serdev_device {
+        self.0.get()
+    }
+}
+
+impl Device<device::Bound> {
+    /// Set the baudrate in bits per second.
+    ///
+    /// Common baudrates are 115200, 9600, 19200, 57600, 4800.
+    ///
+    /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    pub fn set_baudrate(&self, speed: u32) -> Result<(), u32> {
+        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        let ret = unsafe { bindings::serdev_device_set_baudrate(self.as_raw(), speed) };
+        if ret == speed {
+            Ok(())
+        } else {
+            Err(ret)
+        }
+    }
+
+    /// Set if flow control should be enabled.
+    ///
+    /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    pub fn set_flow_control(&self, enable: bool) {
+        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        unsafe { bindings::serdev_device_set_flow_control(self.as_raw(), enable) };
+    }
+
+    /// Set parity to use.
+    ///
+    /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    pub fn set_parity(&self, parity: Parity) -> Result {
+        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        to_result(unsafe { bindings::serdev_device_set_parity(self.as_raw(), parity as u32) })
+    }
+
+    /// Write data to the serial device until the controller has accepted all the data or has
+    /// been interrupted by a timeout or signal.
+    ///
+    /// Note that any accepted data has only been buffered by the controller. Use
+    /// [`Device::wait_until_sent`] to make sure the controller write buffer has actually been
+    /// emptied.
+    ///
+    /// Returns the number of bytes written (less than `data.len()` if interrupted).
+    /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
+    /// before any bytes were written.
+    pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
+        // SAFETY:
+        // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
+        //   `data.len()`.
+        let ret = unsafe {
+            bindings::serdev_device_write(
+                self.as_raw(),
+                data.as_ptr(),
+                data.len(),
+                timeout.into_jiffies(),
+            )
+        };
+        // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
+        // which always fit into a `i32`.
+        to_result(ret as i32).map(|()| ret.unsigned_abs())
+    }
+
+    /// Write data to the serial device.
+    ///
+    /// If you want to write until the controller has accepted all the data, use
+    /// [`Device::write_all`].
+    ///
+    /// Note that any accepted data has only been buffered by the controller. Use
+    /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
+    /// emptied.
+    ///
+    /// Returns the number of bytes written (less than `data.len()` if not enough room in the
+    /// write buffer).
+    pub fn write(&self, data: &[u8]) -> Result<u32> {
+        // SAFETY:
+        // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
+        //   `data.len()`.
+        let ret =
+            unsafe { bindings::serdev_device_write_buf(self.as_raw(), data.as_ptr(), data.len()) };
+
+        to_result(ret as i32).map(|()| ret.unsigned_abs())
+    }
+
+    /// Send data to the serial device immediately.
+    ///
+    /// Note that this doesn't guarantee that the data has been transmitted.
+    /// Use [`Device::wait_until_sent`] for this purpose.
+    pub fn write_flush(&self) {
+        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        unsafe { bindings::serdev_device_write_flush(self.as_raw()) };
+    }
+
+    /// Wait for the data to be sent.
+    ///
+    /// After this function, the write buffer of the controller should be empty.
+    pub fn wait_until_sent(&self, timeout: Timeout) {
+        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout.into_jiffies()) };
+    }
+}
+
+// SAFETY: `serdev::Device` is a transparent wrapper of `struct serdev_device`.
+// The offset is guaranteed to point to a valid device field inside `serdev::Device`.
+unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx> {
+    const OFFSET: usize = offset_of!(bindings::serdev_device, dev);
+}
+
+// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { Device });
+kernel::impl_device_context_into_aref!(Device);
+
+// SAFETY: Instances of `Device` are always reference-counted.
+unsafe impl AlwaysRefCounted for Device {
+    fn inc_ref(&self) {
+        self.as_ref().inc_ref();
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+        unsafe { bindings::serdev_device_put(obj.cast().as_ptr()) }
+    }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+    fn as_ref(&self) -> &device::Device<Ctx> {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct serdev_device`.
+        let dev = unsafe { &raw mut (*self.as_raw()).dev };
+
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+// SAFETY: A `Device` is always reference-counted and can be released from any thread.
+unsafe impl Send for Device {}
+
+// SAFETY: `Device` can be shared among threads because all methods of `Device`
+// (i.e. `Device<Normal>) are thread safe.
+unsafe impl Sync for Device {}

-- 
2.52.0

[PATCH v5 4/4] samples: rust: add Rust serial device bus sample device driver

[Markus Probst]

Add a sample Rust serial device bus device driver illustrating the usage
of the serial device bus abstractions.

This drivers probes through either a match of device / driver name or a
match within the OF ID table.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 samples/rust/Kconfig               | 11 +++++
 samples/rust/Makefile              |  1 +
 samples/rust/rust_driver_serdev.rs | 86 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+)

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index c49ab9106345..31d62533ef25 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -161,6 +161,17 @@ config SAMPLE_RUST_DRIVER_AUXILIARY
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRIVER_SERDEV
+	tristate "Serial Device Bus Device Driver"
+	select RUST_SERIAL_DEV_BUS_ABSTRACTIONS
+	help
+	  This option builds the Rust serial device bus driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_driver_serdev.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_SOC
 	tristate "SoC Driver"
 	select SOC_BUS
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 6c0aaa58cccc..b986b681cde5 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_USB)		+= rust_driver_usb.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_AUXILIARY)	+= rust_driver_auxiliary.o
+obj-$(CONFIG_SAMPLE_RUST_DRIVER_SERDEV)		+= rust_driver_serdev.o
 obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
 obj-$(CONFIG_SAMPLE_RUST_SOC)			+= rust_soc.o
 
diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
new file mode 100644
index 000000000000..8cf3fb451b22
--- /dev/null
+++ b/samples/rust/rust_driver_serdev.rs
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust Serial device bus device driver sample.
+
+use kernel::{
+    acpi,
+    device::{
+        Bound,
+        Core, //
+    },
+    of,
+    prelude::*,
+    serdev,
+    sync::aref::ARef, //
+};
+
+struct SampleDriver {
+    sdev: ARef<serdev::Device>,
+}
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <SampleDriver as serdev::Driver>::IdInfo,
+    [(of::DeviceId::new(c"test,rust_driver_serdev"), ())]
+);
+
+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <SampleDriver as serdev::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c"LNUXBEEF"), ())]
+);
+
+#[vtable]
+impl serdev::Driver for SampleDriver {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
+
+    fn probe(
+        sdev: &serdev::Device<Core>,
+        _info: Option<&Self::IdInfo>,
+    ) -> impl PinInit<Self, Error> {
+        let dev = sdev.as_ref();
+
+        dev_dbg!(dev, "Probe Rust Serial device bus device driver sample.\n");
+
+        if sdev
+            .set_baudrate(
+                dev.fwnode()
+                    .and_then(|fwnode| fwnode.property_read(c"baudrate").optional())
+                    .unwrap_or(115200),
+            )
+            .is_err()
+        {
+            return Err(EINVAL);
+        }
+        sdev.set_flow_control(false);
+        sdev.set_parity(serdev::Parity::None)?;
+
+        Ok(Self { sdev: sdev.into() })
+    }
+
+    fn receive(sdev: &serdev::Device<Bound>, _this: Pin<&Self>, data: &[u8]) -> usize {
+        let _ = sdev.write_all(data, serdev::Timeout::Max);
+        data.len()
+    }
+}
+
+impl Drop for SampleDriver {
+    fn drop(&mut self) {
+        dev_dbg!(
+            self.sdev.as_ref(),
+            "Remove Rust Serial device bus device driver sample.\n"
+        );
+    }
+}
+
+kernel::module_serdev_device_driver! {
+    type: SampleDriver,
+    name: "rust_driver_serdev",
+    authors: ["Markus Probst"],
+    description: "Rust Serial device bus device driver",
+    license: "GPL v2",
+}

-- 
2.52.0

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Overall Series Review

Subject: rust: add basic serial device bus abstractions
Author: Markus Probst <markus.probst@posteo.de>
Patches: 5
Reviewed: 2026-04-23T09:21:38.011857

---

This is a v5 series adding Rust serdev (serial device bus) abstractions to the kernel, consisting of 4 patches: a preparatory devres change, a C-side struct modification, the main Rust abstraction, and a sample driver.

The overall design follows established kernel Rust patterns well. The `Driver` trait, `Adapter` registration mechanism, and `Device` wrapper are consistent with how other bus abstractions (platform, USB, auxiliary) work. The race condition between `devm_serdev_device_open` and probe completion is handled correctly using a `Completion` barrier. The code is generally well-structured and the safety comments are mostly thorough.

Key concerns: (1) the `rust_private_data` field in `struct serdev_device` adds overhead to all serdev devices whether or not a Rust driver is in use; (2) the `devres::register` lifetime extension in patch 1 has a subtle safety argument that deserves careful review from the devres maintainers; (3) there are a few documentation typos/broken links that should be fixed.

---
Generated by Claude Code Patch Reviewer

Claude review: rust: devres: return reference in `devres::register`

[Claude Code Review Bot]

Patch Review

This changes `devres::register` from returning `Result` to `Result<&'a T>`, tying the returned reference's lifetime to the `&'a Device<Bound>` parameter.

The SAFETY comment is the critical part:
```rust
+        // SAFETY: `dev` is valid for the lifetime of 'a. As long as there is a reference to
+        // `Device<Bound>`, it is guaranteed that the device is not unbound and data has not been
+        // dropped. Thus `data_ptr` is also valid for the lifetime of 'a.
+        .map(|()| unsafe { &*data_ptr })
```

This is correct given the semantics of `Device<Bound>` -- the Bound context guarantees the device hasn't been unbound, and devres resources are freed at unbind time. However, the reasoning depends entirely on the invariant that `Device<Bound>` references cannot outlive the bound state. This is an important invariant for the devres subsystem maintainers to validate.

The mechanical updates to `cpufreq.rs` and `drm/driver.rs` (adding `?; Ok(())`) are straightforward and correct -- existing callers that don't use the returned reference simply discard it.

The doc example should also be updated to show the new return type:
```rust
-///     devres::register(dev, Registration::new(), GFP_KERNEL)
+///     devres::register(dev, Registration::new(), GFP_KERNEL)?;
+///     Ok(())
```
This is done correctly.

Already has an `Acked-by` from Viresh Kumar, which is good since it touches cpufreq.

---
Generated by Claude Code Patch Reviewer

Claude review: serdev: add rust private data to serdev_device

[Claude Code Review Bot]

Patch Review

Adds `void *rust_private_data` to `struct serdev_device`:

```c
+	void *rust_private_data;
```

This is the most debatable design choice in the series. It adds a pointer to every `serdev_device` instance, even when no Rust driver is present. The alternative would be using a side data structure (e.g., an IDR/XArray lookup), but that would add complexity and overhead in the receive path.

The doc update fixes the missing colons on `@write_comp` and `@write_lock`:
```c
- * @write_comp	Completion used by serdev_device_write() internally
- * @write_lock	Lock to serialize access when writing data
+ * @write_comp:	       Completion used by serdev_device_write() internally
+ * @write_lock:	       Lock to serialize access when writing data
```
That's a nice cleanup, but the re-alignment to very wide column positions (tabs to column ~24) changes the style relative to the other fields. Might attract style comments from the serdev maintainer.

The field is not initialized anywhere, but `serdev_device_alloc` uses `kzalloc` for the controller, and the device is allocated via `device_register`, so the field should be zero-initialized. Still, it would be more defensive to document or assert that assumption.

This patch is missing a `Signed-off-by` or `Acked-by` from Rob Herring (the serdev maintainer), which is important since it modifies a core C header he maintains.

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Patch Review

This is the main patch adding `rust/kernel/serdev.rs` (536 lines).

**Typo in `Parity` enum:**
```rust
+    /// Even partiy.
+    Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
```
"partiy" should be "parity". The v5 changelog says "fix typo in documentation", but this one appears to still be present.

**Broken doc link in `write` method:**
```rust
+    /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
```
This is malformed -- should be `` [`Device::wait_until_sent`] `` (the opening backtick is missing and there's a spurious space after `[`). Compare with the correct version in `write_all`:
```rust
+    /// [`Device::wait_until_sent`] to make sure the controller write buffer has actually been
```

**`Timeout` enum documentation:**
```rust
+/// Timeout in Jiffies.
+pub enum Timeout {
```
The doc says "Timeout in Jiffies" but the enum also supports `Milliseconds` and `Max` variants. This should be something like "Timeout specification for serial device operations."

**`Timeout::Max` returning 0:**
```rust
+            Self::Max => 0,
```
This is correct. The C `serdev_device_write` documentation says "timeout in jiffies, or 0 to wait indefinitely", so `Max => 0` correctly maps to indefinite wait.

**Race condition handling in `probe_callback`/`receive_buf_callback`:**

The design opens the device *before* completing probe:
```rust
+            to_result(unsafe {
+                bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
+            })?;
+
+            let data = T::probe(sdev, id_info);
```

This means `receive_buf_callback` can fire before driver data is set. The `Completion` + `UnsafeCell<bool>` pattern handles this correctly:
- The receive callback waits on `probe_complete`
- After completion, it checks the `error` flag
- If probe failed, it consumes all data silently (`return length`)

The synchronization is sound because `complete_all()`/`wait_for_completion()` provide the necessary memory barriers for the `UnsafeCell<bool>` access. The receive callback documentation says "may sleep", so blocking on the completion is safe.

One concern: if `T::probe` returns an error-producing initializer, `set_drvdata` will return `Err(...)`. The code correctly handles this by still calling `complete_all()` after setting the error flag. But if the error happens *before* `set_drvdata` (e.g., `devres::register` or `devm_serdev_device_open` fails), the early `?` return would skip `complete_all()`. This is safe only because client_ops are set before `devm_serdev_device_open`, and if the device isn't opened, the receive callback won't be called. The ordering of operations is correct but fragile -- a future refactor that reorders these calls could introduce a deadlock.

**`set_baudrate` return type:**
```rust
+    pub fn set_baudrate(&self, speed: u32) -> Result<(), u32> {
```
Returning `Result<(), u32>` (where the error value is the actual baudrate achieved) is an unusual but defensible API choice. However, it breaks from the kernel Rust convention where `Result` typically means `Result<T, kernel::error::Error>`. Users who want to know the actual baudrate set on failure must handle the `u32` error, which is a different error type than the rest of the serdev API.

**`write_all` return value cast:**
```rust
+        to_result(ret as i32).map(|()| ret.unsigned_abs())
```
Where `ret` is `isize` (from `ssize_t`). The comment correctly notes that negative values fit in `i32`. For positive values, `ret as i32` could overflow if writing >2GB, but this is unrealistic for serial devices.

**Missing `#[cfg]` guard:** The `serdev.c` helper file is compiled unconditionally (added to `helpers.c`). It should arguably be guarded by `#ifdef CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS` to avoid compiling unused helpers, matching the pattern used for the Rust module in `lib.rs`:
```rust
+#[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)]
+pub mod serdev;
```

**Trailing `//` comments:**
```rust
+    types::{
+        AlwaysRefCounted,
+        Opaque, //
+    }, //
```
These trailing `//` comments appear to be rustfmt artifacts to force vertical formatting. This is a known pattern in kernel Rust code but could be noted as intentional.

---
Generated by Claude Code Patch Reviewer

Claude review: samples: rust: add Rust serial device bus sample device driver

[Claude Code Review Bot]

Patch Review

Clean and functional sample driver. A few observations:

**Echoing data in receive callback:**
```rust
+    fn receive(sdev: &serdev::Device<Bound>, _this: Pin<&Self>, data: &[u8]) -> usize {
+        let _ = sdev.write_all(data, serdev::Timeout::Max);
+        data.len()
+    }
```
The `write_all` with `Timeout::Max` could block indefinitely if the device can't accept data. For a sample this is acceptable, but a brief comment noting this is intentional would be helpful. Also, `let _ =` silently discards write errors, which is fine for a sample.

**Baudrate handling:**
```rust
+        if sdev
+            .set_baudrate(
+                dev.fwnode()
+                    .and_then(|fwnode| fwnode.property_read(c"baudrate").optional())
+                    .unwrap_or(115200),
+            )
+            .is_err()
+        {
+            return Err(EINVAL);
+        }
```
Reading a `baudrate` property from firmware and falling back to 115200 is a reasonable pattern. The `set_baudrate` error (which contains the actual baudrate set) is discarded and replaced with `EINVAL`, which is appropriate.

**Kconfig entry properly uses `select`:**
```
+	select RUST_SERIAL_DEV_BUS_ABSTRACTIONS
```
This ensures the abstraction is compiled when the sample is enabled. Correct.

The sample effectively demonstrates OF and ACPI device table setup, probe/remove lifecycle, baudrate/flow-control/parity configuration, and the receive callback -- good coverage for a sample driver.

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Overall Series Review

Subject: rust: add basic serial device bus abstractions
Author: Markus Probst via B4 Relay <devnull+markus.probst.posteo.de@kernel.org>
Patches: 5
Reviewed: 2026-04-28T14:14:44.163925

---

This is a v6 series (4 patches) by Markus Probst adding Rust abstractions for the serial device bus (serdev) to the Linux kernel. The series is well-structured: a preparatory devres API change, a small C-side struct modification, the main Rust abstraction, and a sample driver.

**Strengths:**
- The overall architecture follows established patterns from the platform and PCI Rust driver abstractions closely.
- The probe synchronization design (using `Completion` to handle the race between `devm_serdev_device_open` and probe completion) is a thoughtful solution to a real concurrency problem.
- The API surface is reasonable — covering baudrate, flow control, parity, read, and write operations.
- Good use of `devres` for the abstraction's internal bookkeeping data.

**Concerns:**
- The `Timeout::Max` doc comment says it is "equivalent to `kernel::task::MAX_SCHEDULE_TIMEOUT`" but it actually maps to 0, relying on the C serdev/tty convention that 0 means "wait forever." The doc is misleading — it should reference the C API convention instead.
- The `unwrap_or_default()` in `Timeout::into_jiffies()` silently degrades large timeouts to 0, which happens to be correct (wait forever) by the same C convention, but this is accidental correctness and confusing.
- Adding `void *rust_private_data` to the C `struct serdev_device` is a debatable design choice that has no enforcement against misuse by C drivers.
- A few doc typos and a broken intra-doc link.

---

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Patch Review

This is the main patch. Several observations:

**1. `Parity` enum typo:**
```rust
/// Even partiy.
Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
```
Should be "Even parity."

**2. `Timeout::Max` doc is misleading:**
```rust
/// Wait as long as possible.
///
/// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
Max,
```
The actual value returned is 0, not `MAX_SCHEDULE_TIMEOUT` (which is `LONG_MAX`). This works because the C serdev API (`serdev_device_write`) and tty layer (`tty_wait_until_sent`) both convert `timeout == 0` to `MAX_SCHEDULE_TIMEOUT` internally. The doc should say something like "Passes 0 to the C API, which treats it as 'wait indefinitely'" rather than claiming equivalence with `MAX_SCHEDULE_TIMEOUT`, which is a different numeric value.

**3. `Timeout::into_jiffies()` overflow behavior:**
```rust
Self::Jiffies(value) => value.get().try_into().unwrap_or_default(),
Self::Milliseconds(value) => {
    msecs_to_jiffies(value.get()).try_into().unwrap_or_default()
}
```
If a `Jiffies` value exceeds `isize::MAX`, `try_into()` fails and `unwrap_or_default()` returns 0. By the C convention, 0 means "wait forever," so a too-large timeout becomes "wait forever." This happens to be reasonable behavior, but it's accidental — the code looks like a bug where overflow silently becomes zero. A comment explaining why `unwrap_or_default()` (i.e., 0 = wait forever) is the correct fallback would help future readers. Alternatively, using `.unwrap_or(0)` with an explicit comment about the C convention would be clearer.

**4. Broken doc link in `write()`:**
```rust
/// Note that any accepted data has only been buffered by the controller. Use
/// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
/// emptied.
```
Should be `[`Device::wait_until_sent`]` — missing opening backtick after `[`.

**5. `PrivateData` and `UnsafeCell<bool>` synchronization:**
```rust
#[pin_data]
struct PrivateData {
    #[pin]
    probe_complete: Completion,
    error: UnsafeCell<bool>,
}
```
The `Completion` provides the memory synchronization between the write (in `probe_callback`) and read (in `receive_buf_callback`) of the `error` field. This is correct at runtime. However, `UnsafeCell<bool>` makes `PrivateData` `!Sync`, and the code creates shared `&PrivateData` references across threads via raw pointer casts. The SAFETY comments explain the synchronization correctly, but using `AtomicBool` with `Relaxed` ordering (since the `Completion` provides the barrier) would make the type system happy and avoid the need for unsafe reasoning about `Sync`.

**6. `probe_callback` structure — ordering concern:**

```rust
// Step 1: Register PrivateData via devres
let private_data = devres::register(sdev.as_ref(), ...)?;
// Step 2: Store pointer
(*sdev.as_raw()).rust_private_data = ...;
// Step 3: Set client ops (enables receive_buf callback)
bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::OPS);
// Step 4: Open device (data can start flowing)
bindings::devm_serdev_device_open(...)?;
// Step 5: Driver probe
let data = T::probe(sdev, id_info);
// Step 6: Store driver data
let result = sdev.as_ref().set_drvdata(data);
// Step 7: Signal completion
private_data.probe_complete.complete_all();
```

The ordering of steps 3 and 4 is correct — ops must be set before opening, and the `Completion` in `receive_buf_callback` prevents data access before probe finishes. This is well-designed.

One subtlety: if `T::probe()` itself calls `write_all` with `Timeout::Max` (as shown in the doc example), this happens before `complete_all()`. If a response arrives from the device, `receive_buf_callback` would be invoked, which calls `wait_for_completion()` — but `complete_all()` hasn't been called yet, so this would **deadlock** if `receive_buf_callback` runs on the same thread context. In practice, serdev receive callbacks run from an interrupt/workqueue context different from the probe context, so this should be fine, but it's worth documenting this constraint.

**7. `remove_callback` does not call `drvdata_borrow` through `Device<CoreInternal>`:**

Looking at `remove_callback`:
```rust
let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
let data = unsafe { sdev.as_ref().drvdata_borrow::<T>() };
T::unbind(sdev, data);
```
But `unbind` is defined as:
```rust
fn unbind(sdev: &Device<device::Core>, this: Pin<&Self>) { ... }
```
The `sdev` argument to `unbind` is `&Device<CoreInternal>`, but the trait declares `&Device<device::Core>`. This works because of the deref coercion chain set up by `impl_device_context_deref!`. Fine.

**8. `write_all` return type inconsistency with docs:**
The doc says:
```
/// Returns the number of bytes written (less than `data.len()` if interrupted).
/// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
/// before any bytes were written.
```
The second line reads like it describes error return values, but the formatting makes it look like part of the success return description. It should be something like: "Returns `Err(ETIMEDOUT)` or `Err(ERESTARTSYS)` if interrupted before any bytes were written."

**9. `ret as i32` truncation in `write_all`:**
```rust
let ret = unsafe { bindings::serdev_device_write(...) }; // returns ssize_t (isize)
to_result(ret as i32).map(|()| ret.unsigned_abs())
```
On 64-bit, `ssize_t` is 64 bits. The `as i32` truncation is safe in practice because serial writes never exceed 2GB, but for correctness, converting the full `isize` error check would be more robust: e.g., `if ret < 0 { return Err(...) }` instead of relying on `to_result(ret as i32)`.

---

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Overall Series Review

Subject: rust: add basic serial device bus abstractions
Author: Markus Probst via B4 Relay <devnull+markus.probst.posteo.de@kernel.org>
Patches: 6
Reviewed: 2026-05-05T11:18:28.881410

---

This is a well-structured 4-patch series adding Rust abstractions for the serial device bus (serdev) subsystem. The series is now at v7, showing significant iteration. The architecture follows established kernel Rust patterns: a preparatory devres API change, a C-side struct modification, the main abstraction, and a sample driver.

The code quality is generally good, with proper use of the kernel's Rust driver model (`DriverLayout`, `RegistrationOps`, `Adapter`), correct synchronization for the probe/receive race via `Completion`, and a clean sample driver. There are a few issues ranging from a potential truncation bug in `write_all` to minor doc typos.

The series depends on Danilo Krummrich's driver-lifetime patches, which is noted in the cover letter.

---
Generated by Claude Code Patch Reviewer

Claude review: rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Patch Review

This is the main patch (571 lines). Several observations:

**(1) Typo in Parity doc comment:**

```rust
+    /// Even partiy.
+    Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
```

Should be `Even parity.`

**(2) `Timeout::into_jiffies` silent overflow to "wait forever":**

```rust
+    fn into_jiffies(self) -> isize {
+        match self {
+            Self::Jiffies(value) => value.get().try_into().unwrap_or_default(),
+            Self::Milliseconds(value) => {
+                msecs_to_jiffies(value.get()).try_into().unwrap_or_default()
+            }
+            Self::Max => 0,
+        }
+    }
```

The `unwrap_or_default()` on overflow returns 0, which the C `serdev_device_write` converts to `MAX_SCHEDULE_TIMEOUT` (wait forever). A caller specifying a large but finite timeout would silently get an infinite wait. This is worth noting in the doc or choosing a different fallback (e.g., `isize::MAX`).

The `Self::Max => 0` mapping relies on `serdev_device_write`'s internal `if (timeout == 0) timeout = MAX_SCHEDULE_TIMEOUT;` behavior. This is correct but fragile — it couples to an implementation detail of `serdev_device_write` rather than passing `MAX_SCHEDULE_TIMEOUT` directly. If this function is ever used for other serdev calls that don't do the 0-to-MAX conversion, it would break silently.

**(3) Potential truncation in `write_all`:**

```rust
+    pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
+        let ret = unsafe {
+            bindings::serdev_device_write(
+                self.as_raw(),
+                data.as_ptr(),
+                data.len(),
+                timeout.into_jiffies(),
+            )
+        };
+        // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
+        // which always fit into a `i32`.
+        to_result(ret as i32).map(|()| ret.unsigned_abs())
+    }
```

`serdev_device_write` returns `ssize_t` (isize in Rust). The cast `ret as i32` is safe for negative error codes as the comment states, but on 64-bit systems, a successful return value larger than `i32::MAX` (>2GB) would be truncated to a negative i32, causing `to_result` to return a spurious error. While this is extremely unlikely for serial writes, the same pattern appears in `write()` (where `write_buf` returns `int`, so there's no issue there). A more defensive approach might be to check `ret < 0` directly instead of casting through `to_result`.

**(4) Missing bracket in doc comment for `write`:**

```rust
+    /// Note that any accepted data has only been buffered by the controller. Use
+    /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
```

Should be `[`Device::wait_until_sent`]` (no space after `[`).

**(5) `write_all` naming may confuse Rust developers:**

In `std::io::Write`, `write_all` guarantees all bytes are written or returns an error. Here, `write_all` can return fewer bytes than `data.len()` (if interrupted). The doc is clear about this, but the name may mislead readers used to the standard library convention. The C function being wrapped is `serdev_device_write` (vs `serdev_device_write_buf`), so alternative names like `write_timeout` might be less surprising.

**(6) Probe/receive synchronization — correct:**

The `PrivateData` with `Completion` + `UnsafeCell<bool>` for synchronizing probe completion with receive_buf is correctly structured:

```rust
+            // SAFETY: We have exclusive access to `private_data.error`.
+            unsafe { *private_data.error.get() = result.is_err() };
+            private_data.probe_complete.complete_all();
```

The `complete_all()` provides the necessary memory barrier so that `wait_for_completion()` in `receive_buf_callback` sees the error flag. The ordering in `probe_callback` is also correct: `rust_private_data` is set before `set_client_ops` and `devm_serdev_device_open`, ensuring the pointer is valid when `receive_buf` can first be called.

**(7) OPS struct wires `write_wakeup` to C function directly:**

```rust
+    const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
+        receive_buf: if <F::Of<'static> as Driver<'static>>::HAS_RECEIVE {
+            Some(Self::receive_buf_callback)
+        } else {
+            None
+        },
+        write_wakeup: Some(bindings::serdev_device_write_wakeup),
+    };
```

Using `bindings::serdev_device_write_wakeup` directly as the function pointer is fine since it has the right signature. The `receive_buf` is conditionally set based on whether the driver implements `receive`, which is a nice use of vtable detection.

**(8) `Device` Send/Sync impls:**

```rust
+unsafe impl Send for Device {}
+unsafe impl Sync for Device {}
+unsafe impl Sync for Device<device::Bound> {}
```

The `Send` impl only covers `Device` (i.e., `Device<Normal>`). Missing `Send` for `Device<device::Bound>` and other contexts. However, `Device<Bound>` is typically only used by reference within a driver callback, so it may not need `Send`. This is likely intentional but could use a comment.

---

---
Generated by Claude Code Patch Reviewer