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

[PATCH RFC 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 I am currently working on,
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>
---
Kari Argillander (1):
      serdev: Export internal is_serdev_device() for drivers

Markus Probst (3):
      rust: add basic serial device bus abstractions
      samples: rust: add Rust serial device bus sample device driver
      rust: Add serdev rust abstractions to MAINTAINERS file

 MAINTAINERS                        |   3 +
 drivers/tty/serdev/core.c          |   3 +-
 include/linux/serdev.h             |   2 +
 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              | 815 +++++++++++++++++++++++++++++++++++++
 samples/rust/Kconfig               |  10 +
 samples/rust/Makefile              |   1 +
 samples/rust/rust_driver_serdev.rs | 175 ++++++++
 11 files changed, 1034 insertions(+), 1 deletion(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251217-rust_serdev-ee5481e9085c

[PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers

[Markus Probst]

From: Kari Argillander <kari.argillander@gmail.com>

The serdev core has an internal is_serdev_device() helper, but it was
not accessible to drivers. Make it public by declaring it in serdev.h
and exporting the symbol so that modular serdev drivers can rely on it
instead of duplicating type checks.

This allows example future Rust serdev abstraction to have

    TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>

That way using bus is easy for other substystems. Also some other
subsystems expose similar function:

  - bool is_usb_device(const struct device *dev)
  - bool dev_is_pci(const struct device *dev)

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 drivers/tty/serdev/core.c | 3 ++-
 include/linux/serdev.h    | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index b33e708cb245..1f6bf8e826d8 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -69,10 +69,11 @@ static const struct device_type serdev_device_type = {
 	.release	= serdev_device_release,
 };
 
-static bool is_serdev_device(const struct device *dev)
+bool is_serdev_device(const struct device *dev)
 {
 	return dev->type == &serdev_device_type;
 }
+EXPORT_SYMBOL_GPL(is_serdev_device);
 
 static void serdev_ctrl_release(struct device *dev)
 {
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 34562eb99931..0043b6cc6d01 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -116,6 +116,8 @@ static inline struct serdev_controller *to_serdev_controller(struct device *d)
 	return container_of(d, struct serdev_controller, dev);
 }
 
+bool is_serdev_device(const struct device *dev);
+
 static inline void *serdev_device_get_drvdata(const struct serdev_device *serdev)
 {
 	return dev_get_drvdata(&serdev->dev);

-- 
2.51.2

[PATCH RFC 2/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`.

Co-developed-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
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           | 815 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 841 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index a067038b4b42..bec6c6d0913a 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -79,6 +79,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/sched.h>
 #include <linux/security.h>
+#include <linux/serdev.h>
 #include <linux/slab.h>
 #include <linux/task_work.h>
 #include <linux/tracepoint.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 79c72762ad9c..834e9fbb897d 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -50,6 +50,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 f812cf120042..cc71195466b6 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -136,6 +136,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;
 mod static_assert;
diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
new file mode 100644
index 000000000000..0f5ef325a054
--- /dev/null
+++ b/rust/kernel/serdev.rs
@@ -0,0 +1,815 @@
+// 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,
+    container_of,
+    device,
+    driver,
+    error::{
+        from_result,
+        to_result,
+        VTABLE_DEFAULT_ERROR, //
+    },
+    of,
+    prelude::*,
+    time::Jiffies,
+    types::{AlwaysRefCounted, Opaque}, //
+};
+
+use core::{
+    cell::UnsafeCell,
+    marker::PhantomData,
+    mem::offset_of,
+    num::NonZero,
+    ptr::{
+        addr_of_mut, //
+        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`].
+    Value(NonZero<Jiffies>),
+    /// Wait as long as possible.
+    ///
+    /// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
+    MaxScheduleTimeout,
+}
+
+impl Timeout {
+    fn into_jiffies(self) -> isize {
+        match self {
+            Self::Value(value) => value.get().try_into().unwrap_or_default(),
+            Self::MaxScheduleTimeout => 0,
+        }
+    }
+}
+
+/// An adapter for the registration of serial device bus device drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::serdev_device_driver;
+
+    unsafe fn register(
+        sdrv: &Opaque<Self::RegType>,
+        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 `RegType`.
+        to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
+    }
+
+    unsafe fn unregister(sdrv: &Opaque<Self::RegType>) {
+        // SAFETY: `sdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::serdev_device_driver_unregister(sdrv.get()) };
+    }
+}
+
+#[pin_data]
+struct Drvdata<T: Driver + 'static> {
+    #[pin]
+    driver: T,
+    initial_data: UnsafeCell<Option<T::InitialData>>,
+    late_probe_data: UnsafeCell<Option<Pin<KBox<T::LateProbeData>>>>,
+}
+
+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: if T::HAS_WRITE_WAKEUP {
+            Some(Self::write_wakeup_callback)
+        } else {
+            Some(bindings::serdev_device_write_wakeup)
+        },
+    };
+    const INITIAL_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
+        receive_buf: Some(Self::initial_receive_buf_callback),
+        write_wakeup: if T::HAS_WRITE_WAKEUP_INITIAL {
+            Some(Self::initial_write_wakeup_callback)
+        } else {
+            Some(bindings::serdev_device_write_wakeup)
+        },
+    };
+    const NO_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
+        receive_buf: 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(|| {
+            let data = try_pin_init!(Drvdata {
+                driver <- T::probe(sdev, info),
+                initial_data: Some(Default::default()).into(),
+                late_probe_data: None.into(),
+            });
+
+            sdev.as_ref().set_drvdata(data)?;
+
+            // SAFETY: We just set drvdata to `Drvdata<T>`.
+            let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
+
+            // 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::INITIAL_OPS) };
+
+            // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
+            // to a `struct serdev_device`.
+            to_result(unsafe {
+                bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
+            })?;
+
+            // SAFETY: `&data.driver` is guaranteed to be pinned.
+            T::configure(sdev, unsafe { Pin::new_unchecked(&data.driver) }, info)?;
+
+            if !T::HAS_RECEIVE_INITIAL {
+                // SAFETY:
+                // - It is guaranteed that we have exclusive access to `data.initial_data` and
+                //   `data.late_probe_data`.
+                // - We just initialized `data.initial_data` with Some.
+                unsafe { Self::do_late_probe(sdev, data)? };
+            }
+
+            Ok(0)
+        })
+    }
+
+    /// # Safety
+    ///
+    /// The caller must guarantee, that we have exclusive access to `data.initial_data` and
+    /// `data.late_probe_data`. `data.initial_data` must be Some.
+    /// (i. e. `late_probe` has not been called yet).
+    unsafe fn do_late_probe(sdev: &Device<device::CoreInternal>, data: Pin<&Drvdata<T>>) -> Result {
+        // SAFETY: `&data.driver` is guaranteed to be pinned.
+        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
+
+        // SAFETY: The function contract guarantees that we have exclusive access to
+        // `data.initial_data`.
+        let initial_data = unsafe { &mut *data.initial_data.get() };
+
+        // SAFETY: The function contract guarantees that we have exclusive access to
+        // `data.late_probe_data`.
+        let late_probe_data = unsafe { &mut *data.late_probe_data.get() };
+
+        *late_probe_data = Some(KBox::pin_init(
+            T::late_probe(
+                sdev,
+                data_driver,
+                // SAFETY: The function contract guarantees that `data.initial_data` is Some.
+                unsafe { initial_data.take().unwrap_unchecked() },
+            ),
+            GFP_KERNEL,
+        )?);
+        // 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) };
+        Ok(())
+    }
+
+    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_obtain::<Drvdata<T>>() };
+
+        // SAFETY: `&data.driver` is guaranteed to be pinned.
+        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
+
+        // SAFETY:
+        // - `data.late_probe_data` is guaranteed to be pinned.
+        // - It is guaranteed that we have exclusive access to `data.late_probe_data`.
+        let late_probe_data = unsafe {
+            (*data.late_probe_data.get())
+                .as_deref()
+                .map(|data| Pin::new_unchecked(data))
+        };
+
+        T::unbind(sdev, data_driver, late_probe_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<Drvdata<T>>>`.
+        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<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) };
+
+        // SAFETY: `&data.driver` is guaranteed to be pinned.
+        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
+
+        // SAFETY:
+        // - `data.late_probe_data` is guaranteed to be Some.
+        // - `data.late_probe_data` is guaranteed to be pinned.
+        let late_probe_data = unsafe {
+            Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
+        };
+
+        T::receive(sdev, data_driver, late_probe_data, buf)
+    }
+
+    extern "C" fn write_wakeup_callback(sdev: *mut bindings::serdev_device) {
+        // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
+        // pointer to a `struct serdev_device`.
+        //
+        // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
+        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
+
+        // SAFETY: `write_wakeup_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<Drvdata<T>>>`.
+        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
+
+        // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
+
+        // SAFETY: `&data.driver` is guaranteed to be pinned.
+        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
+
+        // SAFETY:
+        // - `data.late_probe_data` is guaranteed to be Some.
+        // - `data.late_probe_data` is guaranteed to be pinned.
+        let late_probe_data = unsafe {
+            Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
+        };
+
+        // SAFETY: As long as the driver implementation meets the safety requirements, this call
+        // is safe.
+        unsafe { T::write_wakeup(sdev, data_driver, late_probe_data) };
+    }
+
+    extern "C" fn initial_receive_buf_callback(
+        sdev: *mut bindings::serdev_device,
+        buf: *const u8,
+        length: usize,
+    ) -> usize {
+        if !T::HAS_RECEIVE_INITIAL {
+            return 0;
+        }
+
+        // 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: `buf` is guaranteed to be non-null and has the size of `length`.
+        let buf = unsafe { core::slice::from_raw_parts(buf, length) };
+
+        // SAFETY: `initial_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<Drvdata<T>>>`.
+        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
+
+        // SAFETY: `&data.driver` is guaranteed to be pinned.
+        let driver_data = unsafe { Pin::new_unchecked(&data.driver) };
+
+        // SAFETY:
+        // - `data.initial_data` is always Some until `InitialReceiveResult::Ready` is
+        //   returned below.
+        // - It is guaranteed that we have exclusive access to `data.initial_data`.
+        let initial_data = unsafe { (*data.initial_data.get()).as_mut().unwrap_unchecked() };
+
+        match T::receive_initial(sdev, driver_data, initial_data, buf) {
+            Ok(InitialReceiveResult::Pending(size)) => size,
+            Ok(InitialReceiveResult::Ready(size)) => {
+                // SAFETY:
+                // - It is guaranteed that we have exclusive access to `data.initial_data` and
+                //   `data.late_probe_data`.
+                // - We just initialized `data.initial_data` with Some.
+                if let Err(err) = unsafe { Self::do_late_probe(sdev, data) } {
+                    dev_err!(sdev.as_ref(), "Unable to late probe device: {err:?}\n");
+                    // 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::NO_OPS) };
+                    return length;
+                }
+                size
+            }
+            Err(err) => {
+                dev_err!(
+                    sdev.as_ref(),
+                    "Unable to receive initial data for probe: {err:?}.\n"
+                );
+                // 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::NO_OPS) };
+                length
+            }
+        }
+    }
+
+    extern "C" fn initial_write_wakeup_callback(sdev: *mut bindings::serdev_device) {
+        // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
+        // pointer to a `struct serdev_device`.
+        //
+        // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
+        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
+
+        // SAFETY: `initial_write_wakeup_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<Drvdata<T>>>`.
+        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
+
+        // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
+
+        // SAFETY: `&data.driver` is guaranteed to be pinned.
+        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
+
+        // SAFETY: As long as the driver implementation meets the safety requirements, this call
+        // is safe.
+        unsafe { T::write_wakeup_initial(sdev, data_driver) };
+    }
+}
+
+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)* });
+    };
+}
+
+/// Result for `receive_initial` in [`Driver`].
+pub enum InitialReceiveResult {
+    /// More data is required.
+    ///
+    /// The inner data is the number of bytes accepted.
+    Pending(usize),
+    /// Ready for late probe.
+    ///
+    /// The inner data is the number of bytes accepted.
+    Ready(usize),
+}
+
+/// 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 LateProbeData = ();
+///     type InitialData = ();
+///     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,
+///         _id_info: Option<&Self::IdInfo>,
+///     ) -> impl PinInit<Self, Error> {
+///         Err(ENODEV)
+///     }
+///
+///     fn configure(
+///         dev: &serdev::Device<Core>,
+///         _this: Pin<&Self>,
+///         _id_info: Option<&Self::IdInfo>,
+///     ) -> Result {
+///         dev.set_baudrate(115200);
+///         Ok(())
+///     }
+///
+///     fn late_probe(
+///        _dev: &serdev::Device<Bound>,
+///        _this: Pin<&Self>,
+///        _initial_data: Self::InitialData,
+///    ) -> impl PinInit<Self::LateProbeData, Error> {
+///        Ok(())
+///    }
+/// }
+///```
+#[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 LateProbeData: Send + 'static = ();
+    // type InitialData: Default + Send + 'static = ();
+    // ```
+    type IdInfo: 'static;
+
+    /// Data returned in `late_probe` function.
+    type LateProbeData: Send + 'static;
+    /// Data used for initial data.
+    type InitialData: Default + Send + '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, id_info: Option<&Self::IdInfo>) -> impl PinInit<Self, Error>;
+
+    /// Serial device bus device driver configure.
+    ///
+    /// Called directly after the serial device bus device has been opened.
+    /// This should be used for setting up the communication (i. e. baudrate, flow control, parity)
+    /// and sending an initial hello if required.
+    fn configure(
+        sdev: &Device<device::Core>,
+        this: Pin<&Self>,
+        id_info: Option<&Self::IdInfo>,
+    ) -> Result;
+
+    /// Serial device bus device data receive callback (initial).
+    ///
+    /// Called when data got received from device, before `late_probe` has been called.
+    /// This should be used, to get information about the device for `late_probe`.
+    ///
+    /// Returns `InitialReceiveResult::Pending` if more data is still required for `late_probe`.
+    /// Otherwise `InitialReceiveResult::Ready`. The inner data is the number of bytes accepted.
+    fn receive_initial(
+        sdev: &Device<device::Bound>,
+        this: Pin<&Self>,
+        initial_data: &mut Self::InitialData,
+        data: &[u8],
+    ) -> Result<InitialReceiveResult> {
+        let _ = (sdev, this, initial_data, data);
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Serial device bus device write wakeup callback (initial).
+    ///
+    /// Called when ready to transmit more data, before `late_probe` has been called.
+    ///
+    /// # Safety
+    ///
+    /// This function must not sleep.
+    unsafe fn write_wakeup_initial(sdev: &Device<device::Core>, this: Pin<&Self>) {
+        let _ = (sdev, this);
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Serial device bus device late probe.
+    ///
+    /// Called after the initial data is available.
+    /// If `receive_initial` isn't implemented, this will be called directly after configure.
+    fn late_probe(
+        sdev: &Device<device::Bound>,
+        this: Pin<&Self>,
+        initial_data: Self::InitialData,
+    ) -> impl PinInit<Self::LateProbeData, 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>,
+        late_probe_this: Option<Pin<&Self::LateProbeData>>,
+    ) {
+        let _ = (sdev, this, late_probe_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>,
+        late_probe_this: Pin<&Self::LateProbeData>,
+        data: &[u8],
+    ) -> usize {
+        let _ = (sdev, this, late_probe_this, data);
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Serial device bus device write wakeup callback.
+    ///
+    /// Called when ready to transmit more data.
+    ///
+    /// # Safety
+    ///
+    /// This function must not sleep.
+    unsafe fn write_wakeup(
+        sdev: &Device<device::Bound>,
+        this: Pin<&Self>,
+        late_probe_this: Pin<&Self::LateProbeData>,
+    ) {
+        let _ = (sdev, this, late_probe_this);
+        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) -> u32 {
+        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
+        unsafe { bindings::serdev_device_set_baudrate(self.as_raw(), speed) }
+    }
+
+    /// 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 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(),
+            )
+        };
+        if ret < 0 {
+            // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
+            // which always fit into a `i32`.
+            Err(Error::from_errno(ret as i32))
+        } else {
+            Ok(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 if interrupted).
+    /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
+    /// before any bytes were written.
+    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()) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(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 { addr_of_mut!((*self.as_raw()).dev) };
+
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &Device<Ctx> {
+    type Error = kernel::error::Error;
+
+    fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
+        // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
+        // `struct device`.
+        if !unsafe { bindings::is_serdev_device(dev.as_raw()) } {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: We've just verified that the device_type of `dev` is
+        // `serdev_device_type`, hence `dev` must be embedded in a valid `struct
+        // serdev_device_driver` as guaranteed by the corresponding C code.
+        let sdev = unsafe { container_of!(dev.as_raw(), bindings::serdev_device, dev) };
+
+        // SAFETY: `sdev` is a valid pointer to a `struct serdev_device_driver`.
+        Ok(unsafe { &*sdev.cast() })
+    }
+}
+
+// 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.51.2

[PATCH RFC 3/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 platform bus abstractions.

This drivers probes through either a match of device / driver name or a
match within the OF ID table.
---
 samples/rust/Kconfig               |  10 +++
 samples/rust/Makefile              |   1 +
 samples/rust/rust_driver_serdev.rs | 175 +++++++++++++++++++++++++++++++++++++
 3 files changed, 186 insertions(+)

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 3efa51bfc8ef..3b6663b4bc9b 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -161,6 +161,16 @@ config SAMPLE_RUST_DRIVER_AUXILIARY
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRIVER_SERDEV
+	tristate "Serial Device Bus Device Driver"
+	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_HOSTPROGS
 	bool "Host programs"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index f65885d1d62b..ec5cb8065fb7 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
 
 rust_print-y := rust_print_main.o rust_print_events.o
diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
new file mode 100644
index 000000000000..f23b38a26c32
--- /dev/null
+++ b/samples/rust/rust_driver_serdev.rs
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust Serial device bus device driver sample.
+
+use kernel::{
+    acpi,
+    device::{
+        self,
+        property::{
+            FwNodeReferenceArgs,
+            NArgs, //
+        },
+        Bound,
+        Core, //
+    },
+    of,
+    prelude::*,
+    serdev,
+    str::CString,
+    sync::aref::ARef, //
+};
+
+struct SampleDriver {
+    sdev: ARef<serdev::Device>,
+}
+
+struct Info(u32);
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <SampleDriver as serdev::Driver>::IdInfo,
+    [(of::DeviceId::new(c"test,rust_driver_serdev"), Info(42))]
+);
+
+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <SampleDriver as serdev::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c"LNUXBEEF"), Info(0))]
+);
+
+#[vtable]
+impl serdev::Driver for SampleDriver {
+    type IdInfo = Info;
+    type InitialData = ();
+    type LateProbeData = ();
+    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, 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 let Some(info) = info {
+            dev_info!(dev, "Probed with info: '{}'.\n", info.0);
+        }
+
+        if dev.fwnode().is_some_and(|node| node.is_of_node()) {
+            Self::properties_parse(dev)?;
+        }
+
+        Ok(Self { sdev: sdev.into() })
+    }
+
+    fn configure(
+        sdev: &serdev::Device<Core>,
+        _this: Pin<&Self>,
+        _id_info: Option<&Self::IdInfo>,
+    ) -> Result {
+        dev_dbg!(
+            sdev.as_ref(),
+            "Configure Rust Serial device bus device driver sample.\n"
+        );
+
+        sdev.set_baudrate(115200);
+        sdev.set_flow_control(false);
+        sdev.set_parity(serdev::Parity::None)?;
+        Ok(())
+    }
+
+    fn late_probe(
+        sdev: &serdev::Device<Bound>,
+        _this: Pin<&Self>,
+        _initial_data: Self::InitialData,
+    ) -> impl PinInit<Self::LateProbeData, Error> {
+        dev_dbg!(
+            sdev.as_ref(),
+            "Late Probe Rust Serial device bus device driver sample.\n"
+        );
+        Ok(())
+    }
+
+    fn receive(
+        sdev: &serdev::Device<Bound>,
+        _this: Pin<&Self>,
+        _late_probe_this: Pin<&Self::LateProbeData>,
+        data: &[u8],
+    ) -> usize {
+        let _ = sdev.write_all(data, serdev::Timeout::MaxScheduleTimeout);
+        data.len()
+    }
+}
+
+impl SampleDriver {
+    fn properties_parse(dev: &device::Device) -> Result {
+        let fwnode = dev.fwnode().ok_or(ENOENT)?;
+
+        if let Ok(idx) = fwnode.property_match_string(c"compatible", c"test,rust-device") {
+            dev_info!(dev, "matched compatible string idx = {}\n", idx);
+        }
+
+        let name = c"compatible";
+        let prop = fwnode.property_read::<CString>(name).required_by(dev)?;
+        dev_info!(dev, "'{name}'='{prop:?}'\n");
+
+        let name = c"test,bool-prop";
+        let prop = fwnode.property_read_bool(c"test,bool-prop");
+        dev_info!(dev, "'{name}'='{prop}'\n");
+
+        if fwnode.property_present(c"test,u32-prop") {
+            dev_info!(dev, "'test,u32-prop' is present\n");
+        }
+
+        let name = c"test,u32-optional-prop";
+        let prop = fwnode.property_read::<u32>(name).or(0x12);
+        dev_info!(dev, "'{name}'='{prop:#x}' (default = 0x12)\n");
+
+        // A missing required property will print an error. Discard the error to
+        // prevent properties_parse from failing in that case.
+        let name = c"test,u32-required-prop";
+        let _ = fwnode.property_read::<u32>(name).required_by(dev);
+
+        let name = c"test,u32-prop";
+        let prop: u32 = fwnode.property_read(name).required_by(dev)?;
+        dev_info!(dev, "'{name}'='{prop:#x}'\n");
+
+        let name = c"test,i16-array";
+        let prop: [i16; 4] = fwnode.property_read(name).required_by(dev)?;
+        dev_info!(dev, "'{name}'='{prop:?}'\n");
+        let len = fwnode.property_count_elem::<u16>(name)?;
+        dev_info!(dev, "'{name}' length is {len}\n");
+
+        let name = c"test,i16-array";
+        let prop: KVec<i16> = fwnode.property_read_array_vec(name, 4)?.required_by(dev)?;
+        dev_info!(dev, "'{name}'='{prop:?}' (KVec)\n");
+
+        for child in fwnode.children() {
+            let name = c"test,ref-arg";
+            let nargs = NArgs::N(2);
+            let prop: FwNodeReferenceArgs = child.property_get_reference_args(name, nargs, 0)?;
+            dev_info!(dev, "'{name}'='{prop:?}'\n");
+        }
+
+        Ok(())
+    }
+}
+
+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.51.2

[PATCH RFC 4/4] rust: Add serdev rust abstractions to MAINTAINERS file

[Markus Probst]

Adds files related to the serial device bus rust abstractions to the
MAINTAINERS file.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5b11839cba9d..929b1c534173 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23718,6 +23718,9 @@ 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
+F:	samples/rust/rust_driver_serdev.rs
 
 SERIAL IR RECEIVER
 M:	Sean Young <sean@mess.org>

-- 
2.51.2

Re: [PATCH RFC 3/4] samples: rust: add Rust serial device bus sample device driver

[Dirk Behme]

Hi Markus,

On 20.12.25 19:44, Markus Probst wrote:
> Add a sample Rust serial device bus device driver illustrating the usage
> of the platform bus abstractions.
> 
> This drivers probes through either a match of device / driver name or a
> match within the OF ID table.
> ---
>  samples/rust/Kconfig               |  10 +++
>  samples/rust/Makefile              |   1 +
>  samples/rust/rust_driver_serdev.rs | 175 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 186 insertions(+)
...
> diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
> new file mode 100644
> index 000000000000..f23b38a26c32
> --- /dev/null
> +++ b/samples/rust/rust_driver_serdev.rs
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust Serial device bus device driver sample.
> +
> +use kernel::{
> +    acpi,
> +    device::{
> +        self,
> +        property::{
> +            FwNodeReferenceArgs,
> +            NArgs, //
> +        },
> +        Bound,
> +        Core, //
> +    },
> +    of,
> +    prelude::*,
> +    serdev,
> +    str::CString,
> +    sync::aref::ARef, //
> +};
> +
> +struct SampleDriver {
> +    sdev: ARef<serdev::Device>,
> +}
> +
> +struct Info(u32);
> +
> +kernel::of_device_table!(
> +    OF_TABLE,
> +    MODULE_OF_TABLE,
> +    <SampleDriver as serdev::Driver>::IdInfo,
> +    [(of::DeviceId::new(c"test,rust_driver_serdev"), Info(42))]

I stopped reading here regarding the new "rust_driver_serdev" but
re-reading Rob's

https://lore.kernel.org/rust-for-linux/20241022234712.GB1848992-robh@kernel.org/

adding "test,<whatever>" should be fine as-is without any documenation.

> +);
> +
> +kernel::acpi_device_table!(
> +    ACPI_TABLE,
> +    MODULE_ACPI_TABLE,
> +    <SampleDriver as serdev::Driver>::IdInfo,
> +    [(acpi::DeviceId::new(c"LNUXBEEF"), Info(0))]
> +);
> +
> +#[vtable]
> +impl serdev::Driver for SampleDriver {
> +    type IdInfo = Info;
> +    type InitialData = ();
> +    type LateProbeData = ();
> +    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, 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 let Some(info) = info {
> +            dev_info!(dev, "Probed with info: '{}'.\n", info.0);
> +        }

Last time I had a look to the log output from rust_driver_platform.rs
(where this is copied from?) I was slightly confused to see the
"Probed with ..." in the log but not the "Probe Rust ...". Well, I
hadn't DEBUG enabled. So I wonder if the combination of `dev_dbg!()`
and `dev_info!()` this way should be improved? At least in
rust_driver_platform.rs because we could drop that here completely? I
mean in rust_driver_platform.rs it makes sense to demonstrate how
`info` is supposed to work. But do we need that here?


> +        if dev.fwnode().is_some_and(|node| node.is_of_node()) {
> +            Self::properties_parse(dev)?;
> +        }


This is a left over from copy & paste? I mean having all this
`properties_parse()` below and calling it here does not make any sense
here? And should be dropped completely?


> +
> +        Ok(Self { sdev: sdev.into() })
> +    }
> +
> +    fn configure(
> +        sdev: &serdev::Device<Core>,
> +        _this: Pin<&Self>,
> +        _id_info: Option<&Self::IdInfo>,
> +    ) -> Result {
> +        dev_dbg!(
> +            sdev.as_ref(),
> +            "Configure Rust Serial device bus device driver sample.\n"
> +        );
> +
> +        sdev.set_baudrate(115200);
> +        sdev.set_flow_control(false);
> +        sdev.set_parity(serdev::Parity::None)?;
> +        Ok(())
> +    }
> +
> +    fn late_probe(
> +        sdev: &serdev::Device<Bound>,
> +        _this: Pin<&Self>,
> +        _initial_data: Self::InitialData,
> +    ) -> impl PinInit<Self::LateProbeData, Error> {
> +        dev_dbg!(
> +            sdev.as_ref(),
> +            "Late Probe Rust Serial device bus device driver sample.\n"
> +        );
> +        Ok(())
> +    }
> +
> +    fn receive(
> +        sdev: &serdev::Device<Bound>,
> +        _this: Pin<&Self>,
> +        _late_probe_this: Pin<&Self::LateProbeData>,
> +        data: &[u8],
> +    ) -> usize {
> +        let _ = sdev.write_all(data, serdev::Timeout::MaxScheduleTimeout);


Is it intended to have a function with the name `receive()`calling
`write()`?

> +        data.len()
> +    }
> +}
> +
> +impl SampleDriver {
> +    fn properties_parse(dev: &device::Device) -> Result {


As mentioned above I think this is a left over from copy & paste and
should be dropped?

Cheers,

Dirk

Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions

[Dirk Behme]

Hi Markus,

On 20.12.25 19:44, 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`.
> 
> Co-developed-by: Kari Argillander <kari.argillander@gmail.com>
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> 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           | 815 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 841 insertions(+)
> 
...
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> new file mode 100644
> index 000000000000..0f5ef325a054
> --- /dev/null
> +++ b/rust/kernel/serdev.rs
....
> +    /// 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 if interrupted).

Should it be "less than data.len"? Instead of just "data"? Same in the
comment for `write()`below.


> +    /// [`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(),
> +            )
> +        };
> +        if ret < 0 {
> +            // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> +            // which always fit into a `i32`.
> +            Err(Error::from_errno(ret as i32))
> +        } else {
> +            Ok(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 if interrupted).
> +    /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> +    /// before any bytes were written.
> +    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()) };
> +        if ret < 0 {
> +            Err(Error::from_errno(ret))
> +        } else {
> +            Ok(ret.unsigned_abs())
> +        }
> +    }

Best regards

Dirk

Re: [PATCH RFC 3/4] samples: rust: add Rust serial device bus sample device driver

[Markus Probst]

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

Hi Dirk,

On Sun, 2025-12-21 at 10:11 +0100, Dirk Behme wrote:
> Hi Markus,
> 
> On 20.12.25 19:44, Markus Probst wrote:
> > Add a sample Rust serial device bus device driver illustrating the usage
> > of the platform bus abstractions.
> > 
> > This drivers probes through either a match of device / driver name or a
> > match within the OF ID table.
> > ---
> >  samples/rust/Kconfig               |  10 +++
> >  samples/rust/Makefile              |   1 +
> >  samples/rust/rust_driver_serdev.rs | 175 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 186 insertions(+)
> ...
> > diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
> > new file mode 100644
> > index 000000000000..f23b38a26c32
> > --- /dev/null
> > +++ b/samples/rust/rust_driver_serdev.rs
> > @@ -0,0 +1,175 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Rust Serial device bus device driver sample.
> > +
> > +use kernel::{
> > +    acpi,
> > +    device::{
> > +        self,
> > +        property::{
> > +            FwNodeReferenceArgs,
> > +            NArgs, //
> > +        },
> > +        Bound,
> > +        Core, //
> > +    },
> > +    of,
> > +    prelude::*,
> > +    serdev,
> > +    str::CString,
> > +    sync::aref::ARef, //
> > +};
> > +
> > +struct SampleDriver {
> > +    sdev: ARef<serdev::Device>,
> > +}
> > +
> > +struct Info(u32);
> > +
> > +kernel::of_device_table!(
> > +    OF_TABLE,
> > +    MODULE_OF_TABLE,
> > +    <SampleDriver as serdev::Driver>::IdInfo,
> > +    [(of::DeviceId::new(c"test,rust_driver_serdev"), Info(42))]
> 
> I stopped reading here regarding the new "rust_driver_serdev" but
> re-reading Rob's
> 
> https://lore.kernel.org/rust-for-linux/20241022234712.GB1848992-robh@kernel.org/
> 
> adding "test,<whatever>" should be fine as-is without any documenation.
> 
> > +);
> > +
> > +kernel::acpi_device_table!(
> > +    ACPI_TABLE,
> > +    MODULE_ACPI_TABLE,
> > +    <SampleDriver as serdev::Driver>::IdInfo,
> > +    [(acpi::DeviceId::new(c"LNUXBEEF"), Info(0))]
> > +);
> > +
> > +#[vtable]
> > +impl serdev::Driver for SampleDriver {
> > +    type IdInfo = Info;
> > +    type InitialData = ();
> > +    type LateProbeData = ();
> > +    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, 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 let Some(info) = info {
> > +            dev_info!(dev, "Probed with info: '{}'.\n", info.0);
> > +        }
> 
> Last time I had a look to the log output from rust_driver_platform.rs
> (where this is copied from?) I was slightly confused to see the
> "Probed with ..." in the log but not the "Probe Rust ...". Well, I
> hadn't DEBUG enabled. So I wonder if the combination of `dev_dbg!()`
> and `dev_info!()` this way should be improved? At least in
> rust_driver_platform.rs because we could drop that here completely? I
> mean in rust_driver_platform.rs it makes sense to demonstrate how
> `info` is supposed to work. But do we need that here?
> 
> 
> > +        if dev.fwnode().is_some_and(|node| node.is_of_node()) {
> > +            Self::properties_parse(dev)?;
> > +        }
> 
> 
> This is a left over from copy & paste? I mean having all this
> `properties_parse()` below and calling it here does not make any sense
> here? And should be dropped completely?
It was meant to show, that this is the function in which fwnode
properties should be parsed. As I did indeed base the sample on the
platform driver sample, I left it as-is. I will minimize it down to 1
property without an extra function, given that the platform driver
sample already shows how to use properties.
> 
> 
> > +
> > +        Ok(Self { sdev: sdev.into() })
> > +    }
> > +
> > +    fn configure(
> > +        sdev: &serdev::Device<Core>,
> > +        _this: Pin<&Self>,
> > +        _id_info: Option<&Self::IdInfo>,
> > +    ) -> Result {
> > +        dev_dbg!(
> > +            sdev.as_ref(),
> > +            "Configure Rust Serial device bus device driver sample.\n"
> > +        );
> > +
> > +        sdev.set_baudrate(115200);
> > +        sdev.set_flow_control(false);
> > +        sdev.set_parity(serdev::Parity::None)?;
> > +        Ok(())
> > +    }
> > +
> > +    fn late_probe(
> > +        sdev: &serdev::Device<Bound>,
> > +        _this: Pin<&Self>,
> > +        _initial_data: Self::InitialData,
> > +    ) -> impl PinInit<Self::LateProbeData, Error> {
> > +        dev_dbg!(
> > +            sdev.as_ref(),
> > +            "Late Probe Rust Serial device bus device driver sample.\n"
> > +        );
> > +        Ok(())
> > +    }
> > +
> > +    fn receive(
> > +        sdev: &serdev::Device<Bound>,
> > +        _this: Pin<&Self>,
> > +        _late_probe_this: Pin<&Self::LateProbeData>,
> > +        data: &[u8],
> > +    ) -> usize {
> > +        let _ = sdev.write_all(data, serdev::Timeout::MaxScheduleTimeout);
> 
> 
> Is it intended to have a function with the name `receive()`calling
> `write()`?
This sample driver echos any data it receives back to the serial
device.

It shows how to receive data from the serial device and how to send
data to the serial device.

It depends on the driver, if it is necessary to call write inside
receive. The serial device may want a reply or ack from the driver
after it sent a message.

> 
> > +        data.len()
> > +    }
> > +}
> > +
> > +impl SampleDriver {
> > +    fn properties_parse(dev: &device::Device) -> Result {
> 
> 
> As mentioned above I think this is a left over from copy & paste and
> should be dropped?
> 
> Cheers,
> 
> Dirk

Thanks
- Markus Probst

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

Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions

[Markus Probst]

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

Hi Dirk,

On Sun, 2025-12-21 at 10:19 +0100, Dirk Behme wrote:
> Hi Markus,
> 
> On 20.12.25 19:44, 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`.
> > 
> > Co-developed-by: Kari Argillander <kari.argillander@gmail.com>
> > Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> > 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           | 815 ++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 841 insertions(+)
> > 
> ...
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > new file mode 100644
> > index 000000000000..0f5ef325a054
> > --- /dev/null
> > +++ b/rust/kernel/serdev.rs
> ....
> > +    /// 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 if interrupted).
> 
> Should it be "less than data.len"? Instead of just "data"? Same in the
> comment for `write()`below.
Yes.
> 
> 
> > +    /// [`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(),
> > +            )
> > +        };
> > +        if ret < 0 {
> > +            // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> > +            // which always fit into a `i32`.
> > +            Err(Error::from_errno(ret as i32))
> > +        } else {
> > +            Ok(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 if interrupted).
> > +    /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> > +    /// before any bytes were written.
> > +    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()) };
> > +        if ret < 0 {
> > +            Err(Error::from_errno(ret))
> > +        } else {
> > +            Ok(ret.unsigned_abs())
> > +        }
> > +    }
> 
> Best regards
> 
> Dirk

Thanks
- Markus Probst

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

Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers

[Greg Kroah-Hartman]

On Sat, Dec 20, 2025 at 06:44:05PM +0000, Markus Probst wrote:
> From: Kari Argillander <kari.argillander@gmail.com>
> 
> The serdev core has an internal is_serdev_device() helper, but it was
> not accessible to drivers. Make it public by declaring it in serdev.h
> and exporting the symbol so that modular serdev drivers can rely on it
> instead of duplicating type checks.
> 
> This allows example future Rust serdev abstraction to have
> 
>     TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>

But why is that going to be needed?

> That way using bus is easy for other substystems. Also some other
> subsystems expose similar function:
> 
>   - bool is_usb_device(const struct device *dev)
>   - bool dev_is_pci(const struct device *dev)

Yes, and usually that's not a good idea, unless you have a bus with
multiple types of devices on it.  I don't think serdev has that, does
it?

Only under special circumstances should this be required, so I'm curious
as to why you would ever have a pointer to a struct device and not
"know" that it is of this type?  Who is passing that to you?

thanks,

greg k-h

Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers

[Markus Probst]

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

On Sun, 2025-12-21 at 17:10 +0100, Greg Kroah-Hartman wrote:
> On Sat, Dec 20, 2025 at 06:44:05PM +0000, Markus Probst wrote:
> > From: Kari Argillander <kari.argillander@gmail.com>
> > 
> > The serdev core has an internal is_serdev_device() helper, but it was
> > not accessible to drivers. Make it public by declaring it in serdev.h
> > and exporting the symbol so that modular serdev drivers can rely on it
> > instead of duplicating type checks.
> > 
> > This allows example future Rust serdev abstraction to have
> > 
> >     TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>
> 
> But why is that going to be needed?
> 
> > That way using bus is easy for other substystems. Also some other
> > subsystems expose similar function:
> > 
> >   - bool is_usb_device(const struct device *dev)
> >   - bool dev_is_pci(const struct device *dev)
> 
> Yes, and usually that's not a good idea, unless you have a bus with
> multiple types of devices on it.  I don't think serdev has that, does
> it?
> 
> Only under special circumstances should this be required, so I'm curious
> as to why you would ever have a pointer to a struct device and not
> "know" that it is of this type?  Who is passing that to you?
For example, the pwm rust abstraction currently only provides a
`device::Device<device::Bound>` reference in callbacks [1]. If we want
to write data to the serial device in one of the pwm callbacks, we need
to convert the `device::Device<device::Bound>` reference to
`serdev::Device<device::Bound>`. The TryFrom implementation provides a
*safe* abstraction.

At least in the pwm example, this could/should be circumvented with the
`AsBusDevice` trait [2].

But this technically also applies to all other bus device abstractions
and they also have a `TryFrom<&device::Device<Ctx>>` implementation, so
with this it would be consistent across all rust bus device
abstractions.

Thanks
- Markus Probst

[1] https://rust.docs.kernel.org/kernel/pwm/trait.PwmOps.html

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/rust/kernel/device.rs?id=e4addc7cc2dfcc19f1c8c8e47f3834b22cb21559

> 
> thanks,
> 
> greg k-h

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

Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers

[Greg Kroah-Hartman]

On Sun, Dec 21, 2025 at 04:28:11PM +0000, Markus Probst wrote:
> On Sun, 2025-12-21 at 17:10 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Dec 20, 2025 at 06:44:05PM +0000, Markus Probst wrote:
> > > From: Kari Argillander <kari.argillander@gmail.com>
> > > 
> > > The serdev core has an internal is_serdev_device() helper, but it was
> > > not accessible to drivers. Make it public by declaring it in serdev.h
> > > and exporting the symbol so that modular serdev drivers can rely on it
> > > instead of duplicating type checks.
> > > 
> > > This allows example future Rust serdev abstraction to have
> > > 
> > >     TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>
> > 
> > But why is that going to be needed?
> > 
> > > That way using bus is easy for other substystems. Also some other
> > > subsystems expose similar function:
> > > 
> > >   - bool is_usb_device(const struct device *dev)
> > >   - bool dev_is_pci(const struct device *dev)
> > 
> > Yes, and usually that's not a good idea, unless you have a bus with
> > multiple types of devices on it.  I don't think serdev has that, does
> > it?
> > 
> > Only under special circumstances should this be required, so I'm curious
> > as to why you would ever have a pointer to a struct device and not
> > "know" that it is of this type?  Who is passing that to you?
> For example, the pwm rust abstraction currently only provides a
> `device::Device<device::Bound>` reference in callbacks [1]. If we want
> to write data to the serial device in one of the pwm callbacks, we need
> to convert the `device::Device<device::Bound>` reference to
> `serdev::Device<device::Bound>`. The TryFrom implementation provides a
> *safe* abstraction.

While I like the feeling of *safe* you can mark it as *safe* as the
driver core can NOT give you a callback of a pointer that is not of that
type.

That's how the C code works today, and is why the C code does not need
this function exported.  The rust code should do the same thing.

thanks,

greg k-h

Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers

[Danilo Krummrich]

On Sun Dec 21, 2025 at 5:28 PM CET, Markus Probst wrote:
> On Sun, 2025-12-21 at 17:10 +0100, Greg Kroah-Hartman wrote:
>> Yes, and usually that's not a good idea, unless you have a bus with
>> multiple types of devices on it.  I don't think serdev has that, does
>> it?
>> 
>> Only under special circumstances should this be required, so I'm curious
>> as to why you would ever have a pointer to a struct device and not
>> "know" that it is of this type?  Who is passing that to you?
> For example, the pwm rust abstraction currently only provides a
> `device::Device<device::Bound>` reference in callbacks [1]. If we want
> to write data to the serial device in one of the pwm callbacks, we need
> to convert the `device::Device<device::Bound>` reference to
> `serdev::Device<device::Bound>`. The TryFrom implementation provides a
> *safe* abstraction.
>
> At least in the pwm example, this could/should be circumvented with the
> `AsBusDevice` trait [2].

Yes, class device implementations should start using AsBusDevice, so you don't
need the TryFrom upcast.

> But this technically also applies to all other bus device abstractions
> and they also have a `TryFrom<&device::Device<Ctx>>` implementation, so
> with this it would be consistent across all rust bus device
> abstractions.

It should only be PCI and platform and they have it for a reason ()which is the
one Greg already mentioned, i.e. when you potentially have multiple device types
on a bus).

The concrete use-case that motivated adding those for PCI and platform is when
they interact with the auxiliary bus: Imagine a driver that supports devices
from the platform bus and the PCI bus and exports an auxiliary device.

In this case, when you call back into the parent driver from the auxiliary
device the parent driver has to upcast.

This exact constellation is what is implemented by nova-core and nova-drm.

However, outside of such a constellation we should avoid doing upcasts with
TryFrom and make the correct type available in the first place.

- Danilo

Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers

[Danilo Krummrich]

On Sat Dec 20, 2025 at 7:44 PM CET, Markus Probst wrote:
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index b33e708cb245..1f6bf8e826d8 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -69,10 +69,11 @@ static const struct device_type serdev_device_type = {
>  	.release	= serdev_device_release,
>  };
>  
> -static bool is_serdev_device(const struct device *dev)
> +bool is_serdev_device(const struct device *dev)
>  {
>  	return dev->type == &serdev_device_type;
>  }
> +EXPORT_SYMBOL_GPL(is_serdev_device);
>  
>  static void serdev_ctrl_release(struct device *dev)
>  {
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index 34562eb99931..0043b6cc6d01 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -116,6 +116,8 @@ static inline struct serdev_controller *to_serdev_controller(struct device *d)
>  	return container_of(d, struct serdev_controller, dev);
>  }
>  
> +bool is_serdev_device(const struct device *dev);
> +
>  static inline void *serdev_device_get_drvdata(const struct serdev_device *serdev)
>  {
>  	return dev_get_drvdata(&serdev->dev);

Besides what I mentioned in [1] (which should make the patch obsolete anyways),
I want to point out that such cases should be solved with a helper in
rust/helpers/ instead.

[1] https://lore.kernel.org/lkml/DF42RR0I52L3.1JET4R0KFDPPH@kernel.org/

Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions

[Kari Argillander]

On 20.12.2025 klo 20.44 Markus Probst (markus.probst@posteo.de) kirjoitti:
>
> 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`.
>
> Co-developed-by: Kari Argillander <kari.argillander@gmail.com>
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> 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           | 815 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 841 insertions(+)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index a067038b4b42..bec6c6d0913a 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -79,6 +79,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/sched.h>
>  #include <linux/security.h>
> +#include <linux/serdev.h>
>  #include <linux/slab.h>
>  #include <linux/task_work.h>
>  #include <linux/tracepoint.h>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c..834e9fbb897d 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -50,6 +50,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 f812cf120042..cc71195466b6 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -136,6 +136,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;
>  mod static_assert;
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> new file mode 100644
> index 000000000000..0f5ef325a054
> --- /dev/null
> +++ b/rust/kernel/serdev.rs
> @@ -0,0 +1,815 @@
> +// 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,
> +    container_of,
> +    device,
> +    driver,
> +    error::{
> +        from_result,
> +        to_result,
> +        VTABLE_DEFAULT_ERROR, //
> +    },
> +    of,
> +    prelude::*,
> +    time::Jiffies,
> +    types::{AlwaysRefCounted, Opaque}, //
> +};
> +
> +use core::{
> +    cell::UnsafeCell,
> +    marker::PhantomData,
> +    mem::offset_of,
> +    num::NonZero,
> +    ptr::{
> +        addr_of_mut, //
> +        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`].
> +    Value(NonZero<Jiffies>),
> +    /// Wait as long as possible.
> +    ///
> +    /// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
> +    MaxScheduleTimeout,
> +}
> +
> +impl Timeout {
> +    fn into_jiffies(self) -> isize {
> +        match self {
> +            Self::Value(value) => value.get().try_into().unwrap_or_default(),
> +            Self::MaxScheduleTimeout => 0,
> +        }
> +    }
> +}
> +
> +/// An adapter for the registration of serial device bus device drivers.
> +pub struct Adapter<T: Driver>(T);
> +
> +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
> +// a preceding call to `register` has been successful.
> +unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
> +    type RegType = bindings::serdev_device_driver;
> +
> +    unsafe fn register(
> +        sdrv: &Opaque<Self::RegType>,
> +        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 `RegType`.
> +        to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
> +    }
> +
> +    unsafe fn unregister(sdrv: &Opaque<Self::RegType>) {
> +        // SAFETY: `sdrv` is guaranteed to be a valid `RegType`.
> +        unsafe { bindings::serdev_device_driver_unregister(sdrv.get()) };
> +    }
> +}
> +
> +#[pin_data]
> +struct Drvdata<T: Driver + 'static> {
> +    #[pin]
> +    driver: T,
> +    initial_data: UnsafeCell<Option<T::InitialData>>,
> +    late_probe_data: UnsafeCell<Option<Pin<KBox<T::LateProbeData>>>>,
> +}
> +
> +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: if T::HAS_WRITE_WAKEUP {
> +            Some(Self::write_wakeup_callback)
> +        } else {
> +            Some(bindings::serdev_device_write_wakeup)
> +        },
> +    };
> +    const INITIAL_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> +        receive_buf: Some(Self::initial_receive_buf_callback),
> +        write_wakeup: if T::HAS_WRITE_WAKEUP_INITIAL {
> +            Some(Self::initial_write_wakeup_callback)
> +        } else {
> +            Some(bindings::serdev_device_write_wakeup)
> +        },
> +    };
> +    const NO_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> +        receive_buf: 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(|| {
> +            let data = try_pin_init!(Drvdata {
> +                driver <- T::probe(sdev, info),
> +                initial_data: Some(Default::default()).into(),
> +                late_probe_data: None.into(),
> +            });
> +
> +            sdev.as_ref().set_drvdata(data)?;
> +
> +            // SAFETY: We just set drvdata to `Drvdata<T>`.
> +            let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +            // 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::INITIAL_OPS) };
> +
> +            // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
> +            // to a `struct serdev_device`.
> +            to_result(unsafe {
> +                bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
> +            })?;
> +
> +            // SAFETY: `&data.driver` is guaranteed to be pinned.
> +            T::configure(sdev, unsafe { Pin::new_unchecked(&data.driver) }, info)?;
> +
> +            if !T::HAS_RECEIVE_INITIAL {
> +                // SAFETY:
> +                // - It is guaranteed that we have exclusive access to `data.initial_data` and
> +                //   `data.late_probe_data`.
> +                // - We just initialized `data.initial_data` with Some.
> +                unsafe { Self::do_late_probe(sdev, data)? };
> +            }
> +
> +            Ok(0)
> +        })
> +    }
> +
> +    /// # Safety
> +    ///
> +    /// The caller must guarantee, that we have exclusive access to `data.initial_data` and
> +    /// `data.late_probe_data`. `data.initial_data` must be Some.
> +    /// (i. e. `late_probe` has not been called yet).
> +    unsafe fn do_late_probe(sdev: &Device<device::CoreInternal>, data: Pin<&Drvdata<T>>) -> Result {
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY: The function contract guarantees that we have exclusive access to
> +        // `data.initial_data`.
> +        let initial_data = unsafe { &mut *data.initial_data.get() };
> +
> +        // SAFETY: The function contract guarantees that we have exclusive access to
> +        // `data.late_probe_data`.
> +        let late_probe_data = unsafe { &mut *data.late_probe_data.get() };
> +
> +        *late_probe_data = Some(KBox::pin_init(
> +            T::late_probe(
> +                sdev,
> +                data_driver,
> +                // SAFETY: The function contract guarantees that `data.initial_data` is Some.
> +                unsafe { initial_data.take().unwrap_unchecked() },
> +            ),
> +            GFP_KERNEL,
> +        )?);
> +        // 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) };
> +        Ok(())
> +    }
> +
> +    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_obtain::<Drvdata<T>>() };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.late_probe_data` is guaranteed to be pinned.
> +        // - It is guaranteed that we have exclusive access to `data.late_probe_data`.
> +        let late_probe_data = unsafe {
> +            (*data.late_probe_data.get())
> +                .as_deref()
> +                .map(|data| Pin::new_unchecked(data))
> +        };
> +
> +        T::unbind(sdev, data_driver, late_probe_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<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) };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.late_probe_data` is guaranteed to be Some.
> +        // - `data.late_probe_data` is guaranteed to be pinned.
> +        let late_probe_data = unsafe {
> +            Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> +        };
> +
> +        T::receive(sdev, data_driver, late_probe_data, buf)
> +    }
> +
> +    extern "C" fn write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> +        // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> +        // pointer to a `struct serdev_device`.
> +        //
> +        // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
> +        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> +
> +        // SAFETY: `write_wakeup_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +        // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> +        unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.late_probe_data` is guaranteed to be Some.
> +        // - `data.late_probe_data` is guaranteed to be pinned.
> +        let late_probe_data = unsafe {
> +            Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> +        };
> +
> +        // SAFETY: As long as the driver implementation meets the safety requirements, this call
> +        // is safe.
> +        unsafe { T::write_wakeup(sdev, data_driver, late_probe_data) };
> +    }
> +
> +    extern "C" fn initial_receive_buf_callback(
> +        sdev: *mut bindings::serdev_device,
> +        buf: *const u8,
> +        length: usize,
> +    ) -> usize {
> +        if !T::HAS_RECEIVE_INITIAL {
> +            return 0;
> +        }
> +
> +        // 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: `buf` is guaranteed to be non-null and has the size of `length`.
> +        let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> +
> +        // SAFETY: `initial_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let driver_data = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.initial_data` is always Some until `InitialReceiveResult::Ready` is
> +        //   returned below.
> +        // - It is guaranteed that we have exclusive access to `data.initial_data`.
> +        let initial_data = unsafe { (*data.initial_data.get()).as_mut().unwrap_unchecked() };
> +
> +        match T::receive_initial(sdev, driver_data, initial_data, buf) {
> +            Ok(InitialReceiveResult::Pending(size)) => size,
> +            Ok(InitialReceiveResult::Ready(size)) => {
> +                // SAFETY:
> +                // - It is guaranteed that we have exclusive access to `data.initial_data` and
> +                //   `data.late_probe_data`.
> +                // - We just initialized `data.initial_data` with Some.
> +                if let Err(err) = unsafe { Self::do_late_probe(sdev, data) } {
> +                    dev_err!(sdev.as_ref(), "Unable to late probe device: {err:?}\n");
> +                    // 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::NO_OPS) };
> +                    return length;
> +                }
> +                size
> +            }
> +            Err(err) => {
> +                dev_err!(
> +                    sdev.as_ref(),
> +                    "Unable to receive initial data for probe: {err:?}.\n"
> +                );
> +                // 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::NO_OPS) };
> +                length
> +            }
> +        }
> +    }
> +
> +    extern "C" fn initial_write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> +        // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> +        // pointer to a `struct serdev_device`.
> +        //
> +        // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
> +        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> +
> +        // SAFETY: `initial_write_wakeup_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +        // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> +        unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY: As long as the driver implementation meets the safety requirements, this call
> +        // is safe.
> +        unsafe { T::write_wakeup_initial(sdev, data_driver) };
> +    }
> +}
> +
> +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)* });
> +    };
> +}
> +
> +/// Result for `receive_initial` in [`Driver`].
> +pub enum InitialReceiveResult {
> +    /// More data is required.
> +    ///
> +    /// The inner data is the number of bytes accepted.
> +    Pending(usize),
> +    /// Ready for late probe.
> +    ///
> +    /// The inner data is the number of bytes accepted.
> +    Ready(usize),
> +}
> +
> +/// 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 LateProbeData = ();
> +///     type InitialData = ();
> +///     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,
> +///         _id_info: Option<&Self::IdInfo>,
> +///     ) -> impl PinInit<Self, Error> {
> +///         Err(ENODEV)
> +///     }
> +///
> +///     fn configure(
> +///         dev: &serdev::Device<Core>,
> +///         _this: Pin<&Self>,
> +///         _id_info: Option<&Self::IdInfo>,
> +///     ) -> Result {
> +///         dev.set_baudrate(115200);
> +///         Ok(())
> +///     }
> +///
> +///     fn late_probe(
> +///        _dev: &serdev::Device<Bound>,
> +///        _this: Pin<&Self>,
> +///        _initial_data: Self::InitialData,
> +///    ) -> impl PinInit<Self::LateProbeData, Error> {
> +///        Ok(())
> +///    }
> +/// }
> +///```
> +#[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 LateProbeData: Send + 'static = ();
> +    // type InitialData: Default + Send + 'static = ();
> +    // ```
> +    type IdInfo: 'static;
> +
> +    /// Data returned in `late_probe` function.
> +    type LateProbeData: Send + 'static;
> +    /// Data used for initial data.
> +    type InitialData: Default + Send + '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, id_info: Option<&Self::IdInfo>) -> impl PinInit<Self, Error>;
> +
> +    /// Serial device bus device driver configure.
> +    ///
> +    /// Called directly after the serial device bus device has been opened.
> +    /// This should be used for setting up the communication (i. e. baudrate, flow control, parity)
> +    /// and sending an initial hello if required.
> +    fn configure(
> +        sdev: &Device<device::Core>,
> +        this: Pin<&Self>,
> +        id_info: Option<&Self::IdInfo>,
> +    ) -> Result;
> +
> +    /// Serial device bus device data receive callback (initial).
> +    ///
> +    /// Called when data got received from device, before `late_probe` has been called.
> +    /// This should be used, to get information about the device for `late_probe`.
> +    ///
> +    /// Returns `InitialReceiveResult::Pending` if more data is still required for `late_probe`.
> +    /// Otherwise `InitialReceiveResult::Ready`. The inner data is the number of bytes accepted.
> +    fn receive_initial(
> +        sdev: &Device<device::Bound>,
> +        this: Pin<&Self>,
> +        initial_data: &mut Self::InitialData,
> +        data: &[u8],
> +    ) -> Result<InitialReceiveResult> {
> +        let _ = (sdev, this, initial_data, data);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Serial device bus device write wakeup callback (initial).
> +    ///
> +    /// Called when ready to transmit more data, before `late_probe` has been called.
> +    ///
> +    /// # Safety
> +    ///
> +    /// This function must not sleep.
> +    unsafe fn write_wakeup_initial(sdev: &Device<device::Core>, this: Pin<&Self>) {
> +        let _ = (sdev, this);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Serial device bus device late probe.
> +    ///
> +    /// Called after the initial data is available.
> +    /// If `receive_initial` isn't implemented, this will be called directly after configure.
> +    fn late_probe(
> +        sdev: &Device<device::Bound>,
> +        this: Pin<&Self>,
> +        initial_data: Self::InitialData,
> +    ) -> impl PinInit<Self::LateProbeData, 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>,
> +        late_probe_this: Option<Pin<&Self::LateProbeData>>,
> +    ) {
> +        let _ = (sdev, this, late_probe_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>,
> +        late_probe_this: Pin<&Self::LateProbeData>,
> +        data: &[u8],
> +    ) -> usize {
> +        let _ = (sdev, this, late_probe_this, data);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Serial device bus device write wakeup callback.
> +    ///
> +    /// Called when ready to transmit more data.
> +    ///
> +    /// # Safety
> +    ///
> +    /// This function must not sleep.
> +    unsafe fn write_wakeup(
> +        sdev: &Device<device::Bound>,
> +        this: Pin<&Self>,
> +        late_probe_this: Pin<&Self::LateProbeData>,
> +    ) {
> +        let _ = (sdev, this, late_probe_this);
> +        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) -> u32 {
> +        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> +        unsafe { bindings::serdev_device_set_baudrate(self.as_raw(), speed) }
> +    }
> +
> +    /// 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 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(),
> +            )
> +        };
> +        if ret < 0 {
> +            // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> +            // which always fit into a `i32`.
> +            Err(Error::from_errno(ret as i32))
> +        } else {
> +            Ok(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 if interrupted).
> +    /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> +    /// before any bytes were written.
> +    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()) };
> +        if ret < 0 {
> +            Err(Error::from_errno(ret))
> +        } else {
> +            Ok(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 { addr_of_mut!((*self.as_raw()).dev) };
> +
> +        // SAFETY: `dev` points to a valid `struct device`.
> +        unsafe { device::Device::from_raw(dev) }
> +    }
> +}
> +
> +impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &Device<Ctx> {
> +    type Error = kernel::error::Error;
> +
> +    fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
> +        // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
> +        // `struct device`.
> +        if !unsafe { bindings::is_serdev_device(dev.as_raw()) } {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY: We've just verified that the device_type of `dev` is
> +        // `serdev_device_type`, hence `dev` must be embedded in a valid `struct
> +        // serdev_device_driver` as guaranteed by the corresponding C code.
> +        let sdev = unsafe { container_of!(dev.as_raw(), bindings::serdev_device, dev) };
> +
> +        // SAFETY: `sdev` is a valid pointer to a `struct serdev_device_driver`.
> +        Ok(unsafe { &*sdev.cast() })
> +    }
> +}
> +
> +// 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.51.2
>
>

Currently I'm not a huge fan that we need to have configure(),
receive_initial(), write_wakeup_initial() and late_probe(). Also I do not
like that we hide hide open() completely from user. Maybe user want's to do
something if there is error. This does not feel very ergonomic or standard
kernel way. So after thinking a bit more about this idea I have come up with
a different approach.

We will have just probe() as usual. In probe() user will be forced to do open()
and finnish() on serdev device. Between open() and finnish() user can
configure the device and do initial reads / writes as needed. After probe is
finished we will switch to runtime ops.

This is not production ready code and I have just tested the idea locally
without kernel. Here is example how probe() could look like:

```rust
fn probe(sdev: &serdev::Device<Core>, _id_info:
Option<&Self::IdInfo>)-> impl PinInit<(Self, ProbeEndToken), Error> {
    let sdev= sdev.open()?;
    // After open we can configure and do initial reads / writes.
    sdev.set_baudrate(9600);

    // Here we can also write if needed.

    for _ in 0..8 {
        // For initial reads we will have iterator. This cannot be
done after finish().
        let _ = sdev.next_byte(Duration::from_millis(300));
    }

    // We do finnish so user cannot store "initial sdev" anywhere. Also get
    // end_token so probe cannot return unless open() and finnish() are called.
    let (sdev, end_token) = sdev.finish();
    Ok(try_pin_init!((Self {sdev}, end_token)))
}
```

then our receive_initial will look something like this. (Not runnable
on kernel).
Idea is that we have event for next_byte(). So initial read is little bit slower
but that should not be used so much that it matters in practice.

```rust
extern "C" fn initial_receive_buf(dev: *mut serdev_device, buf: *const
u8, count: usize) -> usize {
    let abs = unsafe { &mut *((*dev).client_data as *mut Abstraction) };

    let mut consumed = 0usize;

    loop {
        // Wait until we can either switch or deliver one byte (slot
must be empty).
        let mut st = abs.st.lock().unwrap();
        while !st.probe_finished && !(st.want_byte && st.slot.is_none()) {
            st = abs.ev.wait(st).unwrap();
        }

        if st.probe_finished {
            // TODO: Do we lock write lock so it is really safe to
change OPS here?
            unsafe { bindings::serdev_device_set_client_ops(dev,
&RUNTIME_OPS) };

            // Forward the *remaining* bytes that we have NOT taken
out of this buf.
            let rem_ptr = unsafe { buf.add(consumed) };
            let rem = count - consumed;
            return consumed + Self::runtime_receive_buf(dev, rem_ptr, rem);
        }

        // Deliver exactly one byte to iterator (slot is empty here).
        let b = unsafe { *buf.add(consumed) };
        st.slot = Some(b);
        // wake next_byte()
        abs.ev.notify_all();

        // Now WAIT until next_byte really consumes it (slot becomes None),
        // or probe finishes (in which case we’ll flush it to runtime).
        while !st.probe_finished && st.slot.is_some() {
            st = abs.ev.wait(st).unwrap();
        }
        if st.probe_finished {
            continue;
        }

        // Now it is truly consumed by next_byte().
        consumed += 1;
        if consumed == count {
            return count;
        }
    }
}
```

Does anyone have opinions which is better or is there some other way
for this which
we have not yet discovered?

Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions

[Kari Argillander]

Here are some review comments. Overall it seems that this will be quite hard to
get right, safe and easy to use. Example many null pointer dereferences can
happen with C API if things are not done in right order. That currently force
this abstraction to do those things in right order (like you have done and kudos
for that). That however makes this probably too rigid for many use cases. Maybe
right thing to do is to fix C API so it is safer to use. That way we can call
things in wrong order and that is still safe but just logic bug in driver.

On 20.12.2025 20.44 +0200 Markus Probst (markus.probst@posteo.de) 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`.

Maybe add more info about design decisions here.

> Co-developed-by: Kari Argillander <kari.argillander@gmail.com>
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> 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           | 815 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 841 insertions(+)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index a067038b4b42..bec6c6d0913a 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -79,6 +79,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/sched.h>
>  #include <linux/security.h>
> +#include <linux/serdev.h>
>  #include <linux/slab.h>
>  #include <linux/task_work.h>
>  #include <linux/tracepoint.h>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c..834e9fbb897d 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -50,6 +50,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 f812cf120042..cc71195466b6 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -136,6 +136,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;
>  mod static_assert;
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> new file mode 100644
> index 000000000000..0f5ef325a054
> --- /dev/null
> +++ b/rust/kernel/serdev.rs
> @@ -0,0 +1,815 @@
> +// 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,
> +    container_of,
> +    device,
> +    driver,
> +    error::{
> +        from_result,
> +        to_result,
> +        VTABLE_DEFAULT_ERROR, //
> +    },
> +    of,
> +    prelude::*,
> +    time::Jiffies,
> +    types::{AlwaysRefCounted, Opaque}, //
> +};
> +
> +use core::{
> +    cell::UnsafeCell,
> +    marker::PhantomData,
> +    mem::offset_of,
> +    num::NonZero,
> +    ptr::{
> +        addr_of_mut, //
> +        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`].
> +    Value(NonZero<Jiffies>),

Maybe better name this Jiffies than value. As Jiffies is not yet real unit. Also
many C drivers use msecs_to_jiffies() maybe also have Msecs variant that takes
NonZero<Msecs>. Then into_jiffies() can call msecs_to_jiffies() for that. That
would make construction easier. Also that way people can have const
Timeout::Msecs which is quite nice. Probably people usually will define timeouts
as consts and then use those consts.

> +    /// Wait as long as possible.
> +    ///
> +    /// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
> +    MaxScheduleTimeout,

As enum is already Timeout maybe Max or MaxSchedule is better name than
MaxScheduleTimeout.

> +}
> +
> +impl Timeout {
> +    fn into_jiffies(self) -> isize {
> +        match self {
> +            Self::Value(value) => value.get().try_into().unwrap_or_default(),
> +            Self::MaxScheduleTimeout => 0,
> +        }
> +    }
> +}
> +
> +/// An adapter for the registration of serial device bus device drivers.
> +pub struct Adapter<T: Driver>(T);
> +
> +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
> +// a preceding call to `register` has been successful.
> +unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
> +    type RegType = bindings::serdev_device_driver;
> +
> +    unsafe fn register(
> +        sdrv: &Opaque<Self::RegType>,
> +        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 `RegType`.
> +        to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
> +    }
> +
> +    unsafe fn unregister(sdrv: &Opaque<Self::RegType>) {
> +        // SAFETY: `sdrv` is guaranteed to be a valid `RegType`.
> +        unsafe { bindings::serdev_device_driver_unregister(sdrv.get()) };
> +    }
> +}
> +
> +#[pin_data]
> +struct Drvdata<T: Driver + 'static> {
> +    #[pin]
> +    driver: T,
> +    initial_data: UnsafeCell<Option<T::InitialData>>,

Maybe also box initial_data as that can be destroyed in after late_probe.

> +    late_probe_data: UnsafeCell<Option<Pin<KBox<T::LateProbeData>>>>,
> +}
> +
> +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: if T::HAS_WRITE_WAKEUP {
> +            Some(Self::write_wakeup_callback)
> +        } else {
> +            Some(bindings::serdev_device_write_wakeup)
> +        },
> +    };
> +    const INITIAL_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> +        receive_buf: Some(Self::initial_receive_buf_callback),
> +        write_wakeup: if T::HAS_WRITE_WAKEUP_INITIAL {
> +            Some(Self::initial_write_wakeup_callback)
> +        } else {
> +            Some(bindings::serdev_device_write_wakeup)
> +        },
> +    };
> +    const NO_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> +        receive_buf: 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(|| {
> +            let data = try_pin_init!(Drvdata {
> +                driver <- T::probe(sdev, info),
> +                initial_data: Some(Default::default()).into(),
> +                late_probe_data: None.into(),
> +            });
> +
> +            sdev.as_ref().set_drvdata(data)?;
> +
> +            // SAFETY: We just set drvdata to `Drvdata<T>`.
> +            let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +            // 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::INITIAL_OPS) };
> +
> +            // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
> +            // to a `struct serdev_device`.
> +            to_result(unsafe {
> +                bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
> +            })?;
> +
> +            // SAFETY: `&data.driver` is guaranteed to be pinned.
> +            T::configure(sdev, unsafe { Pin::new_unchecked(&data.driver) }, info)?;
> +
> +            if !T::HAS_RECEIVE_INITIAL {
> +                // SAFETY:
> +                // - It is guaranteed that we have exclusive access to `data.initial_data` and
> +                //   `data.late_probe_data`.
> +                // - We just initialized `data.initial_data` with Some.
> +                unsafe { Self::do_late_probe(sdev, data)? };
> +            }
> +
> +            Ok(0)
> +        })
> +    }
> +
> +    /// # Safety
> +    ///
> +    /// The caller must guarantee, that we have exclusive access to `data.initial_data` and
> +    /// `data.late_probe_data`. `data.initial_data` must be Some.
> +    /// (i. e. `late_probe` has not been called yet).
> +    unsafe fn do_late_probe(sdev: &Device<device::CoreInternal>, data: Pin<&Drvdata<T>>) -> Result {
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY: The function contract guarantees that we have exclusive access to
> +        // `data.initial_data`.
> +        let initial_data = unsafe { &mut *data.initial_data.get() };
> +
> +        // SAFETY: The function contract guarantees that we have exclusive access to
> +        // `data.late_probe_data`.
> +        let late_probe_data = unsafe { &mut *data.late_probe_data.get() };
> +
> +        *late_probe_data = Some(KBox::pin_init(
> +            T::late_probe(
> +                sdev,
> +                data_driver,
> +                // SAFETY: The function contract guarantees that `data.initial_data` is Some.
> +                unsafe { initial_data.take().unwrap_unchecked() },
> +            ),
> +            GFP_KERNEL,
> +        )?);
> +        // 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) };

How can this be safe? What if serdev core is accessing the ops while we are
changing them? SAFETY comment should address also concurrency issues in this
case.

> +        Ok(())
> +    }
> +
> +    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_obtain::<Drvdata<T>>() };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.late_probe_data` is guaranteed to be pinned.
> +        // - It is guaranteed that we have exclusive access to `data.late_probe_data`.
> +        let late_probe_data = unsafe {
> +            (*data.late_probe_data.get())
> +                .as_deref()
> +                .map(|data| Pin::new_unchecked(data))
> +        };
> +
> +        T::unbind(sdev, data_driver, late_probe_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<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) };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.late_probe_data` is guaranteed to be Some.
> +        // - `data.late_probe_data` is guaranteed to be pinned.
> +        let late_probe_data = unsafe {
> +            Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> +        };
> +
> +        T::receive(sdev, data_driver, late_probe_data, buf)
> +    }
> +
> +    extern "C" fn write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> +        // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> +        // pointer to a `struct serdev_device`.
> +        //
> +        // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
> +        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> +
> +        // SAFETY: `write_wakeup_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +        // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> +        unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.late_probe_data` is guaranteed to be Some.
> +        // - `data.late_probe_data` is guaranteed to be pinned.
> +        let late_probe_data = unsafe {
> +            Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> +        };
> +
> +        // SAFETY: As long as the driver implementation meets the safety requirements, this call
> +        // is safe.
> +        unsafe { T::write_wakeup(sdev, data_driver, late_probe_data) };
> +    }
> +
> +    extern "C" fn initial_receive_buf_callback(
> +        sdev: *mut bindings::serdev_device,
> +        buf: *const u8,
> +        length: usize,
> +    ) -> usize {
> +        if !T::HAS_RECEIVE_INITIAL {
> +            return 0;
> +        }
> +
> +        // 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: `buf` is guaranteed to be non-null and has the size of `length`.
> +        let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> +
> +        // SAFETY: `initial_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let driver_data = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY:
> +        // - `data.initial_data` is always Some until `InitialReceiveResult::Ready` is
> +        //   returned below.
> +        // - It is guaranteed that we have exclusive access to `data.initial_data`.
> +        let initial_data = unsafe { (*data.initial_data.get()).as_mut().unwrap_unchecked() };
> +
> +        match T::receive_initial(sdev, driver_data, initial_data, buf) {
> +            Ok(InitialReceiveResult::Pending(size)) => size,
> +            Ok(InitialReceiveResult::Ready(size)) => {
> +                // SAFETY:
> +                // - It is guaranteed that we have exclusive access to `data.initial_data` and
> +                //   `data.late_probe_data`.
> +                // - We just initialized `data.initial_data` with Some.
> +                if let Err(err) = unsafe { Self::do_late_probe(sdev, data) } {
> +                    dev_err!(sdev.as_ref(), "Unable to late probe device: {err:?}\n");

Not an abstraction job to log this.

> +                    // 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::NO_OPS) };

What if write_wakeup is accessed by serdev core while we are changing the ops? I
checked and even taking serdev write lock does not prevent that as checking
write_wakeup ops is done outside the lock. We could probably change that however
if necessary.

> +                    return length;
> +                }
> +                size
> +            }
> +            Err(err) => {
> +                dev_err!(
> +                    sdev.as_ref(),
> +                    "Unable to receive initial data for probe: {err:?}.\n"
> +                );

Same here. Not an abstraction job to log this.

> +                // 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::NO_OPS) };
> +                length
> +            }
> +        }
> +    }
> +
> +    extern "C" fn initial_write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> +        // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> +        // pointer to a `struct serdev_device`.
> +        //
> +        // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.

initial_write_wakeup_callback

> +        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> +
> +        // SAFETY: `initial_write_wakeup_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<Drvdata<T>>>`.
> +        let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> +
> +        // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> +        unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> +
> +        // SAFETY: `&data.driver` is guaranteed to be pinned.
> +        let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> +
> +        // SAFETY: As long as the driver implementation meets the safety requirements, this call
> +        // is safe.
> +        unsafe { T::write_wakeup_initial(sdev, data_driver) };
> +    }
> +}
> +
> +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 {

To be consistent with other bus module macros this should be named
module_serdev_driver. Also change in example.

> +    ($($f:tt)*) => {
> +        $crate::module_driver!(<T>, $crate::serdev::Adapter<T>, { $($f)* });
> +    };
> +}
> +
> +/// Result for `receive_initial` in [`Driver`].
> +pub enum InitialReceiveResult {
> +    /// More data is required.
> +    ///
> +    /// The inner data is the number of bytes accepted.
> +    Pending(usize),
> +    /// Ready for late probe.
> +    ///
> +    /// The inner data is the number of bytes accepted.
> +    Ready(usize),
> +}
> +
> +/// 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 LateProbeData = ();
> +///     type InitialData = ();
> +///     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,
> +///         _id_info: Option<&Self::IdInfo>,
> +///     ) -> impl PinInit<Self, Error> {
> +///         Err(ENODEV)
> +///     }
> +///
> +///     fn configure(
> +///         dev: &serdev::Device<Core>,
> +///         _this: Pin<&Self>,
> +///         _id_info: Option<&Self::IdInfo>,
> +///     ) -> Result {
> +///         dev.set_baudrate(115200);
> +///         Ok(())
> +///     }
> +///
> +///     fn late_probe(
> +///        _dev: &serdev::Device<Bound>,
> +///        _this: Pin<&Self>,
> +///        _initial_data: Self::InitialData,
> +///    ) -> impl PinInit<Self::LateProbeData, Error> {
> +///        Ok(())
> +///    }
> +/// }
> +///```
> +#[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 LateProbeData: Send + 'static = ();
> +    // type InitialData: Default + Send + 'static = ();
> +    // ```
> +    type IdInfo: 'static;
> +
> +    /// Data returned in `late_probe` function.
> +    type LateProbeData: Send + 'static;
> +    /// Data used for initial data.
> +    type InitialData: Default + Send + '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, id_info: Option<&Self::IdInfo>) -> impl PinInit<Self, Error>;

As probe has serdev::Device. This means we cannot example enable let's say
regulators as those needs device::Device<Bound>. So we could enable those just
in configure(). That means open() is called before we can do preparations. This
seems to be a problem to me. It might not be so someone else can comment if this
is actually a problem.

If we cannot do any preparation before open() we might as well call open()
before probe() and have probe() receive Device<Core>. So something does not seem
quite right here.

Also like I said in another thread I think user should be able to control when
open() is called.

> +
> +    /// Serial device bus device driver configure.
> +    ///
> +    /// Called directly after the serial device bus device has been opened.
> +    /// This should be used for setting up the communication (i. e. baudrate, flow control, parity)
> +    /// and sending an initial hello if required.
> +    fn configure(
> +        sdev: &Device<device::Core>,
> +        this: Pin<&Self>,
> +        id_info: Option<&Self::IdInfo>,
> +    ) -> Result;
> +
> +    /// Serial device bus device data receive callback (initial).
> +    ///
> +    /// Called when data got received from device, before `late_probe` has been called.
> +    /// This should be used, to get information about the device for `late_probe`.
> +    ///
> +    /// Returns `InitialReceiveResult::Pending` if more data is still required for `late_probe`.
> +    /// Otherwise `InitialReceiveResult::Ready`. The inner data is the number of bytes accepted.
> +    fn receive_initial(
> +        sdev: &Device<device::Bound>,
> +        this: Pin<&Self>,
> +        initial_data: &mut Self::InitialData,
> +        data: &[u8],
> +    ) -> Result<InitialReceiveResult> {
> +        let _ = (sdev, this, initial_data, data);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }

Maybe just let implementors could just write to `this` in receive_initial and
then we do not need initial_data at all. That would simplify things a bit. That
would mean some "locking" from implementors is required though. But I think that
is acceptable. Maybe same for late_probe data.

> +    /// Serial device bus device write wakeup callback (initial).
> +    ///
> +    /// Called when ready to transmit more data, before `late_probe` has been called.
> +    ///
> +    /// # Safety
> +    ///
> +    /// This function must not sleep.
> +    unsafe fn write_wakeup_initial(sdev: &Device<device::Core>, this: Pin<&Self>) {
> +        let _ = (sdev, this);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }

This seems useless as during probe we can just do sequential writes as nothing
else is happening. After late_probe we can use the normal write_wakeup.

> +
> +    /// Serial device bus device late probe.
> +    ///
> +    /// Called after the initial data is available.
> +    /// If `receive_initial` isn't implemented, this will be called directly after configure.
> +    fn late_probe(
> +        sdev: &Device<device::Bound>,
> +        this: Pin<&Self>,
> +        initial_data: Self::InitialData,
> +    ) -> impl PinInit<Self::LateProbeData, 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>,
> +        late_probe_this: Option<Pin<&Self::LateProbeData>>,
> +    ) {
> +        let _ = (sdev, this, late_probe_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>,
> +        late_probe_this: Pin<&Self::LateProbeData>,
> +        data: &[u8],
> +    ) -> usize {
> +        let _ = (sdev, this, late_probe_this, data);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Serial device bus device write wakeup callback.
> +    ///
> +    /// Called when ready to transmit more data.
> +    ///
> +    /// # Safety
> +    ///
> +    /// This function must not sleep.

Maybe add hint here that implementors should enable DEBUG_ATOMIC_SLEEP when
developing to catch accidental sleeps.

> +    unsafe fn write_wakeup(
> +        sdev: &Device<device::Bound>,
> +        this: Pin<&Self>,
> +        late_probe_this: Pin<&Self::LateProbeData>,
> +    ) {
> +        let _ = (sdev, this, late_probe_this);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }

I do not see how writing can actually be done. Only place we can actually write
after late probe is receive(). This is because all write functions are in
serdev::Device<Bound>. Also we cannot use those in write_wakeup as that is
atomic context. We should schedule work there but if we schedule work there is
no way to actually call any write function in there as we only have
Aref<serdev::Device> and that has no write functions. So this seems like a very
big problem.

Also I fear that as we do OPS swap we lost first write wakeup call. That means
as this usually schedules work we might never schedule that work if we miss the
first call. I'm not sure of if this is a real problem or not.

> +}
> +
> +/// 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) -> u32 {
> +        // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> +        unsafe { bindings::serdev_device_set_baudrate(self.as_raw(), speed) }
> +    }

Here I'm wondering two things. Could we just have Baudare enum instead. That
would mean that return value is not needed as it will always succeed.

Another thing is if we do not use enum is that should we do something like

```rust
    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 {
           Err(ret)
       }

       Ok(())
    }

```

I'm still not quite convinced for this idea. So I still think maybe enum is good
idea. But then again there must be reason why C API does not use enum?
It might be
historical reason only and we should not make same thing again.

Another idea is to have enum which have all bauderates but last one is
Custom(u32). But having this kind of work done during this abstraction layer
seems a bit off so I'm totally ok if we just use u32 here.

> +    /// 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 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(),
> +            )
> +        };

Use to_result()

> +        if ret < 0 {
> +            // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> +            // which always fit into a `i32`.
> +            Err(Error::from_errno(ret as i32))
> +        } else {
> +            Ok(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 if interrupted).
> +    /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> +    /// before any bytes were written.
> +    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()) };

Use to_result()

> +        if ret < 0 {
> +            Err(Error::from_errno(ret))
> +        } else {
> +            Ok(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 { addr_of_mut!((*self.as_raw()).dev) };

Now days `&raw mut` is preferred instead of `addr_of_mut!`.

> +        // SAFETY: `dev` points to a valid `struct device`.
> +        unsafe { device::Device::from_raw(dev) }
> +    }
> +}
> +
> +impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &Device<Ctx> {
> +    type Error = kernel::error::Error;
> +
> +    fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
> +        // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
> +        // `struct device`.
> +        if !unsafe { bindings::is_serdev_device(dev.as_raw()) } {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY: We've just verified that the device_type of `dev` is
> +        // `serdev_device_type`, hence `dev` must be embedded in a valid `struct
> +        // serdev_device_driver` as guaranteed by the corresponding C code.
> +        let sdev = unsafe { container_of!(dev.as_raw(), bindings::serdev_device, dev) };
> +
> +        // SAFETY: `sdev` is a valid pointer to a `struct serdev_device_driver`.
> +        Ok(unsafe { &*sdev.cast() })
> +    }
> +}
> +
> +// 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 {}