usb: octeon-hcd: fix the FIFO-flush timeout computation

cvmx_wait_tx_rx() computes its 100us deadline from (u64)octeon_get_clock_rate - the address of the function, not its return value; the parentheses have been missing since the CVMX_WAIT_FOR_FIELD32 macro became a function. The cast makes it compile silently, and the resulting deadline is effectively infinite.

On a healthy controller the flush bit clears on the first read and nothing is noticed. On a controller whose PHY did not come up (for example when the reference-clock configuration is wrong for the board), txfflsh/rxfflsh never clear and probe spins forever in __delay() - observed as a hard hang with a soft-lockup splat on a CN5020 board, where the board watchdog then resets the system with no console output.

Call the function, restoring the 100us timeout the code always intended.

---

@@ -578,7 +578,7 @@
 	int result;
 	u64 address = CVMX_USBCX_GRSTCTL(usb->index);
 	u64 done = cvmx_get_cycle() + 100 *
-		   (u64)octeon_get_clock_rate / 1000000;
+		   (u64)octeon_get_clock_rate() / 1000000;
 	union cvmx_usbcx_grstctl c;
 
 	while (1) {

2.47.0

If the USBN clock/PHY setup did not actually bring the core up (wrong board reference-clock configuration, for instance), every USBC CSR reads as all-ones and initialization marches on programming garbage, ending in timeouts or a wedge deep in the FIFO flush.

Read GHWCFG3 - a hardwired configuration register that can be neither 0 nor all-ones on a live core - right before core initialization, and give up cleanly if the core is not answering.

---

@@ -853,6 +853,14 @@
 	 *    USBC_GAHBCFG[PTXFEMPLVL]
 	 *    Global interrupt mask, USBC_GAHBCFG[GLBLINTRMSK] = 1
 	 */
+	usbcx_gahbcfg.u32 = cvmx_usb_read_csr32(usb,
+						CVMX_USBCX_GHWCFG3(usb->index));
+	if (usbcx_gahbcfg.u32 == 0xffffffff || usbcx_gahbcfg.u32 == 0) {
+		dev_err(dev, "USB core is not responding (GHWCFG3=0x%08x)\n",
+			usbcx_gahbcfg.u32);
+		return -ENODEV;
+	}
+
 	usbcx_gahbcfg.u32 = 0;
 	usbcx_gahbcfg.s.dmaen = !(usb->init_flags &
 				  CVMX_USB_INITIALIZE_FLAGS_NO_DMA);

2.47.0

cvmx_usb_schedule() enables the SOF interrupt whenever any pipe has next_tx_frame in the future, and leaves it enabled until the transfer is due. An interrupt endpoint keeps such a deadline pending permanently, so a single attached hub (status pipe polled every 256ms) costs an interrupt on every SOF - 8000/s in high-speed mode, forever. On a 500MHz CN5020 that is measurably ~20% of one core spent counting frames.

The frame counter is resynchronized from HFNUM at the top of every poll, so the driver does not actually need to see every SOF to know when a deadline arrives. Sleep on an hrtimer when the nearest deadline is more than a few frames away (one high-speed frame is 125us) and keep SOF interrupts only for deadlines that are imminent - or far enough away to risk the 16383-frame HFNUM wrap, where the interrupt path still tracks the counter extension.

Measured on a CN5020 board with one 4-port hub attached and idle:
8200 -> 4 USB interrupts/s, with no change in enumeration or transfer behavior.

---

@@ -378,8 +378,22 @@
 	struct cvmx_usb_transaction *active_split;
 	struct cvmx_usb_tx_fifo periodic;
 	struct cvmx_usb_tx_fifo nonperiodic;
+	struct hrtimer sof_timer;
 };
 
+static int cvmx_usb_poll(struct octeon_hcd *usb);
+
+static enum hrtimer_restart octeon_usb_sof_timer(struct hrtimer *t)
+{
+	struct octeon_hcd *usb = container_of(t, struct octeon_hcd, sof_timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&usb->lock, flags);
+	cvmx_usb_poll(usb);
+	spin_unlock_irqrestore(&usb->lock, flags);
+	return HRTIMER_NORESTART;
+}
+
 /*
  * This macro logically sets a single field in a CSR. It does the sequence
  * read, modify, and write

@@ -1908,6 +1922,7 @@
 	int channel;
 	struct cvmx_usb_pipe *pipe;
 	int need_sof;
+	u64 min_due;
 	enum cvmx_usb_transfer ttype;
 
 	if (usb->init_flags & CVMX_USB_INITIALIZE_FLAGS_NO_DMA) {

@@ -1948,15 +1963,33 @@
 	 * future that might need to be scheduled
 	 */
 	need_sof = 0;
+	min_due = ~0ull;
 	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) {
-				need_sof = 1;
-				break;
-			}
+			if (pipe->next_tx_frame > usb->frame_number &&
+			    pipe->next_tx_frame < min_due)
+				min_due = pipe->next_tx_frame;
 		}
 	}
+	if (min_due != ~0ull) {
+		u64 delta = min_due - usb->frame_number;
+
+		/*
+		 * frame_number is resynced from HFNUM on every poll, so a
+		 * deadline that is many frames away does not need an
+		 * interrupt on every SOF to count them down - sleep on the
+		 * timer instead and keep SOF interrupts for deadlines within
+		 * 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)
+			need_sof = 1;
+		else
+			hrtimer_start(&usb->sof_timer,
+				      ns_to_ktime((delta - 2) * 125000),
+				      HRTIMER_MODE_REL);
+	}
 	USB_SET_FIELD32(CVMX_USBCX_GINTMSK(usb->index),
 			cvmx_usbcx_gintmsk, sofmsk, need_sof);
 }

@@ -3646,6 +3679,8 @@
 	usb = (struct octeon_hcd *)hcd->hcd_priv;
 
 	spin_lock_init(&usb->lock);
+	hrtimer_setup(&usb->sof_timer, octeon_usb_sof_timer, CLOCK_MONOTONIC,
+		      HRTIMER_MODE_REL);
 
 	usb->init_flags = initialize_flags;
 

@@ -3696,6 +3731,7 @@
 	unsigned long flags;
 
 	usb_remove_hcd(hcd);
+	hrtimer_cancel(&usb->sof_timer);
 	spin_lock_irqsave(&usb->lock, flags);
 	status = cvmx_usb_shutdown(usb);
 	spin_unlock_irqrestore(&usb->lock, flags);

2.47.0

Should this series have an Assisted-by: tag?

How did you find and test it?

thanks,

greg k-h

Yes - my oversight, v2 will carry it on all three patches:

I used Claude Code for the analysis, patches and commit texts. My Signed-off-by stays as the DCO certification; I ran and reviewed it all on hardware.

I'm updating the kernel on our Octeon boards from ancient 2.6 to 7.2, so I walked into it. Bringing USB back up on an AudioCodes M800 (CN5020p1.1-300, board type CN3010_EVB_HS5), modprobing octeon-hcd killed the board: watchdog reset, nothing printed. Serial capture caught a soft lockup in __delay() under cvmx_wait_tx_rx(), where the missing parens on octeon_get_clock_rate are right there in the deadline; git blame lands on 3e195a80e096, and the cast hides it from the compiler.

The flush never finished on my board because the driver got the DTS default 12MHz crystal instead of the board's 24MHz external reference, so the PHY never came up and the USBC registers read all-ones. That is a board/arch problem I'm handling separately, but it made a reliable reproducer: with the clock wrong, probe never returns and the board dies; with 1/3 it times out after 100us and returns an error; with 2/3 it bails earlier at GHWCFG3 and says why. With the clock right, the root hub and on-board 4-port hub enumerate at high speed and run without errors, so the timeout doesn't fire on healthy hardware - same setup 3/3 was measured on (8200 -> 4 USB interrupts/s idle). Builds clean, checkpatch is happy.

Untested: any part other than CN5020, and the HFNUM wrap path in 3/3 beyond reasoning plus long idle runs with a hub attached.

- Orgad

cvmx_wait_tx_rx() computes its 100us deadline from (u64)octeon_get_clock_rate - the address of the function, not its return value; the parentheses have been missing since the CVMX_WAIT_FOR_FIELD32 macro became a function. The cast makes it compile silently, and the resulting deadline is effectively infinite.

On a healthy controller the flush bit clears on the first read and nothing is noticed. On a controller whose PHY did not come up (for example when the reference-clock configuration is wrong for the board), txfflsh/rxfflsh never clear and probe spins forever in __delay() - observed as a hard hang with a soft-lockup splat on a CN5020 board, where the board watchdog then resets the system with no console output.

Call the function, restoring the 100us timeout the code always intended.

---

v2: no code change; added the Assisted-by tag for the AI assistance used
(Documentation/process/coding-assistants.rst).

@@ -578,7 +578,7 @@
 	int result;
 	u64 address = CVMX_USBCX_GRSTCTL(usb->index);
 	u64 done = cvmx_get_cycle() + 100 *
-		   (u64)octeon_get_clock_rate / 1000000;
+		   (u64)octeon_get_clock_rate() / 1000000;
 	union cvmx_usbcx_grstctl c;
 
 	while (1) {

2.47.0

If the USBN clock/PHY setup did not actually bring the core up (wrong board reference-clock configuration, for instance), every USBC CSR reads as all-ones and initialization marches on programming garbage, ending in timeouts or a wedge deep in the FIFO flush.

Read GHWCFG3 - a hardwired configuration register that can be neither 0 nor all-ones on a live core - right before core initialization, and give up cleanly if the core is not answering.

---

v2: no code change; added the Assisted-by tag for the AI assistance used
(Documentation/process/coding-assistants.rst).

@@ -853,6 +853,14 @@
 	 *    USBC_GAHBCFG[PTXFEMPLVL]
 	 *    Global interrupt mask, USBC_GAHBCFG[GLBLINTRMSK] = 1
 	 */
+	usbcx_gahbcfg.u32 = cvmx_usb_read_csr32(usb,
+						CVMX_USBCX_GHWCFG3(usb->index));
+	if (usbcx_gahbcfg.u32 == 0xffffffff || usbcx_gahbcfg.u32 == 0) {
+		dev_err(dev, "USB core is not responding (GHWCFG3=0x%08x)\n",
+			usbcx_gahbcfg.u32);
+		return -ENODEV;
+	}
+
 	usbcx_gahbcfg.u32 = 0;
 	usbcx_gahbcfg.s.dmaen = !(usb->init_flags &
 				  CVMX_USB_INITIALIZE_FLAGS_NO_DMA);

2.47.0

cvmx_usb_schedule() enables the SOF interrupt whenever any pipe has next_tx_frame in the future, and leaves it enabled until the transfer is due. An interrupt endpoint keeps such a deadline pending permanently, so a single attached hub (status pipe polled every 256ms) costs an interrupt on every SOF - 8000/s in high-speed mode, forever. On a 300MHz CN5020 that is measurably ~20% of one core spent counting frames.

The frame counter is resynchronized from HFNUM at the top of every poll, so the driver does not actually need to see every SOF to know when a deadline arrives. Sleep on an hrtimer when the nearest deadline is more than a few frames away (one high-speed frame is 125us) and keep SOF interrupts only for deadlines that are imminent - or far enough away to risk the 16383-frame HFNUM wrap, where the interrupt path still tracks the counter extension.

Measured on a CN5020 board with one 4-port hub attached and idle:
8200 -> 4 USB interrupts/s, with no change in enumeration or transfer behavior.

---

v2: corrected the core clock in the commit message - the board is a
300MHz CN5020, the 500MHz number belongs to a different board here; added the Assisted-by tag (Documentation/process/coding-assistants.rst). No code change.

@@ -378,8 +378,22 @@
 	struct cvmx_usb_transaction *active_split;
 	struct cvmx_usb_tx_fifo periodic;
 	struct cvmx_usb_tx_fifo nonperiodic;
+	struct hrtimer sof_timer;
 };
 
+static int cvmx_usb_poll(struct octeon_hcd *usb);
+
+static enum hrtimer_restart octeon_usb_sof_timer(struct hrtimer *t)
+{
+	struct octeon_hcd *usb = container_of(t, struct octeon_hcd, sof_timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&usb->lock, flags);
+	cvmx_usb_poll(usb);
+	spin_unlock_irqrestore(&usb->lock, flags);
+	return HRTIMER_NORESTART;
+}
+
 /*
  * This macro logically sets a single field in a CSR. It does the sequence
  * read, modify, and write

@@ -1908,6 +1922,7 @@
 	int channel;
 	struct cvmx_usb_pipe *pipe;
 	int need_sof;
+	u64 min_due;
 	enum cvmx_usb_transfer ttype;
 
 	if (usb->init_flags & CVMX_USB_INITIALIZE_FLAGS_NO_DMA) {

@@ -1948,15 +1963,33 @@
 	 * future that might need to be scheduled
 	 */
 	need_sof = 0;
+	min_due = ~0ull;
 	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) {
-				need_sof = 1;
-				break;
-			}
+			if (pipe->next_tx_frame > usb->frame_number &&
+			    pipe->next_tx_frame < min_due)
+				min_due = pipe->next_tx_frame;
 		}
 	}
+	if (min_due != ~0ull) {
+		u64 delta = min_due - usb->frame_number;
+
+		/*
+		 * frame_number is resynced from HFNUM on every poll, so a
+		 * deadline that is many frames away does not need an
+		 * interrupt on every SOF to count them down - sleep on the
+		 * timer instead and keep SOF interrupts for deadlines within
+		 * 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)
+			need_sof = 1;
+		else
+			hrtimer_start(&usb->sof_timer,
+				      ns_to_ktime((delta - 2) * 125000),
+				      HRTIMER_MODE_REL);
+	}
 	USB_SET_FIELD32(CVMX_USBCX_GINTMSK(usb->index),
 			cvmx_usbcx_gintmsk, sofmsk, need_sof);
 }

@@ -3646,6 +3679,8 @@
 	usb = (struct octeon_hcd *)hcd->hcd_priv;
 
 	spin_lock_init(&usb->lock);
+	hrtimer_setup(&usb->sof_timer, octeon_usb_sof_timer, CLOCK_MONOTONIC,
+		      HRTIMER_MODE_REL);
 
 	usb->init_flags = initialize_flags;
 

@@ -3696,6 +3731,7 @@
 	unsigned long flags;
 
 	usb_remove_hcd(hcd);
+	hrtimer_cancel(&usb->sof_timer);
 	spin_lock_irqsave(&usb->lock, flags);
 	status = cvmx_usb_shutdown(usb);
 	spin_unlock_irqrestore(&usb->lock, flags);

2.47.0