[PATCH RESEND] usb: octeon-hcd: keep SOF interrupts while a periodic pipe is due

[PATCH RESEND] usb: octeon-hcd: keep SOF interrupts while a periodic pipe is due

[Orgad Shaneh]

cvmx_usb_schedule() now sleeps on an hrtimer until the nearest future
periodic deadline instead of counting frames down on every SOF. The scan
that finds that deadline only looks at pipes with

	next_tx_frame > usb->frame_number

which is right for picking the deadline, but it means a periodic pipe
that is already due and was *not* started is invisible to the decision.
A pipe can be due and not started for reasons that resolve on their own:
no idle hardware channel this round, its split window is closed
(cvmx_usb_find_ready_pipe() rejects both), or another pipe owns
usb->active_split.

Such a pipe is only retried from a SOF - cvmx_usb_next_pipe() looks at
the isochronous and interrupt lists only when is_sof is true - so when
some other periodic pipe sets a far deadline, the driver masks SOF, arms
the timer for that far deadline and the due pipe waits for it. With a
hub's status pipe as the far one, that is hundreds of frames; the code
allows up to 8000 (one second) before it falls back to SOF.

Before the hrtimer this could not happen: any pipe with a future
deadline set need_sof, so SOF stayed enabled every frame and the due
pipe was retried on the next one.

Note the pipe that is due and not started, and keep SOF enabled in that
case, exactly as the old code did. Nothing changes when no periodic pipe
is due, which is the case the timer was added for.

Control and bulk pipes are excluded deliberately: their next_tx_frame is
not updated on completion, so they are almost always "due" and would
keep SOF enabled permanently.

Fixes: aebbc7d63407 ("usb: octeon-hcd: sleep on an hrtimer for far-off periodic transfers")
Assisted-by: LLM
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
Resend of
https://lore.kernel.org/all/20260915081724.25637-1-orgads@gmail.com/
with no code change; the Assisted-by tag is now in the form
coding-assistants.rst documents.

diff --git a/drivers/usb/host/octeon-hcd.c b/drivers/usb/host/octeon-hcd.c
--- a/drivers/usb/host/octeon-hcd.c
+++ b/drivers/usb/host/octeon-hcd.c
@@ -1923,6 +1923,7 @@ static void cvmx_usb_schedule(struct octeon_hcd *usb, int is_sof)
 	struct cvmx_usb_pipe *pipe;
 	int need_sof;
 	u64 min_due;
+	bool due_now;
 	enum cvmx_usb_transfer ttype;
 
 	if (usb->init_flags & CVMX_USB_INITIALIZE_FLAGS_NO_DMA) {
@@ -1964,12 +1965,23 @@ done:
 	 */
 	need_sof = 0;
 	min_due = ~0ull;
+	due_now = false;
 	for (ttype = CVMX_USB_TRANSFER_CONTROL;
 	     ttype <= CVMX_USB_TRANSFER_INTERRUPT; ttype++) {
 		list_for_each_entry(pipe, &usb->active_pipes[ttype], node) {
-			if (pipe->next_tx_frame > usb->frame_number &&
-			    pipe->next_tx_frame < min_due)
+			if (pipe->next_tx_frame <= usb->frame_number) {
+				/*
+				 * A periodic pipe that is due but was not
+				 * started - no idle channel, or its split
+				 * window is closed - is only retried from a
+				 * SOF, so do not sleep past it.
+				 */
+				if (ttype == CVMX_USB_TRANSFER_ISOCHRONOUS ||
+				    ttype == CVMX_USB_TRANSFER_INTERRUPT)
+					due_now = true;
+			} else if (pipe->next_tx_frame < min_due) {
 				min_due = pipe->next_tx_frame;
+			}
 		}
 	}
 	if (min_due != ~0ull) {
@@ -1983,7 +1995,7 @@ done:
 		 * a few frames. Stay well below the 16383-frame wrap of
 		 * HFNUM. One (micro)frame is 125us in high-speed mode.
 		 */
-		if (delta <= 4 || delta > 8000)
+		if (due_now || delta <= 4 || delta > 8000)
 			need_sof = 1;
 		else
 			hrtimer_start(&usb->sof_timer,
-- 
2.47.0

Re: [PATCH RESEND] usb: octeon-hcd: keep SOF interrupts while a periodic pipe is due

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: octeon-hcd: periodic pipe starvation due to bypassed due_now evaluation
- [High] usb: octeon-hcd: control/bulk split transfers stall with SOF disabled
--

commit ba5443b476ec252047a7c43f65661c3131a5d94c
Author: Orgad Shaneh <orgads@gmail.com>

usb: octeon-hcd: keep SOF interrupts while a periodic pipe is due

This commit modifies cvmx_usb_schedule() to track when a periodic pipe is
due but not yet started, setting a due_now flag. This flag is used to keep
SOF interrupts enabled instead of sleeping on an hrtimer, ensuring due pipes
are retried promptly.

Link: https://lore.kernel.org/all/20260915081724.25637-1-orgads@gmail.com/

> diff --git a/drivers/usb/host/octeon-hcd.c b/drivers/usb/host/octeon-hcd.c
> index 256e459ff6d8d..197e04317a264 100644
> --- a/drivers/usb/host/octeon-hcd.c
> +++ b/drivers/usb/host/octeon-hcd.c
> @@ -1964,12 +1965,23 @@ static void cvmx_usb_schedule(struct octeon_hcd *usb, int is_sof)
>  	 */
>  	need_sof = 0;
>  	min_due = ~0ull;
> +	due_now = false;
>  	for (ttype = CVMX_USB_TRANSFER_CONTROL;
>  	     ttype <= CVMX_USB_TRANSFER_INTERRUPT; ttype++) {
>  		list_for_each_entry(pipe, &usb->active_pipes[ttype], node) {
> -			if (pipe->next_tx_frame > usb->frame_number &&
> -			    pipe->next_tx_frame < min_due)
> +			if (pipe->next_tx_frame <= usb->frame_number) {
> +				/*
> +				 * A periodic pipe that is due but was not
> +				 * started - no idle channel, or its split
> +				 * window is closed - is only retried from a
> +				 * SOF, so do not sleep past it.
> +				 */
> +				if (ttype == CVMX_USB_TRANSFER_ISOCHRONOUS ||
> +				    ttype == CVMX_USB_TRANSFER_INTERRUPT)
> +					due_now = true;

[Severity: High]
Can this explicit exclusion of control and bulk pipes cause SPLIT COMPLETE
transactions to stall indefinitely if SOF interrupts are disabled?

If a low/full-speed device is connected via a high-speed hub using split
transactions, a control or bulk transfer initiates a START SPLIT and
completes with an ACK. The hardware channel is disabled and the transaction
advances to CVMX_USB_STAGE_NON_CONTROL_SPLIT_COMPLETE, but its
next_tx_frame is not advanced.

The pipe must wait until usb->frame_number reaches split_sc_frame:

cvmx_usb_find_ready_pipe() {
    ...
    ((pipe->split_sc_frame == -1) ||
     ((((int)current_frame - pipe->split_sc_frame) & 0x7f) < 0x40)) &&
    ...
}

Since control and bulk pipes skip setting due_now and their next_tx_frame
remains in the past (so they don't update min_due), need_sof will be 0 if
no other periodic pipes are active. Does this mean usb->frame_number stops
updating, permanently stalling the pipe until an unrelated interrupt fires?

> +			} else if (pipe->next_tx_frame < min_due) {
>  				min_due = pipe->next_tx_frame;
> +			}
>  		}
>  	}
>  	if (min_due != ~0ull) {

[Severity: High]
Will the due_now flag be completely ignored if there are only due pipes
and no future periodic deadlines exist?

In the loop above, if a pipe is due (pipe->next_tx_frame <= usb->frame_number),
due_now is set to true but min_due is not updated.

If there are no other active periodic pipes with a future deadline, min_due
remains ~0ull. The code then completely bypasses this entire if
(min_due != ~0ull) block.

As a result, need_sof remains 0 and the timer is not armed. Since periodic
pipes are only retried when is_sof is true, does this result in a permanent
starvation of the unstarted due periodic pipes?

> @@ -1983,7 +1995,7 @@ static void cvmx_usb_schedule(struct octeon_hcd *usb, int is_sof)
>  		 * a few frames. Stay well below the 16383-frame wrap of
>  		 * HFNUM. One (micro)frame is 125us in high-speed mode.
>  		 */
> -		if (delta <= 4 || delta > 8000)
> +		if (due_now || delta <= 4 || delta > 8000)
>  			need_sof = 1;
>  		else
>  			hrtimer_start(&usb->sof_timer,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011072236.2381546-1-orgads@gmail.com?part=1

[PATCH v2] usb: octeon-hcd: keep SOF interrupts while a pipe needs them

[Orgad Shaneh]

cvmx_usb_schedule() now sleeps on an hrtimer until the nearest future
periodic deadline instead of counting frames down on every SOF. The scan
that finds that deadline only looks at pipes with

	next_tx_frame > usb->frame_number

which is right for picking the deadline, but two kinds of pipe that
need a SOF to make progress are invisible to the decision:

- A periodic pipe that is due and was *not* started: no idle hardware
  channel this round, its split window is closed
  (cvmx_usb_find_ready_pipe() rejects both), or another pipe owns
  usb->active_split. cvmx_usb_next_pipe() looks at the isochronous and
  interrupt lists only when is_sof is true.

- Any pipe that has sent its start split and waits for the frame its
  split complete may go out in (split_sc_frame, also checked by
  cvmx_usb_find_ready_pipe()). A control or bulk pipe on a full or low
  speed device behind a hub is in this state between its two halves, and
  with SOF masked nothing guarantees an interrupt that brings
  frame_number there.

When some other periodic pipe sets a far deadline - a hub's status pipe,
whenever a hub is attached - the driver masks SOF, arms the timer for
that deadline and these pipes wait for it. That is hundreds of frames;
the code allows up to 8000 (one second) before it falls back to SOF.

Before the hrtimer this could not happen: any pipe with a future
deadline set need_sof, so SOF stayed enabled every frame and the pipe
was retried on the next one.

Note such a pipe and keep SOF enabled while there is one, whether or not
another pipe has a future deadline. Nothing changes when there is none,
which is the case the timer was added for.

Other control and bulk pipes are excluded deliberately: their
next_tx_frame is not updated on completion, so they are almost always
"due" and would keep SOF enabled permanently.

Fixes: aebbc7d63407 ("usb: octeon-hcd: sleep on an hrtimer for far-off periodic transfers")
Assisted-by: LLM
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
v2: the Sashiko review bot found that v1 missed two cases:
 - due_now was only looked at when some pipe had a future deadline, so
   a due pipe with none still left SOF masked;
 - a control or bulk pipe waiting for its split complete is in the same
   position as a due periodic pipe, but v1 excluded all control and bulk
   pipes. split_sc_frame tells these apart from the always-due ones.
 Compile-tested only: there is no full or low speed device behind a hub
 on the test board, so the split path is not exercised.
v1: https://lore.kernel.org/all/20260915081724.25637-1-orgads@gmail.com/
    and the resend 20261011072236.2381546-1-orgads@gmail.com

diff --git a/drivers/usb/host/octeon-hcd.c b/drivers/usb/host/octeon-hcd.c
index 256e459..9477a1d 100644
--- a/drivers/usb/host/octeon-hcd.c
+++ b/drivers/usb/host/octeon-hcd.c
@@ -1923,6 +1923,7 @@ static void cvmx_usb_schedule(struct octeon_hcd *usb, int is_sof)
 	struct cvmx_usb_pipe *pipe;
 	int need_sof;
 	u64 min_due;
+	bool due_now;
 	enum cvmx_usb_transfer ttype;
 
 	if (usb->init_flags & CVMX_USB_INITIALIZE_FLAGS_NO_DMA) {
@@ -1964,15 +1965,32 @@ done:
 	 */
 	need_sof = 0;
 	min_due = ~0ull;
+	due_now = false;
 	for (ttype = CVMX_USB_TRANSFER_CONTROL;
 	     ttype <= CVMX_USB_TRANSFER_INTERRUPT; ttype++) {
 		list_for_each_entry(pipe, &usb->active_pipes[ttype], node) {
+			/*
+			 * Do not sleep past a pipe that is only retried from a
+			 * SOF: a periodic one that is due but was not started
+			 * (no idle channel, closed split window, another pipe's
+			 * split active), or any pipe waiting for the frame its
+			 * split complete may go out in. Other control and bulk
+			 * pipes are left out: their next_tx_frame is not updated
+			 * on completion, so they always look due.
+			 */
+			if (pipe->split_sc_frame != -1 ||
+			    (pipe->next_tx_frame <= usb->frame_number &&
+			     (ttype == CVMX_USB_TRANSFER_ISOCHRONOUS ||
+			      ttype == CVMX_USB_TRANSFER_INTERRUPT)))
+				due_now = true;
 			if (pipe->next_tx_frame > usb->frame_number &&
 			    pipe->next_tx_frame < min_due)
 				min_due = pipe->next_tx_frame;
 		}
 	}
-	if (min_due != ~0ull) {
+	if (due_now) {
+		need_sof = 1;
+	} else if (min_due != ~0ull) {
 		u64 delta = min_due - usb->frame_number;
 
 		/*

Re: [PATCH v2] usb: octeon-hcd: keep SOF interrupts while a pipe needs them

[sashiko-bot]

> cvmx_usb_schedule() now sleeps on an hrtimer until the nearest future
> periodic deadline instead of counting frames down on every SOF. The scan
> that finds that deadline only looks at pipes with
> 
> 	next_tx_frame > usb->frame_number
> 
> which is right for picking the deadline, but two kinds of pipe that
> need a SOF to make progress are invisible to the decision:
> 
> - A periodic pipe that is due and was *not* started: no idle hardware
>   channel this round, its split window is closed
>   (cvmx_usb_find_ready_pipe() rejects both), or another pipe owns
>   usb->active_split. cvmx_usb_next_pipe() looks at the isochronous and
>   interrupt lists only when is_sof is true.
> 
> [ ... ]
> "due" and would keep SOF enabled permanently.
> 
> Fixes: aebbc7d63407 ("usb: octeon-hcd: sleep on an hrtimer for far-off periodic transfers")
> Assisted-by: LLM
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011080506.2394708-1-orgads@gmail.com?part=1