Skip to contentSkip to conversation

usb: xhci: Fix a format bug

PATCH In review Michal Pecio 2 participants 4 messages Oct 16, 2025 – Oct 31, 2025

Subsystems (2)
USB USB xHCI driver
lore
Share & tools
lore

Michal Pecio

author
#0 lore raw

PATCH 1/2 usb: xhci: Fix a format bug

The width of 'addr' depends on kernel configuration and gibberish is printed in traces and dynamic debug on some 32 bit systems like ARM:

Removing canceled TD starting at 0xf9c96eb0 (dma) in stream 0 URB 54e247b5 Set TR Deq ptr 0x205400000000000, cycle 0

Successful Set TR Deq Ptr cmd, deq = @f9c96ef0

Fix it by casting to 64 bits. No effect on unaffected systems. Remove the newline which casuses an empty line to appear next.

Fixes d1dbfb942c33 ("xhci: introduce a new move_dequeue_past_td() function to replace old code.")
Cc [email protected]
Signed-off-by Michal Pecio <[email protected]>
---
 drivers/usb/host/xhci-ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/host/xhci-ring.c +1 -1
index c7f658d446cd..6d799a5a062d 100644
@@ -776,7 +776,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
 	ep->queued_deq_ptr = new_deq;
 
 	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-		       "Set TR Deq ptr 0x%llx, cycle %u\n", addr, new_cycle);
+		       "Set TR Deq ptr 0x%llx, cycle %u", (u64) addr, new_cycle);
 
 	/* Stop the TD queueing code from ringing the doorbell until
 	 * this command completes.  The HC won't set the dequeue pointer
Signature · 1 line
2.48.1
author
#1 lore raw

PATCH 2/2 usb: xhci: Type check xhci_dbg_trace()

xhci_dbg_trace() is a printf-like function which can be type checked at build time. Do it to catch potential format bugs in the driver.

I found no remaining warnings on x86-64 and ARM.

Signed-off-by Michal Pecio <[email protected]>
---
 drivers/usb/host/xhci.h | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/host/xhci.h +1 -0
index 485ea7fc0433..8facba10fc9c 100644
@@ -1788,6 +1788,7 @@ static inline bool xhci_link_chain_quirk(struct xhci_hcd *xhci, enum xhci_ring_t
 /* xHCI debugging */
 char *xhci_get_slot_state(struct xhci_hcd *xhci,
 		struct xhci_container_ctx *ctx);
+__printf(3, 4)
 void xhci_dbg_trace(struct xhci_hcd *xhci, void (*trace)(struct va_format *),
 			const char *fmt, ...);
Signature · 1 line
2.48.1
#2 lore raw
In reply to Michal Pecio
Quoted 23 lines · Michal Pecio

The width of 'addr' depends on kernel configuration and gibberish is printed in traces and dynamic debug on some 32 bit systems like ARM:

Removing canceled TD starting at 0xf9c96eb0 (dma) in stream 0 URB 54e247b5 Set TR Deq ptr 0x205400000000000, cycle 0

Successful Set TR Deq Ptr cmd, deq = @f9c96ef0

Fix it by casting to 64 bits. No effect on unaffected systems. Remove the newline which casuses an empty line to appear next.

Fixes d1dbfb942c33 ("xhci: introduce a new move_dequeue_past_td() function to replace old code.")
Cc [email protected]
Signed-off-by Michal Pecio <[email protected]>
---
  drivers/usb/host/xhci-ring.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/host/xhci-ring.c +0 -0
index c7f658d446cd..6d799a5a062d 100644

drivers/usb/host/xhci-ring.c static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci, · line 779

Quoted 5 lines
drivers/usb/host/xhci-ring.c +1 -1
@@ -776,7 +776,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
  	ep->queued_deq_ptr = new_deq;
 
  	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-		       "Set TR Deq ptr 0x%llx, cycle %u\n", addr, new_cycle);
+		       "Set TR Deq ptr 0x%llx, cycle %u", (u64) addr, new_cycle);

Why not %pad and &addr instead?

Thanks
Mathias

author
#3 lore raw
Quoted 26 lines · Mathias Nyman
Quoted 25 lines · Michal Pecio

The width of 'addr' depends on kernel configuration and gibberish is printed in traces and dynamic debug on some 32 bit systems like ARM:

Removing canceled TD starting at 0xf9c96eb0 (dma) in stream 0 URB 54e247b5 Set TR Deq ptr 0x205400000000000, cycle 0

Successful Set TR Deq Ptr cmd, deq = @f9c96ef0

Fix it by casting to 64 bits. No effect on unaffected systems. Remove the newline which casuses an empty line to appear next.

Fixes d1dbfb942c33 ("xhci: introduce a new move_dequeue_past_td() function to replace old code.")
Cc [email protected]
Signed-off-by Michal Pecio <[email protected]>
---
  drivers/usb/host/xhci-ring.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/host/xhci-ring.c +0 -0
index c7f658d446cd..6d799a5a062d 100644
@@ -776,7 +776,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
  	ep->queued_deq_ptr = new_deq;
Quoted 5 lines

drivers/usb/host/xhci-ring.c static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci, · line 779

Quoted 3 lines
drivers/usb/host/xhci-ring.c +1 -1
  	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-		       "Set TR Deq ptr 0x%llx, cycle %u\n", addr, new_cycle);
+		       "Set TR Deq ptr 0x%llx, cycle %u", (u64) addr, new_cycle);

Why not %pad and &addr instead?

I thought this would be worth fixing in stable and %pad annoyingly doesn't support precision modifiers. So using it would be
- functional change (implicit %.16 padding) on non-broken 64 bit systems
- difference from other related formats which I am not updating here

I can do a v2, though I think such change would make more sense as a separate non-stable commit updating more of related messages.

And I admit that I am not a fan of %pad in general, it's a hack which defeats compiler type checks and allows passing any invalid type as long as it's passed by reference. Probably made sense when review was the only means of catching format bugs, but not sure today.

Patch 2/2 makes such bugs a build warning on affected systems. With %pad, &addr you can change addr to u32, u64 or 'struct xhci_hcd' and it will build cleanly on every platform, then print garbage.

Regards,
Michal