Re: [PATCH v14 1/3] rust: add basic serial device bus abstractions

Markus Probst <[email protected]> Fri, 17 Jul 2026 11:27:26 +0000
Newsgroups	org.kernel.vger.linux-serial,dev.linux.lists.driver-core,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pm,org.kernel.vger.rust-for-linux
Message-ID	<[email protected]>
On Fri, 2026-07-17 at 12:15 +0100, Gary Guo wrote:
> On Wed Jul 15, 2026 at 10:00 PM BST, Markus Probst wrote:
> > Implement the basic serial device bus abstractions required to write a
> > serial device bus device driver with or without the need for initial device
> > data. This includes the following data structures:
> > 
> > The `serdev::Driver` trait represents the interface to the driver.
> > 
> > The `serdev::Device` abstraction represents a `struct serdev_device`.
> > 
> > In order to provide the Serdev specific parts to a generic
> > `driver::Registration` the `driver::RegistrationOps` trait is
> > implemented by `serdev::Adapter`.
> > 
> > Signed-off-by: Markus Probst <[email protected]>
> > ---
> >  MAINTAINERS                     |   2 +
> >  drivers/tty/serdev/Kconfig      |   7 +
> >  rust/bindings/bindings_helper.h |   1 +
> >  rust/helpers/helpers.c          |   1 +
> >  rust/helpers/serdev.c           |  22 ++
> >  rust/kernel/lib.rs              |   2 +
> >  rust/kernel/serdev.rs           | 599 ++++++++++++++++++++++++++++++++++++++++
> >  7 files changed, 634 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 806bd2d80d15..cd735d227892 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -24568,6 +24568,8 @@ S:	Maintained
> >  F:	Documentation/devicetree/bindings/serial/serial.yaml
> >  F:	drivers/tty/serdev/
> >  F:	include/linux/serdev.h
> > +F:	rust/helpers/serdev.c
> > +F:	rust/kernel/serdev.rs
> >  
> >  SERIAL IR RECEIVER
> >  M:	Sean Young <[email protected]>
> > diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
> > index 46ae732bfc68..e6dfe949ad01 100644
> > --- a/drivers/tty/serdev/Kconfig
> > +++ b/drivers/tty/serdev/Kconfig
> > @@ -9,6 +9,13 @@ menuconfig SERIAL_DEV_BUS
> >  
> >  	  Note that you typically also want to enable TTY port controller support.
> >  
> > +config RUST_SERIAL_DEV_BUS_ABSTRACTIONS
> > +	bool "Rust Serial device bus abstractions"
> > +	depends on RUST
> > +	select SERIAL_DEV_BUS
> > +	help
> > +	  This enables the Rust abstraction for the serial device bus API.
> > +
> >  if SERIAL_DEV_BUS
> >  
> >  config SERIAL_DEV_CTRL_TTYPORT
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index 1124785e210b..fe7c505da236 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -85,6 +85,7 @@
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/sched.h>
> >  #include <linux/security.h>
> > +#include <linux/serdev.h>
> >  #include <linux/slab.h>
> >  #include <linux/sys_soc.h>
> >  #include <linux/task_work.h>
> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> > index 998e31052e66..6921a5ecb78a 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -87,6 +87,7 @@
> >  #include "regulator.c"
> >  #include "scatterlist.c"
> >  #include "security.c"
> > +#include "serdev.c"
> >  #include "signal.c"
> >  #include "slab.c"
> >  #include "spinlock.c"
> > diff --git a/rust/helpers/serdev.c b/rust/helpers/serdev.c
> > new file mode 100644
> > index 000000000000..c52b78ca3fc7
> > --- /dev/null
> > +++ b/rust/helpers/serdev.c
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/serdev.h>
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_driver_unregister(struct serdev_device_driver *sdrv)
> > +{
> > +	serdev_device_driver_unregister(sdrv);
> > +}
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_put(struct serdev_device *serdev)
> > +{
> > +	serdev_device_put(serdev);
> > +}
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_set_client_ops(struct serdev_device *serdev,
> > +					      const struct serdev_device_ops *ops)
> > +{
> > +	serdev_device_set_client_ops(serdev, ops);
> > +}
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 9512af7156df..d5ca17336739 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -119,6 +119,8 @@
> >  pub mod scatterlist;
> >  pub mod security;
> >  pub mod seq_file;
> > +#[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)]
> > +pub mod serdev;
> >  pub mod sizes;
> >  #[cfg(CONFIG_SOC_BUS)]
> >  pub mod soc;
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > new file mode 100644
> > index 000000000000..0ffcef1849d2
> > --- /dev/null
> > +++ b/rust/kernel/serdev.rs
> > @@ -0,0 +1,599 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Abstractions for the serial device bus.
> > +//!
> > +//! C header: [`include/linux/serdev.h`](srctree/include/linux/serdev.h)
> > +
> > +use crate::{
> > +    acpi,
> > +    device,
> > +    driver,
> > +    error::{
> > +        from_result,
> > +        to_result,
> > +        VTABLE_DEFAULT_ERROR, //
> > +    },
> > +    new_mutex,
> > +    of,
> > +    prelude::*,
> > +    sync::{
> > +        aref::AlwaysRefCounted,
> > +        Mutex, //
> > +    },
> > +    time::{
> > +        msecs_to_jiffies,
> > +        Jiffies,
> > +        Msecs, //
> > +    },
> > +    types::{
> > +        Opaque,
> > +        ScopeGuard, //
> > +    }, //
> > +};
> > +
> > +use core::{
> > +    cell::UnsafeCell,
> > +    marker::PhantomData,
> > +    mem::{offset_of, MaybeUninit},
> > +    num::NonZero,
> > +    ptr::NonNull, //
> > +};
> > +
> > +/// Parity bit to use with a serial device.
> > +#[repr(u32)]
> > +pub enum Parity {
> > +    /// No parity bit.
> > +    None = bindings::serdev_parity_SERDEV_PARITY_NONE,
> > +    /// Even partiy.
> > +    Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
> > +    /// Odd parity.
> > +    Odd = bindings::serdev_parity_SERDEV_PARITY_ODD,
> > +}
> > +
> > +/// Timeout in Jiffies.
> > +pub enum Timeout {
> > +    /// Wait for a specific amount of [`Jiffies`].
> > +    Jiffies(NonZero<Jiffies>),
> > +    /// Wait for a specific amount of [`Msecs`].
> > +    Milliseconds(NonZero<Msecs>),
> > +    /// Wait as long as possible.
> > +    ///
> > +    /// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
> > +    Max,
> > +}
> > +
> > +impl Timeout {
> > +    fn into_jiffies(self) -> isize {
> > +        match self {
> > +            Self::Jiffies(value) => value.get().try_into().unwrap_or_default(),
> > +            Self::Milliseconds(value) => {
> > +                msecs_to_jiffies(value.get()).try_into().unwrap_or_default()
> > +            }
> > +            Self::Max => 0,
> > +        }
> > +    }
> > +}
> 
> FWIW this shouldn't be part of serdev abstraction. See
> https://lore.kernel.org/rust-for-linux/[email protected]
> 
> > +
> > +/// An adapter for the registration of serial device bus device drivers.
> > +pub struct Adapter<T: Driver>(T);
> > +
> > +// SAFETY:
> > +// - `bindings::serdev_device_driver` is a C type declared as `repr(C)`.
> > +// - `PrivateData<'bound, T>` is the type of the driver's device private data.
> > +// - `struct serdev_device_driver` embeds a `struct device_driver`.
> > +// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`.
> > +unsafe impl<T: Driver> driver::DriverLayout for Adapter<T> {
> > +    type DriverType = bindings::serdev_device_driver;
> > +    type DriverData<'bound> = PrivateData<'bound, T>;
> > +    const DEVICE_DRIVER_OFFSET: usize = core::mem::offset_of!(Self::DriverType, driver);
> > +}
> > +
> > +// SAFETY: A call to `unregister` for a given instance of `DriverType` is guaranteed to be valid if
> > +// a preceding call to `register` has been successful.
> > +unsafe impl<T: Driver> driver::RegistrationOps for Adapter<T> {
> > +    unsafe fn register(
> > +        sdrv: &Opaque<Self::DriverType>,
> > +        name: &'static CStr,
> > +        module: &'static ThisModule,
> > +    ) -> Result {
> > +        let of_table = match T::OF_ID_TABLE {
> > +            Some(table) => table.as_ptr(),
> > +            None => core::ptr::null(),
> > +        };
> > +
> > +        let acpi_table = match T::ACPI_ID_TABLE {
> > +            Some(table) => table.as_ptr(),
> > +            None => core::ptr::null(),
> > +        };
> > +
> > +        // SAFETY: It's safe to set the fields of `struct serdev_device_driver` on initialization.
> > +        unsafe {
> > +            (*sdrv.get()).driver.name = name.as_char_ptr();
> > +            (*sdrv.get()).probe = Some(Self::probe_callback);
> > +            (*sdrv.get()).remove = Some(Self::remove_callback);
> > +            (*sdrv.get()).driver.of_match_table = of_table;
> > +            (*sdrv.get()).driver.acpi_match_table = acpi_table;
> > +        }
> > +
> > +        // SAFETY: `sdrv` is guaranteed to be a valid `DriverType`.
> > +        to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
> > +    }
> > +
> > +    unsafe fn unregister(sdrv: &Opaque<Self::DriverType>) {
> > +        // SAFETY: `sdrv` is guaranteed to be a valid `DriverType`.
> > +        unsafe { bindings::serdev_device_driver_unregister(sdrv.get()) };
> > +    }
> > +}
> > +
> > +#[doc(hidden)]
> > +#[pin_data(PinnedDrop)]
> > +pub struct PrivateData<'bound, T: Driver> {
> > +    sdev: &'bound Device<device::Bound>,
> > +    #[pin]
> > +    driver: UnsafeCell<MaybeUninit<T::Data<'bound>>>,
> > +    open: UnsafeCell<bool>,
> > +    #[pin]
> > +    active: Mutex<bool>,
> > +}
> > +
> > +#[pinned_drop]
> > +impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> > +    fn drop(self: Pin<&mut Self>) {
> > +        let mut active = self.active.lock();
> > +        if *active {
> > +            // SAFETY:
> > +            // - We have exclusive access to `self.driver`.
> > +            // - `self.driver` is guaranteed to be initialized.
> > +            unsafe { (*self.driver.get()).assume_init_drop() };
> > +            *active = false;
> > +        }
> > +        drop(active);
> > +
> > +        // SAFETY: We have exclusive access to `self.open`.
> > +        if unsafe { *self.open.get() } {
> > +            // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
> > +            // `struct serdev_device`.
> > +            unsafe { bindings::serdev_device_close(self.sdev.as_raw()) };
> > +        }
> > +    }
> > +}
> > +
> > +impl<T: Driver> Adapter<T> {
> > +    const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > +        receive_buf: if T::HAS_RECEIVE {
> > +            Some(Self::receive_buf_callback)
> > +        } else {
> > +            None
> > +        },
> > +        write_wakeup: Some(bindings::serdev_device_write_wakeup),
> > +    };
> > +
> > +    extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi::c_int {
> > +        // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer to
> > +        // a `struct serdev_device`.
> > +        //
> > +        // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
> > +        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
> > +        let info = <Self as driver::Adapter>::id_info(sdev.as_ref());
> > +
> > +        from_result(|| {
> > +            sdev.as_ref().set_drvdata(try_pin_init!(PrivateData::<T> {
> > +                sdev: &**sdev,
> > +                driver: MaybeUninit::<T::Data<'_>>::zeroed().into(),
> > +                open: false.into(),
> > +                active <- new_mutex!(false),
> > +            }))?;
> > +            // SAFETY: We just set drvdata to `PrivateData<'_, T>`.
> > +            let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > +            let private_data = ScopeGuard::new_with_data(private_data, |_| {
> > +                // SAFETY: We just set drvdata to `PrivateData<'_, T>`.
> > +                drop(unsafe { sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() });
> > +            });
> > +            let mut active = private_data.active.lock();
> 
> What's this for? This gives me a strong vibe that something is off. Either the
> serdev subsystem is not having sufficient synchronization or there is an
> abstraction issue.
There is the following issue it trys to solve:

In the rust abstraction, the driver data is only initialized after the
probe. `serdev_device_open` should be called inside or before probe,
because otherwise the driver can't configure the serdev device. But as
a sideeffect, once the serdev device is open, it is possible for
receive_buf_callback to be called, which this mutex prevents while the
driver data is not initialized yet.
> 
> Either way, you really should have more comments about what this active mutex is
> for and why it is needed.
> 
> > +
> > +            // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > +            unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::OPS) };
> > +
> > +            // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
> > +            // to a `serdev_device`.
> > +            to_result(unsafe { bindings::serdev_device_open(sdev.as_raw()) })?;
> > +
> > +            // SAFETY: We have exclusive access to `private_data.open`.
> > +            unsafe { *private_data.open.get() = true };
> > +
> > +            let data = T::probe(sdev, info);
> > +
> > +            // SAFETY: We have exclusive access to `private_data.driver`.
> > +            let driver = unsafe { &mut *private_data.driver.get() };
> > +            // SAFETY:
> > +            // - `driver.as_mut_ptr()` is a valid pointer to uninitialized data.
> > +            // - `private_data.driver` is pinned.
> > +            let result = unsafe { data.__pinned_init(driver.as_mut_ptr()) };
> > +
> > +            *active = result.is_ok();
> > +
> > +            drop(active);
> > +
> > +            result.map(|()| {
> > +                private_data.dismiss();
> > +                0
> > +            })
> > +        })
> > +    }
> > +
> > +    extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> > +        // SAFETY: The serial device bus only ever calls the remove callback with a valid pointer
> > +        // to a `struct serdev_device`.
> > +        //
> > +        // INVARIANT: `sdev` is valid for the duration of `remove_callback()`.
> > +        let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
> > +
> > +        // SAFETY: `remove_callback` is only ever called after a successful call to
> > +        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > +        // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > +        let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > +
> > +        // SAFETY: No one has exclusive access to `private_data.driver`.
> > +        let data = unsafe { &*private_data.driver.get() };
> > +        // SAFETY:
> > +        // - `private_data.driver` is pinned.
> > +        // - `remove_callback` is only ever called after a successful call to `probe_callback`,
> > +        //   hence it's guaranteed that `private_data.driver` was initialized.
> > +        let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> > +
> > +        T::unbind(sdev, data_pinned);
> > +    }
> > +
> > +    extern "C" fn receive_buf_callback(
> > +        sdev: *mut bindings::serdev_device,
> > +        buf: *const u8,
> > +        length: usize,
> > +    ) -> usize {
> > +        // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > +        // pointer to a `struct serdev_device`.
> > +        //
> > +        // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > +        let sdev = unsafe { &*sdev.cast::<Device<device::BoundInternal>>() };
> > +
> > +        // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > +        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > +        // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > +        let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > +        let active = private_data.active.lock();
> > +
> > +        if !*active {
> > +            return length;
> > +        }
> > +
> > +        // SAFETY: No one has exclusive access to `private_data.driver`.
> > +        let data = unsafe { &*private_data.driver.get() };
> > +        // SAFETY:
> > +        // - `private_data.driver` is pinned.
> > +        // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> > +        //   hence it's guaranteed that `private_data.driver` was initialized.
> > +        let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> > +
> > +        // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > +        let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > +
> > +        T::receive(sdev, data_pinned, buf)
> > +    }
> > +}
> > +
> > +impl<T: Driver> driver::Adapter for Adapter<T> {
> > +    type IdInfo = T::IdInfo;
> > +
> > +    fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
> > +        T::OF_ID_TABLE
> > +    }
> > +
> > +    fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
> > +        T::ACPI_ID_TABLE
> > +    }
> > +}
> > +
> > +/// Declares a kernel module that exposes a single serial device bus device driver.
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// kernel::module_serdev_device_driver! {
> > +///     type: MyDriver,
> > +///     name: "Module name",
> > +///     authors: ["Author name"],
> > +///     description: "Description",
> > +///     license: "GPL v2",
> > +/// }
> > +/// ```
> > +#[macro_export]
> > +macro_rules! module_serdev_device_driver {
> > +    ($($f:tt)*) => {
> > +        $crate::module_driver!(<T>, $crate::serdev::Adapter<T>, { $($f)* });
> > +    };
> > +}
> > +
> > +/// The serial device bus device driver trait.
> > +///
> > +/// Drivers must implement this trait in order to get a serial device bus device driver registered.
> > +///
> > +/// # Examples
> > +///
> > +///```
> > +/// # use kernel::{
> > +///     acpi,
> > +///     bindings,
> > +///     device::{
> > +///         Bound,
> > +///         Core, //
> > +///     },
> > +///     of,
> > +///     serdev, //
> > +/// };
> > +///
> > +/// struct MyDriver;
> > +///
> > +/// kernel::of_device_table!(
> > +///     OF_TABLE,
> > +///     MODULE_OF_TABLE,
> > +///     <MyDriver as serdev::Driver>::IdInfo,
> > +///     [
> > +///         (of::DeviceId::new(c"test,device"), ())
> > +///     ]
> > +/// );
> > +///
> > +/// kernel::acpi_device_table!(
> > +///     ACPI_TABLE,
> > +///     MODULE_ACPI_TABLE,
> > +///     <MyDriver as serdev::Driver>::IdInfo,
> > +///     [
> > +///         (acpi::DeviceId::new(c"LNUXBEEF"), ())
> > +///     ]
> > +/// );
> > +///
> > +/// #[vtable]
> > +/// impl serdev::Driver for MyDriver {
> > +///     type IdInfo = ();
> > +///     type Data<'bound> = Self;
> > +///     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> > +///     const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
> > +///
> > +///     fn probe<'bound>(
> > +///         sdev: &'bound serdev::Device<Core<'_>>,
> > +///         _id_info: Option<&'bound Self::IdInfo>,
> > +///     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> > +///         sdev.set_baudrate(115200);
> > +///         sdev.write_all(b"Hello\n", serdev::Timeout::Max)?;
> > +///         Ok(MyDriver)
> > +///     }
> > +/// }
> > +///```
> > +#[vtable]
> > +pub trait Driver {
> > +    /// The type holding driver private data about each device id supported by the driver.
> > +    // TODO: Use associated_type_defaults once stabilized:
> > +    //
> > +    // ```
> > +    // type IdInfo: 'static = ();
> > +    // ```
> > +    type IdInfo: 'static;
> > +
> > +    /// The type of the driver's bus device private data.
> > +    type Data<'bound>: Send + Sync + 'bound;
> > +
> > +    /// The table of OF device ids supported by the driver.
> > +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
> > +
> > +    /// The table of ACPI device ids supported by the driver.
> > +    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
> > +
> > +    /// Serial device bus device driver probe.
> > +    ///
> > +    /// Called when a new serial device bus device is added or discovered.
> > +    /// Implementers should attempt to initialize the device here.
> > +    fn probe<'bound>(
> > +        sdev: &'bound Device<device::Core<'_>>,
> > +        id_info: Option<&'bound Self::IdInfo>,
> > +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
> > +
> > +    /// Serial device bus device driver unbind.
> > +    ///
> > +    /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
> > +    /// is optional.
> > +    ///
> > +    /// This callback serves as a place for drivers to perform teardown operations that require a
> > +    /// `&Device<Core>` or `&Device<Bound>` reference. For instance.
> > +    ///
> > +    /// Otherwise, release operations for driver resources should be performed in `Drop`.
> > +    fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
> > +        let _ = (sdev, this);
> > +    }
> > +
> > +    /// Serial device bus device data receive callback.
> > +    ///
> > +    /// Called when data got received from device.
> > +    ///
> > +    /// Returns the number of bytes accepted.
> > +    fn receive<'bound>(
> > +        sdev: &'bound Device<device::Bound>,
> > +        this: Pin<&Self::Data<'bound>>,
> > +        data: &[u8],
> > +    ) -> usize {
> > +        let _ = (sdev, this, data);
> > +        build_error!(VTABLE_DEFAULT_ERROR)
> > +    }
> > +}
> > +
> > +/// The serial device bus device representation.
> > +///
> > +/// This structure represents the Rust abstraction for a C `struct serdev_device`. The
> > +/// implementation abstracts the usage of an already existing C `struct serdev_device` within Rust
> > +/// code that we get passed from the C side.
> > +///
> > +/// # Invariants
> > +///
> > +/// A [`Device`] instance represents a valid `struct serdev_device` created by the C portion of
> > +/// the kernel.
> > +#[repr(transparent)]
> > +pub struct Device<Ctx: device::DeviceContext = device::Normal>(
> > +    Opaque<bindings::serdev_device>,
> > +    PhantomData<Ctx>,
> > +);
> > +
> > +impl<Ctx: device::DeviceContext> Device<Ctx> {
> > +    fn as_raw(&self) -> *mut bindings::serdev_device {
> > +        self.0.get()
> > +    }
> 
> This and many other functions should be `#[inline]`. I am surprised that Sashiko
> didn't point out.
Yes, inline would make sense here. In this particular case, I used
platform.rs as base, so the missing inline got inherited.

Thanks
- Markus Probst

> 
> Best,
> Gary
signature.asc (application/pgp-signature, 870 B)