[PATCH 0/7] usb: xhci: enhancements to address printing

[PATCH 0/7] usb: xhci: enhancements to address printing

[Niklas Neronin]

DMA Address Printing:
 All DMA address printing is now standardized using the "%pad" specifier.
 This guarantees correct address formatting regardless of the kernel's
 architecture, whether 32-bit or 64-bit.

Address Formatting:
 Printed hexadecimal values can be ambiguous, making it tricky to
 distinguish between values and addresses. To enhance clarity, all
 trusted DMA addresses are now prefixed with the '@' symbol, ensuring
 immediate recognition as addresses.

 However, the printk() specifier '%p' has had its '@' prefix removed.
 This is because it hashes the address before printing, effectively turning
 it into an address ID. The hashing process does not preserve any relation
 between addresses; for instance, if two addresses are X bytes apart, their
 hashed counterparts will not reflect this distinction.

Read 64-bit register printing:
 Debug messages that print an entire register value containing a DMA
 address have been improved. The register fields are now split and printed
 separately, making it easier for users to interpret the information.

These changes collectively improve the readability and consistency of
address printing in the xHCI driver, making it easier for developers and
maintainers to interpret log outputs accurately.

=== Alternative approach ===
An argument can be made for switching from representing a TRB by its DMA
address to using an index, as most printed DMA addresses are TRB addresses.
While this approach offers potential benefits, it also presents challenges.
Calculating the TRB index is straightforward when the segment and TRB are
known. However, when printing TRB addresses read from xHCI registers,
determining the TRB index becomes complex, increasing the risk of producing
incorrect debug messages. Due to these complexities, I have decided not to
switch to TRB indexes (yet).

Niklas Neronin (7):
  usb: xhci-dbgcap: correct DMA address handling
  usb: xhci: use '%pad' specifier for DMA address printing
  usb: xhci: improve Stream Context register debugging
  usb: xhci: improve Endpoint Context register debugging
  usb: xhci: improve Command Ring Control register debugging
  usb: xhci: improve Event Ring Dequeue Pointer Register debugging
  usb: xhci: standardize address format

 drivers/usb/host/xhci-dbgcap.c  | 11 +++++----
 drivers/usb/host/xhci-debugfs.c | 22 ++++++++++-------
 drivers/usb/host/xhci-mem.c     | 13 +++++-----
 drivers/usb/host/xhci-ring.c    | 42 ++++++++++++++++-----------------
 drivers/usb/host/xhci-trace.h   | 37 +++++++++++++++--------------
 drivers/usb/host/xhci.c         | 37 +++++++++++++++--------------
 drivers/usb/host/xhci.h         |  4 ++--
 7 files changed, 86 insertions(+), 80 deletions(-)

-- 
2.50.1

[PATCH 1/7] usb: xhci-dbgcap: correct DMA address handling

[Niklas Neronin]

Address the handling of addresses within the dbc_handle_xfer_event(),
ensuring accurate extraction and comparison.

Variable details:
'r->trb_dma'		    : A DMA address.
'ep_ctx->deq'		    : A le64, bits 63:4 contain the DMA address.
'event->trans_event.buffer' : A le64, bits 63:0 contain the DMA address.

1. Convert 'ep_ctx->deq' and 'event->trans_event.buffer' addresses to
   the CPU's native byte order.

2. Use mask; TR_DEQ_PTR_MASK (bits 63:4) to extract the address from
   'ep_ctx->deq', replacing the previous use of ~TRB_CYCLE (bits 63:1).

Why not use 'dma_addr_t' for the address?
The 'dma_addr_t' type can vary between 32 and 64 bits depending on the
system architecture or xHCI driver flags, whereas the 64-bit address field
size remains constant. Since hardware cannot be fully trusted, it's better
to print the entire 64-bit address to detect any non-zero values in the
upper 32 bits. This approach ensures that potential issues are easily
detected.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-dbgcap.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 06a2edb9e86e..2f24a3a54439 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -724,6 +724,7 @@ static void dbc_handle_xfer_event(struct xhci_dbc *dbc, union xhci_trb *event)
 	u32			comp_code;
 	size_t			remain_length;
 	struct dbc_request	*req = NULL, *r;
+	u64			ep_trb, deq;
 
 	comp_code	= GET_COMP_CODE(le32_to_cpu(event->generic.field[2]));
 	remain_length	= EVENT_TRB_LEN(le32_to_cpu(event->generic.field[2]));
@@ -733,10 +734,11 @@ static void dbc_handle_xfer_event(struct xhci_dbc *dbc, union xhci_trb *event)
 	ep_ctx		= (ep_id == EPID_OUT) ?
 				dbc_bulkout_ctx(dbc) : dbc_bulkin_ctx(dbc);
 	ring		= dep->ring;
+	ep_trb		= le64_to_cpu(event->trans_event.buffer);
 
 	/* Match the pending request: */
 	list_for_each_entry(r, &dep->list_pending, list_pending) {
-		if (r->trb_dma == event->trans_event.buffer) {
+		if (r->trb_dma == ep_trb) {
 			req = r;
 			break;
 		}
@@ -768,8 +770,9 @@ static void dbc_handle_xfer_event(struct xhci_dbc *dbc, union xhci_trb *event)
 		status = -comp_code;
 		break;
 	case COMP_STALL_ERROR:
+		deq = le64_to_cpu(ep_ctx->deq) & TR_DEQ_PTR_MASK;
 		dev_warn(dbc->dev, "Stall error at bulk TRB %llx, remaining %zu, ep deq %llx\n",
-			 event->trans_event.buffer, remain_length, ep_ctx->deq);
+			 ep_trb, remain_length, deq);
 		status = 0;
 		dep->halted = 1;
 
@@ -788,7 +791,7 @@ static void dbc_handle_xfer_event(struct xhci_dbc *dbc, union xhci_trb *event)
 		 * TRB again.
 		 */
 
-		if ((ep_ctx->deq & ~TRB_CYCLE) == event->trans_event.buffer) {
+		if (deq == ep_trb) {
 			dev_dbg(dbc->dev, "Ep stopped on Stalled TRB\n");
 			if (remain_length == req->length) {
 				dev_dbg(dbc->dev, "Spurious stall event, keep req\n");
-- 
2.50.1

[PATCH 2/7] usb: xhci: use '%pad' specifier for DMA address printing

[Niklas Neronin]

Switch all printing of DMA addresses to '%pad' specifier. This specifier
ensures that the address is printed correctly, regardless of whether the
kernel is running in a 32-bit or 64-bit environment.

This patch exclusively modifies variables that are DMA addresses.
Subsequent patches will address variables that partially contain DMA
addresses.

Remove "offset" from dbg trace concerning URBs TD start addresses.
Prefix "0x" is automatically added by '%pad'.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c   |  4 ++--
 drivers/usb/host/xhci-ring.c  | 33 ++++++++++++++++-----------------
 drivers/usb/host/xhci-trace.h | 30 +++++++++++++++---------------
 drivers/usb/host/xhci.c       | 23 ++++++++++++-----------
 4 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 07289333a1e8..2a414dee7233 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1015,10 +1015,10 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
 
 	/* Point to output device context in dcbaa. */
 	xhci->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(dev->out_ctx->dma);
-	xhci_dbg(xhci, "Set slot id %d dcbaa entry %p to 0x%llx\n",
+	xhci_dbg(xhci, "Set slot id %d dcbaa entry %p to %pad\n",
 		 slot_id,
 		 &xhci->dcbaa->dev_context_ptrs[slot_id],
-		 le64_to_cpu(xhci->dcbaa->dev_context_ptrs[slot_id]));
+		 &dev->out_ctx->dma);
 
 	trace_xhci_alloc_virt_device(dev);
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d2ccf19292aa..7d57e2a3abf3 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -781,7 +781,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 %pad, cycle %u\n", &addr, new_cycle);
 
 	/* Stop the TD queueing code from ringing the doorbell until
 	 * this command completes.  The HC won't set the dequeue pointer
@@ -1035,6 +1035,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 	u64			hw_deq;
 	unsigned int		slot_id = ep->vdev->slot_id;
 	int			err;
+	dma_addr_t		dma;
 
 	/*
 	 * This is not going to work if the hardware is changing its dequeue
@@ -1046,11 +1047,10 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 	xhci = ep->xhci;
 
 	list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) {
+		dma = xhci_trb_virt_to_dma(td->start_seg, td->start_trb);
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-			       "Removing canceled TD starting at 0x%llx (dma) in stream %u URB %p",
-			       (unsigned long long)xhci_trb_virt_to_dma(
-				       td->start_seg, td->start_trb),
-			       td->urb->stream_id, td->urb);
+			       "Removing canceled TD starting at %pad (dma) in stream %u URB %p",
+			       &dma, td->urb->stream_id, td->urb);
 		list_del_init(&td->td_list);
 		ring = xhci_urb_to_transfer_ring(xhci, td->urb);
 		if (!ring) {
@@ -2217,6 +2217,7 @@ static void finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 		      u32 trb_comp_code)
 {
 	struct xhci_ep_ctx *ep_ctx;
+	dma_addr_t dma;
 
 	ep_ctx = xhci_get_ep_ctx(xhci, ep->vdev->out_ctx, ep->ep_index);
 
@@ -2252,9 +2253,8 @@ static void finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 			 */
 			if ((ep->ep_state & EP_HALTED) &&
 			    !list_empty(&td->cancelled_td_list)) {
-				xhci_dbg(xhci, "Already resolving halted ep for 0x%llx\n",
-					 (unsigned long long)xhci_trb_virt_to_dma(
-						 td->start_seg, td->start_trb));
+				dma = xhci_trb_virt_to_dma(td->start_seg, td->start_trb);
+				xhci_dbg(xhci, "Already resolving halted ep for %pad\n", &dma);
 				return;
 			}
 			/* endpoint not halted, don't reset it */
@@ -2654,7 +2654,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 	unsigned int slot_id;
 	int ep_index;
 	struct xhci_td *td = NULL;
-	dma_addr_t ep_trb_dma;
+	dma_addr_t ep_trb_dma, deq, td_start, td_end;
 	struct xhci_segment *ep_seg;
 	union xhci_trb *ep_trb;
 	int status = -EINPROGRESS;
@@ -2976,18 +2976,17 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 	return 0;
 
 debug_finding_td:
-	xhci_err(xhci, "Event dma %pad for ep %d status %d not part of TD at %016llx - %016llx\n",
-		 &ep_trb_dma, ep_index, trb_comp_code,
-		 (unsigned long long)xhci_trb_virt_to_dma(td->start_seg, td->start_trb),
-		 (unsigned long long)xhci_trb_virt_to_dma(td->end_seg, td->end_trb));
+	td_start = xhci_trb_virt_to_dma(td->start_seg, td->start_trb);
+	td_end = xhci_trb_virt_to_dma(td->end_seg, td->end_trb);
+	xhci_err(xhci, "Event dma %pad for ep %d status %d not part of TD at %pad - %pad\n",
+		 &ep_trb_dma, ep_index, trb_comp_code, &td_start, &td_end);
 
 	return -ESHUTDOWN;
 
 err_out:
-	xhci_err(xhci, "@%016llx %08x %08x %08x %08x\n",
-		 (unsigned long long) xhci_trb_virt_to_dma(
-			 ir->event_ring->deq_seg,
-			 ir->event_ring->dequeue),
+	deq = xhci_trb_virt_to_dma(ir->event_ring->deq_seg, ir->event_ring->dequeue);
+	xhci_err(xhci, "@%pad %08x %08x %08x %08x\n",
+		 &deq,
 		 lower_32_bits(le64_to_cpu(event->buffer)),
 		 upper_32_bits(le64_to_cpu(event->buffer)),
 		 le32_to_cpu(event->transfer_len),
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index bfb5c5c17012..8451e9386aa9 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -95,9 +95,9 @@ DECLARE_EVENT_CLASS(xhci_log_ctx,
 			((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) *
 			((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1));
 	),
-	TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
+	TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%pad, ctx_va=@%p",
 			__entry->ctx_64, __entry->ctx_type,
-			(unsigned long long) __entry->ctx_dma, __entry->ctx_va
+			&__entry->ctx_dma, __entry->ctx_va
 	)
 );
 
@@ -174,22 +174,22 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
 	TP_ARGS(vdev),
 	TP_STRUCT__entry(
 		__field(void *, vdev)
-		__field(unsigned long long, out_ctx)
-		__field(unsigned long long, in_ctx)
+		__field(dma_addr_t, out_ctx)
+		__field(dma_addr_t, in_ctx)
 		__field(int, slot_id)
 		__field(u16, current_mel)
 
 	),
 	TP_fast_assign(
 		__entry->vdev = vdev;
-		__entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
-		__entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
+		__entry->in_ctx = vdev->in_ctx->dma;
+		__entry->out_ctx = vdev->out_ctx->dma;
 		__entry->slot_id = (int) vdev->slot_id;
 		__entry->current_mel = (u16) vdev->current_mel;
 		),
-	TP_printk("vdev %p slot %d ctx %llx | %llx current_mel %d",
-		__entry->vdev, __entry->slot_id, __entry->in_ctx,
-		__entry->out_ctx, __entry->current_mel
+	TP_printk("vdev %p slot %d ctx %pad | %pad current_mel %d",
+		__entry->vdev, __entry->slot_id, &__entry->in_ctx,
+		&__entry->out_ctx, __entry->current_mel
 	)
 );
 
@@ -203,8 +203,8 @@ DECLARE_EVENT_CLASS(xhci_log_virt_dev,
 	TP_ARGS(vdev),
 	TP_STRUCT__entry(
 		__field(void *, vdev)
-		__field(unsigned long long, out_ctx)
-		__field(unsigned long long, in_ctx)
+		__field(dma_addr_t, out_ctx)
+		__field(dma_addr_t, in_ctx)
 		__field(int, devnum)
 		__field(int, state)
 		__field(int, speed)
@@ -214,8 +214,8 @@ DECLARE_EVENT_CLASS(xhci_log_virt_dev,
 	),
 	TP_fast_assign(
 		__entry->vdev = vdev;
-		__entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
-		__entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
+		__entry->in_ctx = vdev->in_ctx->dma;
+		__entry->out_ctx = vdev->out_ctx->dma;
 		__entry->devnum = vdev->udev->devnum;
 		__entry->state = vdev->udev->state;
 		__entry->speed = vdev->udev->speed;
@@ -223,8 +223,8 @@ DECLARE_EVENT_CLASS(xhci_log_virt_dev,
 		__entry->level = vdev->udev->level;
 		__entry->slot_id = vdev->udev->slot_id;
 	),
-	TP_printk("vdev %p ctx %llx | %llx num %d state %d speed %d port %d level %d slot %d",
-		__entry->vdev, __entry->in_ctx, __entry->out_ctx,
+	TP_printk("vdev %p ctx %pad | %pad num %d state %d speed %d port %d level %d slot %d",
+		__entry->vdev, &__entry->in_ctx, &__entry->out_ctx,
 		__entry->devnum, __entry->state, __entry->speed,
 		__entry->portnum, __entry->level, __entry->slot_id
 	)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 47151ca527bf..5e5681c1eb4e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1818,15 +1818,15 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 	}
 
 	i = urb_priv->num_tds_done;
-	if (i < urb_priv->num_tds)
+	if (i < urb_priv->num_tds) {
+		dma_addr_t dma = xhci_trb_virt_to_dma(urb_priv->td[i].start_seg,
+						      urb_priv->td[i].start_trb);
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-				"Cancel URB %p, dev %s, ep 0x%x, "
-				"starting at offset 0x%llx",
+				"Cancel URB %p, dev %s, ep 0x%x, starting at %pad",
 				urb, urb->dev->devpath,
 				urb->ep->desc.bEndpointAddress,
-				(unsigned long long) xhci_trb_virt_to_dma(
-					urb_priv->td[i].start_seg,
-					urb_priv->td[i].start_trb));
+				&dma);
+	}
 
 	for (; i < urb_priv->num_tds; i++) {
 		td = &urb_priv->td[i];
@@ -4273,6 +4273,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 	struct xhci_input_control_ctx *ctrl_ctx;
 	u64 temp_64;
 	struct xhci_command *command = NULL;
+	dma_addr_t dma;
 
 	mutex_lock(&xhci->mutex);
 
@@ -4413,15 +4414,15 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 	temp_64 = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
 			"Op regs DCBAA ptr = %#016llx", temp_64);
+	dma = le64_to_cpu(xhci->dcbaa->dev_context_ptrs[udev->slot_id]);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
-		"Slot ID %d dcbaa entry @%p = %#016llx",
+		"Slot ID %d dcbaa entry @%p = %pad",
 		udev->slot_id,
 		&xhci->dcbaa->dev_context_ptrs[udev->slot_id],
-		(unsigned long long)
-		le64_to_cpu(xhci->dcbaa->dev_context_ptrs[udev->slot_id]));
+		&dma);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
-			"Output Context DMA address = %#08llx",
-			(unsigned long long)virt_dev->out_ctx->dma);
+			"Output Context DMA address = %pad",
+			&virt_dev->out_ctx->dma);
 	trace_xhci_address_ctx(xhci, virt_dev->in_ctx,
 				le32_to_cpu(slot_ctx->dev_info) >> 27);
 	/*
-- 
2.50.1

[PATCH 3/7] usb: xhci: improve Stream Context register debugging

[Niklas Neronin]

Improve the debugging output for Stream Context registers in the xHCI
driver. The Stream Context registers consist of the following fields:
 bit 0 - Dequeue Cycle State.
 bits 3:1 - Stream Context Type.
 bits 63:4 - TR Dequeue Pointer, is 16-byte aligned.

Instead of printing the entire 64-bit register as a single block, each
field is now printed separately. This approach enhances the readability.

Remove xhci_dbg() in xhci_alloc_stream_info(). A detailed trace message is
printed after xhci_update_stream_mapping() call.

xHCI specification, section 6.2.4.1.

Why not use 'dma_addr_t' for the address?
The 'dma_addr_t' type can vary between 32 and 64 bits depending on the
system architecture or xHCI driver flags, whereas the 64-bit address field
size remains constant. Since hardware cannot be fully trusted, it's better
to print the entire 64-bit address to detect any non-zero values in the
upper 32 bits. This approach ensures that potential issues are easily
detected.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-debugfs.c | 16 ++++++++++------
 drivers/usb/host/xhci-mem.c     |  1 -
 drivers/usb/host/xhci-trace.h   |  5 +++--
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index c6d44977193f..35398b95c5a2 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -521,6 +521,7 @@ static int xhci_stream_context_array_show(struct seq_file *s, void *unused)
 	struct xhci_ep_priv	*epriv = s->private;
 	struct xhci_stream_ctx	*stream_ctx;
 	dma_addr_t		dma;
+	u64			ctx;
 	int			id;
 
 	if (!epriv->stream_info)
@@ -533,12 +534,15 @@ static int xhci_stream_context_array_show(struct seq_file *s, void *unused)
 	for (id = 0; id < epriv->stream_info->num_stream_ctxs; id++) {
 		stream_ctx = epriv->stream_info->stream_ctx_array + id;
 		dma = epriv->stream_info->ctx_array_dma + id * 16;
-		if (id < epriv->stream_info->num_streams)
-			seq_printf(s, "%pad stream id %d deq %016llx\n", &dma,
-				   id, le64_to_cpu(stream_ctx->stream_ring));
-		else
-			seq_printf(s, "%pad stream context entry not used deq %016llx\n",
-				   &dma, le64_to_cpu(stream_ctx->stream_ring));
+
+		if (id < epriv->stream_info->num_streams) {
+			ctx = le64_to_cpu(stream_ctx->stream_ring);
+			seq_printf(s, "%pad stream %d: deq %016llx SCT %llu cycle %llu\n",
+				   &dma, id, ctx & TR_DEQ_PTR_MASK, CTX_TO_SCT(ctx),
+				   ctx & EP_CTX_CYCLE_MASK);
+		} else {
+			seq_printf(s, "%pad stream %d: entry not used\n", &dma, id);
+		}
 	}
 
 	return 0;
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2a414dee7233..9520e7c6e774 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -676,7 +676,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
 			cur_ring->cycle_state;
 		stream_info->stream_ctx_array[cur_stream].stream_ring =
 			cpu_to_le64(addr);
-		xhci_dbg(xhci, "Setting stream %d ring ptr to 0x%08llx\n", cur_stream, addr);
 
 		ret = xhci_update_stream_mapping(cur_ring, mem_flags);
 
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 8451e9386aa9..f6a2b4cedb8d 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -329,9 +329,10 @@ DECLARE_EVENT_CLASS(xhci_log_stream_ctx,
 		__entry->ctx_array_dma = info->ctx_array_dma + stream_id * 16;
 
 	),
-	TP_printk("stream %u ctx @%pad: SCT %llu deq %llx", __entry->stream_id,
+	TP_printk("stream %u ctx @%pad: SCT %llu deq %llx cycle %llu", __entry->stream_id,
 		&__entry->ctx_array_dma, CTX_TO_SCT(__entry->stream_ring),
-		__entry->stream_ring
+		__entry->stream_ring & TR_DEQ_PTR_MASK,
+		__entry->stream_ring & EP_CTX_CYCLE_MASK
 	)
 );
 
-- 
2.50.1

[PATCH 4/7] usb: xhci: improve Endpoint Context register debugging

[Niklas Neronin]

Improve the debugging output for Endpoint Context registers in the xHCI
driver. The Endpoint Context registers consist of the following fields:
 bit 0 - Dequeue Cycle State.
 bits 3:1 - RsvdZ.
 bits 63:4 - TR Dequeue Pointer, is 16-byte aligned.

Instead of printing the entire 64-bit register as a single block, each
field is now printed separately. This approach enhances the readability.

xHCI specification, section 6.2.3.

Why not use 'dma_addr_t' for the address?
The 'dma_addr_t' type can vary between 32 and 64 bits depending on the
system architecture or xHCI driver flags, whereas the 64-bit address field
size remains constant. Since hardware cannot be fully trusted, it's better
to print the entire 64-bit address to detect any non-zero values in the
upper 32 bits. This approach ensures that potential issues are easily
detected.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 59ff84ba2d4a..2662356d048e 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2580,9 +2580,9 @@ static inline const char *xhci_decode_ep_context(char *str, u32 info,
 	ret += sprintf(str + ret, "interval %d us max ESIT payload %d CErr %d ",
 			(1 << interval) * 125, esit, cerr);
 
-	ret += sprintf(str + ret, "Type %s %sburst %d maxp %d deq %016llx ",
+	ret += sprintf(str + ret, "Type %s %sburst %d maxp %d deq %016llx cycle %llu",
 			xhci_ep_type_string(ep_type), hid ? "HID" : "",
-			burst, maxp, deq);
+			burst, maxp, deq & TR_DEQ_PTR_MASK, deq & EP_CTX_CYCLE_MASK);
 
 	ret += sprintf(str + ret, "avg trb len %d", avg);
 
-- 
2.50.1

[PATCH 5/7] usb: xhci: improve Command Ring Control register debugging

[Niklas Neronin]

Improve the debugging output for Command Ring Control registers in the xHCI
driver. The CRCR consist of the following fields:
 bit 0 - Ring Cycle State
 bit 1 - Command Stop
 bit 2 - Command Abort
 bit 3 - Command Ring Running
 bits 5:4 - RsvdP.
 bits 63:4 - TR Dequeue Pointer, is 16-byte aligned.

Instead of printing the entire 64-bit register as a single block, each
field is now printed separately using the appropriate printk() specifiers.
This approach enhances user interpretation and ensures the DMA address
format is displayed accurately.

Except for bit 3, reading the other bits will consistently return '0'.
Therefore, only modified bits and bit 3 are printed.

xHCI specification, section 5.4.5.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5e5681c1eb4e..4526989169a3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -499,7 +499,8 @@ static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
 	crcr &= ~CMD_RING_CYCLE;
 	crcr |= xhci->cmd_ring->cycle_state;
 
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Setting command ring address to 0x%llx", crcr);
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Command ring deq %pad running %u cycle %u",
+		       &deq_dma, crcr & CMD_RING_RUNNING, xhci->cmd_ring->cycle_state);
 	xhci_write_64(xhci, crcr, &xhci->op_regs->cmd_ring);
 }
 
-- 
2.50.1

[PATCH 6/7] usb: xhci: improve Event Ring Dequeue Pointer Register debugging

[Niklas Neronin]

Improve debugging output for Event Ring Dequeue Pointer Register (ERDP).
The ERDP registers consist of the following fields:
 bits 2:0 - Dequeue ERST Segment Index
 bit  3 - Event Handler Busy
 bits 63:4 - Event Ring Dequeue Pointer, is 16-byte aligned.

Instead of printing the entire 64-bit register as a single block, each
field is now printed separately. This approach enhances the readability.

xHCI specification, section 5.5.2.3.3

Why not use 'dma_addr_t' for the address?
The 'dma_addr_t' type can vary between 32 and 64 bits depending on the
system architecture or xHCI driver flags, whereas the 64-bit address field
size remains constant. Since hardware cannot be fully trusted, it's better
to print the entire 64-bit address to detect any non-zero values in the
upper 32 bits. This approach ensures that potential issues are easily
detected.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4526989169a3..c7fa82f8b853 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -634,7 +634,7 @@ static int xhci_run_finished(struct xhci_hcd *xhci)
  */
 int xhci_run(struct usb_hcd *hcd)
 {
-	u64 temp_64;
+	u64 erdp;
 	int ret;
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 	struct xhci_interrupter *ir = xhci->interrupters[0];
@@ -651,10 +651,9 @@ int xhci_run(struct usb_hcd *hcd)
 
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "xhci_run");
 
-	temp_64 = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
-	temp_64 &= ERST_PTR_MASK;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"ERST deq = 64'h%0lx", (long unsigned int) temp_64);
+	erdp = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "ERST ptr %llx DESI %llu EHB %llu",
+		       erdp & ERST_PTR_MASK, erdp & ERST_DESI_MASK, erdp & ERST_EHB);
 
 	xhci_set_interrupter_moderation(ir, xhci->imod_interval);
 
-- 
2.50.1

[PATCH 7/7] usb: xhci: standardize address format

[Niklas Neronin]

In the xHCI driver, printed hexadecimal values can be ambiguous, making it
difficult to distinguish between values and addresses. To enhance clarity,
all DMA addresses are now prefixed with the '@' symbol, ensuring immediate
recognition as addresses.

The '@' prefix is removed from '%p' specifiers, as they represent unique
address IDs rather than actual addresses. The hashing process does not
preserve any relation between addresses; for instance, if two addresses
are X bytes apart, their hashed counterparts will not reflect this
distinction.

Exceptions to the '@' prefix rule are functions xhci_ring_enqueue_show()
and xhci_ring_dequeue_show(), which exclusively print to the enqueue and
dequeue debugfs files, containing only addresses.

Standardize printing of all 64-bit addresses read from registers, using
the "0x%llx" specifier. Adding padding is unnecessary and provides no
useful information. Prefix the value with "0x" to clearly indicate that its
a hexadecimal.

$ git grep -n '0x%' | wc -l
39796
$ git grep -n '%#' | wc -l
5204

Redundant "0x" string prefix is removed from DMA addresses printed using
the '%pad' specifier, since '%pad' automatically includes the "0x" prefix.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-dbgcap.c  |  2 +-
 drivers/usb/host/xhci-debugfs.c | 10 +++++-----
 drivers/usb/host/xhci-mem.c     | 10 +++++-----
 drivers/usb/host/xhci-ring.c    | 17 ++++++++---------
 drivers/usb/host/xhci-trace.h   | 10 +++++-----
 drivers/usb/host/xhci.c         | 12 ++++++------
 drivers/usb/host/xhci.h         |  2 +-
 7 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 2f24a3a54439..5c8f87c24fe7 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -771,7 +771,7 @@ static void dbc_handle_xfer_event(struct xhci_dbc *dbc, union xhci_trb *event)
 		break;
 	case COMP_STALL_ERROR:
 		deq = le64_to_cpu(ep_ctx->deq) & TR_DEQ_PTR_MASK;
-		dev_warn(dbc->dev, "Stall error at bulk TRB %llx, remaining %zu, ep deq %llx\n",
+		dev_warn(dbc->dev, "Stall error at bulk TRB 0x%llx, remaining %zu, ep deq 0x%llx\n",
 			 ep_trb, remain_length, deq);
 		status = 0;
 		dep->halted = 1;
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 35398b95c5a2..698b9fb59630 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -204,7 +204,7 @@ static void xhci_ring_dump_segment(struct seq_file *s,
 	for (i = 0; i < TRBS_PER_SEGMENT; i++) {
 		trb = &seg->trbs[i];
 		dma = seg->dma + i * sizeof(*trb);
-		seq_printf(s, "%2u %pad: %s\n", seg->num, &dma,
+		seq_printf(s, "%2u @%pad: %s\n", seg->num, &dma,
 			   xhci_decode_trb(str, XHCI_MSG_MAX, le32_to_cpu(trb->generic.field[0]),
 					   le32_to_cpu(trb->generic.field[1]),
 					   le32_to_cpu(trb->generic.field[2]),
@@ -254,7 +254,7 @@ static int xhci_slot_context_show(struct seq_file *s, void *unused)
 
 	xhci = hcd_to_xhci(bus_to_hcd(dev->udev->bus));
 	slot_ctx = xhci_get_slot_ctx(xhci, dev->out_ctx);
-	seq_printf(s, "%pad: %s\n", &dev->out_ctx->dma,
+	seq_printf(s, "@%pad: %s\n", &dev->out_ctx->dma,
 		   xhci_decode_slot_context(str,
 					    le32_to_cpu(slot_ctx->dev_info),
 					    le32_to_cpu(slot_ctx->dev_info2),
@@ -279,7 +279,7 @@ static int xhci_endpoint_context_show(struct seq_file *s, void *unused)
 	for (ep_index = 0; ep_index < 31; ep_index++) {
 		ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
 		dma = dev->out_ctx->dma + (ep_index + 1) * CTX_SIZE(xhci->hcc_params);
-		seq_printf(s, "%pad: %s, virt_state:%#x\n", &dma,
+		seq_printf(s, "@%pad: %s, virt_state:%#x\n", &dma,
 			   xhci_decode_ep_context(str,
 						  le32_to_cpu(ep_ctx->ep_info),
 						  le32_to_cpu(ep_ctx->ep_info2),
@@ -537,11 +537,11 @@ static int xhci_stream_context_array_show(struct seq_file *s, void *unused)
 
 		if (id < epriv->stream_info->num_streams) {
 			ctx = le64_to_cpu(stream_ctx->stream_ring);
-			seq_printf(s, "%pad stream %d: deq %016llx SCT %llu cycle %llu\n",
+			seq_printf(s, "@%pad stream %d: deq 0x%llx SCT %llu cycle %llu\n",
 				   &dma, id, ctx & TR_DEQ_PTR_MASK, CTX_TO_SCT(ctx),
 				   ctx & EP_CTX_CYCLE_MASK);
 		} else {
-			seq_printf(s, "%pad stream %d: entry not used\n", &dma, id);
+			seq_printf(s, "@%pad stream %d: entry not used\n", &dma, id);
 		}
 	}
 
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 9520e7c6e774..7a4a878db1c9 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -987,14 +987,14 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
 	if (!dev->out_ctx)
 		goto fail;
 
-	xhci_dbg(xhci, "Slot %d output ctx = 0x%pad (dma)\n", slot_id, &dev->out_ctx->dma);
+	xhci_dbg(xhci, "Slot %d output ctx = @%pad (dma)\n", slot_id, &dev->out_ctx->dma);
 
 	/* Allocate the (input) device context for address device command */
 	dev->in_ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_INPUT, flags);
 	if (!dev->in_ctx)
 		goto fail;
 
-	xhci_dbg(xhci, "Slot %d input ctx = 0x%pad (dma)\n", slot_id, &dev->in_ctx->dma);
+	xhci_dbg(xhci, "Slot %d input ctx = @%pad (dma)\n", slot_id, &dev->in_ctx->dma);
 
 	/* Initialize the cancellation and bandwidth list for each ep */
 	for (i = 0; i < 31; i++) {
@@ -1014,7 +1014,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
 
 	/* Point to output device context in dcbaa. */
 	xhci->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(dev->out_ctx->dma);
-	xhci_dbg(xhci, "Set slot id %d dcbaa entry %p to %pad\n",
+	xhci_dbg(xhci, "Set slot id %d dcbaa entry %p to @%pad\n",
 		 slot_id,
 		 &xhci->dcbaa->dev_context_ptrs[slot_id],
 		 &dev->out_ctx->dma);
@@ -2422,7 +2422,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 
 	xhci->dcbaa->dma = dma;
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-		       "Device context base array address = 0x%pad (DMA), %p (virt)",
+		       "Device context base array address = @%pad (DMA), %p (virt)",
 		       &xhci->dcbaa->dma, xhci->dcbaa);
 
 	/*
@@ -2483,7 +2483,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 		goto fail;
 
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Allocated command ring at %p", xhci->cmd_ring);
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%pad",
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is @%pad",
 		       &xhci->cmd_ring->first_seg->dma);
 
 	/*
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7d57e2a3abf3..1e9c2024df30 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -754,15 +754,14 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
 	}
 
 	if ((ep->ep_state & SET_DEQ_PENDING)) {
-		xhci_warn(xhci, "Set TR Deq already pending, don't submit for 0x%pad\n",
-			  &addr);
+		xhci_warn(xhci, "Set TR Deq already pending, don't submit for @%pad\n", &addr);
 		return -EBUSY;
 	}
 
 	/* This function gets called from contexts where it cannot sleep */
 	cmd = xhci_alloc_command(xhci, false, GFP_ATOMIC);
 	if (!cmd) {
-		xhci_warn(xhci, "Can't alloc Set TR Deq cmd 0x%pad\n", &addr);
+		xhci_warn(xhci, "Can't alloc Set TR Deq cmd @%pad\n", &addr);
 		return -ENOMEM;
 	}
 
@@ -781,7 +780,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 %pad, cycle %u\n", &addr, new_cycle);
+		       "Set TR Deq ptr @%pad, cycle %u\n", &addr, new_cycle);
 
 	/* Stop the TD queueing code from ringing the doorbell until
 	 * this command completes.  The HC won't set the dequeue pointer
@@ -1049,7 +1048,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 	list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) {
 		dma = xhci_trb_virt_to_dma(td->start_seg, td->start_trb);
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-			       "Removing canceled TD starting at %pad (dma) in stream %u URB %p",
+			       "Removing canceled TD starting at @%pad (dma) in stream %u URB %p",
 			       &dma, td->urb->stream_id, td->urb);
 		list_del_init(&td->td_list);
 		ring = xhci_urb_to_transfer_ring(xhci, td->urb);
@@ -1500,7 +1499,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
 			deq = le64_to_cpu(ep_ctx->deq) & TR_DEQ_PTR_MASK;
 		}
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-			"Successful Set TR Deq Ptr cmd, deq = @%08llx", deq);
+			"Successful Set TR Deq Ptr cmd, deq = 0x%llx", deq);
 		if (xhci_trb_virt_to_dma(ep->queued_deq_seg,
 					 ep->queued_deq_ptr) == deq) {
 			/* Update the ring's dequeue segment and dequeue pointer
@@ -2254,7 +2253,7 @@ static void finish_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 			if ((ep->ep_state & EP_HALTED) &&
 			    !list_empty(&td->cancelled_td_list)) {
 				dma = xhci_trb_virt_to_dma(td->start_seg, td->start_trb);
-				xhci_dbg(xhci, "Already resolving halted ep for %pad\n", &dma);
+				xhci_dbg(xhci, "Already resolving halted ep for @%pad\n", &dma);
 				return;
 			}
 			/* endpoint not halted, don't reset it */
@@ -2914,7 +2913,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 			 * transfer or error on last TRB. Ignore it.
 			 */
 			if (xhci_spurious_success_tx_event(xhci, ep_ring)) {
-				xhci_dbg(xhci, "Spurious event dma %pad, comp_code %u after %u\n",
+				xhci_dbg(xhci, "Spurious event dma @%pad, comp_code %u after %u\n",
 					 &ep_trb_dma, trb_comp_code, ep_ring->old_trb_comp_code);
 				ep_ring->old_trb_comp_code = 0;
 				return 0;
@@ -2978,7 +2977,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 debug_finding_td:
 	td_start = xhci_trb_virt_to_dma(td->start_seg, td->start_trb);
 	td_end = xhci_trb_virt_to_dma(td->end_seg, td->end_trb);
-	xhci_err(xhci, "Event dma %pad for ep %d status %d not part of TD at %pad - %pad\n",
+	xhci_err(xhci, "Event dma @%pad for ep %d status %d not part of TD at @%pad - @%pad\n",
 		 &ep_trb_dma, ep_index, trb_comp_code, &td_start, &td_end);
 
 	return -ESHUTDOWN;
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index f6a2b4cedb8d..fd3ed105b5c4 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -95,7 +95,7 @@ DECLARE_EVENT_CLASS(xhci_log_ctx,
 			((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) *
 			((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1));
 	),
-	TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%pad, ctx_va=@%p",
+	TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%pad, ctx_va=%p",
 			__entry->ctx_64, __entry->ctx_type,
 			&__entry->ctx_dma, __entry->ctx_va
 	)
@@ -187,7 +187,7 @@ DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
 		__entry->slot_id = (int) vdev->slot_id;
 		__entry->current_mel = (u16) vdev->current_mel;
 		),
-	TP_printk("vdev %p slot %d ctx %pad | %pad current_mel %d",
+	TP_printk("vdev %p slot %d ctx @%pad | @%pad current_mel %d",
 		__entry->vdev, __entry->slot_id, &__entry->in_ctx,
 		&__entry->out_ctx, __entry->current_mel
 	)
@@ -223,7 +223,7 @@ DECLARE_EVENT_CLASS(xhci_log_virt_dev,
 		__entry->level = vdev->udev->level;
 		__entry->slot_id = vdev->udev->slot_id;
 	),
-	TP_printk("vdev %p ctx %pad | %pad num %d state %d speed %d port %d level %d slot %d",
+	TP_printk("vdev %p ctx @%pad | @%pad num %d state %d speed %d port %d level %d slot %d",
 		__entry->vdev, &__entry->in_ctx, &__entry->out_ctx,
 		__entry->devnum, __entry->state, __entry->speed,
 		__entry->portnum, __entry->level, __entry->slot_id
@@ -329,7 +329,7 @@ DECLARE_EVENT_CLASS(xhci_log_stream_ctx,
 		__entry->ctx_array_dma = info->ctx_array_dma + stream_id * 16;
 
 	),
-	TP_printk("stream %u ctx @%pad: SCT %llu deq %llx cycle %llu", __entry->stream_id,
+	TP_printk("stream %u ctx @%pad: SCT %llu deq 0x%llx cycle %llu", __entry->stream_id,
 		&__entry->ctx_array_dma, CTX_TO_SCT(__entry->stream_ring),
 		__entry->stream_ring & TR_DEQ_PTR_MASK,
 		__entry->stream_ring & EP_CTX_CYCLE_MASK
@@ -505,7 +505,7 @@ DECLARE_EVENT_CLASS(xhci_log_ring,
 		__entry->enq = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
 		__entry->deq = xhci_trb_virt_to_dma(ring->deq_seg, ring->dequeue);
 	),
-	TP_printk("%s %p: enq %pad deq %pad segs %d stream %d bounce %d cycle %d",
+	TP_printk("%s %p: enq @%pad deq @%pad segs %d stream %d bounce %d cycle %d",
 			xhci_ring_type_string(__entry->type), __entry->ring,
 			&__entry->enq,
 			&__entry->deq,
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c7fa82f8b853..17755a567c6c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -499,7 +499,7 @@ static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
 	crcr &= ~CMD_RING_CYCLE;
 	crcr |= xhci->cmd_ring->cycle_state;
 
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Command ring deq %pad running %u cycle %u",
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Command ring deq @%pad running %u cycle %u",
 		       &deq_dma, crcr & CMD_RING_RUNNING, xhci->cmd_ring->cycle_state);
 	xhci_write_64(xhci, crcr, &xhci->op_regs->cmd_ring);
 }
@@ -652,7 +652,7 @@ int xhci_run(struct usb_hcd *hcd)
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "xhci_run");
 
 	erdp = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "ERST ptr %llx DESI %llu EHB %llu",
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "ERST ptr 0x%llx DESI %llu EHB %llu",
 		       erdp & ERST_PTR_MASK, erdp & ERST_DESI_MASK, erdp & ERST_EHB);
 
 	xhci_set_interrupter_moderation(ir, xhci->imod_interval);
@@ -1822,7 +1822,7 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 		dma_addr_t dma = xhci_trb_virt_to_dma(urb_priv->td[i].start_seg,
 						      urb_priv->td[i].start_trb);
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-				"Cancel URB %p, dev %s, ep 0x%x, starting at %pad",
+				"Cancel URB %p, dev %s, ep 0x%x, starting at @%pad",
 				urb, urb->dev->devpath,
 				urb->ep->desc.bEndpointAddress,
 				&dma);
@@ -4413,15 +4413,15 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 		goto out;
 	temp_64 = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
-			"Op regs DCBAA ptr = %#016llx", temp_64);
+			"Op regs DCBAA ptr = 0x%llx", temp_64);
 	dma = le64_to_cpu(xhci->dcbaa->dev_context_ptrs[udev->slot_id]);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
-		"Slot ID %d dcbaa entry @%p = %pad",
+		"Slot ID %d dcbaa entry %p = @%pad",
 		udev->slot_id,
 		&xhci->dcbaa->dev_context_ptrs[udev->slot_id],
 		&dma);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
-			"Output Context DMA address = %pad",
+			"Output Context DMA address = @%pad",
 			&virt_dev->out_ctx->dma);
 	trace_xhci_address_ctx(xhci, virt_dev->in_ctx,
 				le32_to_cpu(slot_ctx->dev_info) >> 27);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2662356d048e..f8d742695176 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2580,7 +2580,7 @@ static inline const char *xhci_decode_ep_context(char *str, u32 info,
 	ret += sprintf(str + ret, "interval %d us max ESIT payload %d CErr %d ",
 			(1 << interval) * 125, esit, cerr);
 
-	ret += sprintf(str + ret, "Type %s %sburst %d maxp %d deq %016llx cycle %llu",
+	ret += sprintf(str + ret, "Type %s %sburst %d maxp %d deq 0x%llx cycle %llu",
 			xhci_ep_type_string(ep_type), hid ? "HID" : "",
 			burst, maxp, deq & TR_DEQ_PTR_MASK, deq & EP_CTX_CYCLE_MASK);
 
-- 
2.50.1