From: "Michał Pecio" <michal.pecio@gmail.com>
To: mathias.nyman@linux.intel.com
Cc: gregkh@linuxfoundation.org, ki.chiang65@gmail.com,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
mathias.nyman@intel.com, stable@vger.kernel.org
Subject: Re: [PATCH v4 1/1] xhci: Correctly handle last TRB of isoc TD on Etron xHCI host
Date: Wed, 5 Feb 2025 23:42:05 +0100 [thread overview]
Message-ID: <20250205234205.73ca4ff8@foxbook> (raw)
In-Reply-To: <c746c10a-d504-48bc-bc8d-ba65230d13f6@linux.intel.com>
> Not giving back the TD when we get an event for the last TRB in the
> TD sounds risky. With this change we assume all old and future ETRON
> hosts will trigger this additional spurious success event.
error_mid_td can cope with hosts which don't produce the extra success
event, it was done this way to deal with buggy NECs. The cost is one
more ESIT of latency on TDs with error.
I don't know how many Etron chips Kuangyi Chiang tested, but I have
one here and it definitely has this double event bug.
These are old chips, not sure if Etron is still making them or if
anyone would want to buy (large chip, USB 3.0 only, barely functional
streams, no UAS on Linux and on Windows with stock MS drivers).
> I think we could handle this more like the XHCI_SPURIOUS_SUCCESS case
> seen with short transfers, and just silence the error message.
That's a little dodgy because it frees the TD before the HC is
completely done with it. *Probably* no problem with data buffers
(no sensible reason to DMA into them after an earlier error), but
we could overwrite the transfer ring in rare cases and IDK if it
would or wouldn't cause problems in this particular case.
Same applies to the "short packet" case existing today. I thought
about fixing it, but IIRC I ran into some differences between HCs
or out of spec behavior and it got tricky.
Maybe it would make sense to separate giveback (and freeing of the
data buffer by class drivers) from transfer ring inc_deq(). Do the
former when we reasonably believe the HC won't touch the buffers
anymore, do the latter when we are sure that it's in the next TD.
Not ideal, but easier and better than the status quo.
Regards,
Michal
next prev parent reply other threads:[~2025-02-05 22:42 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-05 5:37 [PATCH v4 0/1] xhci: Some improvement for Etron xHCI host Kuangyi Chiang
2025-02-05 5:37 ` [PATCH v4 1/1] xhci: Correctly handle last TRB of isoc TD on " Kuangyi Chiang
2025-02-05 14:17 ` Mathias Nyman
2025-02-05 15:17 ` Mathias Nyman
2025-02-07 1:38 ` Kuangyi Chiang
2025-02-10 6:09 ` Kuangyi Chiang
2025-02-05 22:42 ` Michał Pecio [this message]
2025-02-07 12:06 ` Mathias Nyman
2025-02-10 8:57 ` Michał Pecio
2025-02-11 12:36 ` [PATCH] usb: xhci: Handle quirky SuperSpeed isoc error reporting by Etron HCs Michal Pecio
2025-02-12 5:59 ` Kuangyi Chiang
2025-02-12 8:12 ` Michał Pecio
2025-02-28 16:13 ` Mathias Nyman
2025-02-28 16:18 ` [RFT PATCH] xhci: Handle spurious events on Etron host isoc enpoints Mathias Nyman
2025-02-28 19:57 ` kernel test robot
2025-02-28 20:07 ` kernel test robot
2025-03-01 2:05 ` Kuangyi Chiang
2025-03-03 3:29 ` Kuangyi Chiang
2025-03-03 8:28 ` Mathias Nyman
2025-03-03 10:34 ` Michał Pecio
2025-03-03 15:08 ` Mathias Nyman
2025-03-06 8:50 ` Michał Pecio
2025-02-28 17:11 ` [PATCH] usb: xhci: Handle quirky SuperSpeed isoc error reporting by Etron HCs Michał Pecio
2025-02-28 17:14 ` Michał Pecio
2025-02-07 1:28 ` [PATCH v4 1/1] xhci: Correctly handle last TRB of isoc TD on Etron xHCI host Kuangyi Chiang
2025-02-05 21:45 ` Michał Pecio
2025-02-07 6:59 ` Kuangyi Chiang
2025-02-07 9:51 ` Michał Pecio
2025-02-10 6:18 ` Kuangyi Chiang
2025-03-17 10:12 ` [PATCH v4 0/1] xhci: Some improvement for " Michał Pecio
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=20250205234205.73ca4ff8@foxbook \
--to=michal.pecio@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=ki.chiang65@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=mathias.nyman@linux.intel.com \
--cc=stable@vger.kernel.org \
/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