[PATCH 5/5] rust: serdev: Pause receive callback before calling unbind

[Markus Probst]

The receive callback and unbind callback now have exclusive access to
the drivers private data. Provide mutable references in callbacks to
avoid the need for locks in the private data. Remove the Sync
requirement.

Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
---
 rust/kernel/serdev.rs              | 39 ++++++++++++++++++++++----------------
 samples/rust/rust_driver_serdev.rs |  2 +-
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 66543108ec2f..7d47d91e3bc3 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -111,12 +111,12 @@ pub struct PrivateData<'bound, T: Driver> {
 }
 
 impl<'bound, T: Driver> PrivateData<'bound, T> {
-    fn driver_data(self: Pin<&Self>) -> Pin<&T::Data<'bound>> {
+    fn driver_data(self: Pin<&mut Self>) -> Pin<&mut T::Data<'bound>> {
         // SAFETY: We treat the result as pinned.
         let inner = unsafe { Pin::into_inner_unchecked(self) };
 
         // SAFETY: `self.driver` is pinned.
-        unsafe { Pin::new_unchecked(&inner.driver) }
+        unsafe { Pin::new_unchecked(&mut inner.driver) }
     }
 }
 
@@ -175,15 +175,18 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
         // INVARIANT: `sdev` is valid for the duration of `remove_callback()`.
         let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
 
-        // SAFETY: `remove_callback` is only ever called after a successful call to
-        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
-        // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
-        let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
-
-        T::unbind(sdev, private_data.driver_data());
-
         // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
         unsafe { bindings::serdev_device_pause_rx(sdev.as_raw()) };
+
+        // SAFETY:
+        // - `remove_callback` is only ever called after a successful call to `probe_callback`,
+        //   hence it's guaranteed that `Device::set_drvdata()` has been called and stored a
+        //   `Pin<KBox<PrivateData<'_, T>>>`.
+        // - The call to `serdev_device_pause_rx` above guarantees that we do not overlap with
+        //   `receive_buf_callback`, thus it is guaranteed that we have exclusive access.
+        let private_data = unsafe { sdev.as_ref().drvdata_borrow_mut::<PrivateData<'_, T>>() };
+
+        T::unbind(sdev, private_data.driver_data());
     }
 
     extern "C" fn receive_buf_callback(
@@ -200,10 +203,14 @@ extern "C" fn receive_buf_callback(
         // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
         let sdev = unsafe { &*sdev.cast::<Device<device::BoundInternal>>() };
 
-        // SAFETY: `receive_buf_callback` is only ever called after a successful call to
-        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
-        // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
-        let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
+        // SAFETY:
+        // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
+        //   hence it's guaranteed that `Device::set_drvdata()` has been called and stored a
+        //   `Pin<KBox<PrivateData<'_, T>>>`.
+        // - `unbind_callback` calls `serdev_device_pause_rx` before accessing the driver data,
+        //   which guarantees that this function will not overlap with it. Thus we have exclusive
+        //   access.
+        let private_data = unsafe { sdev.as_ref().drvdata_borrow_mut::<PrivateData<'_, T>>() };
 
         T::receive(sdev, private_data.driver_data(), buf)
     }
@@ -305,7 +312,7 @@ pub trait Driver {
     type IdInfo: 'static;
 
     /// The type of the driver's bus device private data.
-    type Data<'bound>: Send + Sync + 'bound;
+    type Data<'bound>: Send + 'bound;
 
     /// The table of OF device ids supported by the driver.
     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
@@ -331,7 +338,7 @@ fn probe<'bound>(
     /// `&Device<Core>` or `&Device<Bound>` reference. For instance.
     ///
     /// Otherwise, release operations for driver resources should be performed in `Drop`.
-    fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
+    fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&mut Self::Data<'bound>>) {
         let _ = (sdev, this);
     }
 
@@ -342,7 +349,7 @@ fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<
     /// Returns the number of bytes accepted.
     fn receive<'bound>(
         sdev: &'bound Device<device::Bound>,
-        this: Pin<&Self::Data<'bound>>,
+        this: Pin<&mut Self::Data<'bound>>,
         data: &[u8],
     ) -> usize {
         let _ = (sdev, this, data);
diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
index 51b4898cd855..d00d547234c8 100644
--- a/samples/rust/rust_driver_serdev.rs
+++ b/samples/rust/rust_driver_serdev.rs
@@ -63,7 +63,7 @@ fn probe<'bound>(
 
     fn receive<'bound>(
         sdev: &'bound serdev::Device<Bound>,
-        _this: Pin<&Self>,
+        _this: Pin<&mut Self>,
         data: &[u8],
     ) -> usize {
         sdev.write(data).unwrap_or_default() as usize

-- 
2.55.0