gateware.usb.usb2.request: add claim to RequestHandlerInterface

[mndza]

This commit adds a handled() method to USBRequestHandler. This method takes a SetupPacket as an argument and returns an Amaranth boolean indicating whether a request has been taken care of.

These signals from the various request handlers are subsequently combined to trigger a stall for unhandled cases.

[martinling]

Hi @mndza,

@miek and I don't think this one depends on the other PRs mentioned. Do you see a reason for this PR to wait for the rest?

@zyp how do you feel about this change from the point of view of OrbTrace, or other downstream users?

[zyp]

From an Orbtrace perspective, this would be a welcome improvement. We've got interface handlers that'll stall nonexistent requests on the interfaces they handle, but nothing that'll stall requests that's not directed to any of the handled interfaces.

[mndza]

Hi @mndza,

@miek and I don't think this one depends on the other PRs mentioned. Do you see a reason for this PR to wait for the rest?

I agree that this PR should be independent. I'm removing any references to the other PRs. Let me know if there are any other concerns or adjustments needed.

[martinling]

I'm pretty happy with this as it stands, and it's a good improvement over the status quo, but I do see one issue:

With this change, the logic for which requests should be handled ends up appearing twice in each request handler: once in handled, and again in elaborate.

This seems like a bit of a recipe for trouble, because it would be easy for the two to get out of sync, such that a handler implementation is lying about which requests it does and doesn't handle. That could lead to some quite hard-to-find bugs.

Given that this is already going to be a breaking change to downstream users of USBRequestHandler, I wonder if we should take the opportunity to change the API a little bit further, such that a request handler won't see a request at all unless its handled precondition is met. That would require some further changes, but could make handlers simpler and less error-prone to write.

[zyp]

It'd be nice to have a request dispatcher that you could just register request handlers with, automatically generating both the handled logic and the dispatch logic in elaborate.

[mndza]

I've just pushed a different approach for this PR.

Instead of a handled method, there's a new claim signal in RequestHandlerInterface.

When a USBRequestHandler wants to manage an incoming request, it must assert this signal. This lets the handler drive the rest of interface outputs.

It makes the logic in USBRequestHandlerMultiplexer simpler, and introduces a fallback mechanism for unhandled requests: when no handler claims a request, or more than 1 does, the multiplexer selects this fallback.

[martinling]

I'm happy with this, and it's being used successfully on the update-dependencies branch of the Cynthion factory test.