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

[FUJITA Tomonori <tomo@flapping.org>]

On Wed, 30 Sep 2026 14:52:50 +0100
"Gary Guo" <gary@garyguo.net> wrote:

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

(snip)

>>>      #[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);
>>> +

(snip)

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

For read_poll_timeout(), where a timeout of 0 means "never time out"
in C, we did not take Option for the timeout. Alice's comment [1]:

| Another thing is the `timeout_delta` option. I would just have
| written it as two methods, one that takes a timeout and one that
| doesn't. That way, callers that don't need a timeout do not need to
| handle timeout errors. (Do we have any users without a timeout? If
| not, maybe just remove the Option.)

serdev is in the same situation. Do we want an API that uses None for
"never" here?

[1] https://lore.kernel.org/rust-for-linux/aJm9A_D-zlJtbV6X@google.com/