All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] rust: serdev: trivial fixes
@ 2026-07-18 12:47 Markus Probst
  2026-07-18 12:47 ` [PATCH v2 1/4] rust: serdev: fix typo in rustdoc Markus Probst
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst,
	kernel test robot

Those are small fixes discovered by the kernel test robot and Gary Guo
after the serdev rust abstraction have already been merged.

This includes
- a fix for a documentation typo
- adding missing `#[inline]` attributes
- a small structual change for timeout

There is no urgency to merge this.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v2:
- resolve Sashiko issues:
  - reword small parts of the `PrivateData::active` documentation
  - fix documentation references left to `serdev::Timeout`.
- Link to v1: https://patch.msgid.link/20260718-rust_serdev_fixes-v1-0-35433b6dce0f@posteo.de

---
Markus Probst (4):
      rust: serdev: fix typo in rustdoc
      rust: serdev: mark small functions as `#[inline]`
      rust: serdev: document `PrivateData::active`
      rust: serdev: remove `serdev::Timeout`

 rust/kernel/serdev.rs | 82 +++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 38 deletions(-)
---
base-commit: 25acf039a5dbb40b6865d9392e05be8e713a39f3
change-id: 20260717-rust_serdev_fixes-9baf5f310278


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/4] rust: serdev: fix typo in rustdoc
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
@ 2026-07-18 12:47 ` Markus Probst
  2026-07-18 12:47 ` [PATCH v2 2/4] rust: serdev: mark small functions as `#[inline]` Markus Probst
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst,
	kernel test robot

Fix trivial typo

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202607172113.hLDQgD4l-lkp@intel.com/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/serdev.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 0ffcef1849d2..47ca57966abf 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -515,7 +515,7 @@ pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
     /// [`Device::write_all`].
     ///
     /// Note that any accepted data has only been buffered by the controller. Use
-    /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
+    /// [`Device::wait_until_sent`] to make sure the controller write buffer has actually been
     /// emptied.
     ///
     /// Returns the number of bytes written (less than `data.len()` if not enough room in the

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] rust: serdev: mark small functions as `#[inline]`
  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 ` Markus Probst
  2026-07-18 12:47 ` [PATCH v2 3/4] rust: serdev: document `PrivateData::active` Markus Probst
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst

These methods should be inlined for optimization reasons.

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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 47ca57966abf..79d61c6ceeec 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -440,6 +440,7 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
 );
 
 impl<Ctx: device::DeviceContext> Device<Ctx> {
+    #[inline]
     fn as_raw(&self) -> *mut bindings::serdev_device {
         self.0.get()
     }
@@ -451,6 +452,7 @@ impl Device<device::Bound> {
     /// Common baudrates are 115200, 9600, 19200, 57600, 4800.
     ///
     /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    #[inline]
     pub fn set_baudrate(&self, speed: u32) -> Result<(), u32> {
         // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
         let ret = unsafe { bindings::serdev_device_set_baudrate(self.as_raw(), speed) };
@@ -464,6 +466,7 @@ pub fn set_baudrate(&self, speed: u32) -> Result<(), u32> {
     /// Set if flow control should be enabled.
     ///
     /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    #[inline]
     pub fn set_flow_control(&self, enable: bool) {
         // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
         unsafe { bindings::serdev_device_set_flow_control(self.as_raw(), enable) };
@@ -472,6 +475,7 @@ pub fn set_flow_control(&self, enable: bool) {
     /// Set parity to use.
     ///
     /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
+    #[inline]
     pub fn set_parity(&self, parity: Parity) -> Result {
         // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
         to_result(unsafe { bindings::serdev_device_set_parity(self.as_raw(), parity as u32) })
@@ -487,6 +491,7 @@ pub fn set_parity(&self, parity: Parity) -> Result {
     /// 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.
+    #[inline]
     pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
         if data.len() > i32::MAX as usize {
             return Err(EINVAL);
@@ -520,6 +525,7 @@ pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
     ///
     /// Returns the number of bytes written (less than `data.len()` if not enough room in the
     /// write buffer).
+    #[inline]
     pub fn write(&self, data: &[u8]) -> Result<u32> {
         if data.len() > i32::MAX as usize {
             return Err(EINVAL);
@@ -539,6 +545,7 @@ pub fn write(&self, data: &[u8]) -> Result<u32> {
     ///
     /// Note that this doesn't guarantee that the data has been transmitted.
     /// Use [`Device::wait_until_sent`] for this purpose.
+    #[inline]
     pub fn write_flush(&self) {
         // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
         unsafe { bindings::serdev_device_write_flush(self.as_raw()) };
@@ -547,6 +554,7 @@ pub fn write_flush(&self) {
     /// Wait for the data to be sent.
     ///
     /// After this function, the write buffer of the controller should be empty.
+    #[inline]
     pub fn wait_until_sent(&self, timeout: Timeout) {
         // 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()) };

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] rust: serdev: document `PrivateData::active`
  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 ` Markus Probst
  2026-07-18 12:47 ` [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout` Markus Probst
  2026-07-18 14:10 ` [PATCH v2 0/4] rust: serdev: trivial fixes Danilo Krummrich
  4 siblings, 0 replies; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst

Gary pointed out that it isn't clear for what the mutex is used for,
thus adding documentation comments for it.

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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 79d61c6ceeec..a78dfa6e2c27 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -132,6 +132,23 @@ pub struct PrivateData<'bound, T: Driver> {
     #[pin]
     driver: UnsafeCell<MaybeUninit<T::Data<'bound>>>,
     open: UnsafeCell<bool>,
+    /// Whether `receive_buf_callback` is allowed to call `Driver::receive`.
+    ///
+    /// If locked, the receive_buf_callback will be blocked on data reception.
+    /// This is the case while the driver is being probed or while [`PrivateData`] is being dropped.
+    /// This is necessary, because we need to open the serdev device before the driver has been
+    /// probed in order to allow it to be configured, which allows `receive_buf_callback` to be
+    /// called. Thus we need to block data until probe completes and the driver data becomes
+    /// initialized.
+    ///
+    /// If unlocked and true, the receive_buf_callback will forward the data to
+    /// `Driver::receive`. This is the normal state of operation.
+    ///
+    /// If unlocked and false, the receive_buf_callback will throw away the data.
+    /// This is only the case, if the serdev device is open and
+    /// - the driver returned an error in probe
+    /// or
+    /// - the driver data already has been dropped, because it was unbound.
     #[pin]
     active: Mutex<bool>,
 }

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
                   ` (2 preceding siblings ...)
  2026-07-18 12:47 ` [PATCH v2 3/4] rust: serdev: document `PrivateData::active` Markus Probst
@ 2026-07-18 12:47 ` Markus Probst
  2026-07-18 15:02   ` Gary Guo
  2026-07-18 14:10 ` [PATCH v2 0/4] rust: serdev: trivial fixes Danilo Krummrich
  4 siblings, 1 reply; 9+ messages in thread
From: Markus Probst @ 2026-07-18 12:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel, Markus Probst

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

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/4] rust: serdev: trivial fixes
  2026-07-18 12:47 [PATCH v2 0/4] rust: serdev: trivial fixes Markus Probst
                   ` (3 preceding siblings ...)
  2026-07-18 12:47 ` [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout` Markus Probst
@ 2026-07-18 14:10 ` Danilo Krummrich
  4 siblings, 0 replies; 9+ messages in thread
From: Danilo Krummrich @ 2026-07-18 14:10 UTC (permalink / raw)
  To: Markus Probst
  Cc: Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
	Onur Özkan, linux-serial, rust-for-linux, linux-kernel,
	kernel test robot

On Sat Jul 18, 2026 at 2:47 PM CEST, Markus Probst wrote:
> Those are small fixes discovered by the kernel test robot and Gary Guo
> after the serdev rust abstraction have already been merged.
>
> This includes
> - a fix for a documentation typo
> - adding missing `#[inline]` attributes
> - a small structual change for timeout

Thanks for the quick follow-up, the patches were only in the -testing branch, so
I dropped them from the queue.

Can you please integrate those changes into the original patches and resend?

Thanks,
Danilo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 12:47 ` [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout` Markus Probst
@ 2026-07-18 15:02   ` Gary Guo
  2026-07-18 15:12     ` Markus Probst
  0 siblings, 1 reply; 9+ messages in thread
From: Gary Guo @ 2026-07-18 15:02 UTC (permalink / raw)
  To: Markus Probst, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 15:02   ` Gary Guo
@ 2026-07-18 15:12     ` Markus Probst
  2026-07-18 15:23       ` Gary Guo
  0 siblings, 1 reply; 9+ messages in thread
From: Markus Probst @ 2026-07-18 15:12 UTC (permalink / raw)
  To: Gary Guo, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel

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

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 <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.
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?

Thanks
- Markus Probst

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] rust: serdev: remove `serdev::Timeout`
  2026-07-18 15:12     ` Markus Probst
@ 2026-07-18 15:23       ` Gary Guo
  0 siblings, 0 replies; 9+ messages in thread
From: Gary Guo @ 2026-07-18 15:23 UTC (permalink / raw)
  To: Markus Probst, Gary Guo, Greg Kroah-Hartman, Miguel Ojeda,
	Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan
  Cc: linux-serial, rust-for-linux, linux-kernel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-07-18 15:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.