| Message ID | 20241001112512.4861-3-fujita.tomonori@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add delay abstraction (sleep functions) | expand |
On Tue, Oct 1, 2024 at 1:27 PM FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > Wait until a PHY becomes ready in the probe callback by using a sleep > function. > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > --- > drivers/net/phy/qt2025.rs | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs > index 28d8981f410b..3a8ef9f73642 100644 > --- a/drivers/net/phy/qt2025.rs > +++ b/drivers/net/phy/qt2025.rs > @@ -93,8 +93,15 @@ fn probe(dev: &mut phy::Device) -> Result<()> { > // The micro-controller will start running from SRAM. > dev.write(C45::new(Mmd::PCS, 0xe854), 0x0040)?; > > - // TODO: sleep here until the hw becomes ready. > - Ok(()) > + // sleep here until the hw becomes ready. > + for _ in 0..60 { > + kernel::delay::sleep(core::time::Duration::from_millis(50)); > + let val = dev.read(C45::new(Mmd::PCS, 0xd7fd))?; > + if val != 0x00 && val != 0x10 { > + return Ok(()); > + } Why not place the sleep after this check? That way, we don't need to sleep if the check succeeds immediately. > + } > + Err(code::ENODEV) This sleeps for up to 3 seconds. Is that the right amount to sleep? Alice
On Tue, Oct 01, 2024 at 01:36:41PM +0200, Alice Ryhl wrote: > On Tue, Oct 1, 2024 at 1:27 PM FUJITA Tomonori > <fujita.tomonori@gmail.com> wrote: > > > > Wait until a PHY becomes ready in the probe callback by using a sleep > > function. > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > > --- > > drivers/net/phy/qt2025.rs | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs > > index 28d8981f410b..3a8ef9f73642 100644 > > --- a/drivers/net/phy/qt2025.rs > > +++ b/drivers/net/phy/qt2025.rs > > @@ -93,8 +93,15 @@ fn probe(dev: &mut phy::Device) -> Result<()> { > > // The micro-controller will start running from SRAM. > > dev.write(C45::new(Mmd::PCS, 0xe854), 0x0040)?; > > > > - // TODO: sleep here until the hw becomes ready. > > - Ok(()) > > + // sleep here until the hw becomes ready. > > + for _ in 0..60 { > > + kernel::delay::sleep(core::time::Duration::from_millis(50)); > > + let val = dev.read(C45::new(Mmd::PCS, 0xd7fd))?; > > + if val != 0x00 && val != 0x10 { > > + return Ok(()); > > + } > > Why not place the sleep after this check? That way, we don't need to > sleep if the check succeeds immediately. Nice, you just made my point :-) I generally point developers at iopoll.h, because developers nearly always get this sort of polling for something to happen wrong. The kernel sleep functions guarantee the minimum sleep time. They say nothing about the maximum sleep time. You can ask it to sleep for 1ms, and in reality, due to something stealing the CPU and not being RT friendly, it actually sleeps for 10ms. This extra long sleep time blows straight past your timeout, if you have a time based timeout. What most developers do is after the sleep() returns they check to see if the timeout has been reached and then exit with -ETIMEDOUT. They don't check the state of the hardware, which given its had a long time to do its thing, probably is now in a good state. But the function returns -ETIMEDOUT. There should always be a check of the hardware state after the sleep, in order to determine ETIMEDOUT vs 0. As i said, most C developers get this wrong. So i don't really see why Rust developers also will not get this wrong. So i like to discourage this sort of code, and have Rust implementations of iopoll.h. Andrew
On 01.10.2024 14:48, Andrew Lunn wrote: > On Tue, Oct 01, 2024 at 01:36:41PM +0200, Alice Ryhl wrote: >> On Tue, Oct 1, 2024 at 1:27 PM FUJITA Tomonori >> <fujita.tomonori@gmail.com> wrote: >>> >>> Wait until a PHY becomes ready in the probe callback by using a sleep >>> function. >>> >>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> >>> --- >>> drivers/net/phy/qt2025.rs | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs >>> index 28d8981f410b..3a8ef9f73642 100644 >>> --- a/drivers/net/phy/qt2025.rs >>> +++ b/drivers/net/phy/qt2025.rs >>> @@ -93,8 +93,15 @@ fn probe(dev: &mut phy::Device) -> Result<()> { >>> // The micro-controller will start running from SRAM. >>> dev.write(C45::new(Mmd::PCS, 0xe854), 0x0040)?; >>> >>> - // TODO: sleep here until the hw becomes ready. >>> - Ok(()) >>> + // sleep here until the hw becomes ready. >>> + for _ in 0..60 { >>> + kernel::delay::sleep(core::time::Duration::from_millis(50)); >>> + let val = dev.read(C45::new(Mmd::PCS, 0xd7fd))?; >>> + if val != 0x00 && val != 0x10 { >>> + return Ok(()); >>> + } >> >> Why not place the sleep after this check? That way, we don't need to >> sleep if the check succeeds immediately. > > Nice, you just made my point :-) > > I generally point developers at iopoll.h, because developers nearly > always get this sort of polling for something to happen wrong. > > The kernel sleep functions guarantee the minimum sleep time. They say > nothing about the maximum sleep time. You can ask it to sleep for 1ms, > and in reality, due to something stealing the CPU and not being RT > friendly, it actually sleeps for 10ms. This extra long sleep time > blows straight past your timeout, if you have a time based timeout. > What most developers do is after the sleep() returns they check to see > if the timeout has been reached and then exit with -ETIMEDOUT. They > don't check the state of the hardware, which given its had a long time > to do its thing, probably is now in a good state. But the function > returns -ETIMEDOUT. > > There should always be a check of the hardware state after the sleep, > in order to determine ETIMEDOUT vs 0. > > As i said, most C developers get this wrong. So i don't really see why > Rust developers also will not get this wrong. So i like to discourage > this sort of code, and have Rust implementations of iopoll.h. Do we talk about some simple Rust wrappers for the macros in iopoll.h? E.g. something like [1]? Or are we talking about some more complex (safety) dependencies which need some more complex abstraction handling? Best regards Dirk [1] int rust_helper_readb_poll_timeout(const volatile void * addr, u64 val, u64 cond, u64 delay_us, u64 timeout_us) { return readb_poll_timeout(addr, val, cond, delay_us, timeout_us); }
On Wed, 2 Oct 2024 06:39:48 +0200 Dirk Behme <dirk.behme@de.bosch.com> wrote: >> I generally point developers at iopoll.h, because developers nearly >> always get this sort of polling for something to happen wrong. >> The kernel sleep functions guarantee the minimum sleep time. They say >> nothing about the maximum sleep time. You can ask it to sleep for 1ms, >> and in reality, due to something stealing the CPU and not being RT >> friendly, it actually sleeps for 10ms. This extra long sleep time >> blows straight past your timeout, if you have a time based timeout. >> What most developers do is after the sleep() returns they check to see >> if the timeout has been reached and then exit with -ETIMEDOUT. They >> don't check the state of the hardware, which given its had a long time >> to do its thing, probably is now in a good state. But the function >> returns -ETIMEDOUT. >> There should always be a check of the hardware state after the sleep, >> in order to determine ETIMEDOUT vs 0. >> As i said, most C developers get this wrong. So i don't really see why >> Rust developers also will not get this wrong. So i like to discourage >> this sort of code, and have Rust implementations of iopoll.h. > > > Do we talk about some simple Rust wrappers for the macros in iopoll.h? > E.g. something like [1]? > > Or are we talking about some more complex (safety) dependencies which > need some more complex abstraction handling? (snip) > int rust_helper_readb_poll_timeout(const volatile void * addr, > u64 val, u64 cond, u64 delay_us, > u64 timeout_us) > { > return readb_poll_timeout(addr, val, cond, delay_us, timeout_us); > } I'm not sure a simple wrapper for iopoll.h works. We need to pass a function. I'm testing a macro like the following (not added ktime timeout yet): macro_rules! read_poll_timeout { ($op:expr, $val:expr, $cond:expr, $sleep:expr, $timeout:expr, $($args:expr),*) => {{ let _ = $val; loop { $val = $op($($args),*); if $cond { break; } kernel::delay::sleep($sleep); } if $cond { Ok(()) } else { Err(kernel::error::code::ETIMEDOUT) } }}; }
On Tue, 1 Oct 2024 14:48:06 +0200 Andrew Lunn <andrew@lunn.ch> wrote: > I generally point developers at iopoll.h, because developers nearly > always get this sort of polling for something to happen wrong. Ah, I had forgotten about iopoll.h. Make senses. I'll try implement an equivalent in Rust.
On Tue, 1 Oct 2024 13:36:41 +0200 Alice Ryhl <aliceryhl@google.com> wrote: >> diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs >> index 28d8981f410b..3a8ef9f73642 100644 >> --- a/drivers/net/phy/qt2025.rs >> +++ b/drivers/net/phy/qt2025.rs >> @@ -93,8 +93,15 @@ fn probe(dev: &mut phy::Device) -> Result<()> { >> // The micro-controller will start running from SRAM. >> dev.write(C45::new(Mmd::PCS, 0xe854), 0x0040)?; >> >> - // TODO: sleep here until the hw becomes ready. >> - Ok(()) >> + // sleep here until the hw becomes ready. >> + for _ in 0..60 { >> + kernel::delay::sleep(core::time::Duration::from_millis(50)); >> + let val = dev.read(C45::new(Mmd::PCS, 0xd7fd))?; >> + if val != 0x00 && val != 0x10 { >> + return Ok(()); >> + } > > Why not place the sleep after this check? That way, we don't need to > sleep if the check succeeds immediately. Yeah, that's better. I copied the logic in the vendor driver without thinking. As Andrew pointed out, the code like this in C drivers isn't recommended; iopoll.h should be used. So I'll try to implement such. >> + } >> + Err(code::ENODEV) > > This sleeps for up to 3 seconds. Is that the right amount to sleep? That's what the vendor driver does.
On Wed, Oct 02, 2024 at 10:13:39AM +0000, FUJITA Tomonori wrote: > On Tue, 1 Oct 2024 14:48:06 +0200 > Andrew Lunn <andrew@lunn.ch> wrote: > > > I generally point developers at iopoll.h, because developers nearly > > always get this sort of polling for something to happen wrong. > > Ah, I had forgotten about iopoll.h. Make senses. I'll try implement an > equivalent in Rust. There are some subtleties involved with PHYs, which is why we have our own wrapper around the macros in iopoll.h: https://elixir.bootlin.com/linux/v6.11.1/source/include/linux/phy.h#L1288 Normally an IO operation cannot fail. But PHYs are different, a read could return -EOPNOTSUPP, -EIO, -ETIMEDOUT etc. That needs to be take into account and checked before evaluating the condition. So you might need to think about something similar for Rust. A generic read_poll_timeout() and a phy_read_poll_timeout() built on top of it. This is a worthwhile set of helpers to have, since the bugs in this are nothing to do with memory safety, but plain simple logic bugs, which Rust itself is unlikely to help with. Andrew
On Wed, 2 Oct 2024 14:31:07 +0200 Andrew Lunn <andrew@lunn.ch> wrote: > There are some subtleties involved with PHYs, which is why we have our > own wrapper around the macros in iopoll.h: > > https://elixir.bootlin.com/linux/v6.11.1/source/include/linux/phy.h#L1288 > > Normally an IO operation cannot fail. But PHYs are different, a read > could return -EOPNOTSUPP, -EIO, -ETIMEDOUT etc. That needs to be take > into account and checked before evaluating the condition. Thanks, I didn't know this function. I think that a generic read_poll_timeout in Rust can handle such case. #define read_poll_timeout(op, val, cond, sleep_us, timeout_us, \ sleep_before_read, args...) \ ({ \ u64 __timeout_us = (timeout_us); \ unsigned long __sleep_us = (sleep_us); \ ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \ might_sleep_if((__sleep_us) != 0); \ if (sleep_before_read && __sleep_us) \ usleep_range((__sleep_us >> 2) + 1, __sleep_us); \ for (;;) { \ (val) = op(args); \ if (cond) \ break; \ The reason why the C version cannot handle such case is that after call `op` function, read_poll_timeout macro can't know whether `val` is an error or not. In Rust version, 'op' function can return Result type, explicitly tells success or an error. It can return an error before evaluating the condition.
On Wed, Oct 02, 2024 at 09:56:36AM +0000, FUJITA Tomonori wrote: > On Wed, 2 Oct 2024 06:39:48 +0200 > Dirk Behme <dirk.behme@de.bosch.com> wrote: > > >> I generally point developers at iopoll.h, because developers nearly > >> always get this sort of polling for something to happen wrong. > >> The kernel sleep functions guarantee the minimum sleep time. They say > >> nothing about the maximum sleep time. You can ask it to sleep for 1ms, > >> and in reality, due to something stealing the CPU and not being RT > >> friendly, it actually sleeps for 10ms. This extra long sleep time > >> blows straight past your timeout, if you have a time based timeout. > >> What most developers do is after the sleep() returns they check to see > >> if the timeout has been reached and then exit with -ETIMEDOUT. They > >> don't check the state of the hardware, which given its had a long time > >> to do its thing, probably is now in a good state. But the function > >> returns -ETIMEDOUT. > >> There should always be a check of the hardware state after the sleep, > >> in order to determine ETIMEDOUT vs 0. > >> As i said, most C developers get this wrong. So i don't really see why > >> Rust developers also will not get this wrong. So i like to discourage > >> this sort of code, and have Rust implementations of iopoll.h. > > > > > > Do we talk about some simple Rust wrappers for the macros in iopoll.h? > > E.g. something like [1]? > > > > Or are we talking about some more complex (safety) dependencies which > > need some more complex abstraction handling? > > (snip) > > > int rust_helper_readb_poll_timeout(const volatile void * addr, > > u64 val, u64 cond, u64 delay_us, > > u64 timeout_us) > > { > > return readb_poll_timeout(addr, val, cond, delay_us, timeout_us); > > } > > I'm not sure a simple wrapper for iopoll.h works. We need to pass a > function. I'm testing a macro like the following (not added ktime > timeout yet): 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) } } note that you don't need the args part, because `op` is a closure that can capature value, so for example, if in C code you need to call foo(a, b), with closure, you can do: <a and b are defined in the caller> read_poll_timeout(|| { foo(a, b) }, ...); with above API. Regards, Boqun > > macro_rules! read_poll_timeout { > ($op:expr, $val:expr, $cond:expr, $sleep:expr, $timeout:expr, $($args:expr),*) => {{ > let _ = $val; > loop { > $val = $op($($args),*); > if $cond { > break; > } > kernel::delay::sleep($sleep); > } > if $cond { > Ok(()) > } else { > Err(kernel::error::code::ETIMEDOUT) > } > }}; > } >
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? 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 }
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 > }
> > 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 > > } This appears to have the usual bug when people implement it themselves and i then point them at iopoll.h, which so far as been bug free. kernel::delay::sleep(sleep) can sleep for an arbitrary amount of time greater than sleep, if the system is busy doing other things. You might only get around this loop once, and exit with ETIMEOUT, but while you have been sleeping a long time the hardware has completed its operation, but you never check. There must be a call to cond() after the timeout to handle this condition. And this is not theoretical. I had a very reproducible case of this during the boot of a device. It is less likely today, with SMP systems, and all the RT patches, but if it does happen, it will be very hard to track down. Andrew
> 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 ;-) iopoll.h has a few variants. The variant being implemented here can only be used in thread context where it can sleep. There is also read_poll_timeout_atomic() which can be used in atomic context, which uses udelay() rather than sleeping, since you are not allowed to sleep in atomic context. So we should keep the naming open to allow for the atomic variant sometime in the future. We probably also want a comment that this helper cannot be used in atomic context. Do we have a Rust equivalent of might_sleep()? https://elixir.bootlin.com/linux/v6.12-rc1/source/include/linux/kernel.h#L93 Andrew
On Thu, 3 Oct 2024 18:09:15 +0200 Andrew Lunn <andrew@lunn.ch> wrote: > We probably also want a comment that this helper cannot be used in > atomic context. Yeah, I'll add such. > Do we have a Rust equivalent of might_sleep()? > > https://elixir.bootlin.com/linux/v6.12-rc1/source/include/linux/kernel.h#L93 No. I'll add bindings for might_sleep() and cpu_relax().
On Thu, 3 Oct 2024 18:00:36 +0200 Andrew Lunn <andrew@lunn.ch> wrote: >> > 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 >> > } > > This appears to have the usual bug when people implement it themselves > and i then point them at iopoll.h, which so far as been bug free. Ah, in the next submission, I'll try to ensure that the Rust version works the same way as the C version.
On Fri, Oct 04, 2024 at 08:48:03PM +0900, FUJITA Tomonori wrote: > On Thu, 3 Oct 2024 18:09:15 +0200 > Andrew Lunn <andrew@lunn.ch> wrote: > > > We probably also want a comment that this helper cannot be used in > > atomic context. > > Yeah, I'll add such. > > > Do we have a Rust equivalent of might_sleep()? > > > > https://elixir.bootlin.com/linux/v6.12-rc1/source/include/linux/kernel.h#L93 > > No. I'll add bindings for might_sleep() and cpu_relax(). Please make sure you involve the scheduler people. This is now well outside of networking, same as the discussion around time has little to do with networking. The might_sleep() is not a strong requirement for iopoll, so you might want to get the basic functionality merged first, and then once might_sleep() is agreed on, add it to iopoll. It is just a debug feature. Andrew
diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs index 28d8981f410b..3a8ef9f73642 100644 --- a/drivers/net/phy/qt2025.rs +++ b/drivers/net/phy/qt2025.rs @@ -93,8 +93,15 @@ fn probe(dev: &mut phy::Device) -> Result<()> { // The micro-controller will start running from SRAM. dev.write(C45::new(Mmd::PCS, 0xe854), 0x0040)?; - // TODO: sleep here until the hw becomes ready. - Ok(()) + // sleep here until the hw becomes ready. + for _ in 0..60 { + kernel::delay::sleep(core::time::Duration::from_millis(50)); + let val = dev.read(C45::new(Mmd::PCS, 0xd7fd))?; + if val != 0x00 && val != 0x10 { + return Ok(()); + } + } + Err(code::ENODEV) } fn read_status(dev: &mut phy::Device) -> Result<u16> {
Wait until a PHY becomes ready in the probe callback by using a sleep function. Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> --- drivers/net/phy/qt2025.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)