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

["Danilo Krummrich" <dakr@kernel.org>]

On Mon Jul 6, 2026 at 11:45 AM CEST, Oliver Neukum wrote:
>
>
> On 03.07.26 05:00, Mike Lothian wrote:
>> 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.

I didn't say we never need interface drivers to deal with the usb device, but
there doesn't seem to be any value in taking this indirection for I/O
primitives.

I.e. there's no value for drivers to write

	let dev = intf.device();
	dev.bulk_recv();

over

	intf.bulk_recv();

With the latter we can enforce that any I/O can only be made for an interface
that is bound to a driver (usb::Interface<Bound>).

AFAIK, it is not necessarily valid to assume that if an interface is bound to an
interface driver, the parent USB device is also bound to a USB device driver
(which is what usb::Device<Bound> represents).

If that is correct, we can't just derive a usb::Device<Bound> from a
usb::Interface<Bound>, and hence can't gate I/O behind the USB device's device
context type state.

IOW, we'd need another layer of indirection if we want to properly gate USB I/O
with the device driver lifecycle.

By providing helpers that operate on the interface directly, this goes away
regardless.

> Hi,
>
> I would say that this is just conceptually wrong.
>
> 1. drivers talk to the common control endpoint of the _device_
> not their interface
> 2. drivers ought to be able to set a configuration (That's a device property)
> 3. Drivers need to be able to claim secondary interfaces (we have an API for that)
> 4. Devices and links (and functions) have states, not interfaces.
>
> These operations operate on the device level. Hiding that fact behind an
> interface (which may not even be accepted at that point) is just a layering
> violation. Even calling a device reset through an interface is strictly speaking
> wrong.
> We even have a driver that can ride piggyback on another driver's interface
> and use only control transfers to endpoint 0.
>
> This patch is fundamentally flawed because it operates on assumptions
> that are just not true. USB does device level operations. Just drop it.

This seems overstated, the only thing the patch does is providing helpers to
avoid the above indirection.

However, I agree that reset_configuration() and set_interface() seem misplaced.

That said, I'm happy with any solution as long as it considers the device driver
lifecycle and gates I/O operations (and other operations that belong in the
bound scope) correspondingly.