Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
"Gary Guo" <[email protected]> Sat, 18 Jul 2026 16:23:05 +0100
| Newsgroups | org.kernel.vger.linux-serial,org.kernel.vger.linux-kernel,org.kernel.vger.rust-for-linux |
|---|---|
| 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