Re: [PATCH 2/2] usb: xhci: Warn about suspected "start-stop" bugs in HCs

Greg KH
2024-10-15T10:40:15Z

On Mon, Oct 14, 2024 at 09:11:22PM +0200, Michal Pecio wrote:
> NEC controllers have a bug, where stopping an endpoint soon after it
> has been restarted doesn't quite work as expected. This forces us to
> track whether each Stop Endpoint command is expected to fail or not.
>
> Reuse this infrastracture to warn about similar bugs on other chips,
> if any are found.
>
> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
> ---
> drivers/usb/host/xhci-ring.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index c0efb4d34ab9..c326b86d713c 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1186,8 +1186,11 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
> * So keep retrying until the command clearly succeeds.
> * Not clear what to do if other HCs have similar bugs.
> */
> - if (!(xhci->quirks & XHCI_NEC_HOST))
> + if (!(xhci->quirks & XHCI_NEC_HOST)) {
> + xhci_warn(xhci, "Unhandled Stop Endpoint failure on slot %d ep_index %d\n",
> + slot_id, ep_index);

If a user sees this, what are they supposed to do? This is a hardware
bug, but with this we are going to get reports of "something broke in
the kernel", right? Why not make it just more informative, like:
xhci_info(xhci, "hardware can not deal with...

or something like that so that people know we know about the bug, and
are working around it, but that it's not our issue, but rather the
hardware that is at fault?

thanks,

greg k-h