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

[Markus Probst <markus.probst@posteo.de>]

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

On Wed, 2026-09-30 at 13:13 +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. Clamp a negative timeout to
> 0, as the current code does for a value larger than isize::MAX.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/serdev.rs | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d..094503f059ff 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, //
> @@ -481,22 +484,19 @@ pub fn set_parity(&self, parity: Parity) -> Result {
>      /// [`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 = isize::max(timeout.as_jiffies(), 0);
> +
>          // 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`.
> @@ -547,14 +547,11 @@ pub fn write_flush(&self) {
>      ///
>      /// Use a timeout of 0 to wait indefinitely.
>      #[inline]
> -    pub fn wait_until_sent(&self, timeout: Jiffies) {
> +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
> +        let timeout = isize::max(timeout.as_jiffies(), 0);
> +
>          // 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: f1850e443b0e4f2429ddf42a8d5033ea54ae8a90

Both functions have "Use a timeout of 0 to wait indefinitely." inside
the rustdoc.

Make sure `Delta::ZERO` is also usable for `Delta<Jiffy>` and replace
the 0 in the rustdoc with "[`Delta::<Jiffy>::ZERO`]". You might also
remove the "timeout of" part, but I don't mind if it stays.

With that fixed,

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 --]