[PATCH v2] rust: serdev: use Delta<Jiffy> for timeouts

[PATCH v2] rust: serdev: use Delta<Jiffy> for timeouts

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

write_all() and wait_until_sent() take the timeout as a raw Jiffies,
which is a c_ulong alias. The type does not show the unit.

Take the timeout as Delta<Jiffy> instead.

The C side treats a timeout of 0 as "wait indefinitely". The Rust API
does not keep that meaning. A timeout of 0 or negative is rounded up
to one jiffy. A caller that wants to wait indefinitely passes
Delta::from_jiffies(isize::MAX). That value is MAX_SCHEDULE_TIMEOUT.
The C side replaces 0 with MAX_SCHEDULE_TIMEOUT before it waits, so
this gives the same wait as a timeout of 0 did before.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
v2:
- Round a timeout of 0 or negative up to one jiffy instead of passing 0,
  which the C side treats as "wait indefinitely" (Gary)
- Document Delta::from_jiffies(isize::MAX) for an indefinite wait
v1: https://lore.kernel.org/rust-for-linux/20260930041339.1551129-1-tomo@flapping.org/
---
 rust/kernel/serdev.rs | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 17ca504b7f8d..656598602d7a 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -20,7 +20,10 @@
         aref::AlwaysRefCounted,
         Mutex, //
     },
-    time::Jiffies,
+    time::{
+        Delta,
+        Jiffy, //
+    },
     types::{
         Opaque,
         ScopeGuard, //
@@ -433,6 +436,16 @@ fn as_raw(&self) -> *mut bindings::serdev_device {
     }
 }
 
+/// Convert a timeout to the `long` the C side takes.
+///
+/// The C side treats a timeout of 0 as "wait indefinitely", so a timeout of 0 or negative is
+/// rounded up to one jiffy. `isize::MAX` is `MAX_SCHEDULE_TIMEOUT`, so it is passed through
+/// unchanged and the C side waits indefinitely.
+#[inline]
+fn timeout_jiffies(timeout: Delta<Jiffy>) -> isize {
+    isize::max(timeout.as_jiffies(), 1)
+}
+
 impl Device<device::Bound> {
     /// Set the baudrate in bits per second.
     ///
@@ -475,28 +488,26 @@ 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.
+    /// A `timeout` of 0 or negative is rounded up to one jiffy. Pass
+    /// `Delta::from_jiffies(isize::MAX)` 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. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
     #[inline]
-    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
+    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
         if data.len() > i32::MAX as usize {
             return Err(EINVAL);
         }
 
+        let timeout = timeout_jiffies(timeout);
+
         // 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(),
-                isize::try_from(timeout).unwrap_or_default(),
-            )
+            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
         };
         // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
         // which always fit into a `i32`.
@@ -545,16 +556,14 @@ pub fn write_flush(&self) {
     /// After this function, the write buffer of the controller should be empty or the timeout
     /// elapsed.
     ///
-    /// Use a timeout of 0 to wait indefinitely.
+    /// A `timeout` of 0 or negative is rounded up to one jiffy. Pass
+    /// `Delta::from_jiffies(isize::MAX)` to wait indefinitely.
     #[inline]
-    pub fn wait_until_sent(&self, timeout: Jiffies) {
+    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
+        let timeout = timeout_jiffies(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(),
-                isize::try_from(timeout).unwrap_or_default(),
-            )
-        };
+        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
     }
 }
 

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0

Re: [PATCH v2] rust: serdev: use Delta<Jiffy> for timeouts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: serdev: outdated timeout type in Driver doctest for write_all()
--

commit 2726c2fee6d94012455c660974ada63f17dd12c7
Author: FUJITA Tomonori <fujita.tomonori@gmail.com>

rust: serdev: use Delta<Jiffy> for timeouts

This commit updates serdev::Device::write_all() and
serdev::Device::wait_until_sent() to accept a Delta<Jiffy> timeout instead
of a raw Jiffies value, rounding non-positive timeouts up to one jiffy.

> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d6..656598602d7a9 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs

[ ... ]

> @@ -475,28 +488,26 @@ 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.
> +    /// A `timeout` of 0 or negative is rounded up to one jiffy. Pass
> +    /// `Delta::from_jiffies(isize::MAX)` 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. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
>      #[inline]
> -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
> +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {

[Severity: Low]
Does the rustdoc example on the serdev::Driver trait in rust/kernel/serdev.rs
also need to be updated for this signature change?

///     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", 0)?;
///         Ok(MyDriver)
///     }

It still passes an integer literal 0 to sdev.write_all(), which fails to
compile when building with CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS=y and
CONFIG_RUST_KERNEL_DOCTESTS=y.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102005125.626125-1-tomo@flapping.org?part=1

Re: [PATCH v2] rust: serdev: use Delta<Jiffy> for timeouts

[Markus Probst]

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

On Mon, 2026-11-02 at 09:51 +0900, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> write_all() and wait_until_sent() take the timeout as a raw Jiffies,
> which is a c_ulong alias. The type does not show the unit.
> 
> Take the timeout as Delta<Jiffy> instead.
> 
> The C side treats a timeout of 0 as "wait indefinitely". The Rust API
> does not keep that meaning. A timeout of 0 or negative is rounded up
> to one jiffy. A caller that wants to wait indefinitely passes
> Delta::from_jiffies(isize::MAX). That value is MAX_SCHEDULE_TIMEOUT.
> The C side replaces 0 with MAX_SCHEDULE_TIMEOUT before it waits, so
> this gives the same wait as a timeout of 0 did before.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> v2:
> - Round a timeout of 0 or negative up to one jiffy instead of passing 0,
>   which the C side treats as "wait indefinitely" (Gary)
> - Document Delta::from_jiffies(isize::MAX) for an indefinite wait
> v1: https://lore.kernel.org/rust-for-linux/20260930041339.1551129-1-tomo@flapping.org/
> ---
>  rust/kernel/serdev.rs | 43 ++++++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d..656598602d7a 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
> @@ -20,7 +20,10 @@
>          aref::AlwaysRefCounted,
>          Mutex, //
>      },
> -    time::Jiffies,
> +    time::{
> +        Delta,
> +        Jiffy, //
> +    },
>      types::{
>          Opaque,
>          ScopeGuard, //
> @@ -433,6 +436,16 @@ fn as_raw(&self) -> *mut bindings::serdev_device {
>      }
>  }
>  
> +/// Convert a timeout to the `long` the C side takes.
> +///
> +/// The C side treats a timeout of 0 as "wait indefinitely", so a timeout of 0 or negative is
> +/// rounded up to one jiffy. `isize::MAX` is `MAX_SCHEDULE_TIMEOUT`, so it is passed through
> +/// unchanged and the C side waits indefinitely.
> +#[inline]
> +fn timeout_jiffies(timeout: Delta<Jiffy>) -> isize {
> +    isize::max(timeout.as_jiffies(), 1)
> +}
> +
>  impl Device<device::Bound> {
>      /// Set the baudrate in bits per second.
>      ///
> @@ -475,28 +488,26 @@ 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.
> +    /// A `timeout` of 0 or negative is rounded up to one jiffy. Pass
> +    /// `Delta::from_jiffies(isize::MAX)` 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. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
>      #[inline]
> -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
> +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
>          if data.len() > i32::MAX as usize {
>              return Err(EINVAL);
>          }
>  
> +        let timeout = timeout_jiffies(timeout);
> +
>          // 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(),
> -                isize::try_from(timeout).unwrap_or_default(),
> -            )
> +            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
>          };
>          // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
>          // which always fit into a `i32`.
> @@ -545,16 +556,14 @@ pub fn write_flush(&self) {
>      /// After this function, the write buffer of the controller should be empty or the timeout
>      /// elapsed.
>      ///
> -    /// Use a timeout of 0 to wait indefinitely.
> +    /// A `timeout` of 0 or negative is rounded up to one jiffy. Pass
> +    /// `Delta::from_jiffies(isize::MAX)` to wait indefinitely.
>      #[inline]
> -    pub fn wait_until_sent(&self, timeout: Jiffies) {
> +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
> +        let timeout = timeout_jiffies(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(),
> -                isize::try_from(timeout).unwrap_or_default(),
> -            )
> -        };
> +        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
>      }
>  }
>  
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

I would have preferred something like `Delta::MAX_TIMEOUT`, but thats
outside the scope of serdev.

The rustdoc example above `serdev::Driver` needs to be fixed.

With that resolved,

Reviewed-by: Markus Probst <markus.probst@posteo.de>

Thanks
- Markus Probst

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