Re: [PATCH v1] rust: serdev: use Delta<Jiffy> for timeouts

["Gary Guo" <gary@garyguo.net>]

On Wed Sep 30, 2026 at 1:59 PM BST, Markus Probst wrote:
> On Wed, 2026-09-30 at 13:13 +0900, FUJITA Tomonori wrote:
>> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> 
>> write_all() and wait_until_sent() take the timeout as a raw Jiffies,
>> which is a c_ulong alias. The type does not show the unit.
>> 
>> Take the timeout as Delta<Jiffy> instead. Clamp a negative timeout to
>> 0, as the current code does for a value larger than isize::MAX.
>> 
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
>>  rust/kernel/serdev.rs | 27 ++++++++++++---------------
>>  1 file changed, 12 insertions(+), 15 deletions(-)
>> 
>> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
>> index 17ca504b7f8d..094503f059ff 100644
>> --- a/rust/kernel/serdev.rs
>> +++ b/rust/kernel/serdev.rs
>> @@ -20,7 +20,10 @@
>>          aref::AlwaysRefCounted,
>>          Mutex, //
>>      },
>> -    time::Jiffies,
>> +    time::{
>> +        Delta,
>> +        Jiffy, //
>> +    },
>>      types::{
>>          Opaque,
>>          ScopeGuard, //
>> @@ -481,22 +484,19 @@ pub fn set_parity(&self, parity: Parity) -> Result {
>>      /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
>>      /// before any bytes were written. [`kernel::error::code::EINVAL`] if `data.len() > i32::MAX`.
>>      #[inline]
>> -    pub fn write_all(&self, data: &[u8], timeout: Jiffies) -> Result<usize> {
>> +    pub fn write_all(&self, data: &[u8], timeout: Delta<Jiffy>) -> Result<usize> {
>>          if data.len() > i32::MAX as usize {
>>              return Err(EINVAL);
>>          }
>>  
>> +        let timeout = isize::max(timeout.as_jiffies(), 0);
>> +
>>          // SAFETY:
>>          // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
>>          // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
>>          //   `data.len()`.
>>          let ret = unsafe {
>> -            bindings::serdev_device_write(
>> -                self.as_raw(),
>> -                data.as_ptr(),
>> -                data.len(),
>> -                isize::try_from(timeout).unwrap_or_default(),
>> -            )
>> +            bindings::serdev_device_write(self.as_raw(), data.as_ptr(), data.len(), timeout)
>>          };
>>          // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
>>          // which always fit into a `i32`.
>> @@ -547,14 +547,11 @@ pub fn write_flush(&self) {
>>      ///
>>      /// Use a timeout of 0 to wait indefinitely.
>>      #[inline]
>> -    pub fn wait_until_sent(&self, timeout: Jiffies) {
>> +    pub fn wait_until_sent(&self, timeout: Delta<Jiffy>) {
>> +        let timeout = isize::max(timeout.as_jiffies(), 0);
>> +
>>          // 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(),
>> -                isize::try_from(timeout).unwrap_or_default(),
>> -            )
>> -        };
>> +        unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout) };
>>      }
>>  }
>>  
>> 
>> base-commit: f1850e443b0e4f2429ddf42a8d5033ea54ae8a90
>
> Both functions have "Use a timeout of 0 to wait indefinitely." inside
> the rustdoc.
>
> Make sure `Delta::ZERO` is also usable for `Delta<Jiffy>` and replace
> the 0 in the rustdoc with "[`Delta::<Jiffy>::ZERO`]". You might also
> remove the "timeout of" part, but I don't mind if it stays.

I think the API should ideally use `Option<Delta<Jiffies>>` for this case, and
use `None` to represent indefinite wait.

We might need to round 0 jiffies to 1 to avoid C API change. But ideally the C
API should be using MAX_JIFFY_OFFSET to mean indefinite..

Best,
Gary