Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH v2 0/4] rust: serdev: trivial fixes
@ 2026-07-18 12:47 Markus Probst
  2026-07-18 12:47 ` [PATCH v2 1/4] rust: serdev: fix typo in rustdoc Markus Probst
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst,
	kernel test robot

Those are small fixes discovered by the kernel test robot and Gary Guo
after the serdev rust abstraction have already been merged.

This includes
- a fix for a documentation typo
- adding missing `#[inline]` attributes
- a small structual change for timeout

There is no urgency to merge this.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v2:
- resolve Sashiko issues:
  - reword small parts of the `PrivateData::active` documentation
  - fix documentation references left to `serdev::Timeout`.
- Link to v1: https://patch.msgid.link/20260718-rust_serdev_fixes-v1-0-35433b6dce0f@posteo.de

---
Markus Probst (4):
      rust: serdev: fix typo in rustdoc
      rust: serdev: mark small functions as `#[inline]`
      rust: serdev: document `PrivateData::active`
      rust: serdev: remove `serdev::Timeout`

 rust/kernel/serdev.rs | 82 +++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 38 deletions(-)
---
base-commit: 25acf039a5dbb40b6865d9392e05be8e713a39f3
change-id: 20260717-rust_serdev_fixes-9baf5f310278


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/4] rust: serdev: fix typo in rustdoc
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
@ 2026-07-18 12:47 ` Markus Probst
  2026-07-18 12:47 ` [PATCH v2 2/4] rust: serdev: mark small functions as `#[inline]` Markus Probst
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst,
	kernel test robot

Fix trivial typo

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202607172113.hLDQgD4l-lkp@intel.com/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/serdev.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 0ffcef1849d2..47ca57966abf 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -515,7 +515,7 @@ pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
     /// [`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
+    /// [`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

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] rust: serdev: mark small functions as `#[inline]`
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
  2026-07-18 12:47 ` [PATCH v2 1/4] rust: serdev: fix typo in rustdoc Markus Probst
@ 2026-07-18 12:47 ` Markus Probst
  2026-07-18 12:47 ` [PATCH v2 3/4] rust: serdev: document `PrivateData::active` Markus Probst
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst

These methods should be inlined for optimization reasons.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DK0SX3P5K1J6.1OV4ETJMXW83R@garyguo.net/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/serdev.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 47ca57966abf..79d61c6ceeec 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -440,6 +440,7 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
 );
 
 impl<Ctx: device::DeviceContext> Device<Ctx> {
+    #[inline]
     fn as_raw(&self) -> *mut bindings::serdev_device {
         self.0.get()
     }
@@ -451,6 +452,7 @@ impl Device<device::Bound> {
     /// Common baudrates are 115200, 9600, 19200, 57600, 4800.
     ///
     /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    #[inline]
     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) };
@@ -464,6 +466,7 @@ pub fn set_baudrate(&self, speed: u32) -> Result<(), u32> {
     /// Set if flow control should be enabled.
     ///
     /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    #[inline]
     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) };
@@ -472,6 +475,7 @@ pub fn set_flow_control(&self, enable: bool) {
     /// Set parity to use.
     ///
     /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    #[inline]
     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) })
@@ -487,6 +491,7 @@ pub fn set_parity(&self, parity: Parity) -> Result {
     /// 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.
+    #[inline]
     pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
         if data.len() > i32::MAX as usize {
             return Err(EINVAL);
@@ -520,6 +525,7 @@ pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
     ///
     /// Returns the number of bytes written (less than `data.len()` if not enough room in the
     /// write buffer).
+    #[inline]
     pub fn write(&self, data: &[u8]) -> Result<u32> {
         if data.len() > i32::MAX as usize {
             return Err(EINVAL);
@@ -539,6 +545,7 @@ pub fn write(&self, data: &[u8]) -> Result<u32> {
     ///
     /// Note that this doesn't guarantee that the data has been transmitted.
     /// Use [`Device::wait_until_sent`] for this purpose.
+    #[inline]
     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()) };
@@ -547,6 +554,7 @@ pub fn write_flush(&self) {
     /// Wait for the data to be sent.
     ///
     /// After this function, the write buffer of the controller should be empty.
+    #[inline]
     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()) };

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] rust: serdev: document `PrivateData::active`
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
  2026-07-18 12:47 ` [PATCH v2 1/4] rust: serdev: fix typo in rustdoc Markus Probst
  2026-07-18 12:47 ` [PATCH v2 2/4] rust: serdev: mark small functions as `#[inline]` Markus Probst
@ 2026-07-18 12:47 ` Markus Probst
  2026-07-18 12:47 ` [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout` Markus Probst
  2026-07-18 14:10 ` [PATCH v2 0/4] rust: serdev: trivial fixes Danilo Krummrich
  4 siblings, 0 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst

Gary pointed out that it isn't clear for what the mutex is used for,
thus adding documentation comments for it.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DK0SX3P5K1J6.1OV4ETJMXW83R@garyguo.net/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/serdev.rs | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 79d61c6ceeec..a78dfa6e2c27 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -132,6 +132,23 @@ pub struct PrivateData<'bound, T: Driver> {
     #[pin]
     driver: UnsafeCell<MaybeUninit<T::Data<'bound>>>,
     open: UnsafeCell<bool>,
+    /// Whether `receive_buf_callback` is allowed to call `Driver::receive`.
+    ///
+    /// If locked, the receive_buf_callback will be blocked on data reception.
+    /// This is the case while the driver is being probed or while [`PrivateData`] is being dropped.
+    /// This is necessary, because we need to open the serdev device before the driver has been
+    /// probed in order to allow it to be configured, which allows `receive_buf_callback` to be
+    /// called. Thus we need to block data until probe completes and the driver data becomes
+    /// initialized.
+    ///
+    /// If unlocked and true, the receive_buf_callback will forward the data to
+    /// `Driver::receive`. This is the normal state of operation.
+    ///
+    /// If unlocked and false, the receive_buf_callback will throw away the data.
+    /// This is only the case, if the serdev device is open and
+    /// - the driver returned an error in probe
+    /// or
+    /// - the driver data already has been dropped, because it was unbound.
     #[pin]
     active: Mutex<bool>,
 }

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
                   ` (2 preceding siblings ...)
  2026-07-18 12:47 ` [PATCH v2 3/4] rust: serdev: document `PrivateData::active` Markus Probst
@ 2026-07-18 12:47 ` Markus Probst
  2026-07-18 15:02   ` Gary Guo
  2026-07-18 14:10 ` [PATCH v2 0/4] rust: serdev: trivial fixes Danilo Krummrich
  4 siblings, 1 reply; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst

Instead of relying on its own timeout types, the abstraction should make
use of `impl Into<Jiffies>`.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DK0SX3P5K1J6.1OV4ETJMXW83R@garyguo.net/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/serdev.rs | 55 +++++++++++++++++----------------------------------
 1 file changed, 18 insertions(+), 37 deletions(-)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index a78dfa6e2c27..0fdb37cff15d 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -20,11 +20,7 @@
         aref::AlwaysRefCounted,
         Mutex, //
     },
-    time::{
-        msecs_to_jiffies,
-        Jiffies,
-        Msecs, //
-    },
+    time::Jiffies,
     types::{
         Opaque,
         ScopeGuard, //
@@ -35,7 +31,6 @@
     cell::UnsafeCell,
     marker::PhantomData,
     mem::{offset_of, MaybeUninit},
-    num::NonZero,
     ptr::NonNull, //
 };
 
@@ -50,30 +45,6 @@ pub enum 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);
 
@@ -379,7 +350,7 @@ macro_rules! module_serdev_device_driver {
 ///         _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)?;
+///         sdev.write_all(b"Hello\n", 0usize)?;
 ///         Ok(MyDriver)
 ///     }
 /// }
@@ -505,11 +476,13 @@ pub fn set_parity(&self, parity: Parity) -> Result {
     /// [`Device::wait_until_sent`] to make sure the controller write buffer has actually been
     /// emptied.
     ///
+    /// Use a timeout of 0 to wait indefinitely.
+    ///
     /// 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.
+    /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
     #[inline]
-    pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
+    pub fn write_all(&self, data: &[u8], timeout: impl Into<Jiffies>) -> Result<usize> {
         if data.len() > i32::MAX as usize {
             return Err(EINVAL);
         }
@@ -523,7 +496,7 @@ pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
                 self.as_raw(),
                 data.as_ptr(),
                 data.len(),
-                timeout.into_jiffies(),
+                isize::try_from(timeout.into()).unwrap_or_default(),
             )
         };
         // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
@@ -570,11 +543,19 @@ pub fn write_flush(&self) {
 
     /// Wait for the data to be sent.
     ///
-    /// After this function, the write buffer of the controller should be empty.
+    /// After this function, the write buffer of the controller should be empty or the timeout
+    /// elapsed.
+    ///
+    /// Use a timeout of 0 to wait indefinitely.
     #[inline]
-    pub fn wait_until_sent(&self, timeout: Timeout) {
+    pub fn wait_until_sent(&self, timeout: impl Into<Jiffies>) {
         // 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()) };
+        unsafe {
+            bindings::serdev_device_wait_until_sent(
+                self.as_raw(),
+                isize::try_from(timeout.into()).unwrap_or_default(),
+            )
+        };
     }
 }
 

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/4] rust: serdev: trivial fixes
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
                   ` (3 preceding siblings ...)
  2026-07-18 12:47 ` [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout` Markus Probst
@ 2026-07-18 14:10 ` Danilo Krummrich
  4 siblings, 0 replies; 9+ messages in thread
From: Danilo Krummrich @ 2026-07-18 14:10 UTC (permalink / raw)
  To: Markus Probst
  Cc: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
	Onur Özkan, linux-serial, rust-for-linux, linux-kernel,
	kernel test robot

On Sat Jul 18, 2026 at 2:47 PM CEST, Markus Probst wrote:
> Those are small fixes discovered by the kernel test robot and Gary Guo
> after the serdev rust abstraction have already been merged.
>
> This includes
> - a fix for a documentation typo
> - adding missing `#[inline]` attributes
> - a small structual change for timeout

Thanks for the quick follow-up, the patches were only in the -testing branch, so
I dropped them from the queue.

Can you please integrate those changes into the original patches and resend?

Thanks,
Danilo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 12:47 ` [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout` Markus Probst
@ 2026-07-18 15:02   ` Gary Guo
  2026-07-18 15:12     ` Markus Probst
  0 siblings, 1 reply; 9+ messages in thread
From: Gary Guo @ 2026-07-18 15:02 UTC (permalink / raw)
  To: Markus Probst, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel

On Sat Jul 18, 2026 at 1:47 PM BST, Markus Probst wrote:
> Instead of relying on its own timeout types, the abstraction should make
> use of `impl Into<Jiffies>`.
>
> Suggested-by: Gary Guo <gary@garyguo.net>
> Link: https://lore.kernel.org/rust-for-linux/DK0SX3P5K1J6.1OV4ETJMXW83R@garyguo.net/
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  rust/kernel/serdev.rs | 55 +++++++++++++++++----------------------------------
>  1 file changed, 18 insertions(+), 37 deletions(-)
>
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index a78dfa6e2c27..0fdb37cff15d 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
> @@ -20,11 +20,7 @@
>          aref::AlwaysRefCounted,
>          Mutex, //
>      },
> -    time::{
> -        msecs_to_jiffies,
> -        Jiffies,
> -        Msecs, //
> -    },
> +    time::Jiffies,
>      types::{
>          Opaque,
>          ScopeGuard, //
> @@ -35,7 +31,6 @@
>      cell::UnsafeCell,
>      marker::PhantomData,
>      mem::{offset_of, MaybeUninit},
> -    num::NonZero,
>      ptr::NonNull, //
>  };
>  
> @@ -50,30 +45,6 @@ pub enum 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);
>  
> @@ -379,7 +350,7 @@ macro_rules! module_serdev_device_driver {
>  ///         _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)?;
> +///         sdev.write_all(b"Hello\n", 0usize)?;

Note that the jiffies type is being converted to a new type (in fact,
`Into<Jiffies>` only make sense with it being a new type.

Best,
Gary

>  ///         Ok(MyDriver)
>  ///     }
>  /// }
> @@ -505,11 +476,13 @@ pub fn set_parity(&self, parity: Parity) -> Result {
>      /// [`Device::wait_until_sent`] to make sure the controller write buffer has actually been
>      /// emptied.
>      ///
> +    /// Use a timeout of 0 to wait indefinitely.
> +    ///
>      /// 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.
> +    /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
>      #[inline]
> -    pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
> +    pub fn write_all(&self, data: &[u8], timeout: impl Into<Jiffies>) -> Result<usize> {
>          if data.len() > i32::MAX as usize {
>              return Err(EINVAL);
>          }
> @@ -523,7 +496,7 @@ pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
>                  self.as_raw(),
>                  data.as_ptr(),
>                  data.len(),
> -                timeout.into_jiffies(),
> +                isize::try_from(timeout.into()).unwrap_or_default(),
>              )
>          };
>          // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> @@ -570,11 +543,19 @@ pub fn write_flush(&self) {
>  
>      /// Wait for the data to be sent.
>      ///
> -    /// After this function, the write buffer of the controller should be empty.
> +    /// After this function, the write buffer of the controller should be empty or the timeout
> +    /// elapsed.
> +    ///
> +    /// Use a timeout of 0 to wait indefinitely.
>      #[inline]
> -    pub fn wait_until_sent(&self, timeout: Timeout) {
> +    pub fn wait_until_sent(&self, timeout: impl Into<Jiffies>) {
>          // 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()) };
> +        unsafe {
> +            bindings::serdev_device_wait_until_sent(
> +                self.as_raw(),
> +                isize::try_from(timeout.into()).unwrap_or_default(),
> +            )
> +        };
>      }
>  }
>  



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 15:02   ` Gary Guo
@ 2026-07-18 15:12     ` Markus Probst
  2026-07-18 15:23       ` Gary Guo
  0 siblings, 1 reply; 9+ messages in thread
From: Markus Probst @ 2026-07-18 15:12 UTC (permalink / raw)
  To: Gary Guo, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel

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

On Sat, 2026-07-18 at 16:02 +0100, Gary Guo wrote:
> On Sat Jul 18, 2026 at 1:47 PM BST, Markus Probst wrote:
> > Instead of relying on its own timeout types, the abstraction should make
> > use of `impl Into<Jiffies>`.
> > 
> > Suggested-by: Gary Guo <gary@garyguo.net>
> > Link: https://lore.kernel.org/rust-for-linux/DK0SX3P5K1J6.1OV4ETJMXW83R@garyguo.net/
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> >  rust/kernel/serdev.rs | 55 +++++++++++++++++----------------------------------
> >  1 file changed, 18 insertions(+), 37 deletions(-)
> > 
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > index a78dfa6e2c27..0fdb37cff15d 100644
> > --- a/rust/kernel/serdev.rs
> > +++ b/rust/kernel/serdev.rs
> > @@ -20,11 +20,7 @@
> >          aref::AlwaysRefCounted,
> >          Mutex, //
> >      },
> > -    time::{
> > -        msecs_to_jiffies,
> > -        Jiffies,
> > -        Msecs, //
> > -    },
> > +    time::Jiffies,
> >      types::{
> >          Opaque,
> >          ScopeGuard, //
> > @@ -35,7 +31,6 @@
> >      cell::UnsafeCell,
> >      marker::PhantomData,
> >      mem::{offset_of, MaybeUninit},
> > -    num::NonZero,
> >      ptr::NonNull, //
> >  };
> >  
> > @@ -50,30 +45,6 @@ pub enum 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);
> >  
> > @@ -379,7 +350,7 @@ macro_rules! module_serdev_device_driver {
> >  ///         _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)?;
> > +///         sdev.write_all(b"Hello\n", 0usize)?;
> 
> Note that the jiffies type is being converted to a new type (in fact,
> `Into<Jiffies>` only make sense with it being a new type.
I assume it won't take long for the patch series for the new type to be
merged. Currently there is no user for any other timeout than 0 (which
corrosponds to MAX_SCHEDULE_TIMEOUT in serdev).

Or should I revert to `serdev::Timeout` until then?

Thanks
- Markus Probst

> 
> Best,
> Gary
> 
> >  ///         Ok(MyDriver)
> >  ///     }
> >  /// }
> > @@ -505,11 +476,13 @@ pub fn set_parity(&self, parity: Parity) -> Result {
> >      /// [`Device::wait_until_sent`] to make sure the controller write buffer has actually been
> >      /// emptied.
> >      ///
> > +    /// Use a timeout of 0 to wait indefinitely.
> > +    ///
> >      /// 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.
> > +    /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
> >      #[inline]
> > -    pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
> > +    pub fn write_all(&self, data: &[u8], timeout: impl Into<Jiffies>) -> Result<usize> {
> >          if data.len() > i32::MAX as usize {
> >              return Err(EINVAL);
> >          }
> > @@ -523,7 +496,7 @@ pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
> >                  self.as_raw(),
> >                  data.as_ptr(),
> >                  data.len(),
> > -                timeout.into_jiffies(),
> > +                isize::try_from(timeout.into()).unwrap_or_default(),
> >              )
> >          };
> >          // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> > @@ -570,11 +543,19 @@ pub fn write_flush(&self) {
> >  
> >      /// Wait for the data to be sent.
> >      ///
> > -    /// After this function, the write buffer of the controller should be empty.
> > +    /// After this function, the write buffer of the controller should be empty or the timeout
> > +    /// elapsed.
> > +    ///
> > +    /// Use a timeout of 0 to wait indefinitely.
> >      #[inline]
> > -    pub fn wait_until_sent(&self, timeout: Timeout) {
> > +    pub fn wait_until_sent(&self, timeout: impl Into<Jiffies>) {
> >          // 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()) };
> > +        unsafe {
> > +            bindings::serdev_device_wait_until_sent(
> > +                self.as_raw(),
> > +                isize::try_from(timeout.into()).unwrap_or_default(),
> > +            )
> > +        };
> >      }
> >  }
> >  
> 

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 15:12     ` Markus Probst
@ 2026-07-18 15:23       ` Gary Guo
  0 siblings, 0 replies; 9+ messages in thread
From: Gary Guo @ 2026-07-18 15:23 UTC (permalink / raw)
  To: Markus Probst, Gary Guo, Greg Kroah-Hartman, Miguel Ojeda,
	Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel

On Sat Jul 18, 2026 at 4:12 PM BST, Markus Probst wrote:
> On Sat, 2026-07-18 at 16:02 +0100, Gary Guo wrote:
>> On Sat Jul 18, 2026 at 1:47 PM BST, Markus Probst wrote:
>> > Instead of relying on its own timeout types, the abstraction should make
>> > use of `impl Into<Jiffies>`.
>> > 
>> > Suggested-by: Gary Guo <gary@garyguo.net>
>> > Link: https://lore.kernel.org/rust-for-linux/DK0SX3P5K1J6.1OV4ETJMXW83R@garyguo.net/
>> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
>> > ---
>> >  rust/kernel/serdev.rs | 55 +++++++++++++++++----------------------------------
>> >  1 file changed, 18 insertions(+), 37 deletions(-)
>> > 
>> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
>> > index a78dfa6e2c27..0fdb37cff15d 100644
>> > --- a/rust/kernel/serdev.rs
>> > +++ b/rust/kernel/serdev.rs
>> > @@ -20,11 +20,7 @@
>> >          aref::AlwaysRefCounted,
>> >          Mutex, //
>> >      },
>> > -    time::{
>> > -        msecs_to_jiffies,
>> > -        Jiffies,
>> > -        Msecs, //
>> > -    },
>> > +    time::Jiffies,
>> >      types::{
>> >          Opaque,
>> >          ScopeGuard, //
>> > @@ -35,7 +31,6 @@
>> >      cell::UnsafeCell,
>> >      marker::PhantomData,
>> >      mem::{offset_of, MaybeUninit},
>> > -    num::NonZero,
>> >      ptr::NonNull, //
>> >  };
>> >  
>> > @@ -50,30 +45,6 @@ pub enum 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);
>> >  
>> > @@ -379,7 +350,7 @@ macro_rules! module_serdev_device_driver {
>> >  ///         _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)?;
>> > +///         sdev.write_all(b"Hello\n", 0usize)?;
>> 
>> Note that the jiffies type is being converted to a new type (in fact,
>> `Into<Jiffies>` only make sense with it being a new type.
> I assume it won't take long for the patch series for the new type to be
> merged. Currently there is no user for any other timeout than 0 (which
> corrosponds to MAX_SCHEDULE_TIMEOUT in serdev).
>
> Or should I revert to `serdev::Timeout` until then?

`Timeout` uses `Jiffies` and `Msec` directly which would be changed too. I
think we just need to treat this as conflict and figure out how to handle the
conflict.

I think realistic there're some designs to still iron out for the jiffy series,
so yours one may land first. The easiest thing is probably just have API that
take `Jiffies` type directly without `Into` (as having it doesn't bring much
value at the moment), and that can be converted by the jiffy series.

Best,
Gary


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-07-18 15:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
2026-07-18 12:47 ` [PATCH v2 1/4] rust: serdev: fix typo in rustdoc Markus Probst
2026-07-18 12:47 ` [PATCH v2 2/4] rust: serdev: mark small functions as `#[inline]` Markus Probst
2026-07-18 12:47 ` [PATCH v2 3/4] rust: serdev: document `PrivateData::active` Markus Probst
2026-07-18 12:47 ` [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout` Markus Probst
2026-07-18 15:02   ` Gary Guo
2026-07-18 15:12     ` Markus Probst
2026-07-18 15:23       ` Gary Guo
2026-07-18 14:10 ` [PATCH v2 0/4] rust: serdev: trivial fixes Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox