Re: [PATCH v32 01/32] usb: host: xhci: Repurpose event handler for skipping interrupter events

["Michał Pecio" <michal.pecio@gmail.com>]

Hi,

> Depending on the interrupter use case, the OS may only be used to
> handle the interrupter event ring clean up.

What do you mean by "cleanup"? Because I see that this patch ends up
acknowledging events to the xHC and I don't know why it would do so?

> In these scenarios, event TRBs don't need to be handled by the OS,
> so introduce an xhci interrupter flag to tag if the events from an
> interrupter needs to be handled or not.

Right, and if the OS isn't handling those events because they are owned
by a coprocessor then it shouldn't be acknowledging them either, which
has the effect that the xHC considers their memory free for reuse.

Also, what happens when Linux goes to sleep and this IRQ stops running?
I expected that the coprocessor itself should be updating the xHC about
its own progress.

Is it a bug? How is this stuff supposed to work?

How are future developers supposed to know how it is supposed to work?
I imagine that few of them will have Qualcomm hardware for testing.


> static int xhci_handle_event_trb(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
> 				 union xhci_trb *event)
> {
> 	u32 trb_type;
> 
> +	/*
> +	 * Some interrupters do not need to handle event TRBs, as they may be
> +	 * managed by another entity, but rely on the OS to clean up.
> +	 */
> +	if (ir->skip_events)
> +		return 0;

This function is only called from one place so the caller could perform
this check and don't waste time calling it.

Regards,
Michalal