Thanks for the review!

On Wed, 31 Jul 2024 14:32:18 +0200
Alice Ryhl [off-list ref] wrote:

quoted

quoted

+    /// # Safety
+    ///
+    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
+    unsafe extern "C" fn probe_callback(phydev: *mut bindings::phy_device) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: This callback is called only in contexts
+            // where we can exclusively access to `phy_device`, so the accessors on
+            // `Device` are okay to call.

This one is slightly different to other callbacks. probe is called
without the mutex. Instead, probe is called before the device is
published. So the comment is correct, but given how important Rust
people take these SAFETY comments, maybe it should indicate it is
different to others?

Interesting. Given that we don't hold the mutex, does that mean that
some of the methods on Device are not safe to call in this context? Or
is there something else that makes it okay to call them despite not
holding the mutex?

Before the callback, the device object was initialized properly by
PHYLIB and no concurrent access so all the methods can be called
safely (no kernel panic), I think.

If the safety comment needs to updated, how about the following?

SAFETY: This callback is called only in contexts where we can
exclusively access to `phy_device` because it's not published yet, so
the accessors on `Device` are okay to call.