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

[PATCH v2 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 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 private data to serdev_device
      rust: add basic serial device bus abstractions
      samples: rust: add Rust serial device bus sample device driver

 include/linux/serdev.h             |  14 +-
 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              | 533 +++++++++++++++++++++++++++++++++++++
 samples/rust/Kconfig               |  11 +
 samples/rust/Makefile              |   1 +
 samples/rust/rust_driver_serdev.rs |  86 ++++++
 12 files changed, 682 insertions(+), 10 deletions(-)
---
base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
change-id: 20251217-rust_serdev-ee5481e9085c

[PATCH v2 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).

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 76faa1ac8501..8cf86bb8e0f4 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -1051,7 +1051,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

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

[Claude Code Review Bot]

Patch Review

This patch modifies `devres::register` to return `Result<&'a T>` instead of `Result`, allowing callers to get a reference to the devres-managed data.

**Safety concern with the returned reference:**

```rust
+    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 })
```

The safety argument is that `data_ptr` remains valid for `'a` because the `Device<Bound>` reference lives that long. This works because `register_foreign` calls `data.into_foreign()` which consumes the `Pin<KBox<T>>` without moving the inner `T` -- the data at `data_ptr` is still at the same address, now owned by the devres system.

However, the safety comment should be more precise: it should mention that `into_foreign()` for `Pin<KBox<T>>` extracts the raw pointer without moving the inner `T`, so `data_ptr` continues pointing to valid data as long as the devres action hasn't fired (which is guaranteed while the device remains bound).

**Existing callers are updated correctly** -- `cpufreq.rs` and `drm/driver.rs` now discard the returned reference with `?; Ok(())`, preserving existing behavior.

Overall this patch is acceptable with a minor safety comment refinement.

---

---
Generated by Claude Code Patch Reviewer

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

[Markus Probst]

Add 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 | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 5654c58eb73c..b591af23faf0 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -33,12 +33,13 @@ 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
+ * @private_data Private data for the device driver.
  */
 struct serdev_device {
 	struct device dev;
@@ -47,6 +48,7 @@ struct serdev_device {
 	const struct serdev_device_ops *ops;
 	struct completion write_comp;
 	struct mutex write_lock;
+	void *private_data;
 };
 
 static inline struct serdev_device *to_serdev_device(struct device *d)

-- 
2.52.0

Re: [PATCH v2 2/4] serdev: add private data to serdev_device

[Randy Dunlap]

Hi--

On 3/6/26 11:35 AM, Markus Probst wrote:
> Add 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 | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index 5654c58eb73c..b591af23faf0 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -33,12 +33,13 @@ 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
> + * @private_data Private data for the device driver.
>   */

I don't quite get why each changed line has an extra tab added to it. ?
Also, struct member names in kernel-doc should with a colon  (':'), e.g.,

 * @private_data: Private data for the device driver.

Please correct that and the 2 lines above it also.
And maybe test it to check for warnings.

>  struct serdev_device {
>  	struct device dev;
> @@ -47,6 +48,7 @@ struct serdev_device {
>  	const struct serdev_device_ops *ops;
>  	struct completion write_comp;
>  	struct mutex write_lock;
> +	void *private_data;
>  };
>  
>  static inline struct serdev_device *to_serdev_device(struct device *d)
> 

thanks.
-- 
~Randy

Re: [PATCH v2 2/4] serdev: add private data to serdev_device

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 2412 bytes --]

On Fri, 2026-03-06 at 11:49 -0800, Randy Dunlap wrote:
> Hi--
> 
> On 3/6/26 11:35 AM, Markus Probst wrote:
> > Add 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 | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> > index 5654c58eb73c..b591af23faf0 100644
> > --- a/include/linux/serdev.h
> > +++ b/include/linux/serdev.h
> > @@ -33,12 +33,13 @@ 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
> > + * @private_data Private data for the device driver.
> >   */
> 
> I don't quite get why each changed line has an extra tab added to it. ?
So all the fields have the same indentation.
Otherwise the description of private_data, because of its length, would
be 1 character more to the right.

> Also, struct member names in kernel-doc should with a colon  (':'), e.g.,
> 
>  * @private_data: Private data for the device driver.
> 
> Please correct that and the 2 lines above it also.
> And maybe test it to check for warnings.
I can do that.

> 
> >  struct serdev_device {
> >  	struct device dev;
> > @@ -47,6 +48,7 @@ struct serdev_device {
> >  	const struct serdev_device_ops *ops;
> >  	struct completion write_comp;
> >  	struct mutex write_lock;
> > +	void *private_data;
> >  };
> >  
> >  static inline struct serdev_device *to_serdev_device(struct device *d)
> > 
> 
> thanks.

Thanks
- Markus Probst


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Claude review: Re: [PATCH v2 2/4] serdev: add private data to serdev_device

[Claude Code Review Bot]

Patch Review

```c
+ * @private_data Private data for the device driver.
  */
 struct serdev_device {
 	...
+	void *private_data;
 };
```

**Missing colon in kernel-doc**: The `@private_data` line is missing the colon separator used by all other fields. Should be `@private_data:`.

**Design concern**: `struct serdev_device` already has driver data via `dev_get_drvdata()`/`dev_set_drvdata()` on the embedded `struct device`. Adding a second `private_data` field is unusual. The commit message explains this is needed because the Rust abstraction uses `drvdata` for the driver type `T`, and needs a separate field for internal `PrivateData`. This is a C ABI change visible to all serdev users and should be discussed with the serdev maintainer (Rob Herring). Consider whether the Rust side could instead embed its `PrivateData` alongside the driver type in a wrapper struct stored in the standard `drvdata`, avoiding the C-side change entirely.

---

---
Generated by Claude Code Patch Reviewer

Re: [PATCH v2 2/4] serdev: add private data to serdev_device

[Danilo Krummrich]

On Fri Mar 6, 2026 at 8:35 PM CET, Markus Probst wrote:
> Add 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 | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index 5654c58eb73c..b591af23faf0 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -33,12 +33,13 @@ 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
> + * @private_data Private data for the device driver.

I think this is a bit misleading, as the driver's device private data is stored
in the embedded struct device. This seems to be more about having a place to
store private data of the Rust abstraction.

So, you may want to name this something along the lines of rust_private_data.
Additionally, you should make it very clear that this field must not be used by
drivers directly (i.e. C drivers could easily be tempted to abuse this).

>   */
>  struct serdev_device {
>  	struct device dev;
> @@ -47,6 +48,7 @@ struct serdev_device {
>  	const struct serdev_device_ops *ops;
>  	struct completion write_comp;
>  	struct mutex write_lock;
> +	void *private_data;
>  };
>  
>  static inline struct serdev_device *to_serdev_device(struct device *d)
>
> -- 
> 2.52.0

[PATCH v2 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>
---
 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           | 533 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 559 insertions(+)

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 3da92f18f4ee..1d337e112922 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -140,6 +140,8 @@
 pub mod scatterlist;
 pub mod security;
 pub mod seq_file;
+#[cfg(CONFIG_SERIAL_DEV_BUS)]
+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..cba5fb6245f7
--- /dev/null
+++ b/rust/kernel/serdev.rs
@@ -0,0 +1,533 @@
+// 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, ForeignOwnable, 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()).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);
+
+        // SAFETY:
+        // - `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        // - `remove_callback` is only ever called after a successful call to
+        //   `probe_callback`, hence it's guaranteed that `private_data` is a valid pointer to
+        //   `PrivateData` and is stored in a `Pin<KBox<PrivateData>>`.
+        // - `private_data` will not be accessed after this function.
+        drop(unsafe { <Pin<KBox<PrivateData>>>::from_foreign((*sdev.as_raw()).private_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: `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:
+        // - 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()).private_data.cast::<PrivateData>() };
+
+        private_data.probe_complete.complete_all();
+
+        // SAFETY: No one has exclusive access to `private_data.error`.
+        if unsafe { *private_data.error.get() } {
+            return length;
+        }
+
+        // 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

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

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 24731 bytes --]

On Fri, 2026-03-06 at 19:35 +0000, Markus Probst wrote:
> 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>
> ---
>  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           | 533 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 559 insertions(+)
> 
> 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 3da92f18f4ee..1d337e112922 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -140,6 +140,8 @@
>  pub mod scatterlist;
>  pub mod security;
>  pub mod seq_file;
> +#[cfg(CONFIG_SERIAL_DEV_BUS)]
> +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..cba5fb6245f7
> --- /dev/null
> +++ b/rust/kernel/serdev.rs
> @@ -0,0 +1,533 @@
> +// 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, ForeignOwnable, 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()).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);
> +
> +        // SAFETY:
> +        // - `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> +        // - `remove_callback` is only ever called after a successful call to
> +        //   `probe_callback`, hence it's guaranteed that `private_data` is a valid pointer to
> +        //   `PrivateData` and is stored in a `Pin<KBox<PrivateData>>`.
> +        // - `private_data` will not be accessed after this function.
> +        drop(unsafe { <Pin<KBox<PrivateData>>>::from_foreign((*sdev.as_raw()).private_data) });
It seems I forgot to remove this drop statement. Please ignore it.

Thanks
- Markus Probst

> +    }
> +
> +    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: `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:
> +        // - 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()).private_data.cast::<PrivateData>() };
> +
> +        private_data.probe_complete.complete_all();
> +
> +        // SAFETY: No one has exclusive access to `private_data.error`.
> +        if unsafe { *private_data.error.get() } {
> +            return length;
> +        }
> +
> +        // 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 {}

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

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

[Danilo Krummrich]

On Fri Mar 6, 2026 at 8:35 PM CET, Markus Probst wrote:
> +    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: `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:
> +        // - 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()).private_data.cast::<PrivateData>() };
> +
> +        private_data.probe_complete.complete_all();

Will do a full review pass later on, but one quick question in advance:

What is this used for? It is completed here and in probe(), but I don't see it ever
being used to actually wait.

> +
> +        // SAFETY: No one has exclusive access to `private_data.error`.
> +        if unsafe { *private_data.error.get() } {
> +            return length;
> +        }
> +
> +        // 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)
> +    }
> +}

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

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 2283 bytes --]

On Fri, 2026-03-06 at 21:40 +0100, Danilo Krummrich wrote:
> On Fri Mar 6, 2026 at 8:35 PM CET, Markus Probst wrote:
> > +    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: `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:
> > +        // - 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()).private_data.cast::<PrivateData>() };
> > +
> > +        private_data.probe_complete.complete_all();
> 
> Will do a full review pass later on, but one quick question in advance:
> 
> What is this used for? It is completed here and in probe(), but I don't see it ever
> being used to actually wait.
Uh, thats a typo. It is supposed to wait here in receive_buf_callback.

Thanks
- Markus Probst

> 
> > +
> > +        // SAFETY: No one has exclusive access to `private_data.error`.
> > +        if unsafe { *private_data.error.get() } {
> > +            return length;
> > +        }
> > +
> > +        // 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)
> > +    }
> > +}

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Claude review: Re: [PATCH v2 3/4] rust: add basic serial device bus abstractions

[Claude Code Review Bot]

Patch Review

This is the core patch. It has several significant issues:

**1. CRITICAL: Double-free of PrivateData**

In `probe_callback`, `PrivateData` is registered with the devres system:
```rust
let private_data = devres::register(
    sdev.as_ref(),
    try_pin_init!(PrivateData { ... }),
    GFP_KERNEL,
)?;
```

Looking at the existing `register_foreign` (devres.rs:275-296), this calls `devm_add_action_or_reset` which registers a callback to `drop(P::from_foreign(ptr))` when the device is unbound.

Then in `remove_callback`, the same data is manually freed:
```rust
drop(unsafe { <Pin<KBox<PrivateData>>>::from_foreign((*sdev.as_raw()).private_data) });
```

Since `remove_callback` runs *before* devres cleanup during device unbind, this frees the memory first, and then the devres action fires and attempts to free the same memory again -- a **double-free**.

**2. CRITICAL: Race condition / UB in `receive_buf_callback`**

The probe sequence is:
1. `devm_serdev_device_open` opens the device (data can now arrive)
2. `T::probe()` runs
3. `set_drvdata()` stores the driver data

But `receive_buf_callback` immediately accesses drvdata:
```rust
let data = unsafe { sdev.as_ref().drvdata_borrow::<T>() };
```

If data arrives between step 1 and step 3, `drvdata_borrow` dereferences uninitialized/null driver data -- **undefined behavior**.

The `probe_complete` Completion appears intended to address this, but it's used incorrectly: `receive_buf_callback` calls `complete_all()` (signals completion) rather than waiting for it. Nobody ever waits on this Completion, making it effectively dead code.

A correct fix would be to either:
- Move `devm_serdev_device_open` to *after* `T::probe()` and `set_drvdata()`, or
- Have `receive_buf_callback` call `wait_for_completion()` instead of `complete_all()` (the callback "may sleep" per the ops contract)

**3. BUG: `Timeout::Max` returns 0 instead of MAX_SCHEDULE_TIMEOUT**

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

The doc says `Timeout::Max` is "equivalent to `kernel::task::MAX_SCHEDULE_TIMEOUT`", which is `c_long::MAX` (i.e., wait indefinitely). But the code returns `0`, which means "don't wait at all" -- the exact opposite. This should be `isize::MAX` or reference `MAX_SCHEDULE_TIMEOUT` directly.

**4. Data race on `error` field**

```rust
error: UnsafeCell<bool>,
```

`UnsafeCell<bool>` provides no synchronization. The `error` field is written in `probe_callback` and read in `receive_buf_callback`, which can run concurrently (on different CPUs). This is a data race. An `AtomicBool` should be used instead.

**5. Typo in doc comment**

```rust
/// Even partiy.
```
Should be "Even parity."

**6. Broken doc links**

In both `write_all` and `write` docs:
```rust
/// [ Device::wait_until_sent`]
```
Missing opening backtick: should be `[`Device::wait_until_sent`]`.

**7. `set_baudrate` return type is unidiomatic**

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

Returning `Err(actual_baudrate)` when the hardware sets a different rate is misleading -- it's not really an error. The C function returns the actual baudrate set, and the caller should decide if it's acceptable. Consider returning just `u32` (the actual baudrate).

**8. Inconsistent return types between `write` and `write_all`**

`write` returns `Result<u32>` while `write_all` returns `Result<usize>`. Both represent byte counts. These should be consistent (both `usize`).

**9. `write_all` truncating cast**

```rust
to_result(ret as i32).map(|()| ret.unsigned_abs())
```

`serdev_device_write` returns `ssize_t` (isize). Casting to `i32` could truncate large values, though practically serial writes shouldn't exceed 2GB. Still, this should use a proper conversion rather than a truncating `as` cast.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v2 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..a421470d2c52 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"
+	depends on SERIAL_DEV_BUS
+	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: samples: rust: add Rust serial device bus sample device driver

[Claude Code Review Bot]

Patch Review

The sample is clean and demonstrates the abstraction well. A few notes:

**`set_baudrate` error handling is odd due to the API design:**

```rust
if sdev
    .set_baudrate(
        dev.fwnode()
            .and_then(|fwnode| fwnode.property_read(c"baudrate").optional())
            .unwrap_or(115200),
    )
    .is_err()
{
    return Err(EINVAL);
}
```

This discards the actual baudrate returned in the error. If `set_baudrate` returned `u32` instead of `Result<(), u32>`, this would be simpler and more informative.

**Echo server with `Timeout::Max` would hang:** Due to the `Timeout::Max` bug (returning 0), `write_all` in receive would return immediately or with ETIMEDOUT rather than actually waiting. This makes the sample non-functional in practice.

**The sample stores an `ARef<serdev::Device>` but never uses it** (except in Drop for a debug message). This is fine for a sample but worth noting.

---
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: 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

[PATCH v3 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 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: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681
change-id: 20251217-rust_serdev-ee5481e9085c

[PATCH v3 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 3da92f18f4ee..90635f44e171 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -140,6 +140,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..d9fea4bd4439
--- /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

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 <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