Re: iopoll abstraction

[Boqun Feng <boqun.feng@gmail.com>]

On Thu, Oct 03, 2024 at 01:45:18PM +0000, FUJITA Tomonori wrote:
> On Thu, 3 Oct 2024 04:52:48 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > You could use closure as a parameter to avoid macro interface, something
> > like:
> > 
> > 	fn read_poll_timeout<Op, Cond, T>(
> > 	    op: Op,
> > 	    cond: Cond,
> > 	    sleep: Delta,
> > 	    timeout: Delta,
> > 	) -> Result<T> where
> > 	    Op: Fn() -> T,
> > 	    cond: Fn() -> bool {
> > 
> > 	    let __timeout = kernel::Ktime::ktime_get() + timeout;
> > 
> > 	    let val = loop {
> > 		let val = op();
> > 		if cond() {
> > 		    break Some(val);
> > 		}
> > 		kernel::delay::sleep(sleep);
> > 
> > 		if __timeout.after(kernel::Ktime::ktime_get()) {
> > 		    break None;
> > 		}
> > 	    };
> > 
> > 	    if cond() {
> > 		val
> > 	    } else {
> > 		Err(kernel::error::code::ETIMEDOUT)
> > 	    }
> > 	}
> 
> Great! I changed couple of things.
> 
> 1. Op typically reads a value from a register and could need mut objects. So I use FnMut.
> 2. reading from hardware could fail so Op had better to return an error [1].
> 3. Cond needs val; typically check the value from a register.
> 
> [1] https://lore.kernel.org/netdev/ec7267b5-ae77-4c4a-94f8-aa933c87a9a2@lunn.ch
> 
> Seems that the following works QT2025 driver. How does it look?
> 

Makes sense to me. Of course, more users will probably tell us whether
we cover all the cases, but this is a good starting point. I would put
the function at kernel::io::poll, but that's my personal preference ;-)

Regards,
Boqun

> fn read_poll_timeout<Op, Cond, T: Copy>(
>     mut op: Op,
>     cond: Cond,
>     sleep: Delta,
>     timeout: Delta,
> ) -> Result<T>
> where
>     Op: FnMut() -> Result<T>,
>     Cond: Fn(T) -> bool,
> {
>     let timeout = Ktime::ktime_get() + timeout;
>     let ret = loop {
>         let val = op()?;
>         if cond(val) {
>             break Ok(val);
>         }
>         kernel::delay::sleep(sleep);
> 
>         if Ktime::ktime_get() > timeout {
>             break Err(code::ETIMEDOUT);
>         }
>     };
> 
>     ret
> }