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

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

[Markus Probst]

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

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

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

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

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

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

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

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

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

[Markus Probst]

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

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

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

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

-- 
2.52.0

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

[Markus Probst]

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

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

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

-- 
2.52.0

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

[Markus Probst]

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

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

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

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

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

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

-- 
2.52.0

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

[Markus Probst]

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

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

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

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

-- 
2.52.0

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

[Greg Kroah-Hartman]

On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> Add rust private data to `struct serdev_device`, as it is required by the
> rust abstraction added in the following commit
> (rust: add basic serial device bus abstractions).

why is rust "special" here?  What's wrong with the existing private
pointer in this structure?  Why must we add another one?

thanks,

greg k-h

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

[Markus Probst]

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

On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > Add rust private data to `struct serdev_device`, as it is required by the
> > rust abstraction added in the following commit
> > (rust: add basic serial device bus abstractions).
> 
> why is rust "special" here?  What's wrong with the existing private
> pointer in this structure?  Why must we add another one?
Because in rust, the device drvdata will be set after probe has run. In
serdev, once the device has been opened, it can receive data. It must
be opened either inside probe or before probe, because it can only be
configured (baudrate, flow control etc.) and data written to after it
has been opened. Because it can receive data before drvdata has been
set yet, we need to ensure it waits on data receival for the probe to
be finished. Otherwise this would be a null pointer dereference. To do
this, we need to store a `Completion` for it to wait and a `bool` in
case the probe exits with an error. We cannot store this data in the
device drvdata, because this is where the drivers drvdata goes. We also
cannot create a wrapper of the drivers drvdata, because
`Device::drvdata::<T>()` would always fail in that case. That is why we
need a "rust_private_data" for this abstraction to store the
`Completion` and `bool`.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

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

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

[Greg Kroah-Hartman]

On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > Add rust private data to `struct serdev_device`, as it is required by the
> > > rust abstraction added in the following commit
> > > (rust: add basic serial device bus abstractions).
> > 
> > why is rust "special" here?  What's wrong with the existing private
> > pointer in this structure?  Why must we add another one?
> Because in rust, the device drvdata will be set after probe has run. In
> serdev, once the device has been opened, it can receive data. It must
> be opened either inside probe or before probe, because it can only be
> configured (baudrate, flow control etc.) and data written to after it
> has been opened. Because it can receive data before drvdata has been
> set yet, we need to ensure it waits on data receival for the probe to
> be finished. Otherwise this would be a null pointer dereference. To do
> this, we need to store a `Completion` for it to wait and a `bool` in
> case the probe exits with an error. We cannot store this data in the
> device drvdata, because this is where the drivers drvdata goes. We also
> cannot create a wrapper of the drivers drvdata, because
> `Device::drvdata::<T>()` would always fail in that case. That is why we
> need a "rust_private_data" for this abstraction to store the
> `Completion` and `bool`.

So why is this any different from any other bus type?  I don't see the
"uniqueness" here that has not required this to happen for PCI or USB or
anything else.

What am I missing?

Also, all of this information MUST be in the changelog text in order for
us to be able to accept it.  You need to say _why_ a change is needed,
not just _what_ the change does, as you know.

thanks,

greg k-h

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

[Markus Probst]

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

On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > rust abstraction added in the following commit
> > > > (rust: add basic serial device bus abstractions).
> > > 
> > > why is rust "special" here?  What's wrong with the existing private
> > > pointer in this structure?  Why must we add another one?
> > Because in rust, the device drvdata will be set after probe has run. In
> > serdev, once the device has been opened, it can receive data. It must
> > be opened either inside probe or before probe, because it can only be
> > configured (baudrate, flow control etc.) and data written to after it
> > has been opened. Because it can receive data before drvdata has been
> > set yet, we need to ensure it waits on data receival for the probe to
> > be finished. Otherwise this would be a null pointer dereference. To do
> > this, we need to store a `Completion` for it to wait and a `bool` in
> > case the probe exits with an error. We cannot store this data in the
> > device drvdata, because this is where the drivers drvdata goes. We also
> > cannot create a wrapper of the drivers drvdata, because
> > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > need a "rust_private_data" for this abstraction to store the
> > `Completion` and `bool`.
> 
> So why is this any different from any other bus type?  I don't see the
> "uniqueness" here that has not required this to happen for PCI or USB or
> anything else.
> 
> What am I missing?
In Short:
In serdev, we have to handle incoming device data (serdev calls on a
function pointer we provide in advance), even in the case that the
driver hasn't completed probe yet.

> 
> Also, all of this information MUST be in the changelog text in order for
> us to be able to accept it.  You need to say _why_ a change is needed,
> not just _what_ the change does, as you know.
Will do.

Thanks
- Markus Probst

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

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

[Alice Ryhl]

On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > rust abstraction added in the following commit
> > > > > (rust: add basic serial device bus abstractions).
> > > > 
> > > > why is rust "special" here?  What's wrong with the existing private
> > > > pointer in this structure?  Why must we add another one?
> > > Because in rust, the device drvdata will be set after probe has run. In
> > > serdev, once the device has been opened, it can receive data. It must
> > > be opened either inside probe or before probe, because it can only be
> > > configured (baudrate, flow control etc.) and data written to after it
> > > has been opened. Because it can receive data before drvdata has been
> > > set yet, we need to ensure it waits on data receival for the probe to
> > > be finished. Otherwise this would be a null pointer dereference. To do
> > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > case the probe exits with an error. We cannot store this data in the
> > > device drvdata, because this is where the drivers drvdata goes. We also
> > > cannot create a wrapper of the drivers drvdata, because
> > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > need a "rust_private_data" for this abstraction to store the
> > > `Completion` and `bool`.
> > 
> > So why is this any different from any other bus type?  I don't see the
> > "uniqueness" here that has not required this to happen for PCI or USB or
> > anything else.
> > 
> > What am I missing?
> In Short:
> In serdev, we have to handle incoming device data (serdev calls on a
> function pointer we provide in advance), even in the case that the
> driver hasn't completed probe yet.

Why can the function pointers be called during probe?

Alice

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

[Greg Kroah-Hartman]

On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > rust abstraction added in the following commit
> > > > > (rust: add basic serial device bus abstractions).
> > > > 
> > > > why is rust "special" here?  What's wrong with the existing private
> > > > pointer in this structure?  Why must we add another one?
> > > Because in rust, the device drvdata will be set after probe has run. In
> > > serdev, once the device has been opened, it can receive data. It must
> > > be opened either inside probe or before probe, because it can only be
> > > configured (baudrate, flow control etc.) and data written to after it
> > > has been opened. Because it can receive data before drvdata has been
> > > set yet, we need to ensure it waits on data receival for the probe to
> > > be finished. Otherwise this would be a null pointer dereference. To do
> > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > case the probe exits with an error. We cannot store this data in the
> > > device drvdata, because this is where the drivers drvdata goes. We also
> > > cannot create a wrapper of the drivers drvdata, because
> > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > need a "rust_private_data" for this abstraction to store the
> > > `Completion` and `bool`.
> > 
> > So why is this any different from any other bus type?  I don't see the
> > "uniqueness" here that has not required this to happen for PCI or USB or
> > anything else.
> > 
> > What am I missing?
> In Short:
> In serdev, we have to handle incoming device data (serdev calls on a
> function pointer we provide in advance), even in the case that the
> driver hasn't completed probe yet.

But how is that any different from a USB or PCI driver doing the same
thing?  Why is serdev so unique here?  What specific serdev function
causes this and why isn't it an issue with the C api?  Can we change the
C code to not require this?

thanks,

greg k-h

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

[Danilo Krummrich]

On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
> On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
>> On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
>> > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
>> > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
>> > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
>> > > > > Add rust private data to `struct serdev_device`, as it is required by the
>> > > > > rust abstraction added in the following commit
>> > > > > (rust: add basic serial device bus abstractions).
>> > > > 
>> > > > why is rust "special" here?  What's wrong with the existing private
>> > > > pointer in this structure?  Why must we add another one?
>> > > Because in rust, the device drvdata will be set after probe has run. In
>> > > serdev, once the device has been opened, it can receive data. It must
>> > > be opened either inside probe or before probe, because it can only be
>> > > configured (baudrate, flow control etc.) and data written to after it
>> > > has been opened. Because it can receive data before drvdata has been
>> > > set yet, we need to ensure it waits on data receival for the probe to
>> > > be finished. Otherwise this would be a null pointer dereference. To do
>> > > this, we need to store a `Completion` for it to wait and a `bool` in
>> > > case the probe exits with an error. We cannot store this data in the
>> > > device drvdata, because this is where the drivers drvdata goes. We also
>> > > cannot create a wrapper of the drivers drvdata, because
>> > > `Device::drvdata::<T>()` would always fail in that case. That is why we
>> > > need a "rust_private_data" for this abstraction to store the
>> > > `Completion` and `bool`.
>> > 
>> > So why is this any different from any other bus type?  I don't see the
>> > "uniqueness" here that has not required this to happen for PCI or USB or
>> > anything else.
>> > 
>> > What am I missing?
>> In Short:
>> In serdev, we have to handle incoming device data (serdev calls on a
>> function pointer we provide in advance), even in the case that the
>> driver hasn't completed probe yet.
>
> But how is that any different from a USB or PCI driver doing the same
> thing?  Why is serdev so unique here?  What specific serdev function
> causes this and why isn't it an issue with the C api?  Can we change the
> C code to not require this?

I think the idea is to avoid bugs as in the mhz19b driver [1].

This driver's probe() looks like this:


	serdev_device_set_client_ops(serdev, &mhz19b_ops);
	ret = devm_serdev_device_open(dev, serdev);

	// Lots of other initialization.

	serdev_device_set_drvdata(serdev, indio_dev);

But the receive_buf() callback from mhz19b_ops dereferences the driver's private
data.

Now, maybe this is actually prevented to become an actual race, since some
regulator is only enabled subsequently:

	devm_regulator_get_enable(dev, "vin");

But in any case in Rust it would be unsound as with this a driver could easily
cause undefined behavior with safe APIs.

Maybe it is as simple as letting the abstraction call serdev_device_open() only
after the driver's probe() has completed, but maybe there are reasons why that
is not an option, that's a serdev question.

[1] https://elixir.bootlin.com/linux/v6.19.7/source/drivers/iio/chemical/mhz19b.c#L260

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

[Markus Probst]

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

On Sat, 2026-03-14 at 14:42 +0100, Danilo Krummrich wrote:
> On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
> > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > > > rust abstraction added in the following commit
> > > > > > > (rust: add basic serial device bus abstractions).
> > > > > > 
> > > > > > why is rust "special" here?  What's wrong with the existing private
> > > > > > pointer in this structure?  Why must we add another one?
> > > > > Because in rust, the device drvdata will be set after probe has run. In
> > > > > serdev, once the device has been opened, it can receive data. It must
> > > > > be opened either inside probe or before probe, because it can only be
> > > > > configured (baudrate, flow control etc.) and data written to after it
> > > > > has been opened. Because it can receive data before drvdata has been
> > > > > set yet, we need to ensure it waits on data receival for the probe to
> > > > > be finished. Otherwise this would be a null pointer dereference. To do
> > > > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > > > case the probe exits with an error. We cannot store this data in the
> > > > > device drvdata, because this is where the drivers drvdata goes. We also
> > > > > cannot create a wrapper of the drivers drvdata, because
> > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > > > need a "rust_private_data" for this abstraction to store the
> > > > > `Completion` and `bool`.
> > > > 
> > > > So why is this any different from any other bus type?  I don't see the
> > > > "uniqueness" here that has not required this to happen for PCI or USB or
> > > > anything else.
> > > > 
> > > > What am I missing?
> > > In Short:
> > > In serdev, we have to handle incoming device data (serdev calls on a
> > > function pointer we provide in advance), even in the case that the
> > > driver hasn't completed probe yet.
> > 
> > But how is that any different from a USB or PCI driver doing the same
> > thing?  Why is serdev so unique here?  What specific serdev function
> > causes this and why isn't it an issue with the C api?  Can we change the
> > C code to not require this?
> 
> I think the idea is to avoid bugs as in the mhz19b driver [1].
> 
> This driver's probe() looks like this:
> 
> 
> 	serdev_device_set_client_ops(serdev, &mhz19b_ops);
> 	ret = devm_serdev_device_open(dev, serdev);
> 
> 	// Lots of other initialization.
> 
> 	serdev_device_set_drvdata(serdev, indio_dev);
> 
> But the receive_buf() callback from mhz19b_ops dereferences the driver's private
> data.
> 
> Now, maybe this is actually prevented to become an actual race, since some
> regulator is only enabled subsequently:
> 
> 	devm_regulator_get_enable(dev, "vin");
> 
> But in any case in Rust it would be unsound as with this a driver could easily
> cause undefined behavior with safe APIs.
> 
> Maybe it is as simple as letting the abstraction call serdev_device_open() only
> after the driver's probe() has completed, but maybe there are reasons why that
> is not an option, that's a serdev question.
If we call it after probe, calls to `serdev_device_set_baudrate`,
`serdev_device_set_flow_control`, `serdev_device_set_parity`,
`serdev_device_write_buf`, `serdev_device_write`,
`serdev_device_write_flush`, which are exposed via the rust abstraction
would result in a null pointer dereference.

Thanks
- Markus Probst

> 
> [1] https://elixir.bootlin.com/linux/v6.19.7/source/drivers/iio/chemical/mhz19b.c#L260

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

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

[Danilo Krummrich]

On Sat Mar 14, 2026 at 2:49 PM CET, Markus Probst wrote:
> On Sat, 2026-03-14 at 14:42 +0100, Danilo Krummrich wrote:
>> On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
>> > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
>> > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
>> > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
>> > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
>> > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
>> > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
>> > > > > > > rust abstraction added in the following commit
>> > > > > > > (rust: add basic serial device bus abstractions).
>> > > > > > 
>> > > > > > why is rust "special" here?  What's wrong with the existing private
>> > > > > > pointer in this structure?  Why must we add another one?
>> > > > > Because in rust, the device drvdata will be set after probe has run. In
>> > > > > serdev, once the device has been opened, it can receive data. It must
>> > > > > be opened either inside probe or before probe, because it can only be
>> > > > > configured (baudrate, flow control etc.) and data written to after it
>> > > > > has been opened. Because it can receive data before drvdata has been
>> > > > > set yet, we need to ensure it waits on data receival for the probe to
>> > > > > be finished. Otherwise this would be a null pointer dereference. To do
>> > > > > this, we need to store a `Completion` for it to wait and a `bool` in
>> > > > > case the probe exits with an error. We cannot store this data in the
>> > > > > device drvdata, because this is where the drivers drvdata goes. We also
>> > > > > cannot create a wrapper of the drivers drvdata, because
>> > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
>> > > > > need a "rust_private_data" for this abstraction to store the
>> > > > > `Completion` and `bool`.
>> > > > 
>> > > > So why is this any different from any other bus type?  I don't see the
>> > > > "uniqueness" here that has not required this to happen for PCI or USB or
>> > > > anything else.
>> > > > 
>> > > > What am I missing?
>> > > In Short:
>> > > In serdev, we have to handle incoming device data (serdev calls on a
>> > > function pointer we provide in advance), even in the case that the
>> > > driver hasn't completed probe yet.
>> > 
>> > But how is that any different from a USB or PCI driver doing the same
>> > thing?  Why is serdev so unique here?  What specific serdev function
>> > causes this and why isn't it an issue with the C api?  Can we change the
>> > C code to not require this?
>> 
>> I think the idea is to avoid bugs as in the mhz19b driver [1].
>> 
>> This driver's probe() looks like this:
>> 
>> 
>> 	serdev_device_set_client_ops(serdev, &mhz19b_ops);
>> 	ret = devm_serdev_device_open(dev, serdev);
>> 
>> 	// Lots of other initialization.
>> 
>> 	serdev_device_set_drvdata(serdev, indio_dev);
>> 
>> But the receive_buf() callback from mhz19b_ops dereferences the driver's private
>> data.
>> 
>> Now, maybe this is actually prevented to become an actual race, since some
>> regulator is only enabled subsequently:
>> 
>> 	devm_regulator_get_enable(dev, "vin");
>> 
>> But in any case in Rust it would be unsound as with this a driver could easily
>> cause undefined behavior with safe APIs.
>> 
>> Maybe it is as simple as letting the abstraction call serdev_device_open() only
>> after the driver's probe() has completed, but maybe there are reasons why that
>> is not an option, that's a serdev question.
> If we call it after probe, calls to `serdev_device_set_baudrate`,
> `serdev_device_set_flow_control`, `serdev_device_set_parity`,
> `serdev_device_write_buf`, `serdev_device_write`,
> `serdev_device_write_flush`, which are exposed via the rust abstraction
> would result in a null pointer dereference.

Then maybe ensure that the driver's receive_buf() callback can only ever be
called after probe() has been completed? E.g. receive_buf() could be optional
and swapped out later on.

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

[Markus Probst]

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

On Sat, 2026-03-14 at 14:54 +0100, Danilo Krummrich wrote:
> On Sat Mar 14, 2026 at 2:49 PM CET, Markus Probst wrote:
> > On Sat, 2026-03-14 at 14:42 +0100, Danilo Krummrich wrote:
> > > On Sat Mar 14, 2026 at 2:31 PM CET, Greg Kroah-Hartman wrote:
> > > > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> > > > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > > > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > > > > > rust abstraction added in the following commit
> > > > > > > > > (rust: add basic serial device bus abstractions).
> > > > > > > > 
> > > > > > > > why is rust "special" here?  What's wrong with the existing private
> > > > > > > > pointer in this structure?  Why must we add another one?
> > > > > > > Because in rust, the device drvdata will be set after probe has run. In
> > > > > > > serdev, once the device has been opened, it can receive data. It must
> > > > > > > be opened either inside probe or before probe, because it can only be
> > > > > > > configured (baudrate, flow control etc.) and data written to after it
> > > > > > > has been opened. Because it can receive data before drvdata has been
> > > > > > > set yet, we need to ensure it waits on data receival for the probe to
> > > > > > > be finished. Otherwise this would be a null pointer dereference. To do
> > > > > > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > > > > > case the probe exits with an error. We cannot store this data in the
> > > > > > > device drvdata, because this is where the drivers drvdata goes. We also
> > > > > > > cannot create a wrapper of the drivers drvdata, because
> > > > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > > > > > need a "rust_private_data" for this abstraction to store the
> > > > > > > `Completion` and `bool`.
> > > > > > 
> > > > > > So why is this any different from any other bus type?  I don't see the
> > > > > > "uniqueness" here that has not required this to happen for PCI or USB or
> > > > > > anything else.
> > > > > > 
> > > > > > What am I missing?
> > > > > In Short:
> > > > > In serdev, we have to handle incoming device data (serdev calls on a
> > > > > function pointer we provide in advance), even in the case that the
> > > > > driver hasn't completed probe yet.
> > > > 
> > > > But how is that any different from a USB or PCI driver doing the same
> > > > thing?  Why is serdev so unique here?  What specific serdev function
> > > > causes this and why isn't it an issue with the C api?  Can we change the
> > > > C code to not require this?
> > > 
> > > I think the idea is to avoid bugs as in the mhz19b driver [1].
> > > 
> > > This driver's probe() looks like this:
> > > 
> > > 
> > > 	serdev_device_set_client_ops(serdev, &mhz19b_ops);
> > > 	ret = devm_serdev_device_open(dev, serdev);
> > > 
> > > 	// Lots of other initialization.
> > > 
> > > 	serdev_device_set_drvdata(serdev, indio_dev);
> > > 
> > > But the receive_buf() callback from mhz19b_ops dereferences the driver's private
> > > data.
> > > 
> > > Now, maybe this is actually prevented to become an actual race, since some
> > > regulator is only enabled subsequently:
> > > 
> > > 	devm_regulator_get_enable(dev, "vin");
> > > 
> > > But in any case in Rust it would be unsound as with this a driver could easily
> > > cause undefined behavior with safe APIs.
> > > 
> > > Maybe it is as simple as letting the abstraction call serdev_device_open() only
> > > after the driver's probe() has completed, but maybe there are reasons why that
> > > is not an option, that's a serdev question.
> > If we call it after probe, calls to `serdev_device_set_baudrate`,
> > `serdev_device_set_flow_control`, `serdev_device_set_parity`,
> > `serdev_device_write_buf`, `serdev_device_write`,
> > `serdev_device_write_flush`, which are exposed via the rust abstraction
> > would result in a null pointer dereference.
> 
> Then maybe ensure that the driver's receive_buf() callback can only ever be
> called after probe() has been completed? E.g. receive_buf() could be optional
> and swapped out later on.
I am not exactly sure what you mean by "could be optional and swapped
out later on".

Also,
the function pointer cannot be changed while the device is open, as
this could introduce a race condition. In addition if it was prior set
to NULL and data was received, this data would be lost.

Thanks
- Markus Probst


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

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

[Markus Probst]

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

On Sat, 2026-03-14 at 14:31 +0100, Greg Kroah-Hartman wrote:
> On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > > rust abstraction added in the following commit
> > > > > > (rust: add basic serial device bus abstractions).
> > > > > 
> > > > > why is rust "special" here?  What's wrong with the existing private
> > > > > pointer in this structure?  Why must we add another one?
> > > > Because in rust, the device drvdata will be set after probe has run. In
> > > > serdev, once the device has been opened, it can receive data. It must
> > > > be opened either inside probe or before probe, because it can only be
> > > > configured (baudrate, flow control etc.) and data written to after it
> > > > has been opened. Because it can receive data before drvdata has been
> > > > set yet, we need to ensure it waits on data receival for the probe to
> > > > be finished. Otherwise this would be a null pointer dereference. To do
> > > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > > case the probe exits with an error. We cannot store this data in the
> > > > device drvdata, because this is where the drivers drvdata goes. We also
> > > > cannot create a wrapper of the drivers drvdata, because
> > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > > need a "rust_private_data" for this abstraction to store the
> > > > `Completion` and `bool`.
> > > 
> > > So why is this any different from any other bus type?  I don't see the
> > > "uniqueness" here that has not required this to happen for PCI or USB or
> > > anything else.
> > > 
> > > What am I missing?
> > In Short:
> > In serdev, we have to handle incoming device data (serdev calls on a
> > function pointer we provide in advance), even in the case that the
> > driver hasn't completed probe yet.
> 
> But how is that any different from a USB or PCI driver doing the same
> thing?  Why is serdev so unique here? 
> 
In PCI or USB we don't need to provide function pointers for callbacks
in advance, which will be can be called any time (even while probe).

> What specific serdev function
> causes this
> 
drivers/tty/serdev/serdev-ttyport.c basically only wraps the serdev
calls to tty calls. This isn't directly caused by a serdev function,
but by the tty part.

> why isn't it an issue with the C api?
> 
In C you can set the drvdata inside the probe and even with it not
being fully initialized.

With Rust the drvdata is only available after the probe.
But there is the posibility of serdev calling the provided callback
inside probe.

> Can we change the
> C code to not require this?
Serdev is very closely linked to tty.

There are 3 options:

1. Add a `rust_serdev_device_open` and `rust_serdev_device_ready`
function. `rust_serdev_device_open` would do the same thing as
`serdev_device_open`, but with calling `tty_buffer_lock_exclusive`
before opening the underlying tty port. `rust_serdev_device_ready`
would call `tty_buffer_unlock_exclusive`. Such functions would then
need to exist for every serdev controller (currently there is only
ttyport as serdev controller).

2. Rewrite parts of the tty subsystem.

Not sure what would need to be changed there yet. But this could also
affect the existing tty drivers, which are a lot in comparision to
serdev.

3. Keep the `rust_private_data` pointer in `serdev_device`.

This seems to be the simplest option to me.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

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

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

[Greg Kroah-Hartman]

On Fri, Mar 20, 2026 at 04:53:09PM +0000, Markus Probst wrote:
> On Sat, 2026-03-14 at 14:31 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > > > rust abstraction added in the following commit
> > > > > > > (rust: add basic serial device bus abstractions).
> > > > > > 
> > > > > > why is rust "special" here?  What's wrong with the existing private
> > > > > > pointer in this structure?  Why must we add another one?
> > > > > Because in rust, the device drvdata will be set after probe has run. In
> > > > > serdev, once the device has been opened, it can receive data. It must
> > > > > be opened either inside probe or before probe, because it can only be
> > > > > configured (baudrate, flow control etc.) and data written to after it
> > > > > has been opened. Because it can receive data before drvdata has been
> > > > > set yet, we need to ensure it waits on data receival for the probe to
> > > > > be finished. Otherwise this would be a null pointer dereference. To do
> > > > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > > > case the probe exits with an error. We cannot store this data in the
> > > > > device drvdata, because this is where the drivers drvdata goes. We also
> > > > > cannot create a wrapper of the drivers drvdata, because
> > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > > > need a "rust_private_data" for this abstraction to store the
> > > > > `Completion` and `bool`.
> > > > 
> > > > So why is this any different from any other bus type?  I don't see the
> > > > "uniqueness" here that has not required this to happen for PCI or USB or
> > > > anything else.
> > > > 
> > > > What am I missing?
> > > In Short:
> > > In serdev, we have to handle incoming device data (serdev calls on a
> > > function pointer we provide in advance), even in the case that the
> > > driver hasn't completed probe yet.
> > 
> > But how is that any different from a USB or PCI driver doing the same
> > thing?  Why is serdev so unique here? 
> > 
> In PCI or USB we don't need to provide function pointers for callbacks
> in advance, which will be can be called any time (even while probe).

All class drivers are like this, once you register with them, the
function pointers you provide can be called before your probe call ends.
So this isn't unique as far as I can tell.

> > What specific serdev function
> > causes this
> > 
> drivers/tty/serdev/serdev-ttyport.c basically only wraps the serdev
> calls to tty calls. This isn't directly caused by a serdev function,
> but by the tty part.
> 
> > why isn't it an issue with the C api?
> > 
> In C you can set the drvdata inside the probe and even with it not
> being fully initialized.
> 
> With Rust the drvdata is only available after the probe.
> But there is the posibility of serdev calling the provided callback
> inside probe.

Other classes have this same issue, so why isn't this a problem for HID
and input and the like?

thanks,

greg k-h

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

[Markus Probst]

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

On Fri, 2026-03-20 at 17:59 +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 20, 2026 at 04:53:09PM +0000, Markus Probst wrote:
> > On Sat, 2026-03-14 at 14:31 +0100, Greg Kroah-Hartman wrote:
> > > On Sat, Mar 14, 2026 at 12:08:09PM +0000, Markus Probst wrote:
> > > > On Sat, 2026-03-14 at 12:52 +0100, Greg Kroah-Hartman wrote:
> > > > > On Sat, Mar 14, 2026 at 11:42:02AM +0000, Markus Probst wrote:
> > > > > > On Sat, 2026-03-14 at 09:07 +0100, Greg Kroah-Hartman wrote:
> > > > > > > On Fri, Mar 13, 2026 at 06:12:31PM +0000, Markus Probst wrote:
> > > > > > > > Add rust private data to `struct serdev_device`, as it is required by the
> > > > > > > > rust abstraction added in the following commit
> > > > > > > > (rust: add basic serial device bus abstractions).
> > > > > > > 
> > > > > > > why is rust "special" here?  What's wrong with the existing private
> > > > > > > pointer in this structure?  Why must we add another one?
> > > > > > Because in rust, the device drvdata will be set after probe has run. In
> > > > > > serdev, once the device has been opened, it can receive data. It must
> > > > > > be opened either inside probe or before probe, because it can only be
> > > > > > configured (baudrate, flow control etc.) and data written to after it
> > > > > > has been opened. Because it can receive data before drvdata has been
> > > > > > set yet, we need to ensure it waits on data receival for the probe to
> > > > > > be finished. Otherwise this would be a null pointer dereference. To do
> > > > > > this, we need to store a `Completion` for it to wait and a `bool` in
> > > > > > case the probe exits with an error. We cannot store this data in the
> > > > > > device drvdata, because this is where the drivers drvdata goes. We also
> > > > > > cannot create a wrapper of the drivers drvdata, because
> > > > > > `Device::drvdata::<T>()` would always fail in that case. That is why we
> > > > > > need a "rust_private_data" for this abstraction to store the
> > > > > > `Completion` and `bool`.
> > > > > 
> > > > > So why is this any different from any other bus type?  I don't see the
> > > > > "uniqueness" here that has not required this to happen for PCI or USB or
> > > > > anything else.
> > > > > 
> > > > > What am I missing?
> > > > In Short:
> > > > In serdev, we have to handle incoming device data (serdev calls on a
> > > > function pointer we provide in advance), even in the case that the
> > > > driver hasn't completed probe yet.
> > > 
> > > But how is that any different from a USB or PCI driver doing the same
> > > thing?  Why is serdev so unique here? 
> > > 
> > In PCI or USB we don't need to provide function pointers for callbacks
> > in advance, which will be can be called any time (even while probe).
> 
> All class drivers are like this, once you register with them, the
> function pointers you provide can be called before your probe call ends.
> So this isn't unique as far as I can tell.
> 
> > > What specific serdev function
> > > causes this
> > > 
> > drivers/tty/serdev/serdev-ttyport.c basically only wraps the serdev
> > calls to tty calls. This isn't directly caused by a serdev function,
> > but by the tty part.
> > 
> > > why isn't it an issue with the C api?
> > > 
> > In C you can set the drvdata inside the probe and even with it not
> > being fully initialized.
> > 
> > With Rust the drvdata is only available after the probe.
> > But there is the posibility of serdev calling the provided callback
> > inside probe.
> 
> Other classes have this same issue, so why isn't this a problem for HID
> and input and the like?
This is unique for a bus device.

A class device has its own Data (e. g. PwmOps), i. e. it will only be
registered after this Data has been initialized by the driver.

The receive_buf callback on the serdev device on the other hand must
happen on the drvdata, as there is no place to store its own Data.

The drvdata is only available at the end of the probe in Rust.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

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

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

[Danilo Krummrich]

On 3/20/26 6:13 PM, Markus Probst wrote:
> This is unique for a bus device.
> 
> A class device has its own Data (e. g. PwmOps), i. e. it will only be
> registered after this Data has been initialized by the driver.
> 
> The receive_buf callback on the serdev device on the other hand must
> happen on the drvdata, as there is no place to store its own Data.
> 
> The drvdata is only available at the end of the probe in Rust.

In other words, devm_serdev_device_open() relies on the driver having its
private data set before this is called, since once devm_serdev_device_open() has
been called, the driver's receive_buf() callback may be called.

So, in C it is the driver's responsibility to do things in the correct order, in
Rust we want to make sure the driver can't do it out of order in the first place.

I'm still not sure whether the current approach is the best option. For
instance, we could also solve this with a separate callback. (Wasn't this even
what you had in a previous version?)

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

[Markus Probst]

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

On Fri, 2026-03-20 at 20:59 +0100, Danilo Krummrich wrote:
> On 3/20/26 6:13 PM, Markus Probst wrote:
> > This is unique for a bus device.
> > 
> > A class device has its own Data (e. g. PwmOps), i. e. it will only be
> > registered after this Data has been initialized by the driver.
> > 
> > The receive_buf callback on the serdev device on the other hand must
> > happen on the drvdata, as there is no place to store its own Data.
> > 
> > The drvdata is only available at the end of the probe in Rust.
> 
> In other words, devm_serdev_device_open() relies on the driver having its
> private data set before this is called, since once devm_serdev_device_open() has
> been called, the driver's receive_buf() callback may be called.
> 
> So, in C it is the driver's responsibility to do things in the correct order, in
> Rust we want to make sure the driver can't do it out of order in the first place.
> 
> I'm still not sure whether the current approach is the best option. For
> instance, we could also solve this with a separate callback.
> 

> Wasn't this even
> what you had in a previous version?
Yes, but Kari pointed out that changing the ops while it is in use is
unsafe [1].

Thanks
- Markus Probst

[1]
https://lore.kernel.org/rust-for-linux/CAC=eVgR2WYdDTW3kOeemyQPP-H0aAUsrzn5Gk5zCe2hQEB709w@mail.gmail.com/

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