* [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
@ 2026-05-18 6:42 Brent Page
2026-05-18 18:24 ` Brent Page
2026-05-22 14:18 ` Alan Stern
0 siblings, 2 replies; 11+ messages in thread
From: Brent Page @ 2026-05-18 6:42 UTC (permalink / raw)
To: linux-usb; +Cc: Alan Stern, Michal Pecio
This is a follow-up on
https://lore.kernel.org/linux-usb/
B66AE752-B09C-49B3-A829-F7ABB36FB250@gmail.com/T/#u.
The goal is to accommodate 1023-byte-endpoint full-speed isochronous-in
transactions on a USB2 bus.
One of the main changes is
- max_tt_usecs[] = { 125, 125, 125, 125, 125, 125, 30, 0 };
+ max_tt_usecs[] = { 145, 145, 145, 145, 145, 145, 35, 0 };
To repeat for documentation's sake, "145 us is longer than a microframe, so
this best-case budget will often not reflect the times when transactions occur.
But, this situation is consistent with the best-case budget in section 11.18.1
of the USB-2 spec, in which 1157 data-bytes are scheduled to occur as though no
bit-stuffing is necessary (i.e., the 188 bytes = 12 megabits/second * (1 byte/8
bits)* 0.125 ms that can run on the full-speed bus in a microframe are all
taken taken be data-bytes). So, after the patch, max_tt_usecs is the same as
the spec's best-case budget but is just scaled by (125 us / 188 bytes) = 1/(12
megabits/s) and by the 7/6 bit-stuffing multiplier to reflect the fact that the
scheduling code works in bit-stuffing-inclusive bus-time instead of
data-bytes." However, as pointed out in the linked email chain, because
ehci-sched.c currently doesn't allow frame-hopping CSPLITS (a CSPLIT for a
given transaction that gets executed in the H-frame following the H-frame in
which the transaction was initiated), "it doesn't support 1023 byte packets at
all. 1023/188=5.4 and if worst case bit stuffing factor is 7/6 then up to 6.3
uframes of transfer time. Completion in [HuFrame 6 or 7] and CSPLIT required in
[HuFrame 0 of the next frame]." So, this patch also adds support for
frame-hopping CSPLITS. Per EHCI-1.0 4.12.3.3.2.1, the sitd containing such
CSPLITS must have a backpointer to the sitd of the previous frame.
The main cases to handle in the backpointer code are period==(1 frame) and
period!=(1 frame), where period is specifically derived from the bInterval of
the endpoint.
Say there's a 3-packet urb for the endpoint (urb0, with labels td0_#) in the
pipeline and another gets added (urb1, with labels td1_#). The result is (the
nodes below are sitds, and "td" stands for "transaction descriptor") ...
if period==(1 frame):
|-----| |-----| |-----| |-----| |-----| |-----|
|td0_0|-->|td0_1|-->|td0_2|-->|td1_0|-->|td1_1|-->|td1_2|
|-----|< |-----|< |-----|< |-----|< |-----|< |-----|
| | | | | | | | | |
|----| |---| |---| |---| |---|
where time increases from left to right, and the lines/arrows on the bottom
represent backpointer relationships. Also, all sitds are initialized in the Do
Complete Split state except for the very first in a stream.
Including a backpointer from the first sitd of urb1 to the last sitd of urb0
(as done here) is not certainly the 'correct' way to handle urb boundaries. An
alternative strategy would be to add an additional sitd, td0_3, to urb0 after
td0_2 that just contains CSPLITS for td0_2. However, td0_3 would have to be in
the same frame as td1_0 (the first sitd of urb1). It's possible that the
HC supports having two sitds for one endpoint in one frame, but it
seems unusual enough that it could produce issues, hence why I avoid this
alternative strategy. Also, the adopted strategy is simpler to code – the only
distinctive sitd is the very first in a stream.
if period!=(1 frame):
|-----| |-----| |-----| |-----| |-----| |-----|
|td0_0|-->|td0_1|-->|td0_2|-->|td0_3|-->|td0_4|-->|td0_5|-->
|-----|< |-----| |-----|< |-----| |-----|< |-----| cont.
| | | | | | below
|----| |----| |----|
|-----| |-----| |-----| |-----| |-----| |-----|
-->|td1_0|-->|td1_1|-->|td1_2|-->|td1_3|-->|td1_4|-->|td1_5|
|-----|< |-----| |-----|< |-----| |-----|< |-----|
| | | | | |
|----| |----| |----|
where the second sitd in each pair is positioned one frame after the first sitd
of the pair, gets initialized in the Do Complete Split state, contains CSPLITS
for the transfer that was started in the first sitd of the pair, and has no
SSPLITS.
I also want to make sure I understand something that line ~2197 of the patched
ehci-sched.c:
sitd_before = list_last_entry(&sched->td_list, struct ehci_sitd, sitd_list);
is based on. Namely, in the current code, am I correct that
insertion of a new urb's sitds into the endpoint's td_list looks like:
td_list initially (say it's loaded up with a 3-packet urb)
|-----| |-----| |-----|
|td0_0|-->|td0_1|-->|td0_2|
|-----| |-----| |-----|
line 2090 in the genuine kernel code:
(executed three times for a new 3-packet urb)
list_add(&sitd->sitd_list, &iso_sched->td_list);
|-----| |-----| |-----| |-----| |-----| |-----|
|td0_0|-->| |-->| |-->| |-->|td0_1|-->|td0_2|
|-----| |-----| |-----| |-----| |-----| |-----|
?
The following later lines in stid_link_urb (again genuine kernel code) then put
these new sitds in their proper place I think:
2177 sitd = list_entry(iso_sched->td_list.next,
2178 struct ehci_itd, sitd_list);
2179 list_move_tail(&sitd->sitd_list, &stream->td_list);
Lastly, I'm still a bit confused by this comment in ehci-sched.c:
/* special case for isoc transfers larger than 125us:
* the first and each subsequent fully used uframe
* must be empty, so as to not illegally delay
* already scheduled transactions
*/
To me, the main issue is that the adopted "carryover" approach to budgeting
doesn't take into account the fact that the TT processes the transactions
sequentially (at least according to all the footprints shown in the figs. in
EHCI1 and USB2) and doesn't, e.g., do part of transaction 1, switch to
transaction 2, then go back to transaction 1. This most obviously is
problematic for long transactions, hence ehci-sched.c handling this case
separately.
I make a point of bringing this up because, given the inflation of the
max_tt_usecs values in the patch, it is actually the case that a newly budgeted
endpoint may "delay already scheduled transactions". For example, say there's a
transaction budgeted for HuFrame 1 with tt_usecs=2 us and a new device gets
plugged in that requires up to tt_usecs=140 us per transaction. The new device
will also get budgeted for HuFrame 1. Then, the device's transactions that
actually do take >125 us (due to unfavorable bit stuffing requirements) will
delay the 2 us transaction (it's maybe worth pointing out that this relies on
the fact that sitd_link in ehci-sched.c puts the newer sitds first in the
periodic queue for each uframe). I don't think there's anything "illegal"
about this though.
---
drivers/usb/host/ehci-sched.c | 132 ++++++++++++++++++++++++++--------
drivers/usb/host/ehci.h | 5 +-
2 files changed, 106 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index a241337c9..7c2f2125d 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -285,13 +285,13 @@ static void compute_tt_budget(u8 budget_table[EHCI_BANDWIDTH_SIZE],
for (uf = ps->phase_uf; uf < 8; ++uf) {
x += budget_line[uf];
- /* Each microframe lasts 125 us */
- if (x <= 125) {
+ /* Cumulative tt_usecs can be as large as 145 us */
+ if (x <= 145) {
budget_line[uf] = x;
break;
}
- budget_line[uf] = 125;
- x -= 125;
+ budget_line[uf] = 145;
+ x -= 145;
}
}
}
@@ -313,7 +313,13 @@ static int __maybe_unused same_tt(struct usb_device *dev1,
#ifdef CONFIG_USB_EHCI_TT_NEWSCHED
static const unsigned char
-max_tt_usecs[] = { 125, 125, 125, 125, 125, 125, 30, 0 };
+/*
+ * tt_usecs includes worst-case bit stuffing time, so the transactions
+ * (complete and/or partial) budgeted for a given 125 us uframe can have a
+ * cumulative tt_usecs as large as 145 us and still possibly fit into the
+ * uframe.
+*/
+max_tt_usecs[] = { 145, 145, 145, 145, 145, 145, 35, 0 };
/* carryover low/fullspeed bandwidth that crosses uframe boundries */
static inline void carryover_tt_bandwidth(unsigned short tt_usecs[8])
@@ -378,13 +384,13 @@ static int tt_available(
if (max_tt_usecs[uframe] <= tt_usecs[uframe])
return 0;
- /* special case for isoc transfers larger than 125us:
+ /* special case for isoc transfers larger than max_tt_usecs:
* the first and each subsequent fully used uframe
* must be empty, so as to not illegally delay
* already scheduled transactions
*/
- if (usecs > 125) {
- int ufs = (usecs / 125);
+ if (usecs > 145) {
+ int ufs = (usecs / 145);
for (i = uframe; i < (uframe + ufs) && i < 8; i++)
if (tt_usecs[i] > 0)
@@ -1388,19 +1394,40 @@ sitd_slot_ok(
struct ehci_tt *tt
)
{
- unsigned mask, tmp;
+ unsigned mask, c_mask2, tmp;
unsigned frame, uf;
mask = stream->ps.cs_mask << (uframe & 7);
+ c_mask2 = mask >> 16;
+ mask = mask & 0xffff;
+ if((c_mask2 & 1) && (c_mask2 & 1<<1) && (c_mask2 & 1<<2)) {
+ /* if BuFrame6 is the last uframe in which a transaction is budgeted,
+ * the transaction will initially be configured to have CSPLITS in
+ * BuFrame7 as well as BuFrames 0 and 1 of the following frame
+ * (HuFrames 0,1,2) */
+ /* below: usb2 spec 11.18.4.3.c paragraph 2 */
+ if(mask & 1) {
+ c_mask2 = 1;
+ } else {
+ c_mask2 = (1<<2)-1;
+ }
+ } else if ((c_mask2 & 1) && (c_mask2 & 1<<1)) {
+ /* if BuFrame5 is the last uframe in which a transaction is budgeted,
+ * the transaction will initially be configured to have CSPLITS in
+ * BuFrames 6 and 7 as well as BuFrame 0 of the following frame
+ * (HuFrames 7,0,1) */
+ /* below: usb2 spec 11.18.4.3.c paragraph 1 */
+ if(mask & 1) {
+ c_mask2 = 1;
+ }
+ }
+ if((c_mask2 & mask & 1<<1))
+ return 0; /* ehci1 4.12.3.1 */
/* for OUT, don't wrap SSPLIT into H-microframe 7 */
if (((stream->ps.cs_mask & 0xff) << (uframe & 7)) >= (1 << 7))
return 0;
- /* for IN, don't wrap CSPLIT into the next frame */
- if (mask & ~0xffff)
- return 0;
-
/* check bandwidth */
uframe &= stream->ps.bw_uperiod - 1;
frame = uframe >> 3;
@@ -1450,8 +1477,10 @@ sitd_slot_ok(
uframe += stream->ps.bw_uperiod;
} while (uframe < EHCI_BANDWIDTH_SIZE);
- stream->ps.cs_mask <<= uframe & 7;
+ stream->ps.cs_mask = mask;
+ stream->ps.c_mask2 = c_mask2;
stream->splits = cpu_to_hc32(ehci, stream->ps.cs_mask);
+ stream->c_splits2 = cpu_to_hc32(ehci, stream->ps.c_mask2);
return 1;
}
@@ -2049,13 +2078,13 @@ sitd_urb_transaction(
/* allocate/init sITDs */
spin_lock_irqsave(&ehci->lock, flags);
- for (i = 0; i < urb->number_of_packets; i++) {
-
- /* NOTE: for now, we don't try to handle wraparound cases
- * for IN (using sitd->hw_backpointer, like a FSTN), which
- * means we never need two sitds for full speed packets.
- */
-
+ for (i = 0; i < 2 * urb->number_of_packets; i++) {
+ /* use 2 * number_of_packets to accommodate frame-hopping CSPLITS. if
+ * there are no such CSPLITS OR if the packet interval is 1 frame
+ * (meaning frame-hopping CSPLITS don't require an extra sitd, ehci
+ * spec 1.0 4.12.3.4), then more sitds than needed are allocated by
+ * this loop. Excess sitds are added to the free list later
+ */
/*
* Use siTDs from the free list, but not siTDs that may
* still be in use by the hardware.
@@ -2107,12 +2136,24 @@ sitd_patch(
{
struct ehci_iso_packet *uf = &iso_sched->packet[index];
u64 bufp;
+ __hc32 transaction;
sitd->hw_next = EHCI_LIST_END(ehci);
sitd->hw_fullspeed_ep = stream->address;
- sitd->hw_uframe = stream->splits;
- sitd->hw_results = uf->transaction;
- sitd->hw_backpointer = EHCI_LIST_END(ehci);
+ sitd->hw_backpointer = cpu_to_hc32(ehci, sitd->backpointer_sitd_dma);
+ transaction = uf->transaction;
+
+ if(sitd->backpointer_sitd_dma==1) { /* null backpointer */
+ sitd->hw_uframe=stream->splits;
+ } else {
+ if(stream->ps.period==1) {
+ sitd->hw_uframe=stream->splits|stream->c_splits2;
+ } else {
+ sitd->hw_uframe=stream->c_splits2;
+ }
+ transaction |= SITD_STS_STS; /* start in Do Complete Split mode, ehci1 4.12.3.3.2.1*/
+ }
+ sitd->hw_results = transaction;
bufp = uf->bufp;
sitd->hw_buf[0] = cpu_to_hc32(ehci, bufp);
@@ -2149,13 +2190,19 @@ static void sitd_link_urb(
unsigned next_uframe;
struct ehci_iso_sched *sched = urb->hcpriv;
struct ehci_sitd *sitd;
+ struct ehci_sitd *sitd_before;
next_uframe = stream->next_uframe;
- if (list_empty(&stream->td_list))
+ if (list_empty(&stream->td_list)) {
/* usbfs ignores TT bandwidth */
ehci_to_hcd(ehci)->self.bandwidth_allocated
+= stream->bandwidth;
+ sitd_before = NULL;
+ } else {
+ sitd_before = list_last_entry(&sched->td_list,
+ struct ehci_sitd, sitd_list);
+ }
if (ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs == 0) {
if (ehci->amd_pll_fix == 1)
@@ -2165,9 +2212,9 @@ static void sitd_link_urb(
ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs++;
/* fill sITDs frame by frame */
- for (packet = sched->first_packet, sitd = NULL;
- packet < urb->number_of_packets;
- packet++) {
+ for (int i = sched->first_packet, sitd = NULL;
+ i < 2 * urb->number_of_packets;
+ i++) {
/* ASSERT: we have all necessary sitds */
BUG_ON(list_empty(&sched->td_list));
@@ -2176,15 +2223,40 @@ static void sitd_link_urb(
sitd = list_entry(sched->td_list.next,
struct ehci_sitd, sitd_list);
+ if((i>=urb->number_of_packets)&&((stream->ps.period==1)||(!(stream->ps.c_mask2)))) {
+ /*
+ * no frame-hopping CSPLITS OR the period is 1, so such CSPLITS are
+ * put into the sitd for the next transfer ; in both cases
+ * #sitds=#transfers, so move the surplus sitd to the free list
+ */
+ list_move_tail(&sitd->sitd_list, &stream->free_list);
+ continue;
+ }
+ if(stream->ps.c_mask2 && !list_empty(&stream->td_list) &&
+ ((stream->ps.period==1)||(i%2==1)) ) {
+ sitd->backpointer_sitd_dma = sitd_before->sitd_dma;
+ } else {
+ sitd->backpointer_sitd_dma = 1;
+ }
list_move_tail(&sitd->sitd_list, &stream->td_list);
sitd->stream = stream;
sitd->urb = urb;
- sitd_patch(ehci, stream, sitd, sched, packet);
+ sitd_patch(ehci, stream, sitd, sched, i);
sitd_link(ehci, (next_uframe >> 3) & (ehci->periodic_size - 1),
sitd);
- next_uframe += stream->uperiod;
+ if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
+ if((i%2)==0) {
+ /* next sitd only has frame-hopping CSPLITS */
+ next_uframe += 8;
+ } else if ((i%2)==1) {
+ next_uframe += stream->uperiod - 8;
+ }
+ } else {
+ next_uframe += stream->uperiod;
+ }
+ sitd_before = sitd;
}
stream->next_uframe = next_uframe & (mod - 1);
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index d7a3c8d13..c1c006bd3 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -51,6 +51,7 @@ struct ehci_per_sched {
struct list_head ps_list; /* node on ehci_tt's ps_list */
u16 tt_usecs; /* time on the FS/LS bus */
u16 cs_mask; /* C-mask and S-mask bytes */
+ u8 c_mask2; /* C-mask for frame-crossing TT-iso transfer */
u16 period; /* actual period in frames */
u16 phase; /* actual phase, frame part */
u8 bw_phase; /* same, for bandwidth
@@ -489,7 +490,8 @@ struct ehci_iso_stream {
/* output of (re)scheduling */
struct ehci_per_sched ps; /* scheduling info */
unsigned next_uframe;
- __hc32 splits;
+ __hc32 splits; /* C-mask and S-mask */
+ __hc32 c_splits2; /* C-mask for frame-hopping CSPLITS */
/* the rest is derived from the endpoint descriptor,
* including the extra info for hw_bufp[0..2]
@@ -579,6 +581,7 @@ struct ehci_sitd {
/* the rest is HCD-private */
dma_addr_t sitd_dma;
+ dma_addr_t backpointer_sitd_dma;
union ehci_shadow sitd_next; /* ptr to periodic q entry */
struct urb *urb;
--
2.39.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
2026-05-18 6:42 [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers Brent Page
@ 2026-05-18 18:24 ` Brent Page
2026-05-20 17:05 ` Brent Page
2026-05-22 14:18 ` Alan Stern
1 sibling, 1 reply; 11+ messages in thread
From: Brent Page @ 2026-05-18 18:24 UTC (permalink / raw)
To: linux-usb; +Cc: Alan Stern, Michal Pecio
This revision is necessary in sitd_link_urb:
< sitd_patch(ehci, stream, sitd, sched, i);
---
> if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
> packet = i/2;
> } else {
> packet = i;
> }
> sitd_patch(ehci, stream, sitd, sched, packet);
I didn't catch this before because I haven't yet tested the patch on a ps.period!=1 peripheral.
For such peripherals, I think some more logic will also have to be added to sitd_complete to
ensure that
if ((urb_index + 1) != urb->number_of_packets)
doesn't evaluate to true twice for the same urb (specifically for both sitds of the urb's last
sitd pair)
> On May 17, 2026, at 11:42 PM, Brent Page <brentfpage@gmail.com> wrote:
>
> This is a follow-up on
> https://lore.kernel.org/linux-usb/
> B66AE752-B09C-49B3-A829-F7ABB36FB250@gmail.com/T/#u.
> The goal is to accommodate 1023-byte-endpoint full-speed isochronous-in
> transactions on a USB2 bus.
>
> One of the main changes is
> - max_tt_usecs[] = { 125, 125, 125, 125, 125, 125, 30, 0 };
> + max_tt_usecs[] = { 145, 145, 145, 145, 145, 145, 35, 0 };
> To repeat for documentation's sake, "145 us is longer than a microframe, so
> this best-case budget will often not reflect the times when transactions occur.
> But, this situation is consistent with the best-case budget in section 11.18.1
> of the USB-2 spec, in which 1157 data-bytes are scheduled to occur as though no
> bit-stuffing is necessary (i.e., the 188 bytes = 12 megabits/second * (1 byte/8
> bits)* 0.125 ms that can run on the full-speed bus in a microframe are all
> taken taken be data-bytes). So, after the patch, max_tt_usecs is the same as
> the spec's best-case budget but is just scaled by (125 us / 188 bytes) = 1/(12
> megabits/s) and by the 7/6 bit-stuffing multiplier to reflect the fact that the
> scheduling code works in bit-stuffing-inclusive bus-time instead of
> data-bytes." However, as pointed out in the linked email chain, because
> ehci-sched.c currently doesn't allow frame-hopping CSPLITS (a CSPLIT for a
> given transaction that gets executed in the H-frame following the H-frame in
> which the transaction was initiated), "it doesn't support 1023 byte packets at
> all. 1023/188=5.4 and if worst case bit stuffing factor is 7/6 then up to 6.3
> uframes of transfer time. Completion in [HuFrame 6 or 7] and CSPLIT required in
> [HuFrame 0 of the next frame]." So, this patch also adds support for
> frame-hopping CSPLITS. Per EHCI-1.0 4.12.3.3.2.1, the sitd containing such
> CSPLITS must have a backpointer to the sitd of the previous frame.
>
> The main cases to handle in the backpointer code are period==(1 frame) and
> period!=(1 frame), where period is specifically derived from the bInterval of
> the endpoint.
>
> Say there's a 3-packet urb for the endpoint (urb0, with labels td0_#) in the
> pipeline and another gets added (urb1, with labels td1_#). The result is (the
> nodes below are sitds, and "td" stands for "transaction descriptor") ...
> if period==(1 frame):
> |-----| |-----| |-----| |-----| |-----| |-----|
> |td0_0|-->|td0_1|-->|td0_2|-->|td1_0|-->|td1_1|-->|td1_2|
> |-----|< |-----|< |-----|< |-----|< |-----|< |-----|
> | | | | | | | | | |
> |----| |---| |---| |---| |---|
>
> where time increases from left to right, and the lines/arrows on the bottom
> represent backpointer relationships. Also, all sitds are initialized in the Do
> Complete Split state except for the very first in a stream.
>
> Including a backpointer from the first sitd of urb1 to the last sitd of urb0
> (as done here) is not certainly the 'correct' way to handle urb boundaries. An
> alternative strategy would be to add an additional sitd, td0_3, to urb0 after
> td0_2 that just contains CSPLITS for td0_2. However, td0_3 would have to be in
> the same frame as td1_0 (the first sitd of urb1). It's possible that the
> HC supports having two sitds for one endpoint in one frame, but it
> seems unusual enough that it could produce issues, hence why I avoid this
> alternative strategy. Also, the adopted strategy is simpler to code – the only
> distinctive sitd is the very first in a stream.
>
> if period!=(1 frame):
> |-----| |-----| |-----| |-----| |-----| |-----|
> |td0_0|-->|td0_1|-->|td0_2|-->|td0_3|-->|td0_4|-->|td0_5|-->
> |-----|< |-----| |-----|< |-----| |-----|< |-----| cont.
> | | | | | | below
> |----| |----| |----|
>
> |-----| |-----| |-----| |-----| |-----| |-----|
> -->|td1_0|-->|td1_1|-->|td1_2|-->|td1_3|-->|td1_4|-->|td1_5|
> |-----|< |-----| |-----|< |-----| |-----|< |-----|
> | | | | | |
> |----| |----| |----|
> where the second sitd in each pair is positioned one frame after the first sitd
> of the pair, gets initialized in the Do Complete Split state, contains CSPLITS
> for the transfer that was started in the first sitd of the pair, and has no
> SSPLITS.
>
>
> I also want to make sure I understand something that line ~2197 of the patched
> ehci-sched.c:
> sitd_before = list_last_entry(&sched->td_list, struct ehci_sitd, sitd_list);
> is based on. Namely, in the current code, am I correct that
> insertion of a new urb's sitds into the endpoint's td_list looks like:
>
> td_list initially (say it's loaded up with a 3-packet urb)
> |-----| |-----| |-----|
> |td0_0|-->|td0_1|-->|td0_2|
> |-----| |-----| |-----|
>
> line 2090 in the genuine kernel code:
> (executed three times for a new 3-packet urb)
> list_add(&sitd->sitd_list, &iso_sched->td_list);
>
> |-----| |-----| |-----| |-----| |-----| |-----|
> |td0_0|-->| |-->| |-->| |-->|td0_1|-->|td0_2|
> |-----| |-----| |-----| |-----| |-----| |-----|
>
> ?
>
> The following later lines in stid_link_urb (again genuine kernel code) then put
> these new sitds in their proper place I think:
> 2177 sitd = list_entry(iso_sched->td_list.next,
> 2178 struct ehci_itd, sitd_list);
> 2179 list_move_tail(&sitd->sitd_list, &stream->td_list);
>
> Lastly, I'm still a bit confused by this comment in ehci-sched.c:
> /* special case for isoc transfers larger than 125us:
> * the first and each subsequent fully used uframe
> * must be empty, so as to not illegally delay
> * already scheduled transactions
> */
> To me, the main issue is that the adopted "carryover" approach to budgeting
> doesn't take into account the fact that the TT processes the transactions
> sequentially (at least according to all the footprints shown in the figs. in
> EHCI1 and USB2) and doesn't, e.g., do part of transaction 1, switch to
> transaction 2, then go back to transaction 1. This most obviously is
> problematic for long transactions, hence ehci-sched.c handling this case
> separately.
> I make a point of bringing this up because, given the inflation of the
> max_tt_usecs values in the patch, it is actually the case that a newly budgeted
> endpoint may "delay already scheduled transactions". For example, say there's a
> transaction budgeted for HuFrame 1 with tt_usecs=2 us and a new device gets
> plugged in that requires up to tt_usecs=140 us per transaction. The new device
> will also get budgeted for HuFrame 1. Then, the device's transactions that
> actually do take >125 us (due to unfavorable bit stuffing requirements) will
> delay the 2 us transaction (it's maybe worth pointing out that this relies on
> the fact that sitd_link in ehci-sched.c puts the newer sitds first in the
> periodic queue for each uframe). I don't think there's anything "illegal"
> about this though.
>
> ---
> drivers/usb/host/ehci-sched.c | 132 ++++++++++++++++++++++++++--------
> drivers/usb/host/ehci.h | 5 +-
> 2 files changed, 106 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index a241337c9..7c2f2125d 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -285,13 +285,13 @@ static void compute_tt_budget(u8 budget_table[EHCI_BANDWIDTH_SIZE],
> for (uf = ps->phase_uf; uf < 8; ++uf) {
> x += budget_line[uf];
> - /* Each microframe lasts 125 us */
> - if (x <= 125) {
> + /* Cumulative tt_usecs can be as large as 145 us */
> + if (x <= 145) {
> budget_line[uf] = x;
> break;
> }
> - budget_line[uf] = 125;
> - x -= 125;
> + budget_line[uf] = 145;
> + x -= 145;
> }
> }
> }
> @@ -313,7 +313,13 @@ static int __maybe_unused same_tt(struct usb_device *dev1,
> #ifdef CONFIG_USB_EHCI_TT_NEWSCHED
> static const unsigned char
> -max_tt_usecs[] = { 125, 125, 125, 125, 125, 125, 30, 0 };
> +/*
> + * tt_usecs includes worst-case bit stuffing time, so the transactions
> + * (complete and/or partial) budgeted for a given 125 us uframe can have a
> + * cumulative tt_usecs as large as 145 us and still possibly fit into the
> + * uframe.
> +*/
> +max_tt_usecs[] = { 145, 145, 145, 145, 145, 145, 35, 0 };
> /* carryover low/fullspeed bandwidth that crosses uframe boundries */
> static inline void carryover_tt_bandwidth(unsigned short tt_usecs[8])
> @@ -378,13 +384,13 @@ static int tt_available(
> if (max_tt_usecs[uframe] <= tt_usecs[uframe])
> return 0;
> - /* special case for isoc transfers larger than 125us:
> + /* special case for isoc transfers larger than max_tt_usecs:
> * the first and each subsequent fully used uframe
> * must be empty, so as to not illegally delay
> * already scheduled transactions
> */
> - if (usecs > 125) {
> - int ufs = (usecs / 125);
> + if (usecs > 145) {
> + int ufs = (usecs / 145);
> for (i = uframe; i < (uframe + ufs) && i < 8; i++)
> if (tt_usecs[i] > 0)
> @@ -1388,19 +1394,40 @@ sitd_slot_ok(
> struct ehci_tt *tt
> )
> {
> - unsigned mask, tmp;
> + unsigned mask, c_mask2, tmp;
> unsigned frame, uf;
> mask = stream->ps.cs_mask << (uframe & 7);
> + c_mask2 = mask >> 16;
> + mask = mask & 0xffff;
> + if((c_mask2 & 1) && (c_mask2 & 1<<1) && (c_mask2 & 1<<2)) {
> + /* if BuFrame6 is the last uframe in which a transaction is budgeted,
> + * the transaction will initially be configured to have CSPLITS in
> + * BuFrame7 as well as BuFrames 0 and 1 of the following frame
> + * (HuFrames 0,1,2) */
> + /* below: usb2 spec 11.18.4.3.c paragraph 2 */
> + if(mask & 1) {
> + c_mask2 = 1;
> + } else {
> + c_mask2 = (1<<2)-1;
> + }
> + } else if ((c_mask2 & 1) && (c_mask2 & 1<<1)) {
> + /* if BuFrame5 is the last uframe in which a transaction is budgeted,
> + * the transaction will initially be configured to have CSPLITS in
> + * BuFrames 6 and 7 as well as BuFrame 0 of the following frame
> + * (HuFrames 7,0,1) */
> + /* below: usb2 spec 11.18.4.3.c paragraph 1 */
> + if(mask & 1) {
> + c_mask2 = 1;
> + }
> + }
> + if((c_mask2 & mask & 1<<1))
> + return 0; /* ehci1 4.12.3.1 */
> /* for OUT, don't wrap SSPLIT into H-microframe 7 */
> if (((stream->ps.cs_mask & 0xff) << (uframe & 7)) >= (1 << 7))
> return 0;
> - /* for IN, don't wrap CSPLIT into the next frame */
> - if (mask & ~0xffff)
> - return 0;
> -
> /* check bandwidth */
> uframe &= stream->ps.bw_uperiod - 1;
> frame = uframe >> 3;
> @@ -1450,8 +1477,10 @@ sitd_slot_ok(
> uframe += stream->ps.bw_uperiod;
> } while (uframe < EHCI_BANDWIDTH_SIZE);
> - stream->ps.cs_mask <<= uframe & 7;
> + stream->ps.cs_mask = mask;
> + stream->ps.c_mask2 = c_mask2;
> stream->splits = cpu_to_hc32(ehci, stream->ps.cs_mask);
> + stream->c_splits2 = cpu_to_hc32(ehci, stream->ps.c_mask2);
> return 1;
> }
> @@ -2049,13 +2078,13 @@ sitd_urb_transaction(
> /* allocate/init sITDs */
> spin_lock_irqsave(&ehci->lock, flags);
> - for (i = 0; i < urb->number_of_packets; i++) {
> -
> - /* NOTE: for now, we don't try to handle wraparound cases
> - * for IN (using sitd->hw_backpointer, like a FSTN), which
> - * means we never need two sitds for full speed packets.
> - */
> -
> + for (i = 0; i < 2 * urb->number_of_packets; i++) {
> + /* use 2 * number_of_packets to accommodate frame-hopping CSPLITS. if
> + * there are no such CSPLITS OR if the packet interval is 1 frame
> + * (meaning frame-hopping CSPLITS don't require an extra sitd, ehci
> + * spec 1.0 4.12.3.4), then more sitds than needed are allocated by
> + * this loop. Excess sitds are added to the free list later
> + */
> /*
> * Use siTDs from the free list, but not siTDs that may
> * still be in use by the hardware.
> @@ -2107,12 +2136,24 @@ sitd_patch(
> {
> struct ehci_iso_packet *uf = &iso_sched->packet[index];
> u64 bufp;
> + __hc32 transaction;
> sitd->hw_next = EHCI_LIST_END(ehci);
> sitd->hw_fullspeed_ep = stream->address;
> - sitd->hw_uframe = stream->splits;
> - sitd->hw_results = uf->transaction;
> - sitd->hw_backpointer = EHCI_LIST_END(ehci);
> + sitd->hw_backpointer = cpu_to_hc32(ehci, sitd->backpointer_sitd_dma);
> + transaction = uf->transaction;
> +
> + if(sitd->backpointer_sitd_dma==1) { /* null backpointer */
> + sitd->hw_uframe=stream->splits;
> + } else {
> + if(stream->ps.period==1) {
> + sitd->hw_uframe=stream->splits|stream->c_splits2;
> + } else {
> + sitd->hw_uframe=stream->c_splits2;
> + }
> + transaction |= SITD_STS_STS; /* start in Do Complete Split mode, ehci1 4.12.3.3.2.1*/
> + }
> + sitd->hw_results = transaction;
> bufp = uf->bufp;
> sitd->hw_buf[0] = cpu_to_hc32(ehci, bufp);
> @@ -2149,13 +2190,19 @@ static void sitd_link_urb(
> unsigned next_uframe;
> struct ehci_iso_sched *sched = urb->hcpriv;
> struct ehci_sitd *sitd;
> + struct ehci_sitd *sitd_before;
> next_uframe = stream->next_uframe;
> - if (list_empty(&stream->td_list))
> + if (list_empty(&stream->td_list)) {
> /* usbfs ignores TT bandwidth */
> ehci_to_hcd(ehci)->self.bandwidth_allocated
> += stream->bandwidth;
> + sitd_before = NULL;
> + } else {
> + sitd_before = list_last_entry(&sched->td_list,
> + struct ehci_sitd, sitd_list);
> + }
> if (ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs == 0) {
> if (ehci->amd_pll_fix == 1)
> @@ -2165,9 +2212,9 @@ static void sitd_link_urb(
> ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs++;
> /* fill sITDs frame by frame */
> - for (packet = sched->first_packet, sitd = NULL;
> - packet < urb->number_of_packets;
> - packet++) {
> + for (int i = sched->first_packet, sitd = NULL;
> + i < 2 * urb->number_of_packets;
> + i++) {
> /* ASSERT: we have all necessary sitds */
> BUG_ON(list_empty(&sched->td_list));
> @@ -2176,15 +2223,40 @@ static void sitd_link_urb(
> sitd = list_entry(sched->td_list.next,
> struct ehci_sitd, sitd_list);
> + if((i>=urb->number_of_packets)&&((stream->ps.period==1)||(!(stream->ps.c_mask2)))) {
> + /*
> + * no frame-hopping CSPLITS OR the period is 1, so such CSPLITS are
> + * put into the sitd for the next transfer ; in both cases
> + * #sitds=#transfers, so move the surplus sitd to the free list
> + */
> + list_move_tail(&sitd->sitd_list, &stream->free_list);
> + continue;
> + }
> + if(stream->ps.c_mask2 && !list_empty(&stream->td_list) &&
> + ((stream->ps.period==1)||(i%2==1)) ) {
> + sitd->backpointer_sitd_dma = sitd_before->sitd_dma;
> + } else {
> + sitd->backpointer_sitd_dma = 1;
> + }
> list_move_tail(&sitd->sitd_list, &stream->td_list);
> sitd->stream = stream;
> sitd->urb = urb;
> - sitd_patch(ehci, stream, sitd, sched, packet);
> + sitd_patch(ehci, stream, sitd, sched, i);
> sitd_link(ehci, (next_uframe >> 3) & (ehci->periodic_size - 1),
> sitd);
> - next_uframe += stream->uperiod;
> + if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
> + if((i%2)==0) {
> + /* next sitd only has frame-hopping CSPLITS */
> + next_uframe += 8;
> + } else if ((i%2)==1) {
> + next_uframe += stream->uperiod - 8;
> + }
> + } else {
> + next_uframe += stream->uperiod;
> + }
> + sitd_before = sitd;
> }
> stream->next_uframe = next_uframe & (mod - 1);
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index d7a3c8d13..c1c006bd3 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -51,6 +51,7 @@ struct ehci_per_sched {
> struct list_head ps_list; /* node on ehci_tt's ps_list */
> u16 tt_usecs; /* time on the FS/LS bus */
> u16 cs_mask; /* C-mask and S-mask bytes */
> + u8 c_mask2; /* C-mask for frame-crossing TT-iso transfer */
> u16 period; /* actual period in frames */
> u16 phase; /* actual phase, frame part */
> u8 bw_phase; /* same, for bandwidth
> @@ -489,7 +490,8 @@ struct ehci_iso_stream {
> /* output of (re)scheduling */
> struct ehci_per_sched ps; /* scheduling info */
> unsigned next_uframe;
> - __hc32 splits;
> + __hc32 splits; /* C-mask and S-mask */
> + __hc32 c_splits2; /* C-mask for frame-hopping CSPLITS */
> /* the rest is derived from the endpoint descriptor,
> * including the extra info for hw_bufp[0..2]
> @@ -579,6 +581,7 @@ struct ehci_sitd {
> /* the rest is HCD-private */
> dma_addr_t sitd_dma;
> + dma_addr_t backpointer_sitd_dma;
> union ehci_shadow sitd_next; /* ptr to periodic q entry */
> struct urb *urb;
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
2026-05-18 18:24 ` Brent Page
@ 2026-05-20 17:05 ` Brent Page
2026-05-21 5:52 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Brent Page @ 2026-05-20 17:05 UTC (permalink / raw)
To: linux-usb; +Cc: Alan Stern, Michal Pecio
OK, a coherent set of additions/edits to the patch:
---
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 7c2f2125d..04cf55484 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1478,7 +1478,7 @@ sitd_slot_ok(
} while (uframe < EHCI_BANDWIDTH_SIZE);
stream->ps.cs_mask = mask;
- stream->ps.c_mask2 = c_mask2;
+ stream->ps.c_mask2 = c_mask2 << 8;
stream->splits = cpu_to_hc32(ehci, stream->ps.cs_mask);
stream->c_splits2 = cpu_to_hc32(ehci, stream->ps.c_mask2);
return 1;
@@ -2242,7 +2242,14 @@ static void sitd_link_urb(
sitd->stream = stream;
sitd->urb = urb;
- sitd_patch(ehci, stream, sitd, sched, i);
+ if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
+ packet = i/2;
+ sitd->last_in_urb = i==(2*urb->number_of_packets - 1);
+ } else {
+ packet = i;
+ sitd->last_in_urb = i==(urb->number_of_packets-1);
+ }
+ sitd_patch(ehci, stream, sitd, sched, packet);
sitd_link(ehci, (next_uframe >> 3) & (ehci->periodic_size - 1),
sitd);
@@ -2291,13 +2298,17 @@ static bool sitd_complete(struct ehci_hcd *ehci, struct ehci_sitd *sitd)
int urb_index;
struct ehci_iso_stream *stream = sitd->stream;
bool retval = false;
+ bool has_ssplits;
urb_index = sitd->index;
desc = &urb->iso_frame_desc[urb_index];
t = hc32_to_cpup(ehci, &sitd->hw_results);
+ has_ssplits = hc32_to_cpu(ehci, sitd->hw_uframe) & 0x00ff;
/* report transfer status */
- if (unlikely(t & SITD_ERRS)) {
+ if(!has_ssplits) { /* just contains frame-hopping CSPLITS */
+ desc->status = 0; /* actual completion status reported by previous sitd */
+ } else if (unlikely(t & SITD_ERRS)) {
urb->error_count++;
if (t & SITD_STS_DBE)
desc->status = usb_pipein(urb->pipe)
@@ -2317,7 +2328,7 @@ static bool sitd_complete(struct ehci_hcd *ehci, struct ehci_sitd *sitd)
}
/* handle completion now? */
- if ((urb_index + 1) != urb->number_of_packets)
+ if (!sitd->last_in_urb)
goto done;
/*
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index c1c006bd3..6c9b6f390 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -583,6 +583,7 @@ struct ehci_sitd {
dma_addr_t sitd_dma;
dma_addr_t backpointer_sitd_dma;
union ehci_shadow sitd_next; /* ptr to periodic q entry */
+ bool last_in_urb;
struct urb *urb;
struct ehci_iso_stream *stream; /* endpoint's queue */
---
I configured my 1023-byte-endpoint peripheral to have a bInterval of 2 so that I could test the patch's period!=1 code. The patch appeared to perform well for this case. That said, the peripheral's endpoint is actually not that demanding because the number of data bytes per frame is only ~750. It just has to set wMaxPacketSize to 1023 due to hardware limitations and the fact that 750>512. So, I plan to conduct some more intensive tests requiring closer to 1023 iso-in data bytes and maybe a few interrupt bytes for good measure.
From,
Brent Page
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
2026-05-20 17:05 ` Brent Page
@ 2026-05-21 5:52 ` Greg KH
0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2026-05-21 5:52 UTC (permalink / raw)
To: Brent Page; +Cc: linux-usb, Alan Stern, Michal Pecio
On Wed, May 20, 2026 at 10:05:39AM -0700, Brent Page wrote:
>
> OK, a coherent set of additions/edits to the patch:
> ---
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index 7c2f2125d..04cf55484 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -1478,7 +1478,7 @@ sitd_slot_ok(
> } while (uframe < EHCI_BANDWIDTH_SIZE);
> stream->ps.cs_mask = mask;
> - stream->ps.c_mask2 = c_mask2;
> + stream->ps.c_mask2 = c_mask2 << 8;
> stream->splits = cpu_to_hc32(ehci, stream->ps.cs_mask);
> stream->c_splits2 = cpu_to_hc32(ehci, stream->ps.c_mask2);
> return 1;
Whitespace corrupted and can not be applied :(
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
2026-05-18 6:42 [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers Brent Page
2026-05-18 18:24 ` Brent Page
@ 2026-05-22 14:18 ` Alan Stern
2026-06-02 6:42 ` [PATCH] " Brent Page
2026-06-02 6:52 ` [PATCH] USB: EHCI: " Brent Page
1 sibling, 2 replies; 11+ messages in thread
From: Alan Stern @ 2026-05-22 14:18 UTC (permalink / raw)
To: Brent Page; +Cc: linux-usb, Michal Pecio
On Sun, May 17, 2026 at 11:42:35PM -0700, Brent Page wrote:
> This is a follow-up on
> https://lore.kernel.org/linux-usb/
> B66AE752-B09C-49B3-A829-F7ABB36FB250@gmail.com/T/#u.
> The goal is to accommodate 1023-byte-endpoint full-speed isochronous-in
> transactions on a USB2 bus.
I've been very busy and haven't had time to review this. It would help
if you post your changes in their final form, and do so in a way that
doesn't get corrupted by your email client.
> One of the main changes is
> - max_tt_usecs[] = { 125, 125, 125, 125, 125, 125, 30, 0 };
> + max_tt_usecs[] = { 145, 145, 145, 145, 145, 145, 35, 0 };
...
> I also want to make sure I understand something that line ~2197 of the patched
> ehci-sched.c:
> sitd_before = list_last_entry(&sched->td_list, struct ehci_sitd, sitd_list);
> is based on. Namely, in the current code, am I correct that
> insertion of a new urb's sitds into the endpoint's td_list looks like:
>
> td_list initially (say it's loaded up with a 3-packet urb)
> |-----| |-----| |-----|
> |td0_0|-->|td0_1|-->|td0_2|
> |-----| |-----| |-----|
That picture is wrong, because the list is doubly linked and there is a
separate list head. It actually looks like this:
+-> stream <--> td0_0 <--> td0_1 <--> td0_2 <-+
| |
+---------------------------------------------+
> line 2090 in the genuine kernel code:
Genuine as opposed to fake? :-) Just call it the current or existing
code, and give the kernel version number if you want to be very
specific.
> (executed three times for a new 3-packet urb)
> list_add(&sitd->sitd_list, &iso_sched->td_list);
>
> |-----| |-----| |-----| |-----| |-----| |-----|
> |td0_0|-->| |-->| |-->| |-->|td0_1|-->|td0_2|
> |-----| |-----| |-----| |-----| |-----| |-----|
>
> ?
That loop adds three new "blank" sitds to iso_sched. At the end of
sitd_urb_transaction(), the picture looks like this:
+-> iso_sched <--> new2 <--> new1 <--> new0 <-+
| |
+---------------------------------------------+
where urb->hcpriv points to iso_sched. (The entries end up in reverse
order, since each one is added immediately following the head, but since
they are all blank this doesn't matter.)
> The following later lines in stid_link_urb (again genuine kernel code) then put
> these new sitds in their proper place I think:
> 2177 sitd = list_entry(iso_sched->td_list.next,
> 2178 struct ehci_itd, sitd_list);
> 2179 list_move_tail(&sitd->sitd_list, &stream->td_list);
This loop moves the new sitds to the end of the stream's queue, and the
following calls to sitd_patch() and sitd_link() fill in the entries and
put them on the hardware schedule. The end result is:
+-> stream <--> td0_0 <--> td0_1 <--> td0_2 <--
| --> td1_0 <--> td1_1 <--> td1_2 <-+
| |
+-------------------------------------------------+
> Lastly, I'm still a bit confused by this comment in ehci-sched.c:
> /* special case for isoc transfers larger than 125us:
> * the first and each subsequent fully used uframe
> * must be empty, so as to not illegally delay
> * already scheduled transactions
> */
That comment is from a 20-year-old patch and should be regarded as
somewhat out of date. As far as I can see, there's nothing wrong with
allowing the first uframe to be partially occupied as long as the new
sitd is added to the end of the uframe's queue, so it doesn't delay the
sitd's already there.
> To me, the main issue is that the adopted "carryover" approach to budgeting
> doesn't take into account the fact that the TT processes the transactions
> sequentially (at least according to all the footprints shown in the figs. in
> EHCI1 and USB2) and doesn't, e.g., do part of transaction 1, switch to
> transaction 2, then go back to transaction 1. This most obviously is
> problematic for long transactions, hence ehci-sched.c handling this case
> separately.
>
> I make a point of bringing this up because, given the inflation of the
> max_tt_usecs values in the patch, it is actually the case that a newly budgeted
> endpoint may "delay already scheduled transactions". For example, say there's a
> transaction budgeted for HuFrame 1 with tt_usecs=2 us and a new device gets
> plugged in that requires up to tt_usecs=140 us per transaction. The new device
> will also get budgeted for HuFrame 1. Then, the device's transactions that
> actually do take >125 us (due to unfavorable bit stuffing requirements) will
> delay the 2 us transaction (it's maybe worth pointing out that this relies on
> the fact that sitd_link in ehci-sched.c puts the newer sitds first in the
> periodic queue for each uframe). I don't think there's anything "illegal"
> about this though.
It's okay for sitd_link() to add new sitds to the beginning of the
uframe's queue, provided this doesn't push the budgeted start of the
last sitd in the queue beyond the 145-us limit. But in general, things
will be simpler if each new sitd is always added to the end of the
queue. The code should be changed to do this.
Alan Stern
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] inflate max_tt_usecs and implement sitd backpointers
2026-05-22 14:18 ` Alan Stern
@ 2026-06-02 6:42 ` Brent Page
2026-06-02 19:35 ` Alan Stern
2026-06-02 6:52 ` [PATCH] USB: EHCI: " Brent Page
1 sibling, 1 reply; 11+ messages in thread
From: Brent Page @ 2026-06-02 6:42 UTC (permalink / raw)
To: linux-usb; +Cc: Brent Page
---
This is a version of the patch with new formatting; the content
otherwise is equivalent to the combination of the two prior patches in
this thread. I ran a number of tests of the patch's functionality for
iso-in transfers from a full-speed peripheral with a single endpoint
with a wMaxPacketSize of 1023. The transfers themselves were 1000
bytes. I tried bIntervals of both 1 frame as well as 2 frames.
Setup 1:
- configure this "main" peripheral to send (48x 1s followed by 32x 0s)
repeated 100 times in each transfer (1000 total data bytes)
- "transfer" here means the data that is sent in one frame
- the 48x 1s will require substantial bit stuffing, and the 32x 0s
will require none; the actual number of bytes will be something
like (7/6*48 + 32)/80 * 1000 = 1100 bus bytes
- on a single USB 2 port going in to the computer, plug in a USB 2
one-to-four splitter
- plug in the main peripheral along with a wireless mouse receiver,
wireless keyboard receiver, and a high-speed webcam
- Result: for both bInterval=1 frame and bInterval=2 frames for the main
peripheral, all four connected devices function well
- notes: the main peripheral has to be plugged in before the wireless
mouse and keyboard receivers; otherwise the main peripheral's
1023-byte endpoint is rejected
Setup 2:
- same as Setup 1, except configure the main peripheral to send (72x 1s
followed by 8x 0s) repeated 100 times in each transfer
- ~(7/6*72 + 8)/80 * 1000 = 1150 bus bytes
- Result: for both bInterval=1 frame and bInterval=2 frames, the main
peripheral's 1023-byte endpoint is accepted by the computer, but all
attempts by user space to retrieve urbs (ioctl-REAPURBNDELAY sys
calls) yield error code -11, EAGAIN (instructing the caller to try
again).
- notes: if the data transfers are instead configured to be (64x 1s
followed by 16x 0s) repeated 100 times, then an occasional urb is
successfully retrieved. So, it does seem like the bit-stuffing
requirements are an influential factor
Setup 3:
- same data for the main peripheral as in the first bullet of Setup 2
- don't use a splitter; plug the main peripheral directly into the
computer's USB port
- Result: for both bInterval=1 frame and bInterval=2 frames, the main
peripheral's usb connection is robust
Test summary: the patch works for iso-in transfers that are
- 1150 bus bytes in a frame if the associated peripheral is plugged
directly into a USB port
- 1100 bus bytes in a frame alongside transfers from two full-speed
interrupt devices and a high-speed webcam, with all devices plugged
into the same USB port via a splitter
Weak point: The patch doubles the number of sitds that are allocated for
any given urb. This is true regardless of whether the urb's transfers
require frame-hopping CSPLITS. I figured that the memory in these
structs was negligible, but I actually get ENOMEM from line ~1400 of
drivers/usb/core/devio.c in kernel version 3.18.120 when I tried to submit
4x urbs with 33 transfers each (requiring 4*33*2=264 sitds for this
patch's code) on my 2017 Android tablet. Things work if I only submit
100 iso transfers instead. Also, submitting 120 transfers doesn't cause
any issues on my 2022 Android smartphone. Maybe the extra sitd
allocation should be done on-demand instead of by default? So, start
off by allocating an sitd for each transfer, and only add an extra sitd
if a transfer has frame-hopping CSPLITS?
drivers/usb/host/ehci-sched.c | 152 +++++++++++++++++++++++++++-------
drivers/usb/host/ehci.h | 9 +-
2 files changed, 131 insertions(+), 30 deletions(-)
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index a241337c9..9fdfdb5fe 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -285,13 +285,14 @@ static void compute_tt_budget(u8 budget_table[EHCI_BANDWIDTH_SIZE],
for (uf = ps->phase_uf; uf < 8; ++uf) {
x += budget_line[uf];
- /* Each microframe lasts 125 us */
- if (x <= 125) {
+ /* Cumulative tt_usecs can be as large as 145
+ * us */
+ if (x <= 145) {
budget_line[uf] = x;
break;
}
- budget_line[uf] = 125;
- x -= 125;
+ budget_line[uf] = 145;
+ x -= 145;
}
}
}
@@ -313,7 +314,12 @@ static int __maybe_unused same_tt(struct usb_device *dev1,
#ifdef CONFIG_USB_EHCI_TT_NEWSCHED
static const unsigned char
-max_tt_usecs[] = { 125, 125, 125, 125, 125, 125, 30, 0 };
+/*
+ * tt_usecs includes worst-case bit stuffing time, so the transactions
+ * budgeted for a given 125 us uframe can have cumulative tt_usecs as large
+ * as 145 us and still possibly fit into the uframe.
+ */
+max_tt_usecs[] = { 145, 145, 145, 145, 145, 145, 35, 0 };
/* carryover low/fullspeed bandwidth that crosses uframe boundries */
static inline void carryover_tt_bandwidth(unsigned short tt_usecs[8])
@@ -378,13 +384,13 @@ static int tt_available(
if (max_tt_usecs[uframe] <= tt_usecs[uframe])
return 0;
- /* special case for isoc transfers larger than 125us:
+ /* special case for isoc transfers larger than max_tt_usecs:
* the first and each subsequent fully used uframe
* must be empty, so as to not illegally delay
* already scheduled transactions
*/
- if (usecs > 125) {
- int ufs = (usecs / 125);
+ if (usecs > 145) {
+ int ufs = (usecs / 145);
for (i = uframe; i < (uframe + ufs) && i < 8; i++)
if (tt_usecs[i] > 0)
@@ -1388,19 +1394,42 @@ sitd_slot_ok(
struct ehci_tt *tt
)
{
- unsigned mask, tmp;
+ unsigned mask, c_mask2, tmp;
unsigned frame, uf;
mask = stream->ps.cs_mask << (uframe & 7);
+ c_mask2 = mask >> 16;
+ mask = mask & 0xffff;
+ if((c_mask2 & 1) && (c_mask2 & 1<<1) && (c_mask2 & 1<<2)) {
+ /* if BuFrame6 is the last uframe in which a transaction is
+ * budgeted, the transaction will initially be configured to
+ * have CSPLITS in BuFrame7 as well as BuFrames 0 and 1 of
+ * the following frame (HuFrames 0,1,2)
+ */
+ /* below: usb2 spec 11.18.4.3.c paragraph 2 */
+ if(mask & 1) {
+ c_mask2 = 1;
+ } else {
+ c_mask2 = (1<<2)-1;
+ }
+ } else if ((c_mask2 & 1) && (c_mask2 & 1<<1)) {
+ /* if BuFrame5 is the last uframe in which a transaction is
+ * budgeted, the transaction will initially be configured to
+ * have CSPLITS in BuFrames 6 and 7 as well as BuFrame 0 of
+ * the following frame (HuFrames 7,0,1)
+ */
+ /* below: usb2 spec 11.18.4.3.c paragraph 1 */
+ if(mask & 1) {
+ c_mask2 = 1;
+ }
+ }
+ if((c_mask2 & mask & 1<<1))
+ return 0; /* ehci1 4.12.3.1 */
/* for OUT, don't wrap SSPLIT into H-microframe 7 */
if (((stream->ps.cs_mask & 0xff) << (uframe & 7)) >= (1 << 7))
return 0;
- /* for IN, don't wrap CSPLIT into the next frame */
- if (mask & ~0xffff)
- return 0;
-
/* check bandwidth */
uframe &= stream->ps.bw_uperiod - 1;
frame = uframe >> 3;
@@ -1450,8 +1479,11 @@ sitd_slot_ok(
uframe += stream->ps.bw_uperiod;
} while (uframe < EHCI_BANDWIDTH_SIZE);
- stream->ps.cs_mask <<= uframe & 7;
+ stream->ps.cs_mask = mask;
+ stream->ps.c_mask2 = c_mask2 << 8;
stream->splits = cpu_to_hc32(ehci, stream->ps.cs_mask);
+ stream->c_splits2 = cpu_to_hc32(ehci, stream->ps.c_mask2);
+
return 1;
}
@@ -2049,11 +2081,13 @@ sitd_urb_transaction(
/* allocate/init sITDs */
spin_lock_irqsave(&ehci->lock, flags);
- for (i = 0; i < urb->number_of_packets; i++) {
-
- /* NOTE: for now, we don't try to handle wraparound cases
- * for IN (using sitd->hw_backpointer, like a FSTN), which
- * means we never need two sitds for full speed packets.
+ for (i = 0; i < 2 * urb->number_of_packets; i++) {
+ /* use 2 * number_of_packets to accommodate frame-hopping
+ * CSPLITS. if there are no such CSPLITS OR if the packet
+ * interval is 1 frame (meaning frame-hopping CSPLITS don't
+ * require an extra sitd, ehci spec 1.0 4.12.3.4), then more
+ * sitds than needed are allocated by this loop. Excess
+ * sitds are added to the free list later
*/
/*
@@ -2107,12 +2141,25 @@ sitd_patch(
{
struct ehci_iso_packet *uf = &iso_sched->packet[index];
u64 bufp;
+ __hc32 transaction;
sitd->hw_next = EHCI_LIST_END(ehci);
sitd->hw_fullspeed_ep = stream->address;
- sitd->hw_uframe = stream->splits;
- sitd->hw_results = uf->transaction;
- sitd->hw_backpointer = EHCI_LIST_END(ehci);
+ sitd->hw_backpointer = cpu_to_hc32(ehci, sitd->backpointer_sitd_dma);
+ transaction = uf->transaction;
+
+ if(sitd->backpointer_sitd_dma==1) { /* null backpointer */
+ sitd->hw_uframe=stream->splits;
+ } else {
+ if(stream->ps.period==1) {
+ sitd->hw_uframe=stream->splits|stream->c_splits2;
+ } else {
+ sitd->hw_uframe=stream->c_splits2;
+ }
+ /* start in Do Complete Split mode, ehci1 4.12.3.3.2.1*/
+ transaction |= SITD_STS_STS;
+ }
+ sitd->hw_results = transaction;
bufp = uf->bufp;
sitd->hw_buf[0] = cpu_to_hc32(ehci, bufp);
@@ -2149,13 +2196,19 @@ static void sitd_link_urb(
unsigned next_uframe;
struct ehci_iso_sched *sched = urb->hcpriv;
struct ehci_sitd *sitd;
+ struct ehci_sitd *sitd_before;
next_uframe = stream->next_uframe;
- if (list_empty(&stream->td_list))
+ if (list_empty(&stream->td_list)) {
/* usbfs ignores TT bandwidth */
ehci_to_hcd(ehci)->self.bandwidth_allocated
+= stream->bandwidth;
+ sitd_before = NULL;
+ } else {
+ sitd_before = list_last_entry(&sched->td_list,
+ struct ehci_sitd, sitd_list);
+ }
if (ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs == 0) {
if (ehci->amd_pll_fix == 1)
@@ -2165,9 +2218,9 @@ static void sitd_link_urb(
ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs++;
/* fill sITDs frame by frame */
- for (packet = sched->first_packet, sitd = NULL;
- packet < urb->number_of_packets;
- packet++) {
+ for (int i = sched->first_packet, sitd = NULL;
+ i < 2 * urb->number_of_packets;
+ i++) {
/* ASSERT: we have all necessary sitds */
BUG_ON(list_empty(&sched->td_list));
@@ -2176,15 +2229,51 @@ static void sitd_link_urb(
sitd = list_entry(sched->td_list.next,
struct ehci_sitd, sitd_list);
+ if((i>=urb->number_of_packets)&&
+ (stream->ps.period==1 || !stream->ps.c_mask2)) {
+ /*
+ * no frame-hopping CSPLITS OR the period is 1, so
+ * such CSPLITS are put into the sitd for the next
+ * transfer ; in both cases #sitds=#transfers, so
+ * move the surplus sitd to the free list
+ */
+ list_move_tail(&sitd->sitd_list, &stream->free_list);
+ continue;
+ }
+ if(stream->ps.c_mask2 && !list_empty(&stream->td_list) &&
+ ((stream->ps.period==1)||(i%2==1)) ) {
+ sitd->backpointer_sitd_dma = sitd_before->sitd_dma;
+ } else {
+ sitd->backpointer_sitd_dma = 1;
+ }
+
list_move_tail(&sitd->sitd_list, &stream->td_list);
sitd->stream = stream;
sitd->urb = urb;
+ if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
+ packet = i/2;
+ sitd->last_in_urb = i==(2*urb->number_of_packets - 1);
+ } else {
+ packet = i;
+ sitd->last_in_urb = i==(urb->number_of_packets-1);
+ }
+
sitd_patch(ehci, stream, sitd, sched, packet);
sitd_link(ehci, (next_uframe >> 3) & (ehci->periodic_size - 1),
sitd);
- next_uframe += stream->uperiod;
+ if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
+ if((i%2)==0) {
+ /* next sitd only has frame-hopping CSPLITS */
+ next_uframe += 8;
+ } else if ((i%2)==1) {
+ next_uframe += stream->uperiod - 8;
+ }
+ } else {
+ next_uframe += stream->uperiod;
+ }
+ sitd_before = sitd;
}
stream->next_uframe = next_uframe & (mod - 1);
@@ -2219,13 +2308,18 @@ static bool sitd_complete(struct ehci_hcd *ehci, struct ehci_sitd *sitd)
int urb_index;
struct ehci_iso_stream *stream = sitd->stream;
bool retval = false;
+ bool has_ssplits;
urb_index = sitd->index;
desc = &urb->iso_frame_desc[urb_index];
t = hc32_to_cpup(ehci, &sitd->hw_results);
+ has_ssplits = hc32_to_cpu(ehci, sitd->hw_uframe) & 0x00ff;
/* report transfer status */
- if (unlikely(t & SITD_ERRS)) {
+ if(!has_ssplits) { /* just contains frame-hopping CSPLITS */
+ /* actual completion status reported by previous sitd */
+ desc->status = 0;
+ } else if (unlikely(t & SITD_ERRS)) {
urb->error_count++;
if (t & SITD_STS_DBE)
desc->status = usb_pipein(urb->pipe)
@@ -2245,7 +2339,7 @@ static bool sitd_complete(struct ehci_hcd *ehci, struct ehci_sitd *sitd)
}
/* handle completion now? */
- if ((urb_index + 1) != urb->number_of_packets)
+ if (!sitd->last_in_urb)
goto done;
/*
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index d7a3c8d13..cb91fc5e3 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -51,6 +51,8 @@ struct ehci_per_sched {
struct list_head ps_list; /* node on ehci_tt's ps_list */
u16 tt_usecs; /* time on the FS/LS bus */
u16 cs_mask; /* C-mask and S-mask bytes */
+ u8 c_mask2; /* C-mask for frame-hopping CSPLITS */
+
u16 period; /* actual period in frames */
u16 phase; /* actual phase, frame part */
u8 bw_phase; /* same, for bandwidth
@@ -489,7 +491,8 @@ struct ehci_iso_stream {
/* output of (re)scheduling */
struct ehci_per_sched ps; /* scheduling info */
unsigned next_uframe;
- __hc32 splits;
+ __hc32 splits; /* C-mask and S-mask */
+ __hc32 c_splits2; /* C-mask for frame-hopping CSPLITS */
/* the rest is derived from the endpoint descriptor,
* including the extra info for hw_bufp[0..2]
@@ -579,7 +582,11 @@ struct ehci_sitd {
/* the rest is HCD-private */
dma_addr_t sitd_dma;
+ dma_addr_t backpointer_sitd_dma;
+
union ehci_shadow sitd_next; /* ptr to periodic q entry */
+ bool last_in_urb;
+
struct urb *urb;
struct ehci_iso_stream *stream; /* endpoint's queue */
--
2.39.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
2026-05-22 14:18 ` Alan Stern
2026-06-02 6:42 ` [PATCH] " Brent Page
@ 2026-06-02 6:52 ` Brent Page
2026-06-02 14:26 ` Alan Stern
1 sibling, 1 reply; 11+ messages in thread
From: Brent Page @ 2026-06-02 6:52 UTC (permalink / raw)
To: Alan Stern; +Cc: linux-usb, Michal Pecio
> That loop adds three new "blank" sitds to iso_sched. At the end of
> sitd_urb_transaction(), the picture looks like this:
>
> +-> iso_sched <--> new2 <--> new1 <--> new0 <-+
> | |
> +---------------------------------------------+
But for an iso endpoint, the schedule should never be empty (except for
when the very first urb comes in). So, there are sitds that are already
scheduled before the new sitds come in, and I think in total the picture
will look like, e.g.,
+-> stream <--> new2 <--> new1 <--> new0 <--
| --> prev2 <--> prev1 <--> prev0 <-+
| |
+-------------------------------------------------+
Then, after
2177 sitd = list_entry(iso_sched->td_list.next,
2178 struct ehci_itd, sitd_list);
2179 list_move_tail(&sitd->sitd_list, &stream->td_list);
is executed 3x, the schedule will look like
+-> stream <--> prev2 <--> prev1 <--> prev0 <--
| --> new2 <--> new1 <--> new0 <-+
| |
+-------------------------------------------------+
> As far as I can see, there's nothing wrong with
> allowing the first uframe to be partially occupied as long as the new
> sitd is added to the end of the uframe's queue, so it doesn't delay the
> sitd's already there.
Oh yeah, I see that – because then the large sitd can partially occupy
the uframe without needing to be broken up.
> But in general, things will be simpler if each new sitd is always added
> to the end of the queue. The code should be changed to do this.
To be conservative, I didn't change sitd_link() in the patch. I just
submitted the newest version to this thread. The whitespace formatting
should be correct now.
From,
Brent Page
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
2026-06-02 6:52 ` [PATCH] USB: EHCI: " Brent Page
@ 2026-06-02 14:26 ` Alan Stern
2026-06-22 5:19 ` Brent Page
0 siblings, 1 reply; 11+ messages in thread
From: Alan Stern @ 2026-06-02 14:26 UTC (permalink / raw)
To: Brent Page; +Cc: linux-usb, Michal Pecio
On Mon, Jun 01, 2026 at 11:52:29PM -0700, Brent Page wrote:
> > That loop adds three new "blank" sitds to iso_sched. At the end of
> > sitd_urb_transaction(), the picture looks like this:
> >
> > +-> iso_sched <--> new2 <--> new1 <--> new0 <-+
> > | |
> > +---------------------------------------------+
>
> But for an iso endpoint, the schedule should never be empty (except for
> when the very first urb comes in). So, there are sitds that are already
> scheduled before the new sitds come in, and I think in total the picture
> will look like, e.g.,
> +-> stream <--> new2 <--> new1 <--> new0 <--
> | --> prev2 <--> prev1 <--> prev0 <-+
> | |
> +-------------------------------------------------+
I'm not sure what point in the code you're talking about. Also, you
don't seem to have noticed that iso_sched is different from stream.
> Then, after
>
> 2177 sitd = list_entry(iso_sched->td_list.next,
> 2178 struct ehci_itd, sitd_list);
> 2179 list_move_tail(&sitd->sitd_list, &stream->td_list);
>
> is executed 3x, the schedule will look like
> +-> stream <--> prev2 <--> prev1 <--> prev0 <--
> | --> new2 <--> new1 <--> new0 <-+
> | |
> +-------------------------------------------------+
These labels for the sitd's aren't useful. What matters is not the
order in which they were allocated but rather the order they occupy in
the queue, which must match the order of the submitted URBs and of the
iso_packet_descriptors within each URB. That is, the picture should be:
+-> stream <--> urb0_0 <--> urb0_1 <--> urb0_2 <--
| --> urb1_0 <--> urb1_1 <--> urb1_2 <-+
| |
+------------------------------------------------------+
which is what I wrote earlier. Here "urb1_0" is merely a different name
for new2 (after sitd_patch() has filled in its details), and the same
goes for "urb1_1" and new1, and "urb1_2" and new0.
Alan Stern
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] inflate max_tt_usecs and implement sitd backpointers
2026-06-02 6:42 ` [PATCH] " Brent Page
@ 2026-06-02 19:35 ` Alan Stern
2026-06-22 5:43 ` Brent Page
0 siblings, 1 reply; 11+ messages in thread
From: Alan Stern @ 2026-06-02 19:35 UTC (permalink / raw)
To: Brent Page; +Cc: linux-usb
On Tue, Jun 02, 2026 at 06:42:01AM +0000, Brent Page wrote:
> ---
> This is a version of the patch with new formatting; the content
> otherwise is equivalent to the combination of the two prior patches in
> this thread. I ran a number of tests of the patch's functionality for
> iso-in transfers from a full-speed peripheral with a single endpoint
> with a wMaxPacketSize of 1023. The transfers themselves were 1000
> bytes. I tried bIntervals of both 1 frame as well as 2 frames.
> Setup 1:
> - configure this "main" peripheral to send (48x 1s followed by 32x 0s)
You mean six 0xff bytes followed by four 0x00 bytes, right?
> repeated 100 times in each transfer (1000 total data bytes)
> - "transfer" here means the data that is sent in one frame
> - the 48x 1s will require substantial bit stuffing, and the 32x 0s
> will require none; the actual number of bytes will be something
> like (7/6*48 + 32)/80 * 1000 = 1100 bus bytes
> - on a single USB 2 port going in to the computer, plug in a USB 2
> one-to-four splitter
The correct term is "four-port hub". "Splitter" implies something
different (the same signals going in and out of all the ports, which
is not what happens with USB).
> - plug in the main peripheral along with a wireless mouse receiver,
> wireless keyboard receiver, and a high-speed webcam
> - Result: for both bInterval=1 frame and bInterval=2 frames for the main
> peripheral, all four connected devices function well
> - notes: the main peripheral has to be plugged in before the wireless
> mouse and keyboard receivers; otherwise the main peripheral's
> 1023-byte endpoint is rejected
Unfortunate, but very difficult to fix, as I'm sure you're aware.
> Setup 2:
> - same as Setup 1, except configure the main peripheral to send (72x 1s
> followed by 8x 0s) repeated 100 times in each transfer
> - ~(7/6*72 + 8)/80 * 1000 = 1150 bus bytes
> - Result: for both bInterval=1 frame and bInterval=2 frames, the main
> peripheral's 1023-byte endpoint is accepted by the computer, but all
> attempts by user space to retrieve urbs (ioctl-REAPURBNDELAY sys
> calls) yield error code -11, EAGAIN (instructing the caller to try
> again).
Why is this? What goes wrong? That sort of issue should never arise.
> - notes: if the data transfers are instead configured to be (64x 1s
> followed by 16x 0s) repeated 100 times, then an occasional urb is
> successfully retrieved. So, it does seem like the bit-stuffing
> requirements are an influential factor
> Setup 3:
> - same data for the main peripheral as in the first bullet of Setup 2
> - don't use a splitter; plug the main peripheral directly into the
> computer's USB port
> - Result: for both bInterval=1 frame and bInterval=2 frames, the main
> peripheral's usb connection is robust
Why does this case behave differently? Does the computer have an OHCI
or UHCI host controller in addition to EHCI?
> Test summary: the patch works for iso-in transfers that are
> - 1150 bus bytes in a frame if the associated peripheral is plugged
> directly into a USB port
> - 1100 bus bytes in a frame alongside transfers from two full-speed
> interrupt devices and a high-speed webcam, with all devices plugged
> into the same USB port via a splitter
>
> Weak point: The patch doubles the number of sitds that are allocated for
> any given urb. This is true regardless of whether the urb's transfers
> require frame-hopping CSPLITS. I figured that the memory in these
> structs was negligible, but I actually get ENOMEM from line ~1400 of
> drivers/usb/core/devio.c in kernel version 3.18.120 when I tried to submit
> 4x urbs with 33 transfers each (requiring 4*33*2=264 sitds for this
> patch's code) on my 2017 Android tablet. Things work if I only submit
> 100 iso transfers instead. Also, submitting 120 transfers doesn't cause
> any issues on my 2022 Android smartphone. Maybe the extra sitd
> allocation should be done on-demand instead of by default? So, start
> off by allocating an sitd for each transfer, and only add an extra sitd
> if a transfer has frame-hopping CSPLITS?
Yes, that is the approach the driver uses everywhere else. Not to
mention that it's foolish to allocate twice as many sitd's as the URB
will actually use.
Alan Stern
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers
2026-06-02 14:26 ` Alan Stern
@ 2026-06-22 5:19 ` Brent Page
0 siblings, 0 replies; 11+ messages in thread
From: Brent Page @ 2026-06-22 5:19 UTC (permalink / raw)
To: Alan Stern; +Cc: linux-usb, Michal Pecio
> I'm not sure what point in the code you're talking about.
I've been talking about line 2086
list_add(&sitd->sitd_list, &iso_sched->td_list);
and lines 2177-2179
sitd = list_entry(sched->td_list.next,
struct ehci_sitd, sitd_list);
list_move_tail(&sitd->sitd_list, &stream->td_list);
in the v7.1 kernel.
> Also, you don't seem to have noticed that iso_sched is different from
> stream.
Yes, I did mix these up. The new patch (v3 soon) is better (no bug when crossing
urb boundaries).
From,
Brent Page
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] inflate max_tt_usecs and implement sitd backpointers
2026-06-02 19:35 ` Alan Stern
@ 2026-06-22 5:43 ` Brent Page
0 siblings, 0 replies; 11+ messages in thread
From: Brent Page @ 2026-06-22 5:43 UTC (permalink / raw)
To: Alan Stern; +Cc: linux-usb
---
> You mean six 0xff bytes followed by four 0x00 bytes, right?
Yes
> Why is this? What goes wrong? That sort of issue should never arise.
Now, the test results are as follows:
Setup 3:
- configure the "main" (1023-byte-endpoint full-speed iso-in) peripheral to
send nine 0xff bytes followed by one 0x00 byte 100 times per transfer
- plug the main peripheral into a four-port USB 2 hub that plugs in to a
computer's USB 2 port. Don't plug anything else into the hub.
- for both bInterval=1 and 2, there's a good data link between the peripheral
and the computer
- I see a bit error in the last word of each transfer; it's typically 1101
or 1110 instead of 1111, but I see these for USB 3 ports as well. Maybe a
ferrite bead on the peripheral would help? That's the only error I see in
the data link.
Setup 4:
- same data as for Setup 3
- also plug a high-speed webcam into the four-port hub
- bInterval=2: both the "main" peripheral and the webcam work well
- bInterval=1: ~50% of the raster scan lines in the webcam's stream are blank.
The main peripheral's data link remains good (same near-optimal quality as
for Setup 3).
Setup 5:
- same data for the main peripheral as in Setups 3 and 4.
- in addition to the main peripheral, plug in a full-speed HID with an 8-byte
interrupt endpoint to the four-port hub
- bInterval=1: the HID doesn't work
- bInterval=2: two interrupt full-speed HIDs work well alongside the main
peripheral. Would it make sense if the first time plugging one in has a
50% success rate (presumably if it fortuitously gets scheduled for the
free frame) and all subsequent unplug/replug events work after the first
successful one? Seems like that might be the case.
Setup 1:
- repeating from my previous email
- configure the main peripheral to send five 0xff bytes followed by five 0x00
bytes 100 times per transfer
- plug in the main peripheral, two full-speed HIDs, and the high-speed webcam
into the four-port hub
- bInterval=1 and bInterval=2: everything works great. Considering that this
worked even before I fixed up the patch, it doesn't depend on backpointers at
all.
So, the patch improves things (it can do a data stream from the main peripheral
with 90% ones through the four-port hub. Without the patch, only 50% ones is
possible), but isn't perfect. One change that's necessary for the patch to work
is in scan_isoc, which now doesn't consider an sitd to be expired until it is
three frames in the past. Looking through the history of ehci-sched, I see
that sitds originally were considered expired if they were a single frame in
the past. Then, commit 22e1869 changed this to two frames in the past to
accommodate some aberrant HW. This current backpointer patch allows sitds to
complete one frame after they appear in the queue as a matter of normal course,
and I guess that the four-port hub that I'm using occasionally reports sitd
completion late. So, the expiration frame needs to be delayed one frame
further. BTW, this change for whatever reason is only required to get the
bInterval=1 case to work.
This frame delay I think is a possible cause of remaining failures in the
test results. The TT is clearly busy on the main peripheral's data until past
the primary frame, so maybe getting an HID to work for the Setup 5: bInterval=1
case would require FSTNs? Also, maybe the missed raster scan lines from the
webcam can be mitigated by allowing itds to complete in future frames,
specifically by tweaking scan_isoc? Considering that the itds don't use the
TT, I don't know why this would matter, but I still plan to try it.
> Why does this case behave differently? Does the computer have an OHCI
or UHCI host controller in addition to EHCI?
Just EHCI according to the output of /sys/kernel/debug/usb/devices
> > So, start off by allocating an sitd for each transfer, and only add an extra
> > sitd if a transfer has frame-hopping CSPLITS?
> Yes, that is the approach the driver uses everywhere else. Not to mention
> that it's foolish to allocate twice as many sitd's as the URB will actually
> use.
Done now. I broke out the sitd allocation part of sitd_urb_transaction into
new function: allocate_sitds. This new function is called once in
sitd_urb_transacton, making this latter function nearly* equivalent to what it
is in the current kernel. Then, iso_stream_schedule gets
called, which determines whether the urb requires 2 sitds for each packet. If
the urb does, allocate_sitds is called a second time.
An alternative would be to not allocate any sitds until after
iso_stream_schedule gets called, and then do either 1 or 2 for each packet as
appropriate. This would conserve fewer the choices made in the current driver,
which allocates sitds before iso_stream_schedule, but seems harmless and would
make the patched driver slightly more streamlined. Do you have any immediate
objections to this alternative?
The exception to *, above, is that allocate_sitds now does not recycle sitds
from the free list until they are two frames in the past, as opposed to just
one frame in the past like before. I guess this may lead to an occasional
extra sitd being allocated if urbs are resubmitted soon after they finish,
i.e., when the last sitd of the finished urb isn't 3 frames in the past yet.
Is the harm here minimal enough?
One more thing on this topic: allocate_sitds gets called with ehci->lock held
and also ends with ehci->lock held, but in the middle of allocate_sitds, the
lock gets unlocked and then relocked surrounding a call to dma_pool_alloc. For
correct unlocking here, allocate sitds takes the associated irq save/restore
flags as an argument and passes it on in its call to spin_unlock_irqrestore.
Easy enough. But, allocate_sitds also needs to be able to communicate the
flags that are set when it relocks the lock back up to the caller of
allocate_sitds. So, I made the flags argument of allocate_sitds a pointer that
gets dereferenced in allocate_sitds calls to spin_(un)lock_irq(restore/save).
Is that objectionable? Seems to work (specifically, ensures that the whole
computer doesn't crash, which is what I saw when I got this wrong), but maybe
diverges from current practice.
In principal, there's a nasty failure mode for the bInterval=1 case. It stems
from ehci1's prescription - not this patch's particular implementation.
Namely, a single sitd failure breaks the stream for all subsequent sitds. This
is because those subsequent sitds are initialized in a Do Complete Split state.
An error in a single sitd causes the subsequent sitd to never transition out of
the Do Complete Split state. This subsequent sitd then also isn't successful,
causing the next sitd to also fail, and so forth.
I haven't encountered this failure mode, so it seems that it is addressed in
HW. Maybe after a few failed sitds, the HC simply ignores the initial Do
Complete Split flag? I didn't see any comment like that when reading ehci1,
but maybe it's in there.
v3 of the patch is below.
drivers/usb/host/ehci-sched.c | 248 +++++++++++++++++++++++++++-------
drivers/usb/host/ehci.h | 12 +-
2 files changed, 206 insertions(+), 54 deletions(-)
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index a241337c9..67ef1a918 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -285,13 +285,14 @@ static void compute_tt_budget(u8 budget_table[EHCI_BANDWIDTH_SIZE],
for (uf = ps->phase_uf; uf < 8; ++uf) {
x += budget_line[uf];
- /* Each microframe lasts 125 us */
- if (x <= 125) {
+ /* Cumulative tt_usecs can be as large as 145
+ * us */
+ if (x <= 145) {
budget_line[uf] = x;
break;
}
- budget_line[uf] = 125;
- x -= 125;
+ budget_line[uf] = 145;
+ x -= 145;
}
}
}
@@ -313,7 +314,12 @@ static int __maybe_unused same_tt(struct usb_device *dev1,
#ifdef CONFIG_USB_EHCI_TT_NEWSCHED
static const unsigned char
-max_tt_usecs[] = { 125, 125, 125, 125, 125, 125, 30, 0 };
+/*
+ * tt_usecs includes worst-case bit stuffing time, so the transactions
+ * budgeted for a given 125 us uframe can have cumulative tt_usecs as large
+ * as 145 us and still possibly fit into the uframe.
+ */
+max_tt_usecs[] = { 145, 145, 145, 145, 145, 145, 35, 0 };
/* carryover low/fullspeed bandwidth that crosses uframe boundries */
static inline void carryover_tt_bandwidth(unsigned short tt_usecs[8])
@@ -378,13 +384,13 @@ static int tt_available(
if (max_tt_usecs[uframe] <= tt_usecs[uframe])
return 0;
- /* special case for isoc transfers larger than 125us:
+ /* special case for isoc transfers larger than max_tt_usecs:
* the first and each subsequent fully used uframe
* must be empty, so as to not illegally delay
* already scheduled transactions
*/
- if (usecs > 125) {
- int ufs = (usecs / 125);
+ if (usecs > 145) {
+ int ufs = (usecs / 145);
for (i = uframe; i < (uframe + ufs) && i < 8; i++)
if (tt_usecs[i] > 0)
@@ -1302,7 +1308,7 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd *ehci,
{
unsigned uframe;
unsigned i, j;
- unsigned s_mask, c_mask, m;
+ unsigned s_mask, c_mask, c_mask2, m;
int usecs = stream->ps.usecs;
int c_usecs = stream->ps.c_usecs;
int tt_usecs = stream->ps.tt_usecs;
@@ -1328,16 +1334,18 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd *ehci,
} else { /* Full speed */
s_mask = stream->ps.cs_mask;
c_mask = s_mask >> 8;
+ c_mask2 = stream->ps.c_mask2 >> 8;
- /* NOTE: adjustment needed for frame overflow */
for (i = uframe; i < EHCI_BANDWIDTH_SIZE;
i += stream->ps.bw_uperiod) {
- for ((j = stream->ps.phase_uf, m = 1 << j); j < 8;
+ for ((j = 0, m = 1 << j); j < 8;
(++j, m <<= 1)) {
if (s_mask & m)
ehci->bandwidth[i+j] += usecs;
else if (c_mask & m)
ehci->bandwidth[i+j] += c_usecs;
+ if (c_mask2 & m)
+ ehci->bandwidth[(i+8+j)%EHCI_BANDWIDTH_SIZE] += c_usecs;
}
}
@@ -1388,7 +1396,7 @@ sitd_slot_ok(
struct ehci_tt *tt
)
{
- unsigned mask, tmp;
+ unsigned mask, c_mask2, tmp;
unsigned frame, uf;
mask = stream->ps.cs_mask << (uframe & 7);
@@ -1397,10 +1405,38 @@ sitd_slot_ok(
if (((stream->ps.cs_mask & 0xff) << (uframe & 7)) >= (1 << 7))
return 0;
- /* for IN, don't wrap CSPLIT into the next frame */
- if (mask & ~0xffff)
+ /* for IN, don't wrap SSPLIT too far into next frame */
+ if (mask & ~0x7ffff)
return 0;
+ c_mask2 = mask >> 16;
+ mask = mask & 0xffff;
+ if((c_mask2 & 1) && (c_mask2 & 1<<1) && (c_mask2 & 1<<2)) {
+ /* if BuFrame6 is the last uframe in which a transaction is
+ * budgeted, the transaction will initially be configured to
+ * have CSPLITS in BuFrame7 as well as BuFrames 0 and 1 of
+ * the following frame (HuFrames 0,1,2)
+ */
+ /* below: usb2 spec 11.18.4.3.c paragraph 3 */
+ if(mask & 1) {
+ c_mask2 = 1;
+ } else {
+ c_mask2 = (1<<2)-1;
+ }
+ } else if ((c_mask2 & 1) && (c_mask2 & 1<<1)) {
+ /* if BuFrame5 is the last uframe in which a transaction is
+ * budgeted, the transaction will initially be configured to
+ * have CSPLITS in BuFrames 6 and 7 as well as BuFrame 0 of
+ * the following frame (HuFrames 7,0,1)
+ */
+ /* below: usb2 spec 11.18.4.3.c paragraph 2 */
+ if(mask & 1) {
+ c_mask2 = 1;
+ }
+ }
+ if((c_mask2 & mask & 1<<1))
+ return 0; /* ehci1 4.12.3.1 */
+
/* check bandwidth */
uframe &= stream->ps.bw_uperiod - 1;
frame = uframe >> 3;
@@ -1445,13 +1481,22 @@ sitd_slot_ok(
if (ehci->bandwidth[uf+i] > max_used)
return 0;
}
+ tmp = 1;
+ for (i = 0; i < 2; (++i, tmp <<= 1)) {
+ if ((stream->ps.c_mask2 & tmp) == 0)
+ continue;
+ if (ehci->bandwidth[(uf+8+i) % EHCI_BANDWIDTH_SIZE] > max_used)
+ return 0;
}
uframe += stream->ps.bw_uperiod;
} while (uframe < EHCI_BANDWIDTH_SIZE);
- stream->ps.cs_mask <<= uframe & 7;
+ stream->ps.cs_mask = mask;
+ stream->ps.c_mask2 = c_mask2 << 8;
stream->splits = cpu_to_hc32(ehci, stream->ps.cs_mask);
+ stream->c_splits2 = cpu_to_hc32(ehci, stream->ps.c_mask2);
+
return 1;
}
@@ -2028,54 +2073,45 @@ sitd_sched_init(
}
static int
-sitd_urb_transaction(
+allocate_sitds(
struct ehci_iso_stream *stream,
struct ehci_hcd *ehci,
struct urb *urb,
- gfp_t mem_flags
+ struct ehci_iso_sched *iso_sched,
+ gfp_t mem_flags,
+ unsigned long *ehci_lock_flags
)
{
+ /* caller must hold ehci->lock and provide associated irq flags */
struct ehci_sitd *sitd;
dma_addr_t sitd_dma;
int i;
- struct ehci_iso_sched *iso_sched;
- unsigned long flags;
-
- iso_sched = iso_sched_alloc(urb->number_of_packets, mem_flags);
- if (iso_sched == NULL)
- return -ENOMEM;
-
- sitd_sched_init(ehci, iso_sched, stream, urb);
/* allocate/init sITDs */
- spin_lock_irqsave(&ehci->lock, flags);
for (i = 0; i < urb->number_of_packets; i++) {
- /* NOTE: for now, we don't try to handle wraparound cases
- * for IN (using sitd->hw_backpointer, like a FSTN), which
- * means we never need two sitds for full speed packets.
- */
-
/*
- * Use siTDs from the free list, but not siTDs that may
+ * Use iTDs from the free list, but not iTDs that may
* still be in use by the hardware.
*/
if (likely(!list_empty(&stream->free_list))) {
sitd = list_first_entry(&stream->free_list,
struct ehci_sitd, sitd_list);
- if (sitd->frame == ehci->now_frame)
+ /* include frame+1 to accommodate frame-hopping CSPLITS */
+ if (sitd->frame == ehci->now_frame ||
+ sitd->frame + 1 == ehci->now_frame)
goto alloc_sitd;
list_del(&sitd->sitd_list);
sitd_dma = sitd->sitd_dma;
} else {
- alloc_sitd:
- spin_unlock_irqrestore(&ehci->lock, flags);
+alloc_sitd:
+ spin_unlock_irqrestore(&ehci->lock, *ehci_lock_flags);
sitd = dma_pool_alloc(ehci->sitd_pool, mem_flags,
&sitd_dma);
- spin_lock_irqsave(&ehci->lock, flags);
+ spin_lock_irqsave(&ehci->lock, *ehci_lock_flags);
if (!sitd) {
iso_sched_free(stream, iso_sched);
- spin_unlock_irqrestore(&ehci->lock, flags);
+ /* caller should release ehic->lock */
return -ENOMEM;
}
}
@@ -2085,6 +2121,32 @@ sitd_urb_transaction(
sitd->frame = NO_FRAME;
list_add(&sitd->sitd_list, &iso_sched->td_list);
}
+ return 0;
+}
+
+static int
+sitd_urb_transaction(
+ struct ehci_iso_stream *stream,
+ struct ehci_hcd *ehci,
+ struct urb *urb,
+ gfp_t mem_flags
+)
+{
+ struct ehci_iso_sched *iso_sched;
+ unsigned long flags;
+ int status;
+
+ iso_sched = iso_sched_alloc(urb->number_of_packets, mem_flags);
+ if (iso_sched == NULL)
+ return -ENOMEM;
+
+ sitd_sched_init(ehci, iso_sched, stream, urb);
+ spin_lock_irqsave(&ehci->lock, flags);
+ status = allocate_sitds(stream, ehci, urb, iso_sched, mem_flags, &flags);
+ if(status) {
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ return status;
+ }
/* temporarily store schedule info in hcpriv */
urb->hcpriv = iso_sched;
@@ -2107,12 +2169,29 @@ sitd_patch(
{
struct ehci_iso_packet *uf = &iso_sched->packet[index];
u64 bufp;
+ __hc32 transaction;
sitd->hw_next = EHCI_LIST_END(ehci);
sitd->hw_fullspeed_ep = stream->address;
- sitd->hw_uframe = stream->splits;
- sitd->hw_results = uf->transaction;
- sitd->hw_backpointer = EHCI_LIST_END(ehci);
+ sitd->hw_backpointer = cpu_to_hc32(ehci, sitd->backpointer_sitd_dma);
+ transaction = uf->transaction;
+
+ if(stream->ps.period!=1 && sitd->backpointer_sitd_dma!=1) {
+ sitd->hw_uframe=stream->c_splits2;
+ } else {
+ /*
+ * frame-hopping CSPLITS get skipped if the sitd doesn't
+ * start in the Do Complete Split state.
+ */
+ sitd->hw_uframe=stream->splits|stream->c_splits2;
+ }
+
+ if(sitd->backpointer_sitd_dma!=1) {
+ /* start in Do Complete Split mode, ehci1 4.12.3.3.2.1 */
+ transaction |= cpu_to_hc32(ehci, SITD_STS_STS);
+ }
+
+ sitd->hw_results = transaction;
bufp = uf->bufp;
sitd->hw_buf[0] = cpu_to_hc32(ehci, bufp);
@@ -2149,13 +2228,26 @@ static void sitd_link_urb(
unsigned next_uframe;
struct ehci_iso_sched *sched = urb->hcpriv;
struct ehci_sitd *sitd;
+ struct ehci_sitd *sitd_before;
+ int sitd_mult;
next_uframe = stream->next_uframe;
- if (list_empty(&stream->td_list))
+ if (list_empty(&stream->td_list)) {
/* usbfs ignores TT bandwidth */
ehci_to_hcd(ehci)->self.bandwidth_allocated
+= stream->bandwidth;
+ sitd_before = NULL;
+ } else {
+ sitd_before = list_last_entry(&stream->td_list,
+ struct ehci_sitd, sitd_list);
+ /* only add a bkptr to sitds scheduled in frames past now_frame */
+ if(sitd_before->frame == ehci->now_frame ||
+ sitd_before->frame + 1 == ehci->now_frame)
+ {
+ sitd_before = NULL;
+ }
+ }
if (ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs == 0) {
if (ehci->amd_pll_fix == 1)
@@ -2163,11 +2255,15 @@ static void sitd_link_urb(
}
ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs++;
+ if(stream->ps.c_mask2 && stream->ps.period!=1)
+ sitd_mult = 2;
+ else
+ sitd_mult = 1;
/* fill sITDs frame by frame */
- for (packet = sched->first_packet, sitd = NULL;
- packet < urb->number_of_packets;
- packet++) {
+ for (int i = sched->first_packet, sitd = NULL;
+ i < sitd_mult * urb->number_of_packets;
+ i++) {
/* ASSERT: we have all necessary sitds */
BUG_ON(list_empty(&sched->td_list));
@@ -2176,15 +2272,41 @@ static void sitd_link_urb(
sitd = list_entry(sched->td_list.next,
struct ehci_sitd, sitd_list);
+ if(stream->ps.cs_mask2 && sitd_before!=NULL &&
+ !(sched->first_packet!=0 && i==sched->first_packet) &&
+ (stream->ps.period==1 || i%2==1) ) {
+
+ sitd->backpointer_sitd_dma = sitd_before->sitd_dma;
+ } else {
+ sitd->backpointer_sitd_dma = 1;
+ }
+
list_move_tail(&sitd->sitd_list, &stream->td_list);
sitd->stream = stream;
sitd->urb = urb;
+ if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
+ packet = i/2;
+ sitd->first_in_pair = i%2 == 0;
+ } else {
+ packet = i;
+ }
+
sitd_patch(ehci, stream, sitd, sched, packet);
sitd_link(ehci, (next_uframe >> 3) & (ehci->periodic_size - 1),
sitd);
- next_uframe += stream->uperiod;
+ if(stream->ps.c_mask2 && (stream->ps.period!=1)) {
+ if((i%2)==0) {
+ /* next sitd only has frame-hopping CSPLITS */
+ next_uframe += 8;
+ } else if ((i%2)==1) {
+ next_uframe += stream->uperiod - 8;
+ }
+ } else {
+ next_uframe += stream->uperiod;
+ }
+ sitd_before = sitd;
}
stream->next_uframe = next_uframe & (mod - 1);
@@ -2220,6 +2342,13 @@ static bool sitd_complete(struct ehci_hcd *ehci, struct ehci_sitd *sitd)
struct ehci_iso_stream *stream = sitd->stream;
bool retval = false;
+ if(stream->force_sitd_done) {
+ /* sitd is the 2nd in a pair for a bIntrvl!=1 case w/ bkptrs */
+ stream->force_sitd_done = false;
+ goto done;
+ }
+ stream->force_sitd_done = sitd->first_in_pair; /* for next sitd */
+
urb_index = sitd->index;
desc = &urb->iso_frame_desc[urb_index];
t = hc32_to_cpup(ehci, &sitd->hw_results);
@@ -2245,7 +2374,7 @@ static bool sitd_complete(struct ehci_hcd *ehci, struct ehci_sitd *sitd)
}
/* handle completion now? */
- if ((urb_index + 1) != urb->number_of_packets)
+ if (likely ((urb_index + 1) != urb->number_of_packets))
goto done;
/*
@@ -2293,6 +2422,7 @@ static int sitd_submit(struct ehci_hcd *ehci, struct urb *urb,
gfp_t mem_flags)
{
int status = -EINVAL;
+ int status2 = 0;
unsigned long flags;
struct ehci_iso_stream *stream;
@@ -2335,13 +2465,21 @@ static int sitd_submit(struct ehci_hcd *ehci, struct urb *urb,
goto done_not_linked;
status = iso_stream_schedule(ehci, urb, stream);
if (likely(status == 0)) {
+ if(stream->ps.c_mask2 && stream->ps.period!=1) {
+ /*
+ * stream has frame-hopping CSPLITS and period isn't 1:
+ * 2 sitds required for each packet
+ */
+ status2 = allocate_sitds(stream, ehci, urb, urb->hcpriv, mem_flags,
+ &flags);
+ }
sitd_link_urb(ehci, urb, ehci->periodic_size << 3, stream);
} else if (status > 0) {
status = 0;
ehci_urb_done(ehci, urb, 0);
- } else {
- usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
}
+ if(status<0 || status2<0)
+ usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
done_not_linked:
spin_unlock_irqrestore(&ehci->lock, flags);
done:
@@ -2433,10 +2571,16 @@ static void scan_isoc(struct ehci_hcd *ehci)
* No need to check for activity unless the
* frame is current.
*/
+ /* delay the expiration frame by one frame past now_frame to
+ * accommodate HW delay (commit 22e1869). delay it one further
+ * frame to accommodate frame-hopping csplits
+ */
if (((frame == now_frame) ||
- (((frame + 1) & fmask) == now_frame))
+ (((frame + 1) & fmask) == now_frame) ||
+ (((frame + 2) & fmask) == now_frame))
&& live
- && (q.sitd->hw_results & SITD_ACTIVE(ehci))) {
+ && (q.sitd->hw_results & SITD_ACTIVE(ehci))
+ && !q.sitd->stream->force_sitd_done) {
q_p = &q.sitd->sitd_next;
hw_p = &q.sitd->hw_next;
@@ -2482,8 +2626,8 @@ static void scan_isoc(struct ehci_hcd *ehci)
if (frame == now_frame)
return;
- /* The last frame may still have active siTDs */
- ehci->last_iso_frame = frame;
+ /* next call's scan begins at this call's now_frame-2 */
+ ehci->last_iso_frame = (frame - 1) & fmask;
frame = (frame + 1) & fmask;
goto restart;
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index d7a3c8d13..e4a387976 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -50,7 +50,9 @@ struct ehci_per_sched {
struct usb_host_endpoint *ep;
struct list_head ps_list; /* node on ehci_tt's ps_list */
u16 tt_usecs; /* time on the FS/LS bus */
- u16 cs_mask; /* C-mask and S-mask bytes */
+ u32 cs_mask; /* C-mask and S-mask bytes */
+ u16 c_mask2; /* C-mask for frame-hopping CSPLITS */
+
u16 period; /* actual period in frames */
u16 phase; /* actual phase, frame part */
u8 bw_phase; /* same, for bandwidth
@@ -485,11 +487,13 @@ struct ehci_iso_stream {
u8 highspeed;
struct list_head td_list; /* queued itds/sitds */
struct list_head free_list; /* list of unused itds/sitds */
+ bool force_sitd_done;/* for bIntrvl!=1 case w/ bkptrs */
/* output of (re)scheduling */
struct ehci_per_sched ps; /* scheduling info */
unsigned next_uframe;
- __hc32 splits;
+ __hc32 splits; /* C-mask and S-mask */
+ __hc32 c_splits2; /* C-mask for frame-hopping CSPLITS */
/* the rest is derived from the endpoint descriptor,
* including the extra info for hw_bufp[0..2]
@@ -579,7 +583,11 @@ struct ehci_sitd {
/* the rest is HCD-private */
dma_addr_t sitd_dma;
+ dma_addr_t backpointer_sitd_dma;
+
union ehci_shadow sitd_next; /* ptr to periodic q entry */
+ bool first_in_pair; /* for period!=1 w/ bkptrs */
+
struct urb *urb;
struct ehci_iso_stream *stream; /* endpoint's queue */
--
2.39.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-22 5:43 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18 6:42 [PATCH] USB: EHCI: inflate max_tt_usecs and implement sitd backpointers Brent Page
2026-05-18 18:24 ` Brent Page
2026-05-20 17:05 ` Brent Page
2026-05-21 5:52 ` Greg KH
2026-05-22 14:18 ` Alan Stern
2026-06-02 6:42 ` [PATCH] " Brent Page
2026-06-02 19:35 ` Alan Stern
2026-06-22 5:43 ` Brent Page
2026-06-02 6:52 ` [PATCH] USB: EHCI: " Brent Page
2026-06-02 14:26 ` Alan Stern
2026-06-22 5:19 ` Brent Page
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox