mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Nicolai Buchwitz <nb@tipi-net.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Steve Bayless <steve_bayless@keysight.com>,
	Guido Kiener <guido@kiener-muenchen.de>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, Mathias Nyman <mathias.nyman@intel.com>
Subject: Re: [PATCH 0/4] usb: usbtmc: fix undersized high-speed bulk packets on xHCI
Date: Wed, 21 Oct 2026 00:56:54 +0200	[thread overview]
Message-ID: <20261021005654.23517441.michal.pecio@gmail.com> (raw)
In-Reply-To: <a642dbc2e4ba0efc381c69e26821762a@tipi-net.de>

On Tue, 20 Oct 2026 20:33:17 +0200, Nicolai Buchwitz wrote:
> On 20.10.2026 18:42, Michal Pecio wrote:
> > But SDM4065A is apparently pretty modern (2025) gear marketed for
> > "automatic production lines", so it's odd that it has such issues.
> > Has anyone looked if the same out-of-spec descriptor shows up on
> > Windows or (don't laught at my optimism) consulted vendor support?  
> 
> Not yet, but I'll check on Windows. The vendor(s) don't really seem to
> care about the bug (surprise!). It is known for quite a while and even
> not fixed with the latest 2026 firmware for the siglent. This makes me
> assume that either most of the professional users a) use Windows or b)
> use the lan interface (which we do for most of the stations).

It's worth trying, because sporadically we see devices which enumerate
differently on Windows and on Linux for some stupid reason. Although I
have a feeling that this won't be one of those times.

> > Maybe Windows xHCI driver tries to enable out-of-spec maxpacket?
> > We could do that too, though IIRC not all controllers are happy,
> > so a SW workaround may be unavoidable.  
> 
> Did some quick tests and it looks good so far. If the usb maintainers
> can agree on making it less restrictive (/ compliant) it would save
> lots of efforts and the quirk would be down to the ZLP patch. In my
> tests all three devices worked with
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 8a8636279e844..c4306482874eb 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1505,7 +1505,9 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
>                  err_count = 3;
>          /* HS bulk max packet should be 512, FS bulk supports 8, 16, 32 or 64 */
>          if (usb_endpoint_xfer_bulk(&ep->desc)) {
> -               if (udev->speed == USB_SPEED_HIGH)
> +               if (udev->speed == USB_SPEED_HIGH && max_packet != 512)
> +                       xhci_warn(xhci, "EXPERIMENT: keeping HS bulk maxp %u\n", max_packet);
> +               else if (udev->speed == USB_SPEED_HIGH)
>                          max_packet = 512;
>                  if (udev->speed == USB_SPEED_FULL) {
>                          max_packet = rounddown_pow_of_two(max_packet);
> 
> So this looks like the better fix. How would we want it gated, a USB
> core quirk flag per device, or per controller?

[Cc Mathias]

Good question. Do you need this fixed in stable?

Depending on regression risk maybe we could get away with no quirk
flag at all. That's what ehci and dwc2 seem to do, which means that
hopefully no device specifies illegal packet size while using 512.

Other possible risk is some device doesn't truly care (no transfer
is ever large enough to matter) and works with 512, but setting its
requested packet size (needlessly) breaks some xHCI controller.

Such devices seem not very likely to exist, though more likely than
the former kind, so I will try to test whether such xHCI breakage is
possible. Preliminary results are encouraging: although several of
my HCs reject packet size of 1024 or corrupt data, 64 is accepted by
all but one. I haven't checked for corruption yet.

I will also see if I can figure out what Windows does in such case,
because doing the same thing as Windows may be the safest bet.

> >> The Rigols still have one issue I can't work around. They split
> >> replies over 500 bytes into several transfers, and a follow-on
> >> transfer that ends exactly on a packet boundary never arrives. Raw
> >> libusb shows the same, so I guess it's another firmware bug. Should
> >> be fine for most use cases though.  
> > 
> > What exactly is the sequence of URBs you see vs would want to see?  
> 
> The Rigols split anything over 500 bytes into two transfers:
> 
>    REQUEST_DEV_DEP_MSG_IN #1
>    IN: 8 x 64 bytes (12 byte header + 500 data), then a ZLP
>    REQUEST_DEV_DEP_MSG_IN #2
>    IN: header + the rest
> 
> That works fine, unless the second transfer happens to end exactly on
> a 64 byte boundary (e.g. 640 bytes). Then the device just never
> answers request #2 and the first IN times out. I also tried leaving
> the ZLP from #1 pending until after request #2. That fixes the
> boundary case, but then the device randomly ignores request #2
> instead (roughly 1 in 5). Raw libusb behaves the same, also with
> maxpacket 64 in xHCI, so I'm pretty sure it's just another firmware
> bug. What I'd want to see is simply the second transfer arriving :)

Doesn't look like any obvious xHCI problem to me. Did it also happen
with those USB2 controllers that you used previously?

Regards,
Michal

      parent reply	other threads:[~2026-10-20 22:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 14:33 Nicolai Buchwitz
2026-10-20 14:33 ` [PATCH 1/4] usb: usbtmc: handle undersized high-speed bulk IN packets Nicolai Buchwitz
2026-10-20 14:33 ` [PATCH 2/4] usb: usbtmc: handle undersized high-speed bulk OUT packets Nicolai Buchwitz
2026-10-20 14:33 ` [PATCH 3/4] usb: usbtmc: add Rigol DP800 and DS1000Z to the packet I/O quirk table Nicolai Buchwitz
2026-10-20 14:33 ` [PATCH 4/4] usb: usbtmc: send a ZLP after a full last packet in packet I/O mode Nicolai Buchwitz
2026-10-20 16:42 ` [PATCH 0/4] usb: usbtmc: fix undersized high-speed bulk packets on xHCI Michal Pecio
2026-10-20 18:33   ` Nicolai Buchwitz
2026-10-20 21:12     ` guido
2026-10-20 22:56     ` Michal Pecio [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261021005654.23517441.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=guido@kiener-muenchen.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=nb@tipi-net.de \
    --cc=stable@vger.kernel.org \
    --cc=steve_bayless@keysight.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®