Skip to contentSkip to conversation

xhci: allow TRACE to work with EVENT ring dequeue

PATCH Superseded Adam Wallis 3 participants 6 messages Sep 25, 2017 – Oct 5, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 xhci: allow TRACE to work with EVENT ring dequeue

inc_deq() currently bails earlier for EVENT rings than the common return point of the function, due to the fact that EVENT rings do not have link TRBs. The unfortunate side effect of this is that the very useful trace_xhci_inc_deq() function is not called/usable for EVENT ring debug.

This patch provides a refactor by removing the multiple return exit points into a single return which additionally allows for all rings to use the trace function.

---
Changes in v2: undo accidental line removal at end of patch Changes in v3: reduced complexity by using goto as recommended by Mathias

 drivers/usb/host/xhci-ring.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-ring.c +3 -2
index a944365..8d3df2f 100644
@@ -171,13 +171,13 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
 	if (ring->type == TYPE_EVENT) {
 		if (!last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
 			ring->dequeue++;
-			return;
+			goto out;
 		}
 		if (last_trb_on_ring(ring, ring->deq_seg, ring->dequeue))
 			ring->cycle_state ^= 1;
 		ring->deq_seg = ring->deq_seg->next;
 		ring->dequeue = ring->deq_seg->trbs;
-		return;
+		goto out;
 	}
 
 	/* All other rings have link trbs */
@@ -190,6 +190,7 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
 		ring->dequeue = ring->deq_seg->trbs;
 	}
 
+out:
 	trace_xhci_inc_deq(ring);
 
 	return;
Signature · 3 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
#1 lore raw

PATCH v3 1/1 xhci: allow TRACE to work with EVENT ring dequeue

From: Adam Wallis

Quoted 6 lines

Sent: 25 September 2017 13:26
inc_deq() currently bails earlier for EVENT rings than the common return point of the function, due to the fact that EVENT rings do not have link TRBs. The unfortunate side effect of this is that the very useful trace_xhci_inc_deq() function is not called/usable for EVENT ring debug.

Is it actually worth using different functions for the different ring types?

Quoted 1 line

From what I remember there are conditionals in a lot of the functions

but they are fixed for most of the call sites.

David

#2 lore raw

PATCH v3 1/1 xhci: allow TRACE to work with EVENT ring dequeue

Quoted 10 lines · David Laight

From: Adam Wallis

Quoted 6 lines

Sent: 25 September 2017 13:26
inc_deq() currently bails earlier for EVENT rings than the common return point of the function, due to the fact that EVENT rings do not have link TRBs. The unfortunate side effect of this is that the very useful trace_xhci_inc_deq() function is not called/usable for EVENT ring debug.

Is it actually worth using different functions for the different ring types?

David, that is what this patch attempts to add/fix. Instead of adding another TRACE function for event rings, it hooks the existing trace functionality already used in inc_deq()

Quoted 2 lines

From what I remember there are conditionals in a lot of the functions but they are fixed for most of the call sites.

That's basically all this patch is...fixing a conditional. Thanks for the feedback, let me know if I misunderstood your comments!

Quoted 9 lines

David

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 4 lines
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#3 lore raw

PATCH v3 1/1 xhci: allow TRACE to work with EVENT ring dequeue

In reply to David Laight
Quoted 8 lines · David Laight

From: Adam Wallis

Quoted 6 lines

Sent: 25 September 2017 13:26
inc_deq() currently bails earlier for EVENT rings than the common return point of the function, due to the fact that EVENT rings do not have link TRBs. The unfortunate side effect of this is that the very useful trace_xhci_inc_deq() function is not called/usable for EVENT ring debug.

Quoted 5 lines

Is it actually worth using different functions for the different ring types?
From what I remember there are conditionals in a lot of the functions
but they are fixed for most of the call sites.

There's some restructuring and refactoring that could be done in xhci, but that's not part of this patch.

This will just enable better debugging.

Applying this patch

Thanks
-Mathias

#4 lore raw

PATCH v3 1/1 xhci: allow TRACE to work with EVENT ring dequeue

Quoted 15 lines · Mathias Nyman
Quoted 8 lines · David Laight

From: Adam Wallis

Quoted 6 lines

Sent: 25 September 2017 13:26
inc_deq() currently bails earlier for EVENT rings than the common return point of the function, due to the fact that EVENT rings do not have link TRBs. The unfortunate side effect of this is that the very useful trace_xhci_inc_deq() function is not called/usable for EVENT ring debug.

Quoted 5 lines

Is it actually worth using different functions for the different ring types?
?From what I remember there are conditionals in a lot of the functions but they are fixed for most of the call sites.

Quoted 6 lines

There's some restructuring and refactoring that could be done in xhci, but that's not part of this patch.

This will just enable better debugging.

Applying this patch

Sounds great, thanks! Will this be going in on 4.14 sometime shortly? I hadn't seen it in your tree and was curious since we are tracking internally. Thanks!

Quoted 3 lines

Thanks
-Mathias

Signature · 4 lines
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#5 lore raw

PATCH v3 1/1 xhci: allow TRACE to work with EVENT ring dequeue

Quoted 16 lines · Adam Wallis
Quoted 15 lines · Mathias Nyman
Quoted 13 lines · David Laight

From: Adam Wallis

Quoted 6 lines

Sent: 25 September 2017 13:26
inc_deq() currently bails earlier for EVENT rings than the common return point of the function, due to the fact that EVENT rings do not have link TRBs. The unfortunate side effect of this is that the very useful trace_xhci_inc_deq() function is not called/usable for EVENT ring debug.

Is it actually worth using different functions for the different ring types?
From what I remember there are conditionals in a lot of the functions
but they are fixed for most of the call sites.

Quoted 9 lines
Quoted 6 lines

There's some restructuring and refactoring that could be done in xhci, but that's not part of this patch.

This will just enable better debugging.

Applying this patch

Sounds great, thanks! Will this be going in on 4.14 sometime shortly? I hadn't seen it in your tree and was curious since we are tracking internally. Thanks!

Now pushed to my for-usb-next branch and sent forward to Greg. On its way to 4.15

-Mathias