On Fri, 2026-07-17 at 12:15 +0100, Gary Guo wrote: > On Wed Jul 15, 2026 at 10:00 PM BST, 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 <[email protected]> > > --- > > MAINTAINERS | 2 + > > 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 | 599 > > ++++++++++++++++++++++++++++++++++++++++ > > 7 files changed, 634 insertions(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 806bd2d80d15..cd735d227892 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -24568,6 +24568,8 @@ S: Maintained > > F: Documentation/devicetree/bindings/serial/serial.yaml > > F: drivers/tty/serdev/ > > F: include/linux/serdev.h > > +F: rust/helpers/serdev.c > > +F: rust/kernel/serdev.rs > > > > SERIAL IR RECEIVER > > M: Sean Young <[email protected]> > > 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 1124785e210b..fe7c505da236 100644 > > --- a/rust/bindings/bindings_helper.h > > +++ b/rust/bindings/bindings_helper.h > > @@ -85,6 +85,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 998e31052e66..6921a5ecb78a 100644 > > --- a/rust/helpers/helpers.c > > +++ b/rust/helpers/helpers.c > > @@ -87,6 +87,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 9512af7156df..d5ca17336739 100644 > > --- a/rust/kernel/lib.rs > > +++ b/rust/kernel/lib.rs > > @@ -119,6 +119,8 @@ > > pub mod scatterlist; > > pub mod security; > > pub mod seq_file; > > +#[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)] > > +pub mod serdev; > > pub mod sizes; > > #[cfg(CONFIG_SOC_BUS)] > > pub mod soc; > > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs > > new file mode 100644 > > index 000000000000..0ffcef1849d2 > > --- /dev/null > > +++ b/rust/kernel/serdev.rs > > @@ -0,0 +1,599 @@ > > +// 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, > > + driver, > > + error::{ > > + from_result, > > + to_result, > > + VTABLE_DEFAULT_ERROR, // > > + }, > > + new_mutex, > > + of, > > + prelude::*, > > + sync::{ > > + aref::AlwaysRefCounted, > > + Mutex, // > > + }, > > + time::{ > > + msecs_to_jiffies, > > + Jiffies, > > + Msecs, // > > + }, > > + types::{ > > + Opaque, > > + ScopeGuard, // > > + }, // > > +}; > > + > > +use core::{ > > + cell::UnsafeCell, > > + marker::PhantomData, > > + mem::{offset_of, MaybeUninit}, > > + 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, > > + } > > + } > > +} > > FWIW this shouldn't be part of serdev abstraction. See > https://lore.kernel.org/rust-for-linux/[email protected] > > > + > > +/// 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)`. > > +// - `PrivateData<'bound, 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> driver::DriverLayout for Adapter<T> { > > + type DriverType = bindings::serdev_device_driver; > > + type DriverData<'bound> = PrivateData<'bound, 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> 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()) }; > > + } > > +} > > + > > +#[doc(hidden)] > > +#[pin_data(PinnedDrop)] > > +pub struct PrivateData<'bound, T: Driver> { > > + sdev: &'bound Device<device::Bound>, > > + #[pin] > > + driver: UnsafeCell<MaybeUninit<T::Data<'bound>>>, > > + open: UnsafeCell<bool>, > > + #[pin] > > + active: Mutex<bool>, > > +} > > + > > +#[pinned_drop] > > +impl<T: Driver> PinnedDrop for PrivateData<'_, T> { > > + fn drop(self: Pin<&mut Self>) { > > + let mut active = self.active.lock(); > > + if *active { > > + // SAFETY: > > + // - We have exclusive access to `self.driver`. > > + // - `self.driver` is guaranteed to be initialized. > > + unsafe { (*self.driver.get()).assume_init_drop() }; > > + *active = false; > > + } > > + drop(active); > > + > > + // SAFETY: We have exclusive access to `self.open`. > > + if unsafe { *self.open.get() } { > > + // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer > > to a valid > > + // `struct serdev_device`. > > + unsafe { bindings::serdev_device_close(self.sdev.as_raw()) }; > > + } > > + } > > +} > > + > > +impl<T: Driver> 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 info = <Self as driver::Adapter>::id_info(sdev.as_ref()); > > + > > + from_result(|| { > > + sdev.as_ref().set_drvdata(try_pin_init!(PrivateData::<T> { > > + sdev: &**sdev, > > + driver: MaybeUninit::<T::Data<'_>>::zeroed().into(), > > + open: false.into(), > > + active <- new_mutex!(false), > > + }))?; > > + // SAFETY: We just set drvdata to `PrivateData<'_, T>`. > > + let private_data = unsafe { > > sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() }; > > + let private_data = ScopeGuard::new_with_data(private_data, |_| > > { > > + // SAFETY: We just set drvdata to `PrivateData<'_, T>`. > > + drop(unsafe { > > sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() }); > > + }); > > + let mut active = private_data.active.lock(); > > What's this for? This gives me a strong vibe that something is off. Either the > serdev subsystem is not having sufficient synchronization or there is an > abstraction issue. There is the following issue it trys to solve:
In the rust abstraction, the driver data is only initialized after the
probe. `serdev_device_open` should be called inside or before probe,
because otherwise the driver can't configure the serdev device. But as
a sideeffect, once the serdev device is open, it is possible for
receive_buf_callback to be called, which this mutex prevents while the
driver data is not initialized yet.
>
> Either way, you really should have more comments about what this active mutex
> is
> for and why it is needed.
>
> > +
> > + // 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::serdev_device_open(sdev.as_raw())
> > })?;
> > +
> > + // SAFETY: We have exclusive access to `private_data.open`.
> > + unsafe { *private_data.open.get() = true };
> > +
> > + let data = T::probe(sdev, info);
> > +
> > + // SAFETY: We have exclusive access to `private_data.driver`.
> > + let driver = unsafe { &mut *private_data.driver.get() };
> > + // SAFETY:
> > + // - `driver.as_mut_ptr()` is a valid pointer to uninitialized
> > data.
> > + // - `private_data.driver` is pinned.
> > + let result = unsafe { data.__pinned_init(driver.as_mut_ptr())
> > };
> > +
> > + *active = result.is_ok();
> > +
> > + drop(active);
> > +
> > + result.map(|()| {
> > + private_data.dismiss();
> > + 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<PrivateData<'_, T>>>`.
> > + let private_data = unsafe {
> > sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > +
> > + // SAFETY: No one has exclusive access to `private_data.driver`.
> > + let data = unsafe { &*private_data.driver.get() };
> > + // SAFETY:
> > + // - `private_data.driver` is pinned.
> > + // - `remove_callback` is only ever called after a successful call
> > to `probe_callback`,
> > + // hence it's guaranteed that `private_data.driver` was
> > initialized.
> > + let data_pinned = unsafe {
> > Pin::new_unchecked(data.assume_init_ref()) };
> > +
> > + T::unbind(sdev, data_pinned);
> > + }
> > +
> > + 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::BoundInternal>>()
> > };
> > +
> > + // 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<PrivateData<'_, T>>>`.
> > + let private_data = unsafe {
> > sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > + let active = private_data.active.lock();
> > +
> > + if !*active {
> > + return length;
> > + }
> > +
> > + // SAFETY: No one has exclusive access to `private_data.driver`.
> > + let data = unsafe { &*private_data.driver.get() };
> > + // SAFETY:
> > + // - `private_data.driver` is pinned.
> > + // - `receive_buf_callback` is only ever called after a successful
> > call to `probe_callback`,
> > + // hence it's guaranteed that `private_data.driver` was
> > initialized.
> > + let data_pinned = unsafe {
> > Pin::new_unchecked(data.assume_init_ref()) };
> > +
> > + // 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_pinned, buf)
> > + }
> > +}
> > +
> > +impl<T: Driver> 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 = ();
> > +/// type Data<'bound> = Self;
> > +/// 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<'bound>(
> > +/// sdev: &'bound serdev::Device<Core<'_>>,
> > +/// _id_info: Option<&'bound Self::IdInfo>,
> > +/// ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> > +/// sdev.set_baudrate(115200);
> > +/// sdev.write_all(b"Hello\n", serdev::Timeout::Max)?;
> > +/// Ok(MyDriver)
> > +/// }
> > +/// }
> > +///```
> > +#[vtable]
> > +pub trait Driver {
> > + /// 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 type of the driver's bus device private data.
> > + type Data<'bound>: Send + Sync + 'bound;
> > +
> > + /// 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<'bound>(
> > + sdev: &'bound Device<device::Core<'_>>,
> > + id_info: Option<&'bound Self::IdInfo>,
> > + ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
> > +
> > + /// 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 `Drop`.
> > + fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this:
> > Pin<&Self::Data<'bound>>) {
> > + 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<'bound>(
> > + sdev: &'bound Device<device::Bound>,
> > + this: Pin<&Self::Data<'bound>>,
> > + 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()
> > + }
>
> This and many other functions should be `#[inline]`. I am surprised that
> Sashiko
> didn't point out.
Yes, inline would make sense here. In this particular case, I used
platform.rs as base, so the missing inline got inherited.
Thanks
- Markus Probst
>
> Best,
> Gary
signature.asc
Description: This is a digitally signed message part
