Re: [PATCH v11 7/8] rust: Add read_poll_timeout functions

Danilo Krummrich
2025-08-05T13:48:23Z

On Tue Aug 5, 2025 at 3:37 PM CEST, FUJITA Tomonori wrote:
> On Sat, 02 Aug 2025 13:06:04 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>
>> On Sat Aug 2, 2025 at 3:42 AM CEST, FUJITA Tomonori wrote:
>>> On Mon, 28 Jul 2025 15:13:45 +0200
>>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>>>> On Thu Feb 20, 2025 at 8:06 AM CET, FUJITA Tomonori wrote:
>>>>> +/// This process continues until either `cond` returns `true` or the timeout,
>>>>> +/// specified by `timeout_delta`, is reached. If `timeout_delta` is `None`,
>>>>> +/// polling continues indefinitely until `cond` evaluates to `true` or an error occurs.
>>>>> +///
>>>>> +/// # Examples
>>>>> +///
>>>>> +/// ```rust,ignore
>>>>
>>>> Why ignore? This should be possible to compile test.
>>>
>>> https://lore.kernel.org/rust-for-linux/CEF87294-8580-4C84-BEA3-EB72E63ED7DF@collabora.com/
>>
>> I disagree with that. 'ignore' should only be used if we can't make it compile.
>>
>> In this case we can make it compile, we just can't run it, since there's no real
>> HW underneath that we can read registers from.
>>
>> An example that isn't compiled will eventually be forgotten to be updated when
>> things are changed.
>
> I also prefer the example that can be compiled however I can't think
> of a compilable example that is similar to actual use cases (for
> example, waiting for some hardware condition). Do you have any ideas?

With my suggestion below, it should be compilable.

When you just take a &Io<SIZE> as argument in wait_for_hardware() you can call
io.read(). Then define HW_READY as some random value and it should compile.

>>>>> +/// fn wait_for_hardware(dev: &mut Device) -> Result<()> {
>>>>
>>>> I think the parameter here can just be `&Io<SIZE>`.
>>>>
>>>>> +/// // The `op` closure reads the value of a specific status register.
>>>>> +/// let op = || -> Result<u16> { dev.read_ready_register() };
>>>>> +///
>>>>> +/// // The `cond` closure takes a reference to the value returned by `op`
>>>>> +/// // and checks whether the hardware is ready.
>>>>> +/// let cond = |val: &u16| *val == HW_READY;
>>>>> +///
>>>>> +/// match read_poll_timeout(op, cond, Delta::from_millis(50), Some(Delta::from_secs(3))) {
>>>>> +/// Ok(_) => {
>>>>> +/// // The hardware is ready. The returned value of the `op`` closure isn't used.
>>>>> +/// Ok(())
>>>>> +/// }
>>>>> +/// Err(e) => Err(e),
>>>>> +/// }
>>>>> +/// }
>>>>> +/// ```