Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`

"Gary Guo" <[email protected]>
Newsgroups gmane.linux.serial,gmane.linux.kernel.rust,gmane.linux.kernel
Message-ID <[email protected]>
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 <[email protected]>
> Link: https://lore.kernel.org/rust-for-linux/[email protected]/
> Signed-off-by: Markus Probst <[email protected]>
> ---
>  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(),
> +            )
> +        };
>      }
>  }
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.