Re: [PATCH RFC 1/2] rust: usb: add endpoint abstraction

On Mon, Aug 03, 2026 at 03:46:44PM +0300, Alrexandru Radovici wrote:
> On Sun Aug 2, 2026 at 11:39 AM EEST, Greg Kroah-Hartman wrote:
> > On Sat, Aug 01, 2026 at 03:01:07AM +0300, Alexandru Radovici wrote:
> >> Add an abstraction for `struct usb_host_endpoint`, together with the
> >> accessors needed to reach one: `AlternateSetting` wrapping
> >> `struct usb_host_interface`, `Interface::alternate_settings()` and
> >> `Interface::current_alternate_setting()`, and `Device::control_endpoint()`
> >> for the default control endpoint, which no interface descriptor lists.
> >
> > Why?  USB drivers shouldn't be messing with usb_host_endpoint structures
> > for the most part, what user do you have for this?
> 
> The more I think of this, I think you are right. `HostEndpoint`'s accessor
> methods are only used for debug, as the `kernel` crate can access
> the actual `usb_host_endpoint` underneeth. For debug purposes, we should
> just derive the `Debug` trait instead.

Great, if it's even really needed.  Let's see how that works out, as I
don't know what you want to provide for debugging.

> >> `HostEndpoint` is generic over two sealed marker traits,
> >> `EndpointDirection` and `EndpointTransferType`, whose implementors are
> >> 1-ZSTs held in `PhantomData`. An endpoint borrowed from an alternate
> >> setting starts out generic in both; `as_in()`, `as_out()` and
> >> `as_control()` check the descriptor once and return a reference
> >> carrying the corresponding marker, so a function taking
> >> `&HostEndpoint<In, Bulk>` needs no check of its own. The type is
> >> `#[repr(transparent)]` over the C struct and the markers are
> >> zero-sized, so the refinement costs nothing and a slice of endpoints
> >> can be borrowed directly from the C array.
> >> 
> >> Control endpoints get a distinct `Bidirectional` marker rather than an
> >> IN or OUT one. A control transfer takes its direction from bit 7 of the
> >> setup packet's bmRequestType, and USB 2.0 section 9.6.6 defines the
> >> corresponding bit of bEndpointAddress as ignored for control endpoints.
> >> `as_in()` and `as_out()` are not implemented for `Bidirectional`, making
> >> calling them a compile error rather than a misleading result.
> >
> > Don't over-think USB endpoints, they are "just" a pipe that contain a
> > numbering scheme that the USB core uses.  Is that what you are trying to
> > create here?  What are you trying to "enforce" here that the C code does
> > not already do?
> 
> My USB knowledge is limited, so I hope I am not saying something
> stupid here. My understanding is that drivers should not expect
> interfaces to map the same endpoints (numbers) every time.

Why not?  Well, they can, or can not, depending on the device, and the
driver knows this.  For some drivers, a specific endpoint will _ALWAYS_
be a specific number, while for others, they are dynamically determined.
It depends on the device/protocol being used.

> A driver should expect an interface to expose a certain number of
> endpoints, each one with a certain type, but the actual number of each
> exposed endpoint is not to be considered hardcoded. This means that drivers
> should anyway iterate over the endpoints to discover the numbers
> of the required endpoints.

Again, sometimes, but not always.  What a driver SHOULD always do is
verify that the device is providing the specific number and types of
endpoints that it is expecting at probe time and call the core to "find"
the expected endpoints that are present.  In the C api we do that with
the usb_find_common_endpoints() or the usb_check_bulk_endpoints() type
functions.

> My idea is to leaverage Rust's type system to prevent users from supplying
> the wrong endpoint type at compile time rather then at runtime. By making
> the `HostEndpoint` its own Rust type with no public constructor,
> users will be forced to iterate the endpoints to discover the correct
> number for each endpoint that they require. Once they have it, users
> can hold to the reference as long as the interface is valid.

Having a reference is great, but really, these are things that you
should just call the core for and get a reference back.  No need for the
special encoding logic, see how "simple" the C code is for this please.

> By adding the `Dir` and `Type` generic markers, suplying the wrong endpoint
> to a function will be caught at compile time rather than at runtime. This
> should hopefully shorthen the debug work needed for a driver, as some of
> the errors become impossible.

functions should be taking any "type" of endpoint as this will be
checked when the USB core actually submits the data to the device, so no
driver will get very far if all is not correct.  No real need to attempt
to provide many different types and check it all in the api as the api
needs to handle all endpoint types, right?

> As endpoint 0 is always provided and basically _almost hardcoded_``, I added
> the `control_endpoint` function. 

That's great, but again, we "know" what that endpoint type is, and it
will be used for both read and write operations, BUT you need to specify
it somehow which way you want that operation to happen when you make the
API call, right?

I would recommend actually porting/writing a USB driver using the apis
while you are attempting to make these bindings, as I think a lot of
these issues will fall out automatically when doing so.  USB really
isn't that complicated, it's just a dumb and slow "pipe" that for every
message, is triggered by a host request, no matter which way the data is
flowing.

thanks,

greg k-h