From: Adam Wallis <hidden> Date: 2017-09-25 12:26:05
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.
Signed-off-by: Adam Wallis <redacted>
---
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(-)
@@ -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;+gotoout;}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;+gotoout;}/* All other rings have link trbs */
--
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.
From: David Laight <hidden> Date: 2017-09-25 16:09:56
From: Adam Wallis
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.
David
From: Adam Wallis <hidden> Date: 2017-09-25 17:03:58
On 9/25/2017 12:09 PM, David Laight wrote:
From: Adam Wallis
quoted
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()
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!
--
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.
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.
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
From: Adam Wallis <hidden> Date: 2017-10-04 16:07:52
On 9/26/2017 2:44 AM, Mathias Nyman wrote:
On 25.09.2017 19:09, David Laight wrote:
quoted
From: Adam Wallis
quoted
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.
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!
Thanks
-Mathias
--
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.
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.
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