* [PATCH v2][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue
@ 2017-09-22 19:55 Adam Wallis
2017-09-25 9:59 ` Mathias Nyman
0 siblings, 1 reply; 3+ messages in thread
From: Adam Wallis @ 2017-09-22 19:55 UTC (permalink / raw)
To: linux-arm-kernel
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 <awallis@codeaurora.org>
---
Changes in v2: undo accidental line removal at end of patch
drivers/usb/host/xhci-ring.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a944365..3960ba9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -171,23 +171,23 @@ 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;
+ } else {
+ 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;
+ }
+ } else {
+ /* All other rings have link trbs */
+ if (!trb_is_link(ring->dequeue)) {
+ ring->dequeue++;
+ ring->num_trbs_free++;
+ }
+ while (trb_is_link(ring->dequeue)) {
+ ring->deq_seg = ring->deq_seg->next;
+ ring->dequeue = ring->deq_seg->trbs;
}
- 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;
- }
-
- /* All other rings have link trbs */
- if (!trb_is_link(ring->dequeue)) {
- ring->dequeue++;
- ring->num_trbs_free++;
- }
- while (trb_is_link(ring->dequeue)) {
- ring->deq_seg = ring->deq_seg->next;
- ring->dequeue = ring->deq_seg->trbs;
}
trace_xhci_inc_deq(ring);
--
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.
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue
2017-09-22 19:55 [PATCH v2][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue Adam Wallis
@ 2017-09-25 9:59 ` Mathias Nyman
2017-09-25 12:02 ` Adam Wallis
0 siblings, 1 reply; 3+ messages in thread
From: Mathias Nyman @ 2017-09-25 9:59 UTC (permalink / raw)
To: linux-arm-kernel
On 22.09.2017 22:55, Adam Wallis wrote:
> 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.
True, makes sense to trace the event ring deq ptr as well.
>
> 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 <awallis@codeaurora.org>
> ---
> Changes in v2: undo accidental line removal at end of patch
>
> drivers/usb/host/xhci-ring.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index a944365..3960ba9 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -171,23 +171,23 @@ 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;
> + } else {
> + 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;
> + }
> + } else {
> + /* All other rings have link trbs */
> + if (!trb_is_link(ring->dequeue)) {
> + ring->dequeue++;
> + ring->num_trbs_free++;
> + }
> + while (trb_is_link(ring->dequeue)) {
> + ring->deq_seg = ring->deq_seg->next;
> + ring->dequeue = ring->deq_seg->trbs;
The added level of indentation makes it a little bit messier.
How about just using goto out; instead of return?
out:
trace_xhci_inc_deq(ring);
-Mathias
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue
2017-09-25 9:59 ` Mathias Nyman
@ 2017-09-25 12:02 ` Adam Wallis
0 siblings, 0 replies; 3+ messages in thread
From: Adam Wallis @ 2017-09-25 12:02 UTC (permalink / raw)
To: linux-arm-kernel
On 9/25/2017 5:59 AM, Mathias Nyman wrote:
> On 22.09.2017 22:55, Adam Wallis wrote:
>> 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.
>
> True, makes sense to trace the event ring deq ptr as well.
>
>>
>> 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 <awallis@codeaurora.org>
>> ---
>> Changes in v2: undo accidental line removal at end of patch
>>
>> ? drivers/usb/host/xhci-ring.c | 32 ++++++++++++++++----------------
>> ? 1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index a944365..3960ba9 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -171,23 +171,23 @@ 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;
>> +??????? } else {
>> +??????????? 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;
>> +??????? }
>> +??? } else {
>> +??????? /* All other rings have link trbs */
>> +??????? if (!trb_is_link(ring->dequeue)) {
>> +??????????? ring->dequeue++;
>> +??????????? ring->num_trbs_free++;
>> +??????? }
>> +??????? while (trb_is_link(ring->dequeue)) {
>> +??????????? ring->deq_seg = ring->deq_seg->next;
>> +??????????? ring->dequeue = ring->deq_seg->trbs;
>
> The added level of indentation makes it a little bit messier.
> How about just using goto out; instead of return?
>
> out:
> ????trace_xhci_inc_deq(ring);
My first version of this patch (before submission) actually had gotos, but I
wasn't sure if you would accept. I agree that this will be much cleaner. v3
patch en route!
>
> -Mathias
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at? http://vger.kernel.org/majordomo-info.html
--
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-09-25 12:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-22 19:55 [PATCH v2][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue Adam Wallis
2017-09-25 9:59 ` Mathias Nyman
2017-09-25 12:02 ` Adam Wallis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).