Linux Serial subsystem development
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Markus Probst" <markus.probst@posteo.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>
Cc: <linux-serial@vger.kernel.org>, <rust-for-linux@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
Date: Sat, 18 Jul 2026 16:02:31 +0100	[thread overview]
Message-ID: <DK1SDT57QY13.3ZXZYQGOTK2P@garyguo.net> (raw)
In-Reply-To: <20260718-rust_serdev_fixes-v2-4-e5214e03de13@posteo.de>

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(),
> +            )
> +        };
>      }
>  }
>  



  reply	other threads:[~2026-07-18 15:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DK1SDT57QY13.3ZXZYQGOTK2P@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=markus.probst@posteo.de \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox