[RFC PATCH v2 10/11] rust: usb: keep usb::Device private and gate transfers on Interface<Bound>

[Mike Lothian]

Address the v1 RFC review (Danilo Krummrich):

 - Do not make `usb::Device` public. Writing a `usb::Interface` driver should
   not require naming the underlying `usb::Device` (cf. commit 22d693e45d4a
   ("rust: usb: keep usb::Device private for now")), so the struct is
   private again and the device-wide transfer operations are exposed on
   the interface.

 - Gate the transfer methods (`bulk_send`/`bulk_recv`/`interrupt_recv`/
   `control_send`/`control_recv`/`set_interface`/`reset_configuration`/
   `clear_halt`/`bulk_in_queue`/`bulk_out_queue`) on `Interface<Bound>`, so a
   transfer cannot be issued before the interface's device is bound to the
   driver. Each forwards to the (now private) `Device` helper.

 - Add `Interface::as_bound()` for drivers that keep a refcounted
   `ARef<Interface>` and perform transfers from a deferred context (a work
   item), with a safety contract requiring the work be flushed before unbind.

Signed-off-by: Mike Lothian <mike@xxxxxxxxxxxxxx>
Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
---
 rust/kernel/usb.rs | 162 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 140 insertions(+), 22 deletions(-)

diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 6c49de422b30..1fcaf34433cc 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -368,6 +368,130 @@ pub fn number(&self) -> u8 {
             (*altsetting).desc.bInterfaceNumber
         }
     }
+
+    /// Returns a [`Bound`](device::Bound)-context view of this interface, on which
+    /// the device-wide transfer operations (`bulk_send`, `control_recv`, …) become
+    /// callable.
+    ///
+    /// During `probe()` the interface is already available in a bound context
+    /// ([`Core`](device::Core) dereferences to [`Bound`](device::Bound)), so this
+    /// is only needed by drivers that keep a refcounted [`ARef<Interface>`] and
+    /// perform transfers from a deferred context (e.g. a work item).
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure the interface stays bound to this driver for the
+    /// entire lifetime of the returned reference — for example by cancelling and
+    /// flushing any such deferred work in `disconnect()`. Issuing a transfer on an
+    /// unbound interface is undefined behaviour.
+    ///
+    /// [`ARef<Interface>`]: crate::sync::aref::ARef
+    pub unsafe fn as_bound(&self) -> &Interface<device::Bound> {
+        // SAFETY: `Interface<Ctx>` is `#[repr(transparent)]` over the same
+        // `Opaque<usb_interface>` regardless of the `Ctx` marker, so this cast only
+        // changes the zero-sized context type; the caller upholds boundness.
+        unsafe { &*(self as *const Self as *const Interface<device::Bound>) }
+    }
+}
+
+/// Device-wide USB transfer operations, exposed only on a [`Bound`](device::Bound)
+/// interface so a transfer cannot be issued before the interface's device is bound
+/// to this driver. Each method forwards to the (sleeping) USB core helper for the
+/// interface's [`struct usb_device`]; all must be called from process context.
+impl Interface<device::Bound> {
+    fn device(&self) -> &Device {
+        // SAFETY: `self.as_raw()` is a valid `struct usb_interface` by the type
+        // invariant; `interface_to_usbdev()` returns its valid `struct usb_device`,
+        // and `Device` is `#[repr(transparent)]` over `Opaque<usb_device>`. The
+        // borrow is tied to `&self`, which keeps the interface (and device) alive.
+        let usb_dev = unsafe { bindings::interface_to_usbdev(self.as_raw()) };
+        // SAFETY: `usb_dev` is a valid `struct usb_device` pointer (see above).
+        unsafe { &*(usb_dev.cast::<Device>()) }
+    }
+
+    /// Clears a halt/stall on bulk `endpoint` (both the device-side stall and the
+    /// host-side data toggle). The direction is taken from bit 7 of the endpoint
+    /// address, so it works for IN and OUT endpoints. Sleeps.
+    pub fn clear_halt(&self, endpoint: u8) -> Result {
+        self.device().clear_halt(endpoint)
+    }
+
+    /// Issues a synchronous bulk OUT transfer of `data` to `endpoint`, returning
+    /// the number of bytes transferred. `data` must be DMA-capable. Sleeps.
+    pub fn bulk_send(&self, endpoint: u8, data: &[u8], timeout: Delta) -> Result<usize> {
+        self.device().bulk_send(endpoint, data, timeout)
+    }
+
+    /// Issues a synchronous bulk IN transfer into `data`, returning the number of
+    /// bytes received. `data` must be DMA-capable. Sleeps.
+    pub fn bulk_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> Result<usize> {
+        self.device().bulk_recv(endpoint, data, timeout)
+    }
+
+    /// Issues a synchronous interrupt IN transfer into `data`, returning the number
+    /// of bytes received. `data` must be DMA-capable. Sleeps.
+    pub fn interrupt_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> Result<usize> {
+        self.device().interrupt_recv(endpoint, data, timeout)
+    }
+
+    /// Issues a synchronous control OUT transfer on the default control endpoint.
+    /// The buffer is copied internally, so `data` need not be DMA-capable. Sleeps.
+    pub fn control_send(
+        &self,
+        request: u8,
+        request_type: u8,
+        value: u16,
+        index: u16,
+        data: &[u8],
+        timeout: Delta,
+    ) -> Result {
+        self.device()
+            .control_send(request, request_type, value, index, data, timeout)
+    }
+
+    /// Issues a synchronous control IN transfer on the default control endpoint,
+    /// filling `data` with exactly `data.len()` bytes. Sleeps.
+    pub fn control_recv(
+        &self,
+        request: u8,
+        request_type: u8,
+        value: u16,
+        index: u16,
+        data: &mut [u8],
+        timeout: Delta,
+    ) -> Result {
+        self.device()
+            .control_recv(request, request_type, value, index, data, timeout)
+    }
+
+    /// Selects alternate setting `alternate` of interface `interface`
+    /// (`SET_INTERFACE`). Sleeps.
+    pub fn set_interface(&self, interface: u8, alternate: u8) -> Result {
+        self.device().set_interface(interface, alternate)
+    }
+
+    /// Re-issues `SET_CONFIGURATION` for the current configuration, resetting all
+    /// endpoint data toggles without re-enumerating the device. Sleeps.
+    pub fn reset_configuration(&self) -> Result {
+        self.device().reset_configuration()
+    }
+
+    /// Opens a persistently-queued asynchronous bulk IN reader on `endpoint`
+    /// ([`BulkInQueue`]). Must be called from process context.
+    pub fn bulk_in_queue(&self, endpoint: u8, depth: usize, buf_len: usize) -> Result<BulkInQueue> {
+        self.device().bulk_in_queue(endpoint, depth, buf_len)
+    }
+
+    /// Opens an asynchronous, pipelined bulk OUT writer on `endpoint`
+    /// ([`BulkOutQueue`]). Must be called from process context.
+    pub fn bulk_out_queue(
+        &self,
+        endpoint: u8,
+        depth: usize,
+        buf_len: usize,
+    ) -> Result<BulkOutQueue> {
+        self.device().bulk_out_queue(endpoint, depth, buf_len)
+    }
 }
 
 // SAFETY: `usb::Interface` is a transparent wrapper of `struct usb_interface`.
@@ -392,17 +516,6 @@ fn as_ref(&self) -> &device::Device<Ctx> {
     }
 }
 
-impl<Ctx: device::DeviceContext> AsRef<Device> for Interface<Ctx> {
-    fn as_ref(&self) -> &Device {
-        // SAFETY: `self.as_raw()` is valid by the type invariants.
-        let usb_dev = unsafe { bindings::interface_to_usbdev(self.as_raw()) };
-
-        // SAFETY: For a valid `struct usb_interface` pointer, the above call to
-        // `interface_to_usbdev()` guarantees to return a valid pointer to a `struct usb_device`.
-        unsafe { &*(usb_dev.cast()) }
-    }
-}
-
 // SAFETY: Instances of `Interface` are always reference-counted.
 unsafe impl AlwaysRefCounted for Interface {
     fn inc_ref(&self) {
@@ -431,6 +544,11 @@ unsafe impl Sync for Interface {}
 /// The implementation abstracts the usage of a C [`struct usb_device`] passed in
 /// from the C side.
 ///
+/// It is kept private to this module: a `usb::Interface` driver should never need
+/// to name the underlying `usb::Device`. The device-wide transfer operations are
+/// instead exposed on [`Interface<Bound>`](Interface), which proves the interface
+/// (and thus its device) is bound to this driver for the duration of the call.
+///
 /// # Invariants
 ///
 /// A [`Device`] instance represents a valid [`struct usb_device`] created by the C portion of the
@@ -438,7 +556,7 @@ unsafe impl Sync for Interface {}
 ///
 /// [`struct usb_device`]: https://www.kernel.org/doc/html/latest/driver-api/usb/usb.html#c.usb_device
 #[repr(transparent)]
-pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+struct Device<Ctx: device::DeviceContext = device::Normal>(
     Opaque<bindings::usb_device>,
     PhantomData<Ctx>,
 );
@@ -454,7 +572,7 @@ fn as_raw(&self) -> *mut bindings::usb_device {
     /// address (`USB_DIR_IN`), so this works for both bulk IN and bulk OUT endpoints.
     /// Must be called from process context (it sleeps). Needed e.g. when another
     /// driver left a streaming endpoint stalled.
-    pub fn clear_halt(&self, endpoint: u8) -> Result {
+    pub(crate) fn clear_halt(&self, endpoint: u8) -> Result {
         // `usb_clear_halt()` must be given a pipe whose direction matches the endpoint:
         // it issues CLEAR_FEATURE to that endpoint address and resets the matching data
         // toggle. Build an IN or OUT pipe from the `USB_DIR_IN` bit of the address;
@@ -491,7 +609,7 @@ pub fn clear_halt(&self, endpoint: u8) -> Result {
     /// not arrange DMA-capable storage themselves.
     ///
     /// [`usb_bulk_msg()`]: https://docs.kernel.org/driver-api/usb/usb.html#c.usb_bulk_msg
-    pub fn bulk_send(&self, endpoint: u8, data: &[u8], timeout: Delta) -> Result<usize> {
+    pub(crate) fn bulk_send(&self, endpoint: u8, data: &[u8], timeout: Delta) -> Result<usize> {
         let mut actual: kernel::ffi::c_int = 0;
 
         // `usb_bulk_msg()` requires a DMA-capable buffer; `data` may live on the
@@ -537,7 +655,7 @@ pub fn bulk_send(&self, endpoint: u8, data: &[u8], timeout: Delta) -> Result<usi
     /// `data` may be any slice.
     ///
     /// [`usb_bulk_msg()`]: https://docs.kernel.org/driver-api/usb/usb.html#c.usb_bulk_msg
-    pub fn bulk_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> Result<usize> {
+    pub(crate) fn bulk_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> Result<usize> {
         let mut actual: kernel::ffi::c_int = 0;
 
         // `usb_bulk_msg()` requires a DMA-capable buffer; receive into a kmalloc'd
@@ -579,7 +697,7 @@ pub fn bulk_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> Result
     /// milliseconds; a [`Delta`] of zero, or any non-zero value below 1 ms, waits indefinitely.
     ///
     /// [`bulk_recv`]: Self::bulk_recv
-    pub fn interrupt_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> Result<usize> {
+    pub(crate) fn interrupt_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> Result<usize> {
         let mut actual: kernel::ffi::c_int = 0;
 
         // `usb_interrupt_msg()` requires a DMA-capable buffer; receive into a kmalloc'd
@@ -623,7 +741,7 @@ pub fn interrupt_recv(&self, endpoint: u8, data: &mut [u8], timeout: Delta) -> R
     /// non-zero value below 1 ms — waits indefinitely.
     ///
     /// [`usb_control_msg_send()`]: https://docs.kernel.org/driver-api/usb/usb.html#c.usb_control_msg_send
-    pub fn control_send(
+    pub(crate) fn control_send(
         &self,
         request: u8,
         request_type: u8,
@@ -658,7 +776,7 @@ pub fn control_send(
     /// rules are as for [`Device::control_send`].
     ///
     /// [`usb_control_msg_recv()`]: https://docs.kernel.org/driver-api/usb/usb.html#c.usb_control_msg_recv
-    pub fn control_recv(
+    pub(crate) fn control_recv(
         &self,
         request: u8,
         request_type: u8,
@@ -691,7 +809,7 @@ pub fn control_recv(
     /// a blocking, sleeping call and must only be invoked from process context.
     ///
     /// [`usb_set_interface()`]: https://docs.kernel.org/driver-api/usb/usb.html#c.usb_set_interface
-    pub fn set_interface(&self, interface: u8, alternate: u8) -> Result {
+    pub(crate) fn set_interface(&self, interface: u8, alternate: u8) -> Result {
         // SAFETY: `self.as_raw()` is a valid `struct usb_device` by the type invariant.
         to_result(unsafe {
             bindings::usb_set_interface(self.as_raw(), interface.into(), alternate.into())
@@ -708,7 +826,7 @@ pub fn set_interface(&self, interface: u8, alternate: u8) -> Result {
     /// drivers that are mid-transfer.
     ///
     /// [`usb_reset_configuration()`]: https://docs.kernel.org/driver-api/usb/usb.html#c.usb_reset_configuration
-    pub fn reset_configuration(&self) -> Result {
+    pub(crate) fn reset_configuration(&self) -> Result {
         // SAFETY: `self.as_raw()` is a valid `struct usb_device` by the type invariant;
         // `usb_reset_configuration()` only re-issues SET_CONFIGURATION and updates the
         // host-side endpoint state for this device.
@@ -734,7 +852,7 @@ pub fn reset_configuration(&self) -> Result {
     ///
     /// [`bulk_recv`]: Self::bulk_recv
     /// [`BulkInQueue::recv`]: BulkInQueue::recv
-    pub fn bulk_in_queue(&self, endpoint: u8, depth: usize, buf_len: usize) -> Result<BulkInQueue> {
+    pub(crate) fn bulk_in_queue(&self, endpoint: u8, depth: usize, buf_len: usize) -> Result<BulkInQueue> {
         let dev = self.as_raw();
 
         // SAFETY: `dev` is a valid `struct usb_device` by the type invariant; take a
@@ -818,7 +936,7 @@ pub fn bulk_in_queue(&self, endpoint: u8, depth: usize, buf_len: usize) -> Resul
     /// [`bulk_in_queue`]: Self::bulk_in_queue
     /// [`bulk_send`]: Self::bulk_send
     /// [`send`]: BulkOutQueue::send
-    pub fn bulk_out_queue(
+    pub(crate) fn bulk_out_queue(
         &self,
         endpoint: u8,
         depth: usize,
-- 
2.55.0