| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <76491897ad6e0ff2749935c39702b93adc9951d6.camel@posteo.de>
Date: Tue, 13 Jan 2026 16:15:31 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Kari Argillander <kari.argillander@...il.com>
Cc: Rob Herring <robh@...nel.org>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, Miguel
Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Gary Guo
<gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, Andreas
Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions
On Thu, 2025-12-25 at 17:13 +0200, Kari Argillander wrote:
> On 20.12.2025 klo 20.44 Markus Probst (markus.probst@...teo.de) kirjoitti:
> >
> > Implement the basic serial device bus abstractions required to write a
> > serial device bus device driver with or without the need for initial device
> > data. This includes the following data structures:
> >
> > The `serdev::Driver` trait represents the interface to the driver.
> >
> > The `serdev::Device` abstraction represents a `struct serdev_device`.
> >
> > In order to provide the Serdev specific parts to a generic
> > `driver::Registration` the `driver::RegistrationOps` trait is
> > implemented by `serdev::Adapter`.
> >
> > Co-developed-by: Kari Argillander <kari.argillander@...il.com>
> > Signed-off-by: Kari Argillander <kari.argillander@...il.com>
> > Signed-off-by: Markus Probst <markus.probst@...teo.de>
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/helpers/helpers.c | 1 +
> > rust/helpers/serdev.c | 22 ++
> > rust/kernel/lib.rs | 2 +
> > rust/kernel/serdev.rs | 815 ++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 841 insertions(+)
> >
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index a067038b4b42..bec6c6d0913a 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -79,6 +79,7 @@
> > #include <linux/regulator/consumer.h>
> > #include <linux/sched.h>
> > #include <linux/security.h>
> > +#include <linux/serdev.h>
> > #include <linux/slab.h>
> > #include <linux/task_work.h>
> > #include <linux/tracepoint.h>
> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> > index 79c72762ad9c..834e9fbb897d 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -50,6 +50,7 @@
> > #include "regulator.c"
> > #include "scatterlist.c"
> > #include "security.c"
> > +#include "serdev.c"
> > #include "signal.c"
> > #include "slab.c"
> > #include "spinlock.c"
> > diff --git a/rust/helpers/serdev.c b/rust/helpers/serdev.c
> > new file mode 100644
> > index 000000000000..c52b78ca3fc7
> > --- /dev/null
> > +++ b/rust/helpers/serdev.c
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/serdev.h>
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_driver_unregister(struct serdev_device_driver *sdrv)
> > +{
> > + serdev_device_driver_unregister(sdrv);
> > +}
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_put(struct serdev_device *serdev)
> > +{
> > + serdev_device_put(serdev);
> > +}
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_set_client_ops(struct serdev_device *serdev,
> > + const struct serdev_device_ops *ops)
> > +{
> > + serdev_device_set_client_ops(serdev, ops);
> > +}
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index f812cf120042..cc71195466b6 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -136,6 +136,8 @@
> > pub mod scatterlist;
> > pub mod security;
> > pub mod seq_file;
> > +#[cfg(CONFIG_SERIAL_DEV_BUS)]
> > +pub mod serdev;
> > pub mod sizes;
> > pub mod slice;
> > mod static_assert;
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > new file mode 100644
> > index 000000000000..0f5ef325a054
> > --- /dev/null
> > +++ b/rust/kernel/serdev.rs
> > @@ -0,0 +1,815 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Abstractions for the serial device bus.
> > +//!
> > +//! C header: [`include/linux/serdev.h`](srctree/include/linux/serdev.h)
> > +
> > +use crate::{
> > + acpi,
> > + container_of,
> > + device,
> > + driver,
> > + error::{
> > + from_result,
> > + to_result,
> > + VTABLE_DEFAULT_ERROR, //
> > + },
> > + of,
> > + prelude::*,
> > + time::Jiffies,
> > + types::{AlwaysRefCounted, Opaque}, //
> > +};
> > +
> > +use core::{
> > + cell::UnsafeCell,
> > + marker::PhantomData,
> > + mem::offset_of,
> > + num::NonZero,
> > + ptr::{
> > + addr_of_mut, //
> > + NonNull,
> > + }, //
> > +};
> > +
> > +/// Parity bit to use with a serial device.
> > +#[repr(u32)]
> > +pub enum Parity {
> > + /// No parity bit.
> > + None = bindings::serdev_parity_SERDEV_PARITY_NONE,
> > + /// Even partiy.
> > + Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
> > + /// Odd parity.
> > + Odd = bindings::serdev_parity_SERDEV_PARITY_ODD,
> > +}
> > +
> > +/// Timeout in Jiffies.
> > +pub enum Timeout {
> > + /// Wait for a specific amount of [`Jiffies`].
> > + Value(NonZero<Jiffies>),
> > + /// Wait as long as possible.
> > + ///
> > + /// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
> > + MaxScheduleTimeout,
> > +}
> > +
> > +impl Timeout {
> > + fn into_jiffies(self) -> isize {
> > + match self {
> > + Self::Value(value) => value.get().try_into().unwrap_or_default(),
> > + Self::MaxScheduleTimeout => 0,
> > + }
> > + }
> > +}
> > +
> > +/// An adapter for the registration of serial device bus device drivers.
> > +pub struct Adapter<T: Driver>(T);
> > +
> > +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
> > +// a preceding call to `register` has been successful.
> > +unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
> > + type RegType = bindings::serdev_device_driver;
> > +
> > + unsafe fn register(
> > + sdrv: &Opaque<Self::RegType>,
> > + name: &'static CStr,
> > + module: &'static ThisModule,
> > + ) -> Result {
> > + let of_table = match T::OF_ID_TABLE {
> > + Some(table) => table.as_ptr(),
> > + None => core::ptr::null(),
> > + };
> > +
> > + let acpi_table = match T::ACPI_ID_TABLE {
> > + Some(table) => table.as_ptr(),
> > + None => core::ptr::null(),
> > + };
> > +
> > + // SAFETY: It's safe to set the fields of `struct serdev_device_driver` on initialization.
> > + unsafe {
> > + (*sdrv.get()).driver.name = name.as_char_ptr();
> > + (*sdrv.get()).probe = Some(Self::probe_callback);
> > + (*sdrv.get()).remove = Some(Self::remove_callback);
> > + (*sdrv.get()).driver.of_match_table = of_table;
> > + (*sdrv.get()).driver.acpi_match_table = acpi_table;
> > + }
> > +
> > + // SAFETY: `sdrv` is guaranteed to be a valid `RegType`.
> > + to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
> > + }
> > +
> > + unsafe fn unregister(sdrv: &Opaque<Self::RegType>) {
> > + // SAFETY: `sdrv` is guaranteed to be a valid `RegType`.
> > + unsafe { bindings::serdev_device_driver_unregister(sdrv.get()) };
> > + }
> > +}
> > +
> > +#[pin_data]
> > +struct Drvdata<T: Driver + 'static> {
> > + #[pin]
> > + driver: T,
> > + initial_data: UnsafeCell<Option<T::InitialData>>,
> > + late_probe_data: UnsafeCell<Option<Pin<KBox<T::LateProbeData>>>>,
> > +}
> > +
> > +impl<T: Driver + 'static> Adapter<T> {
> > + const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > + receive_buf: if T::HAS_RECEIVE {
> > + Some(Self::receive_buf_callback)
> > + } else {
> > + None
> > + },
> > + write_wakeup: if T::HAS_WRITE_WAKEUP {
> > + Some(Self::write_wakeup_callback)
> > + } else {
> > + Some(bindings::serdev_device_write_wakeup)
> > + },
> > + };
> > + const INITIAL_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > + receive_buf: Some(Self::initial_receive_buf_callback),
> > + write_wakeup: if T::HAS_WRITE_WAKEUP_INITIAL {
> > + Some(Self::initial_write_wakeup_callback)
> > + } else {
> > + Some(bindings::serdev_device_write_wakeup)
> > + },
> > + };
> > + const NO_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > + receive_buf: None,
> > + write_wakeup: Some(bindings::serdev_device_write_wakeup),
> > + };
> > +
> > + extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi::c_int {
> > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer to
> > + // a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > + let info = <Self as driver::Adapter>::id_info(sdev.as_ref());
> > +
> > + from_result(|| {
> > + let data = try_pin_init!(Drvdata {
> > + driver <- T::probe(sdev, info),
> > + initial_data: Some(Default::default()).into(),
> > + late_probe_data: None.into(),
> > + });
> > +
> > + sdev.as_ref().set_drvdata(data)?;
> > +
> > + // SAFETY: We just set drvdata to `Drvdata<T>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::INITIAL_OPS) };
> > +
> > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
> > + // to a `struct serdev_device`.
> > + to_result(unsafe {
> > + bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
> > + })?;
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + T::configure(sdev, unsafe { Pin::new_unchecked(&data.driver) }, info)?;
> > +
> > + if !T::HAS_RECEIVE_INITIAL {
> > + // SAFETY:
> > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
> > + // `data.late_probe_data`.
> > + // - We just initialized `data.initial_data` with Some.
> > + unsafe { Self::do_late_probe(sdev, data)? };
> > + }
> > +
> > + Ok(0)
> > + })
> > + }
> > +
> > + /// # Safety
> > + ///
> > + /// The caller must guarantee, that we have exclusive access to `data.initial_data` and
> > + /// `data.late_probe_data`. `data.initial_data` must be Some.
> > + /// (i. e. `late_probe` has not been called yet).
> > + unsafe fn do_late_probe(sdev: &Device<device::CoreInternal>, data: Pin<&Drvdata<T>>) -> Result {
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY: The function contract guarantees that we have exclusive access to
> > + // `data.initial_data`.
> > + let initial_data = unsafe { &mut *data.initial_data.get() };
> > +
> > + // SAFETY: The function contract guarantees that we have exclusive access to
> > + // `data.late_probe_data`.
> > + let late_probe_data = unsafe { &mut *data.late_probe_data.get() };
> > +
> > + *late_probe_data = Some(KBox::pin_init(
> > + T::late_probe(
> > + sdev,
> > + data_driver,
> > + // SAFETY: The function contract guarantees that `data.initial_data` is Some.
> > + unsafe { initial_data.take().unwrap_unchecked() },
> > + ),
> > + GFP_KERNEL,
> > + )?);
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::OPS) };
> > + Ok(())
> > + }
> > +
> > + extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> > + // SAFETY: The serial device bus only ever calls the remove callback with a valid pointer
> > + // to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `remove_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `remove_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<T>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_obtain::<Drvdata<T>>() };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.late_probe_data` is guaranteed to be pinned.
> > + // - It is guaranteed that we have exclusive access to `data.late_probe_data`.
> > + let late_probe_data = unsafe {
> > + (*data.late_probe_data.get())
> > + .as_deref()
> > + .map(|data| Pin::new_unchecked(data))
> > + };
> > +
> > + T::unbind(sdev, data_driver, late_probe_data);
> > + }
> > +
> > + extern "C" fn receive_buf_callback(
> > + sdev: *mut bindings::serdev_device,
> > + buf: *const u8,
> > + length: usize,
> > + ) -> usize {
> > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.late_probe_data` is guaranteed to be Some.
> > + // - `data.late_probe_data` is guaranteed to be pinned.
> > + let late_probe_data = unsafe {
> > + Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> > + };
> > +
> > + T::receive(sdev, data_driver, late_probe_data, buf)
> > + }
> > +
> > + extern "C" fn write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> > + // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `write_wakeup_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.late_probe_data` is guaranteed to be Some.
> > + // - `data.late_probe_data` is guaranteed to be pinned.
> > + let late_probe_data = unsafe {
> > + Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> > + };
> > +
> > + // SAFETY: As long as the driver implementation meets the safety requirements, this call
> > + // is safe.
> > + unsafe { T::write_wakeup(sdev, data_driver, late_probe_data) };
> > + }
> > +
> > + extern "C" fn initial_receive_buf_callback(
> > + sdev: *mut bindings::serdev_device,
> > + buf: *const u8,
> > + length: usize,
> > + ) -> usize {
> > + if !T::HAS_RECEIVE_INITIAL {
> > + return 0;
> > + }
> > +
> > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > +
> > + // SAFETY: `initial_receive_buf_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let driver_data = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.initial_data` is always Some until `InitialReceiveResult::Ready` is
> > + // returned below.
> > + // - It is guaranteed that we have exclusive access to `data.initial_data`.
> > + let initial_data = unsafe { (*data.initial_data.get()).as_mut().unwrap_unchecked() };
> > +
> > + match T::receive_initial(sdev, driver_data, initial_data, buf) {
> > + Ok(InitialReceiveResult::Pending(size)) => size,
> > + Ok(InitialReceiveResult::Ready(size)) => {
> > + // SAFETY:
> > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
> > + // `data.late_probe_data`.
> > + // - We just initialized `data.initial_data` with Some.
> > + if let Err(err) = unsafe { Self::do_late_probe(sdev, data) } {
> > + dev_err!(sdev.as_ref(), "Unable to late probe device: {err:?}\n");
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to
> > + // `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
> > + return length;
> > + }
> > + size
> > + }
> > + Err(err) => {
> > + dev_err!(
> > + sdev.as_ref(),
> > + "Unable to receive initial data for probe: {err:?}.\n"
> > + );
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
> > + length
> > + }
> > + }
> > + }
> > +
> > + extern "C" fn initial_write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> > + // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `initial_write_wakeup_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY: As long as the driver implementation meets the safety requirements, this call
> > + // is safe.
> > + unsafe { T::write_wakeup_initial(sdev, data_driver) };
> > + }
> > +}
> > +
> > +impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
> > + type IdInfo = T::IdInfo;
> > +
> > + fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
> > + T::OF_ID_TABLE
> > + }
> > +
> > + fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
> > + T::ACPI_ID_TABLE
> > + }
> > +}
> > +
> > +/// Declares a kernel module that exposes a single serial device bus device driver.
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// kernel::module_serdev_device_driver! {
> > +/// type: MyDriver,
> > +/// name: "Module name",
> > +/// authors: ["Author name"],
> > +/// description: "Description",
> > +/// license: "GPL v2",
> > +/// }
> > +/// ```
> > +#[macro_export]
> > +macro_rules! module_serdev_device_driver {
> > + ($($f:tt)*) => {
> > + $crate::module_driver!(<T>, $crate::serdev::Adapter<T>, { $($f)* });
> > + };
> > +}
> > +
> > +/// Result for `receive_initial` in [`Driver`].
> > +pub enum InitialReceiveResult {
> > + /// More data is required.
> > + ///
> > + /// The inner data is the number of bytes accepted.
> > + Pending(usize),
> > + /// Ready for late probe.
> > + ///
> > + /// The inner data is the number of bytes accepted.
> > + Ready(usize),
> > +}
> > +
> > +/// The serial device bus device driver trait.
> > +///
> > +/// Drivers must implement this trait in order to get a serial device bus device driver registered.
> > +///
> > +/// # Examples
> > +///
> > +///```
> > +/// # use kernel::{
> > +/// acpi,
> > +/// bindings,
> > +/// device::{
> > +/// Bound,
> > +/// Core, //
> > +/// },
> > +/// of,
> > +/// serdev, //
> > +/// };
> > +///
> > +/// struct MyDriver;
> > +///
> > +/// kernel::of_device_table!(
> > +/// OF_TABLE,
> > +/// MODULE_OF_TABLE,
> > +/// <MyDriver as serdev::Driver>::IdInfo,
> > +/// [
> > +/// (of::DeviceId::new(c"test,device"), ())
> > +/// ]
> > +/// );
> > +///
> > +/// kernel::acpi_device_table!(
> > +/// ACPI_TABLE,
> > +/// MODULE_ACPI_TABLE,
> > +/// <MyDriver as serdev::Driver>::IdInfo,
> > +/// [
> > +/// (acpi::DeviceId::new(c"LNUXBEEF"), ())
> > +/// ]
> > +/// );
> > +///
> > +/// #[vtable]
> > +/// impl serdev::Driver for MyDriver {
> > +/// type IdInfo = ();
> > +/// type LateProbeData = ();
> > +/// type InitialData = ();
> > +/// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> > +/// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
> > +///
> > +/// fn probe(
> > +/// _sdev: &serdev::Device,
> > +/// _id_info: Option<&Self::IdInfo>,
> > +/// ) -> impl PinInit<Self, Error> {
> > +/// Err(ENODEV)
> > +/// }
> > +///
> > +/// fn configure(
> > +/// dev: &serdev::Device<Core>,
> > +/// _this: Pin<&Self>,
> > +/// _id_info: Option<&Self::IdInfo>,
> > +/// ) -> Result {
> > +/// dev.set_baudrate(115200);
> > +/// Ok(())
> > +/// }
> > +///
> > +/// fn late_probe(
> > +/// _dev: &serdev::Device<Bound>,
> > +/// _this: Pin<&Self>,
> > +/// _initial_data: Self::InitialData,
> > +/// ) -> impl PinInit<Self::LateProbeData, Error> {
> > +/// Ok(())
> > +/// }
> > +/// }
> > +///```
> > +#[vtable]
> > +pub trait Driver: Send {
> > + /// The type holding driver private data about each device id supported by the driver.
> > + // TODO: Use associated_type_defaults once stabilized:
> > + //
> > + // ```
> > + // type IdInfo: 'static = ();
> > + // type LateProbeData: Send + 'static = ();
> > + // type InitialData: Default + Send + 'static = ();
> > + // ```
> > + type IdInfo: 'static;
> > +
> > + /// Data returned in `late_probe` function.
> > + type LateProbeData: Send + 'static;
> > + /// Data used for initial data.
> > + type InitialData: Default + Send + 'static;
> > +
> > + /// The table of OF device ids supported by the driver.
> > + const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
> > +
> > + /// The table of ACPI device ids supported by the driver.
> > + const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
> > +
> > + /// Serial device bus device driver probe.
> > + ///
> > + /// Called when a new serial device bus device is added or discovered.
> > + /// Implementers should attempt to initialize the device here.
> > + fn probe(sdev: &Device, id_info: Option<&Self::IdInfo>) -> impl PinInit<Self, Error>;
> > +
> > + /// Serial device bus device driver configure.
> > + ///
> > + /// Called directly after the serial device bus device has been opened.
> > + /// This should be used for setting up the communication (i. e. baudrate, flow control, parity)
> > + /// and sending an initial hello if required.
> > + fn configure(
> > + sdev: &Device<device::Core>,
> > + this: Pin<&Self>,
> > + id_info: Option<&Self::IdInfo>,
> > + ) -> Result;
> > +
> > + /// Serial device bus device data receive callback (initial).
> > + ///
> > + /// Called when data got received from device, before `late_probe` has been called.
> > + /// This should be used, to get information about the device for `late_probe`.
> > + ///
> > + /// Returns `InitialReceiveResult::Pending` if more data is still required for `late_probe`.
> > + /// Otherwise `InitialReceiveResult::Ready`. The inner data is the number of bytes accepted.
> > + fn receive_initial(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + initial_data: &mut Self::InitialData,
> > + data: &[u8],
> > + ) -> Result<InitialReceiveResult> {
> > + let _ = (sdev, this, initial_data, data);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +
> > + /// Serial device bus device write wakeup callback (initial).
> > + ///
> > + /// Called when ready to transmit more data, before `late_probe` has been called.
> > + ///
> > + /// # Safety
> > + ///
> > + /// This function must not sleep.
> > + unsafe fn write_wakeup_initial(sdev: &Device<device::Core>, this: Pin<&Self>) {
> > + let _ = (sdev, this);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +
> > + /// Serial device bus device late probe.
> > + ///
> > + /// Called after the initial data is available.
> > + /// If `receive_initial` isn't implemented, this will be called directly after configure.
> > + fn late_probe(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + initial_data: Self::InitialData,
> > + ) -> impl PinInit<Self::LateProbeData, Error>;
> > +
> > + /// Serial device bus device driver unbind.
> > + ///
> > + /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
> > + /// is optional.
> > + ///
> > + /// This callback serves as a place for drivers to perform teardown operations that require a
> > + /// `&Device<Core>` or `&Device<Bound>` reference. For instance.
> > + ///
> > + /// Otherwise, release operations for driver resources should be performed in `Self::drop`.
> > + fn unbind(
> > + sdev: &Device<device::Core>,
> > + this: Pin<&Self>,
> > + late_probe_this: Option<Pin<&Self::LateProbeData>>,
> > + ) {
> > + let _ = (sdev, this, late_probe_this);
> > + }
> > +
> > + /// Serial device bus device data receive callback.
> > + ///
> > + /// Called when data got received from device.
> > + ///
> > + /// Returns the number of bytes accepted.
> > + fn receive(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + late_probe_this: Pin<&Self::LateProbeData>,
> > + data: &[u8],
> > + ) -> usize {
> > + let _ = (sdev, this, late_probe_this, data);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +
> > + /// Serial device bus device write wakeup callback.
> > + ///
> > + /// Called when ready to transmit more data.
> > + ///
> > + /// # Safety
> > + ///
> > + /// This function must not sleep.
> > + unsafe fn write_wakeup(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + late_probe_this: Pin<&Self::LateProbeData>,
> > + ) {
> > + let _ = (sdev, this, late_probe_this);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +}
> > +
> > +/// The serial device bus device representation.
> > +///
> > +/// This structure represents the Rust abstraction for a C `struct serdev_device`. The
> > +/// implementation abstracts the usage of an already existing C `struct serdev_device` within Rust
> > +/// code that we get passed from the C side.
> > +///
> > +/// # Invariants
> > +///
> > +/// A [`Device`] instance represents a valid `struct serdev_device` created by the C portion of
> > +/// the kernel.
> > +#[repr(transparent)]
> > +pub struct Device<Ctx: device::DeviceContext = device::Normal>(
> > + Opaque<bindings::serdev_device>,
> > + PhantomData<Ctx>,
> > +);
> > +
> > +impl<Ctx: device::DeviceContext> Device<Ctx> {
> > + fn as_raw(&self) -> *mut bindings::serdev_device {
> > + self.0.get()
> > + }
> > +}
> > +
> > +impl Device<device::Bound> {
> > + /// Set the baudrate in bits per second.
> > + ///
> > + /// Common baudrates are 115200, 9600, 19200, 57600, 4800.
> > + ///
> > + /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
> > + pub fn set_baudrate(&self, speed: u32) -> u32 {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_set_baudrate(self.as_raw(), speed) }
> > + }
> > +
> > + /// Set if flow control should be enabled.
> > + ///
> > + /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
> > + pub fn set_flow_control(&self, enable: bool) {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_set_flow_control(self.as_raw(), enable) };
> > + }
> > +
> > + /// Set parity to use.
> > + ///
> > + /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
> > + pub fn set_parity(&self, parity: Parity) -> Result {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + to_result(unsafe { bindings::serdev_device_set_parity(self.as_raw(), parity as u32) })
> > + }
> > +
> > + /// Write data to the serial device until the controller has accepted all the data or has
> > + /// been interrupted by a timeout or signal.
> > + ///
> > + /// Note that any accepted data has only been buffered by the controller. Use
> > + /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
> > + /// emptied.
> > + ///
> > + /// Returns the number of bytes written (less than data if interrupted).
> > + /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> > + /// before any bytes were written.
> > + pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
> > + // SAFETY:
> > + // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
> > + // `data.len()`.
> > + let ret = unsafe {
> > + bindings::serdev_device_write(
> > + self.as_raw(),
> > + data.as_ptr(),
> > + data.len(),
> > + timeout.into_jiffies(),
> > + )
> > + };
> > + if ret < 0 {
> > + // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> > + // which always fit into a `i32`.
> > + Err(Error::from_errno(ret as i32))
> > + } else {
> > + Ok(ret.unsigned_abs())
> > + }
> > + }
> > +
> > + /// Write data to the serial device.
> > + ///
> > + /// If you want to write until the controller has accepted all the data, use
> > + /// [`Device::write_all`].
> > + ///
> > + /// Note that any accepted data has only been buffered by the controller. Use
> > + /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
> > + /// emptied.
> > + ///
> > + /// Returns the number of bytes written (less than data if interrupted).
> > + /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> > + /// before any bytes were written.
> > + pub fn write(&self, data: &[u8]) -> Result<u32> {
> > + // SAFETY:
> > + // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
> > + // `data.len()`.
> > + let ret =
> > + unsafe { bindings::serdev_device_write_buf(self.as_raw(), data.as_ptr(), data.len()) };
> > + if ret < 0 {
> > + Err(Error::from_errno(ret))
> > + } else {
> > + Ok(ret.unsigned_abs())
> > + }
> > + }
> > +
> > + /// Send data to the serial device immediately.
> > + ///
> > + /// Note that this doesn't guarantee that the data has been transmitted.
> > + /// Use [`Device::wait_until_sent`] for this purpose.
> > + pub fn write_flush(&self) {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_write_flush(self.as_raw()) };
> > + }
> > +
> > + /// Wait for the data to be sent.
> > + ///
> > + /// After this function, the write buffer of the controller should be empty.
> > + pub fn wait_until_sent(&self, timeout: Timeout) {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout.into_jiffies()) };
> > + }
> > +}
> > +
> > +// SAFETY: `serdev::Device` is a transparent wrapper of `struct serdev_device`.
> > +// The offset is guaranteed to point to a valid device field inside `serdev::Device`.
> > +unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx> {
> > + const OFFSET: usize = offset_of!(bindings::serdev_device, dev);
> > +}
> > +
> > +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
> > +// argument.
> > +kernel::impl_device_context_deref!(unsafe { Device });
> > +kernel::impl_device_context_into_aref!(Device);
> > +
> > +// SAFETY: Instances of `Device` are always reference-counted.
> > +unsafe impl AlwaysRefCounted for Device {
> > + fn inc_ref(&self) {
> > + self.as_ref().inc_ref();
> > + }
> > +
> > + unsafe fn dec_ref(obj: NonNull<Self>) {
> > + // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> > + unsafe { bindings::serdev_device_put(obj.cast().as_ptr()) }
> > + }
> > +}
> > +
> > +impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> > + fn as_ref(&self) -> &device::Device<Ctx> {
> > + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> > + // `struct serdev_device`.
> > + let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
> > +
> > + // SAFETY: `dev` points to a valid `struct device`.
> > + unsafe { device::Device::from_raw(dev) }
> > + }
> > +}
> > +
> > +impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &Device<Ctx> {
> > + type Error = kernel::error::Error;
> > +
> > + fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
> > + // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
> > + // `struct device`.
> > + if !unsafe { bindings::is_serdev_device(dev.as_raw()) } {
> > + return Err(EINVAL);
> > + }
> > +
> > + // SAFETY: We've just verified that the device_type of `dev` is
> > + // `serdev_device_type`, hence `dev` must be embedded in a valid `struct
> > + // serdev_device_driver` as guaranteed by the corresponding C code.
> > + let sdev = unsafe { container_of!(dev.as_raw(), bindings::serdev_device, dev) };
> > +
> > + // SAFETY: `sdev` is a valid pointer to a `struct serdev_device_driver`.
> > + Ok(unsafe { &*sdev.cast() })
> > + }
> > +}
> > +
> > +// SAFETY: A `Device` is always reference-counted and can be released from any thread.
> > +unsafe impl Send for Device {}
> > +
> > +// SAFETY: `Device` can be shared among threads because all methods of `Device`
> > +// (i.e. `Device<Normal>) are thread safe.
> > +unsafe impl Sync for Device {}
> >
> > --
> > 2.51.2
> >
> >
>
> Currently I'm not a huge fan that we need to have configure(),
> receive_initial(), write_wakeup_initial() and late_probe(). Also I do not
> like that we hide hide open() completely from user. Maybe user want's to do
> something if there is error. This does not feel very ergonomic or standard
> kernel way. So after thinking a bit more about this idea I have come up with
> a different approach.
>
> We will have just probe() as usual. In probe() user will be forced to do open()
> and finnish() on serdev device. Between open() and finnish() user can
> configure the device and do initial reads / writes as needed. After probe is
> finished we will switch to runtime ops.
>
> This is not production ready code and I have just tested the idea locally
> without kernel. Here is example how probe() could look like:
>
> ```rust
> fn probe(sdev: &serdev::Device<Core>, _id_info:
> Option<&Self::IdInfo>)-> impl PinInit<(Self, ProbeEndToken), Error> {
> let sdev= sdev.open()?;
> // After open we can configure and do initial reads / writes.
> sdev.set_baudrate(9600);
>
> // Here we can also write if needed.
>
> for _ in 0..8 {
> // For initial reads we will have iterator. This cannot be
> done after finish().
> let _ = sdev.next_byte(Duration::from_millis(300));
> }
>
> // We do finnish so user cannot store "initial sdev" anywhere. Also get
> // end_token so probe cannot return unless open() and finnish() are called.
> let (sdev, end_token) = sdev.finish();
> Ok(try_pin_init!((Self {sdev}, end_token)))
> }
> ```
>
> then our receive_initial will look something like this. (Not runnable
> on kernel).
> Idea is that we have event for next_byte(). So initial read is little bit slower
> but that should not be used so much that it matters in practice.
>
> ```rust
> extern "C" fn initial_receive_buf(dev: *mut serdev_device, buf: *const
> u8, count: usize) -> usize {
> let abs = unsafe { &mut *((*dev).client_data as *mut Abstraction) };
>
> let mut consumed = 0usize;
>
> loop {
> // Wait until we can either switch or deliver one byte (slot
> must be empty).
> let mut st = abs.st.lock().unwrap();
> while !st.probe_finished && !(st.want_byte && st.slot.is_none()) {
> st = abs.ev.wait(st).unwrap();
> }
>
> if st.probe_finished {
> // TODO: Do we lock write lock so it is really safe to
> change OPS here?
> unsafe { bindings::serdev_device_set_client_ops(dev,
> &RUNTIME_OPS) };
>
> // Forward the *remaining* bytes that we have NOT taken
> out of this buf.
> let rem_ptr = unsafe { buf.add(consumed) };
> let rem = count - consumed;
> return consumed + Self::runtime_receive_buf(dev, rem_ptr, rem);
> }
>
> // Deliver exactly one byte to iterator (slot is empty here).
> let b = unsafe { *buf.add(consumed) };
> st.slot = Some(b);
> // wake next_byte()
> abs.ev.notify_all();
>
> // Now WAIT until next_byte really consumes it (slot becomes None),
> // or probe finishes (in which case we’ll flush it to runtime).
> while !st.probe_finished && st.slot.is_some() {
> st = abs.ev.wait(st).unwrap();
> }
> if st.probe_finished {
> continue;
> }
>
> // Now it is truly consumed by next_byte().
> consumed += 1;
> if consumed == count {
> return count;
> }
> }
> }
> ```
>
> Does anyone have opinions which is better or is there some other way
> for this which
> we have not yet discovered?
I am not sure if using this approach is intended by the serdev
subsystem. Otherwise I assume there already would be a C function in
the subsystem for reading and waiting for data on probe.
There currently is only one serial device bus driver in the kernel,
which needs initial data:
drivers/bluetooth/hci_uart.h
drivers/bluetooth/hci_ldisc.c
drivers/bluetooth/hci_serdev.c
This driver retrieves this initial data after probe (not in the probe)
and then initializes it with a workqueue. Given it is part of the
kernel, I assume this is the intended behaviour.
@Danilo: Can you share your opinion on this?
Thanks
- Markus Probst
Download attachment "signature.asc" of type "application/pgp-signature" (871 bytes)
Powered by blists - more mailing lists