[PATCH 0/7] rust: treewide: remove driver unbind callbacks

[PATCH 0/7] rust: treewide: remove driver unbind callbacks

[Danilo Krummrich]

The driver model uses native Rust lifetimes, allowing driver data to
retain device resources and a bound device reference for use in its
destructor. Separate teardown callbacks are therefore no longer needed.

Remove unbind() from the PCI, platform, auxiliary, serdev and I2C driver
traits, and disconnect() from the USB driver trait. Move the affected
sample teardown into destructors and update the driver documentation.

Danilo Krummrich (7):
  rust: pci: remove the unbind() callback
  rust: platform: remove the unbind() callback
  rust: auxiliary: remove the unbind() callback
  rust: serdev: remove the unbind() callback
  rust: i2c: remove the unbind() callback
  rust: usb: remove the disconnect() callback
  rust: driver: document teardown through destructors

 rust/kernel/auxiliary.rs        | 34 ++++-----------------------
 rust/kernel/driver.rs           | 18 ++++++---------
 rust/kernel/i2c.rs              | 31 ++++---------------------
 rust/kernel/pci.rs              | 34 ++++-----------------------
 rust/kernel/platform.rs         | 34 ++++-----------------------
 rust/kernel/serdev.rs           | 41 ++++-----------------------------
 rust/kernel/usb.rs              | 36 +++++------------------------
 samples/rust/rust_driver_i2c.rs | 18 ++++++++++-----
 samples/rust/rust_driver_pci.rs |  7 ++----
 samples/rust/rust_driver_usb.rs | 11 +++++----
 samples/rust/rust_i2c_client.rs | 12 +++++-----
 11 files changed, 59 insertions(+), 217 deletions(-)


base-commit: de7625abe4e92f1fb0047ed5f163b2ecaba77e50
-- 
2.56.0

[PATCH 1/7] rust: pci: remove the unbind() callback

[Danilo Krummrich]

The driver model uses native Rust lifetimes to let driver data retain
device resources and a Device<Bound> reference for the duration of the
binding. Both remain accessible from the destructor, so a separate
unbind() callback is no longer needed.

Remove pci::Driver::unbind() and the adapter's remove callback and
document that device teardown belongs in Drop or PinnedDrop.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/pci.rs              | 34 ++++-----------------------------
 samples/rust/rust_driver_pci.rs |  7 ++-----
 2 files changed, 6 insertions(+), 35 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 0e9455fd544d..8101c58da6f4 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -90,7 +90,6 @@ unsafe fn register(
         unsafe {
             (*pdrv.get()).name = name.as_char_ptr();
             (*pdrv.get()).probe = Some(Self::probe_callback);
-            (*pdrv.get()).remove = Some(Self::remove_callback);
             (*pdrv.get()).id_table = T::ID_TABLE.as_ptr();
         }
 
@@ -133,21 +132,6 @@ extern "C" fn probe_callback(
             Ok(0)
         })
     }
-
-    extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {
-        // SAFETY: The PCI bus only ever calls the remove callback with a valid pointer to a
-        // `struct pci_dev`.
-        //
-        // INVARIANT: `pdev` is valid for the duration of `remove_callback()`.
-        let pdev = unsafe { &*pdev.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<T::Data<'_>>>`.
-        let data = unsafe { pdev.as_ref().drvdata_borrow::<T::Data<'_>>() };
-
-        T::unbind(pdev, data);
-    }
 }
 
 /// Declares a kernel module that exposes a single PCI driver.
@@ -305,6 +289,10 @@ pub trait Driver {
     type IdInfo: 'static;
 
     /// The type of the driver's bus device private data.
+    ///
+    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
+    /// for this type. The `'bound` lifetime allows the data to retain device resources and a
+    /// [`Device<device::Bound>`] reference for use during teardown.
     type Data<'bound>: Send + 'bound;
 
     /// The table of device ids supported by the driver.
@@ -318,20 +306,6 @@ fn probe<'bound>(
         dev: &'bound Device<device::Core<'_>>,
         id_info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
-
-    /// PCI driver unbind.
-    ///
-    /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
-    /// is optional.
-    ///
-    /// This callback serves as a place for drivers to perform teardown operations that require a
-    /// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O
-    /// operations to gracefully tear down the device.
-    ///
-    /// Otherwise, release operations for driver resources should be performed in `Drop`.
-    fn unbind<'bound>(dev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        let _ = (dev, this);
-    }
 }
 
 /// The PCI device representation.
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 13b035a95756..6bd4c1ff11ee 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -176,15 +176,12 @@ fn probe<'bound>(
             index: *info,
         })
     }
-
-    fn unbind<'bound>(_pdev: &'bound pci::Device<Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        this.bar
-            .write_reg(regs::TEST::zeroed().with_index(this.index));
-    }
 }
 
 impl Drop for SampleDriverData<'_> {
     fn drop(&mut self) {
+        self.bar
+            .write_reg(regs::TEST::zeroed().with_index(self.index));
         dev_dbg!(self.pdev, "Remove Rust PCI driver sample.\n");
     }
 }
-- 
2.56.0

[PATCH 2/7] rust: platform: remove the unbind() callback

[Danilo Krummrich]

The driver model uses native Rust lifetimes to let driver data retain
device resources and a Device<Bound> reference for the duration of the
binding. Both remain accessible from the destructor, so a separate
unbind() callback is no longer needed.

Remove platform::Driver::unbind() and the adapter's remove callback and
document that device teardown belongs in Drop or PinnedDrop.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/platform.rs         | 34 ++++-----------------------------
 samples/rust/rust_i2c_client.rs | 12 ++++++------
 2 files changed, 10 insertions(+), 36 deletions(-)

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index f49aedcc94b1..09dabd6022b6 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -75,7 +75,6 @@ unsafe fn register(
         unsafe {
             (*pdrv.get()).driver.name = name.as_char_ptr();
             (*pdrv.get()).probe = Some(Self::probe_callback);
-            (*pdrv.get()).remove = Some(Self::remove_callback);
             (*pdrv.get()).driver.of_match_table = of_table;
             (*pdrv.get()).driver.acpi_match_table = acpi_table;
         }
@@ -109,21 +108,6 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
             Ok(0)
         })
     }
-
-    extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {
-        // SAFETY: The platform bus only ever calls the remove callback with a valid pointer to a
-        // `struct platform_device`.
-        //
-        // INVARIANT: `pdev` is valid for the duration of `remove_callback()`.
-        let pdev = unsafe { &*pdev.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<T::Data<'_>>>`.
-        let data = unsafe { pdev.as_ref().drvdata_borrow::<T::Data<'_>>() };
-
-        T::unbind(pdev, data);
-    }
 }
 
 impl<T: Driver> driver::Adapter for Adapter<T> {
@@ -214,6 +198,10 @@ pub trait Driver {
     type IdInfo: 'static;
 
     /// The type of the driver's bus device private data.
+    ///
+    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
+    /// for this type. The `'bound` lifetime allows the data to retain device resources and a
+    /// [`Device<device::Bound>`] reference for use during teardown.
     type Data<'bound>: Send + 'bound;
 
     /// The table of OF device ids supported by the driver.
@@ -230,20 +218,6 @@ fn probe<'bound>(
         dev: &'bound Device<device::Core<'_>>,
         id_info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
-
-    /// Platform driver unbind.
-    ///
-    /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
-    /// is optional.
-    ///
-    /// This callback serves as a place for drivers to perform teardown operations that require a
-    /// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O
-    /// operations to gracefully tear down the device.
-    ///
-    /// Otherwise, release operations for driver resources should be performed in `Drop`.
-    fn unbind<'bound>(dev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        let _ = (dev, this);
-    }
 }
 
 /// The platform device representation.
diff --git a/samples/rust/rust_i2c_client.rs b/samples/rust/rust_i2c_client.rs
index c8a23875ef5b..e25492612f9c 100644
--- a/samples/rust/rust_i2c_client.rs
+++ b/samples/rust/rust_i2c_client.rs
@@ -78,7 +78,7 @@
     sync::aref::ARef, //
 };
 
-#[pin_data]
+#[pin_data(PinnedDrop)]
 struct SampleDriver {
     parent_dev: ARef<platform::Device>,
     #[pin]
@@ -127,13 +127,13 @@ fn probe<'bound>(
             }
         })
     }
+}
 
-    fn unbind<'bound>(
-        pdev: &'bound platform::Device<device::Core<'_>>,
-        _this: Pin<&Self::Data<'bound>>,
-    ) {
+#[pinned_drop]
+impl PinnedDrop for SampleDriver {
+    fn drop(self: Pin<&mut Self>) {
         dev_info!(
-            pdev.as_ref(),
+            self.parent_dev,
             "Unbind Rust I2C Client registration sample.\n"
         );
     }
-- 
2.56.0

[PATCH 3/7] rust: auxiliary: remove the unbind() callback

[Danilo Krummrich]

The driver model uses native Rust lifetimes to let driver data retain
device resources and a Device<Bound> reference for the duration of the
binding. Both remain accessible from the destructor, so a separate
unbind() callback is no longer needed.

Remove auxiliary::Driver::unbind() and the adapter's remove callback and
document that device teardown belongs in Drop or PinnedDrop.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/auxiliary.rs | 34 ++++------------------------------
 1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 9d3de109b381..d4618c92181f 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -64,7 +64,6 @@ unsafe fn register(
         unsafe {
             (*adrv.get()).name = name.as_char_ptr();
             (*adrv.get()).probe = Some(Self::probe_callback);
-            (*adrv.get()).remove = Some(Self::remove_callback);
             (*adrv.get()).id_table = T::ID_TABLE.as_ptr();
         }
 
@@ -105,21 +104,6 @@ extern "C" fn probe_callback(
             Ok(0)
         })
     }
-
-    extern "C" fn remove_callback(adev: *mut bindings::auxiliary_device) {
-        // SAFETY: The auxiliary bus only ever calls the probe callback with a valid pointer to a
-        // `struct auxiliary_device`.
-        //
-        // INVARIANT: `adev` is valid for the duration of `remove_callback()`.
-        let adev = unsafe { &*adev.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<T::Data<'_>>>`.
-        let data = unsafe { adev.as_ref().drvdata_borrow::<T::Data<'_>>() };
-
-        T::unbind(adev, data);
-    }
 }
 
 /// Declares a kernel module that exposes a single auxiliary driver.
@@ -205,6 +189,10 @@ pub trait Driver {
     type IdInfo: 'static;
 
     /// The type of the driver's bus device private data.
+    ///
+    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
+    /// for this type. The `'bound` lifetime allows the data to retain device resources and a
+    /// [`Device<device::Bound>`] reference for use during teardown.
     type Data<'bound>: Send + 'bound;
 
     /// The table of device ids supported by the driver.
@@ -217,20 +205,6 @@ fn probe<'bound>(
         dev: &'bound Device<device::Core<'_>>,
         id_info: &'bound Self::IdInfo,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
-
-    /// Auxiliary driver unbind.
-    ///
-    /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
-    /// is optional.
-    ///
-    /// This callback serves as a place for drivers to perform teardown operations that require a
-    /// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O
-    /// operations to gracefully tear down the device.
-    ///
-    /// Otherwise, release operations for driver resources should be performed in `Drop`.
-    fn unbind<'bound>(dev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        let _ = (dev, this);
-    }
 }
 
 /// The auxiliary device representation.
-- 
2.56.0

[PATCH 4/7] rust: serdev: remove the unbind() callback

[Danilo Krummrich]

The driver model uses native Rust lifetimes to let driver data retain
device resources and a Device<Bound> reference for the duration of the
binding. Both remain accessible from the destructor, so a separate
unbind() callback is no longer needed.

Remove serdev::Driver::unbind() and the adapter's remove callback and
document that device teardown belongs in Drop or PinnedDrop.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/serdev.rs | 41 ++++-------------------------------------
 1 file changed, 4 insertions(+), 37 deletions(-)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 17ca504b7f8d..fd998b68b74f 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -81,7 +81,6 @@ unsafe fn register(
         unsafe {
             (*sdrv.get()).driver.name = name.as_char_ptr();
             (*sdrv.get()).probe = Some(Self::probe_callback);
-            (*sdrv.get()).remove = Some(Self::remove_callback);
             (*sdrv.get()).driver.of_match_table = of_table;
             (*sdrv.get()).driver.acpi_match_table = acpi_table;
         }
@@ -210,29 +209,6 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi:
         })
     }
 
-    extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
-        // SAFETY: The serial device bus only ever calls the remove callback with a valid pointer
-        // to a `struct 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>>() };
-
-        // SAFETY: No one has exclusive access to `private_data.driver`.
-        let data = unsafe { &*private_data.driver.get() };
-        // SAFETY:
-        // - `private_data.driver` is pinned.
-        // - `remove_callback` is only ever called after a successful call to `probe_callback`,
-        //   hence it's guaranteed that `private_data.driver` was initialized.
-        let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
-
-        T::unbind(sdev, data_pinned);
-    }
-
     extern "C" fn receive_buf_callback(
         sdev: *mut bindings::serdev_device,
         buf: *const u8,
@@ -365,6 +341,10 @@ pub trait Driver {
     type IdInfo: 'static;
 
     /// The type of the driver's bus device private data.
+    ///
+    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
+    /// for this type. The `'bound` lifetime allows the data to retain device resources and a
+    /// [`Device<device::Bound>`] reference for use during teardown.
     type Data<'bound>: Send + Sync + 'bound;
 
     /// The table of OF device ids supported by the driver.
@@ -382,19 +362,6 @@ fn probe<'bound>(
         id_info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
 
-    /// Serial device bus device driver unbind.
-    ///
-    /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
-    /// is optional.
-    ///
-    /// This callback serves as a place for drivers to perform teardown operations that require a
-    /// `&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>>) {
-        let _ = (sdev, this);
-    }
-
     /// Serial device bus device data receive callback.
     ///
     /// Called when data got received from device.
-- 
2.56.0

[PATCH 5/7] rust: i2c: remove the unbind() callback

[Danilo Krummrich]

The driver model uses native Rust lifetimes to let driver data retain
device resources and a Device<Bound> reference for the duration of the
binding. Both remain accessible from the destructor, so a separate
unbind() callback is no longer needed.

Remove i2c::Driver::unbind() and the adapter's remove callback and
document that device teardown belongs in Drop or PinnedDrop.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/i2c.rs              | 31 ++++---------------------------
 samples/rust/rust_driver_i2c.rs | 18 ++++++++++++------
 2 files changed, 16 insertions(+), 33 deletions(-)

diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index b4ff25c830fc..b15e73bf827a 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -123,7 +123,6 @@ unsafe fn register(
         unsafe {
             (*idrv.get()).driver.name = name.as_char_ptr();
             (*idrv.get()).probe = Some(Self::probe_callback);
-            (*idrv.get()).remove = Some(Self::remove_callback);
             (*idrv.get()).shutdown = Some(Self::shutdown_callback);
             (*idrv.get()).id_table = i2c_table;
             (*idrv.get()).driver.of_match_table = of_table;
@@ -161,18 +160,6 @@ extern "C" fn probe_callback(idev: *mut bindings::i2c_client) -> kernel::ffi::c_
         })
     }
 
-    extern "C" fn remove_callback(idev: *mut bindings::i2c_client) {
-        // SAFETY: `idev` is a valid pointer to a `struct i2c_client`.
-        let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };
-
-        // SAFETY: `remove_callback` is only ever called after a successful call to
-        // `probe_callback`, hence it's guaranteed that `I2cClient::set_drvdata()` has been called
-        // and stored a `Pin<KBox<T::Data<'_>>>`.
-        let data = unsafe { idev.as_ref().drvdata_borrow::<T::Data<'_>>() };
-
-        T::unbind(idev, data);
-    }
-
     extern "C" fn shutdown_callback(idev: *mut bindings::i2c_client) {
         // SAFETY: `shutdown_callback` is only ever called for a valid `idev`
         let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };
@@ -312,6 +299,10 @@ pub trait Driver {
     type IdInfo: 'static;
 
     /// The type of the driver's bus device private data.
+    ///
+    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
+    /// for this type. The `'bound` lifetime allows the data to retain device resources and a
+    /// [`I2cClient<device::Bound>`] reference for use during teardown.
     type Data<'bound>: Send + 'bound;
 
     /// The table of device ids supported by the driver.
@@ -346,20 +337,6 @@ fn probe<'bound>(
     fn shutdown<'bound>(dev: &'bound I2cClient<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
         let _ = (dev, this);
     }
-
-    /// I2C driver unbind.
-    ///
-    /// Called when the [`I2cClient`] is unbound from its bound [`Driver`]. Implementing this
-    /// callback is optional.
-    ///
-    /// This callback serves as a place for drivers to perform teardown operations that require a
-    /// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O
-    /// operations to gracefully tear down the device.
-    ///
-    /// Otherwise, release operations for driver resources should be performed in `Drop`.
-    fn unbind<'bound>(dev: &'bound I2cClient<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        let _ = (dev, this);
-    }
 }
 
 /// The i2c adapter representation.
diff --git a/samples/rust/rust_driver_i2c.rs b/samples/rust/rust_driver_i2c.rs
index a0df0c6097c4..4f5047ab4e84 100644
--- a/samples/rust/rust_driver_i2c.rs
+++ b/samples/rust/rust_driver_i2c.rs
@@ -12,6 +12,10 @@
 
 struct SampleDriver;
 
+struct SampleDriverData<'bound> {
+    idev: &'bound i2c::I2cClient,
+}
+
 kernel::acpi_device_table! {
     ACPI_TABLE,
     <SampleDriver as i2c::Driver>::IdInfo,
@@ -32,7 +36,7 @@
 
 impl i2c::Driver for SampleDriver {
     type IdInfo = u32;
-    type Data<'bound> = Self;
+    type Data<'bound> = SampleDriverData<'bound>;
 
     const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
     const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_TABLE);
@@ -41,7 +45,7 @@ impl i2c::Driver for SampleDriver {
     fn probe<'bound>(
         idev: &'bound i2c::I2cClient<Core<'_>>,
         info: Option<&'bound Self::IdInfo>,
-    ) -> impl PinInit<Self, Error> + 'bound {
+    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let dev = idev.as_ref();
 
         dev_info!(dev, "Probe Rust I2C driver sample.\n");
@@ -50,15 +54,17 @@ fn probe<'bound>(
             dev_info!(dev, "Probed with info: '{}'.\n", info);
         }
 
-        Ok(Self)
+        Ok(SampleDriverData { idev })
     }
 
-    fn shutdown<'bound>(idev: &'bound i2c::I2cClient<Core<'_>>, _this: Pin<&Self>) {
+    fn shutdown<'bound>(idev: &'bound i2c::I2cClient<Core<'_>>, _this: Pin<&Self::Data<'bound>>) {
         dev_info!(idev.as_ref(), "Shutdown Rust I2C driver sample.\n");
     }
+}
 
-    fn unbind<'bound>(idev: &'bound i2c::I2cClient<Core<'_>>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Unbind Rust I2C driver sample.\n");
+impl Drop for SampleDriverData<'_> {
+    fn drop(&mut self) {
+        dev_info!(self.idev, "Unbind Rust I2C driver sample.\n");
     }
 }
 
-- 
2.56.0

[PATCH 6/7] rust: usb: remove the disconnect() callback

[Danilo Krummrich]

The driver model uses native Rust lifetimes to let driver data retain
device resources and an Interface<Bound> reference for the duration of
the binding. Both remain accessible from the destructor, so a separate
disconnect() callback is no longer needed.

Remove usb::Driver::disconnect() and document that device teardown
belongs in Drop or PinnedDrop.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/usb.rs              | 36 ++++++---------------------------
 samples/rust/rust_driver_usb.rs | 11 +++++-----
 2 files changed, 12 insertions(+), 35 deletions(-)

diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 993760ff332b..2126378abc95 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -98,22 +98,8 @@ extern "C" fn probe_callback(
         })
     }
 
-    extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) {
-        // SAFETY: The USB core only ever calls the disconnect callback with a valid pointer to a
-        // `struct usb_interface`.
-        //
-        // INVARIANT: `intf` is valid for the duration of `disconnect_callback()`.
-        let intf = unsafe { &*intf.cast::<Interface<device::CoreInternal<'_>>>() };
-
-        let dev: &device::Device<device::CoreInternal<'_>> = intf.as_ref();
-
-        // SAFETY: `disconnect_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<T::Data<'_>>>`.
-        let data = unsafe { dev.drvdata_borrow::<T::Data<'_>>() };
-
-        T::disconnect(intf, data);
-    }
+    // The USB core calls `disconnect` unconditionally.
+    extern "C" fn disconnect_callback(_intf: *mut bindings::usb_interface) {}
 }
 
 /// Abstraction for the USB device ID structure, i.e. [`struct usb_device_id`].
@@ -278,12 +264,6 @@ macro_rules! usb_device_table {
 ///     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
 ///         Err(ENODEV)
 ///     }
-///
-///     fn disconnect<'bound>(
-///         _interface: &'bound usb::Interface<Core<'_>>,
-///         _data: Pin<&Self::Data<'bound>>,
-///     ) {
-///     }
 /// }
 ///```
 pub trait Driver {
@@ -291,6 +271,10 @@ pub trait Driver {
     type IdInfo: 'static;
 
     /// The type of the driver's bus device private data.
+    ///
+    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
+    /// for this type. The `'bound` lifetime allows the data to retain device resources and an
+    /// [`Interface<device::Bound>`] reference for use during teardown.
     type Data<'bound>: Send + 'bound;
 
     /// The table of device ids supported by the driver.
@@ -305,14 +289,6 @@ fn probe<'bound>(
         id: &DeviceId,
         id_info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
-
-    /// USB driver disconnect.
-    ///
-    /// Called when the USB interface is about to be unbound from this driver.
-    fn disconnect<'bound>(
-        interface: &'bound Interface<device::Core<'_>>,
-        data: Pin<&Self::Data<'bound>>,
-    );
 }
 
 /// A USB interface.
diff --git a/samples/rust/rust_driver_usb.rs b/samples/rust/rust_driver_usb.rs
index 7ef04e177c80..1349fa1a66d1 100644
--- a/samples/rust/rust_driver_usb.rs
+++ b/samples/rust/rust_driver_usb.rs
@@ -14,7 +14,7 @@
 };
 
 struct SampleDriver {
-    _intf: ARef<usb::Interface>,
+    intf: ARef<usb::Interface>,
 }
 
 kernel::usb_device_table!(
@@ -36,12 +36,13 @@ fn probe<'bound>(
         let dev: &device::Device<Core<'_>> = intf.as_ref();
         dev_info!(dev, "Rust USB driver sample probed\n");
 
-        Ok(Self { _intf: intf.into() })
+        Ok(Self { intf: intf.into() })
     }
+}
 
-    fn disconnect<'bound>(intf: &'bound usb::Interface<Core<'_>>, _data: Pin<&Self>) {
-        let dev: &device::Device<Core<'_>> = intf.as_ref();
-        dev_info!(dev, "Rust USB driver sample disconnected\n");
+impl Drop for SampleDriver {
+    fn drop(&mut self) {
+        dev_info!(self.intf, "Rust USB driver sample disconnected\n");
     }
 }
 
-- 
2.56.0

[PATCH 7/7] rust: driver: document teardown through destructors

[Danilo Krummrich]

The bus driver traits no longer provide an unbind() callback. Remove it
from the example driver trait and describe how the generic registration
infrastructure drops driver data during unbind cleanup.

Document that device teardown belongs in Drop or PinnedDrop, where the
bound device reference and device resources remain accessible.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/driver.rs | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index c9c74c4dde8f..c228d82ace9d 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -31,14 +31,6 @@
 //!         dev: &'bound Device<device::Core<'_>>,
 //!         id_info: &'bound Self::IdInfo,
 //!     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
-//!
-//!     /// Driver unbind (optional).
-//!     fn unbind<'bound>(
-//!         dev: &'bound Device<device::Core<'_>>,
-//!         this: Pin<&Self::Data<'bound>>,
-//!     ) {
-//!         let _ = (dev, this);
-//!     }
 //! }
 //! ```
 //!
@@ -57,9 +49,12 @@
 //! [`Device`] infrastructure provides common helpers for this purpose on its
 //! [`Device<CoreInternal>`] implementation.
 //!
-//! All driver callbacks should provide a reference to the driver's private data. Once the driver
-//! is unbound from the device, the bus abstraction should take back the ownership of the driver's
-//! private data from the corresponding [`Device`] and [`drop`] it.
+//! All driver callbacks should provide a reference to the driver's private data. The generic
+//! [`Registration`] infrastructure drops the driver's private data during unbind cleanup.
+//!
+//! Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop) for the
+//! driver's private data. The `'bound` lifetime allows the data to retain device resources and a
+//! [`Device<Bound>`] reference for use during teardown.
 //!
 //! All driver callbacks should provide a [`Device<Core>`] reference (see also [`device::Core`]).
 //!
@@ -98,6 +93,7 @@
 //!
 //! [`Core`]: device::Core
 //! [`Device`]: device::Device
+//! [`Device<Bound>`]: device::Device<device::Bound>
 //! [`Device<Core>`]: device::Device<device::Core>
 //! [`Device<CoreInternal>`]: device::Device<device::CoreInternal>
 //! [`DeviceContext`]: device::DeviceContext
-- 
2.56.0

Re: [PATCH 3/7] rust: auxiliary: remove the unbind() callback

[sashiko-bot]

> The driver model uses native Rust lifetimes to let driver data retain
> device resources and a Device<Bound> reference for the duration of the
> binding. Both remain accessible from the destructor, so a separate
> unbind() callback is no longer needed.
> 
> Remove auxiliary::Driver::unbind() and the adapter's remove callback and
> document that device teardown belongs in Drop or PinnedDrop.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112005455.1224352-1-dakr@kernel.org?part=3

Re: [PATCH 2/7] rust: platform: remove the unbind() callback

[sashiko-bot]

> The driver model uses native Rust lifetimes to let driver data retain
> device resources and a Device<Bound> reference for the duration of the
> binding. Both remain accessible from the destructor, so a separate
> unbind() callback is no longer needed.
> 
> Remove platform::Driver::unbind() and the adapter's remove callback and
> document that device teardown belongs in Drop or PinnedDrop.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112005455.1224352-1-dakr@kernel.org?part=2

Re: [PATCH 7/7] rust: driver: document teardown through destructors

[sashiko-bot]

> The bus driver traits no longer provide an unbind() callback. Remove it
> from the example driver trait and describe how the generic registration
> infrastructure drops driver data during unbind cleanup.
> 
> Document that device teardown belongs in Drop or PinnedDrop, where the
> bound device reference and device resources remain accessible.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112005455.1224352-1-dakr@kernel.org?part=7

Re: [PATCH 5/7] rust: i2c: remove the unbind() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] rust: i2c: dropping Driver::Data in post_unbind_rust runs after
  devres release
- [Medium] rust: i2c: missing Sync impl for I2cClient<device::Bound>
--

commit 5a5ad706d767f7788672a5c38d2627a235e83cd2
Author: Danilo Krummrich <dakr@kernel.org>

rust: i2c: remove the unbind() callback

This commit removes i2c::Driver::unbind() and the adapter's remove
callback, documenting that I2C driver teardown should be performed in
Drop or PinnedDrop of Driver::Data.

> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> index b4ff25c830fc0..b15e73bf827ab 100644
> --- a/rust/kernel/i2c.rs
> +++ b/rust/kernel/i2c.rs
> @@ -123,7 +123,6 @@ unsafe fn register(
>          unsafe {
>              (*idrv.get()).driver.name = name.as_char_ptr();
>              (*idrv.get()).probe = Some(Self::probe_callback);
> -            (*idrv.get()).remove = Some(Self::remove_callback);
>              (*idrv.get()).shutdown = Some(Self::shutdown_callback);
>              (*idrv.get()).id_table = i2c_table;
>              (*idrv.get()).driver.of_match_table = of_table;
> @@ -161,18 +160,6 @@ extern "C" fn probe_callback(idev: *mut bindings::i2c_client) -> kernel::ffi::c_
>          })
>      }
>  
> -    extern "C" fn remove_callback(idev: *mut bindings::i2c_client) {
> -        // SAFETY: `idev` is a valid pointer to a `struct i2c_client`.
> -        let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };
> -
> -        // SAFETY: `remove_callback` is only ever called after a successful call to
> -        // `probe_callback`, hence it's guaranteed that `I2cClient::set_drvdata()` has been called
> -        // and stored a `Pin<KBox<T::Data<'_>>>`.
> -        let data = unsafe { idev.as_ref().drvdata_borrow::<T::Data<'_>>() };
> -
> -        T::unbind(idev, data);
> -    }
> -
>      extern "C" fn shutdown_callback(idev: *mut bindings::i2c_client) {
>          // SAFETY: `shutdown_callback` is only ever called for a valid `idev`
>          let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };

[Severity: High]
Does moving I2C driver teardown from remove_callback() to the Drop or
PinnedDrop destructor of Driver::Data cause driver teardown to run after
the client's devres resources have already been released?

i2c_device_probe() opens an unclosed devres group on client->dev before
calling driver->probe(), so devres entries registered for the client land
in client->devres_group_id.  On unbind, __device_release_driver() calls
device_remove() -> i2c_device_remove() before device_unbind_cleanup() ->
post_unbind_rust():

drivers/i2c/i2c-core-base.c:i2c_device_remove() {
	...
	if (driver->remove) {
		...
		driver->remove(client);
	}

	debugfs_remove_recursive(client->debugfs);

	devres_release_group(&client->dev, client->devres_group_id);
	...
}

Because post_unbind_rust() drops Driver::Data in device_unbind_cleanup()
after i2c_device_remove() has already released client->devres_group_id,
all devres resources on the client are already revoked and dropped before
Driver::Data::drop() runs.  If Driver::Data retains a &'bound reference
from Devres::access() or calls Devres::access() with a retained &'bound
Device<Bound> in its destructor, Devres::access() skips the Revocable
availability check and causes a use-after-free in safe Rust.

> @@ -312,6 +299,10 @@ pub trait Driver {
>      type IdInfo: 'static;
>  
>      /// The type of the driver's bus device private data.
> +    ///
> +    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
> +    /// for this type. The `'bound` lifetime allows the data to retain device resources and a
> +    /// [`I2cClient<device::Bound>`] reference for use during teardown.
>      type Data<'bound>: Send + 'bound;

[Severity: Medium]
Can a driver actually store &'bound I2cClient<device::Bound> in
Driver::Data<'bound> as documented here?

Driver::Data<'bound> requires Send, so &'bound I2cClient<device::Bound>
is only Send if I2cClient<device::Bound> implements Sync.  However, Sync
is only implemented for the default context parameter I2cClient
(I2cClient<device::Normal>), not for I2cClient<Ctx> or
I2cClient<device::Bound>, so storing &'bound I2cClient<device::Bound> in
Driver::Data<'bound> fails to compile.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112005455.1224352-1-dakr@kernel.org?part=5

Re: [PATCH 1/7] rust: pci: remove the unbind() callback

[sashiko-bot]

> The driver model uses native Rust lifetimes to let driver data retain
> device resources and a Device<Bound> reference for the duration of the
> binding. Both remain accessible from the destructor, so a separate
> unbind() callback is no longer needed.
> 
> Remove pci::Driver::unbind() and the adapter's remove callback and
> document that device teardown belongs in Drop or PinnedDrop.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112005455.1224352-1-dakr@kernel.org?part=1

Re: [PATCH 4/7] rust: serdev: remove the unbind() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] rust: serdev: null pointer dereference in
  receive_buf_callback() [1]
--

commit 957ca5c83392506736074a2417dcd46b9e299666
Author: Danilo Krummrich <dakr@kernel.org>

rust: serdev: remove the unbind() callback

This commit removes the serdev::Driver::unbind() callback and the
corresponding adapter remove_callback(), documenting that device teardown
belongs in Drop or PinnedDrop.

> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d6..fd998b68b74f3 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
[ ... ]
> @@ -210,29 +209,6 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi:
>          })
>      }
>  
> -    extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> -        // SAFETY: The serial device bus only ever calls the remove callback with a valid pointer
> -        // to a `struct 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>>() };
> -
> -        // SAFETY: No one has exclusive access to `private_data.driver`.
> -        let data = unsafe { &*private_data.driver.get() };
> -        // SAFETY:
> -        // - `private_data.driver` is pinned.
> -        // - `remove_callback` is only ever called after a successful call to `probe_callback`,
> -        //   hence it's guaranteed that `private_data.driver` was initialized.
> -        let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> -
> -        T::unbind(sdev, data_pinned);
> -    }
> -
>      extern "C" fn receive_buf_callback(
>          sdev: *mut bindings::serdev_device,
>          buf: *const u8,

[Severity: High]
This problem wasn't introduced by this patch, but can receive_buf_callback()
trigger a NULL pointer dereference [1] if T::probe() fails after the
serdev device has been opened?

In probe_callback(), the error-path ScopeGuard calls drvdata_obtain(), which
clears the device's drvdata pointer to NULL before dropping PrivateData and
calling serdev_device_close():

CPU0                                CPU1
probe_callback()
  serdev_device_open()
  T::probe() fails
  ScopeGuard::drop()
    drvdata_obtain() // sets NULL
                                    receive_buf_callback()
                                      drvdata_borrow() <-- NULL deref
    PrivateData::drop()
      serdev_device_close()

rust/kernel/serdev.rs:probe_callback() {
    ...
            let private_data = ScopeGuard::new_with_data(private_data, |_| {
                // SAFETY: We just set drvdata to `PrivateData<'_, T>`.
                drop(unsafe { sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() });
            });
    ...
}

rust/kernel/serdev.rs:receive_buf_callback() {
    ...
        let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
    ...
}

If a serial RX interrupt arrives between drvdata_obtain() and
serdev_device_close(), receive_buf_callback() dereferences the NULL drvdata
pointer.

[1] https://sashiko.dev/#/bug/linux-790dcb6c-6485-4b14-be30-8851e99a0542

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112005455.1224352-1-dakr@kernel.org?part=4

Re: [PATCH 6/7] rust: usb: remove the disconnect() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] rust: usb: missing Sync impl for Interface<Bound> breaks
  Driver::Data
--

commit 92376533106fabfcec7a799d05c1ddbb68a85c37
Author: Danilo Krummrich <dakr@kernel.org>

rust: usb: remove the disconnect() callback

This commit removes the usb::Driver::disconnect() callback and updates
the trait documentation and USB sample driver to perform device teardown
in Drop or PinnedDrop instead.

> diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
> index 993760ff332b8..2126378abc959 100644
> --- a/rust/kernel/usb.rs
> +++ b/rust/kernel/usb.rs

[ ... ]

> @@ -291,6 +271,10 @@ pub trait Driver {
>      type IdInfo: 'static;
>  
>      /// The type of the driver's bus device private data.
> +    ///
> +    /// Device teardown should be performed in [`Drop`] or [`PinnedDrop`](pin_init::PinnedDrop)
> +    /// for this type. The `'bound` lifetime allows the data to retain device resources and an
> +    /// [`Interface<device::Bound>`] reference for use during teardown.
>      type Data<'bound>: Send + 'bound;

[Severity: Medium]
Can a driver store &'bound Interface<device::Bound> in Driver::Data<'bound>
when Interface<device::Bound> does not implement Sync?

Interface<Ctx> wraps Opaque<bindings::usb_interface> and only implements
Sync for the default Interface<device::Normal> parameter (whereas Sync for
device::Bound was implemented on the private usb::Device type instead).
Because Interface<device::Bound> is !Sync, &'bound Interface<device::Bound>
is !Send and fails to satisfy the Data<'bound>: Send bound at compile time.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112005455.1224352-1-dakr@kernel.org?part=6