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

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

[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: stable@vger.kernel.org
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
 drivers/usb/host/xhci-ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c7f658d446cd..6d799a5a062d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -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
-- 
2.48.1

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

[Michal Pecio]

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 <michal.pecio@gmail.com>
---
 drivers/usb/host/xhci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 485ea7fc0433..8facba10fc9c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -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, ...);
 
-- 
2.48.1

Re: [PATCH 1/2] usb: xhci: Fix a format bug

[Mathias Nyman]

On 10/16/25 19:28, Michal Pecio wrote:
> 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: stable@vger.kernel.org
> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
> ---
>   drivers/usb/host/xhci-ring.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index c7f658d446cd..6d799a5a062d 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -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

Re: [PATCH 1/2] usb: xhci: Fix a format bug

[Michal Pecio]

On Fri, 31 Oct 2025 17:00:42 +0200, Mathias Nyman wrote:
> On 10/16/25 19:28, Michal Pecio wrote:
> > 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: stable@vger.kernel.org
> > Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
> > ---
> >   drivers/usb/host/xhci-ring.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index c7f658d446cd..6d799a5a062d 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -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?

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