Re: [PATCH v3 1/5] rust: usb: add revocable typed interface I/O

[Mike Lothian]

On Wed, 26 Aug 2026, Danilo Krummrich wrote:
> > + /// Asks the driver core to unbind whatever driver is currently bound to this interface.
>
> What is this needed for? Where do you use it?

Nothing in this posting uses it. It's a leftover from something I
removed, and I'm not sure it should have stayed removed.

It backed a write-only sysfs file, /sys/devices/vino/remove_all, which
walked the driver's list of held interfaces and called
device_release_driver() on each. evdi has had the same file since 2015
as /sys/devices/evdi/remove_all, so it's a name people with DisplayLink
hardware already know.

I added it because unloading the module used to be unsafe. disconnect()
could deadlock against usb_hub_wq, DRM references were held past unbind
so every replug leaked a card minor, and one unload took the machine
down. Having one write that unbound everything from process context,
with each disconnect running somewhere I could watch it, made those a
lot easier to find.

They're all fixed now, modprobe -r is reliable, and the sysfs file got
cut from the series along with the rest of the development scaffolding.
The binding method survived the cut without its caller.

I'll drop it for v4 either way. The bit I'd like your opinion on is
whether it should come back later with the user attached. Against:
/sys/bus/usb/drivers/vino/unbind already does this, and all remove_all
saves you is knowing which interfaces the driver kept. For: a dock is
the kind of device where someone wants to hand the hardware back without
knowing that, and evdi has shipped the file for ten years.

> > +/// A revocable window during which USB I/O is permitted on an interface.
>
> How is this different or narrower than the device's Bound type state
> represents?

There's one way it's narrower, and that's why I wrote it. The USB core
forbids I/O between suspend() and resume(), and between pre_reset()
and post_reset(), and the interface stays bound across all four. So a
driver holding proof of Bound can be holding it when a transfer isn't
legal. Bound means the driver owns the interface; the window means the
core will currently carry a transfer.

> Also, this seems to reinvent Devres, which we superseded with Rust
> native lifetimes and higher-ranked types. Please use that instead.

The open/closed flag and the wait-for-quiescence are hand-rolled, and
they shouldn't be.

What I don't know is whether the lifetime and higher-ranked machinery
can express a window that closes and then reopens, several times, within
one bind. That's what suspend/resume and pre_reset/post_reset need, and
revocation as I understand it is one-way.

If it can, I'll respin on top of it. If it can't, then I think this
guarantee is USB-specific and belongs behind a USB type rather than a
general one. I'd rather know which before I rewrite it.

Thanks,
Mike


On Wed, 26 Aug 2026 at 19:59, Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>
> On Wed Aug 26, 2026 at 6:30 PM CEST, Mike Lothian wrote:
> > +    /// Asks the driver core to unbind whatever driver is currently bound to this interface.
>
> What is this needed for? Where do you use it?
>
> > +    /// This is the narrow, reviewed replacement for handing out a raw `struct device` pointer: it
> > +    /// performs exactly one operation (`device_release_driver()`) on this interface's own device,
> > +    /// and cannot be used to reach the device-wide state of a composite peer.
> > +    ///
> > +    /// It is intended for a driver-provided "release my devices" control (e.g. a sysfs attribute),
> > +    /// and must not be called from the driver's own `probe()` or `disconnect()` callback: the
> > +    /// driver core already holds the device lock across those.
>
> IOW, it must not be available for Interface<Core>, which due to the deref chain
> is not that trivial to model. So, if this is really needed I think this needs a
> an abstraction where you get a different device newtype from the scope where
> this *should* be called from that allows you to do this and can never leave the
> scope.
>
> > +    pub fn release_driver(&self) {
> > +        // SAFETY: `self.as_raw()` is a valid `struct usb_interface` by the type invariant, so the
> > +        // address of its embedded `dev` is a valid `struct device`. `device_release_driver()`
> > +        // takes the device lock itself and tolerates a device with no driver bound.
> > +        unsafe { bindings::device_release_driver(&raw mut (*self.as_raw()).dev) };
> > +    }
>
> [...]
>
> > +/// A revocable window during which USB I/O is permitted on an interface.
> > +///
> > +/// A driver-`Bound` interface is *not* on its own proof that a transfer may be issued: the USB
> > +/// core forbids I/O outside the window that opens after a successful `probe()`/resume/reset-resume
> > +/// and must be closed again before `disconnect()`, `suspend()` or `pre_reset()` returns. This type
> > +/// represents exactly that narrower state.
>
> How is this different or narrower than the device's Bound type state represents?
> Also, this seems to reinvent Devres, which we superseded with Rust native
> lifetimes and higher-ranked types. Please use that instead.