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

"Gary Guo" <[email protected]>
Newsgroups gmane.linux.kernel.rust,gmane.linux.serial,gmane.linux.kernel
Message-ID <[email protected]>
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 <[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.
> 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
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.