[LINUX PATCH] usb: xhci: Add toggle cycle bit for the last seg trb when cached ring is used

[LINUX PATCH] usb: xhci: Add toggle cycle bit for the last seg trb when cached ring is used

[Anurag Kumar Vulisha]

At present USB 2.0 cameras fail to play when run for 2nd time or greater.
This issue occurs only when previous endpoint cached ring is used instead
of allocating a new endpoint ring. The root cause of the problem narrows
down to the Toggle bit, which is not getting set on the last trb of the
cached ring last segment. When the controller fetches the last link trb
with no toggle bit set, abnormal behaviour is generated.
This patch solves that problem by adding the TOGGLE bit for the last trb
of the last segment of the cached ring.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/usb/host/xhci-mem.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 1f1687e..3e59ba9 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -455,6 +455,14 @@ static void xhci_reinit_cached_ring(struct xhci_hcd *xhci,
 	} while (seg != ring->first_seg);
 	ring->type = type;
 	xhci_initialize_ring_info(ring, cycle_state);
+
+	/* Only event ring does not use link TRB */
+	if (type != TYPE_EVENT) {
+		/* See xhci spec section 4.9.2.1 and 6.4.4.1 */
+		ring->last_seg->trbs[TRBS_PER_SEGMENT - 1].link.control |=
+			cpu_to_le32(LINK_TOGGLE);
+	}
+
 	/* td list should be empty since all URBs have been cancelled,
 	 * but just in case...
 	 */
-- 
2.1.1

Re: [LINUX PATCH] usb: xhci: Add toggle cycle bit for the last seg trb when cached ring is used

[Mathias Nyman]

On 31.05.2017 16:41, Anurag Kumar Vulisha wrote:
> At present USB 2.0 cameras fail to play when run for 2nd time or greater.
> This issue occurs only when previous endpoint cached ring is used instead
> of allocating a new endpoint ring. The root cause of the problem narrows
> down to the Toggle bit, which is not getting set on the last trb of the
> cached ring last segment. When the controller fetches the last link trb
> with no toggle bit set, abnormal behaviour is generated.
> This patch solves that problem by adding the TOGGLE bit for the last trb
> of the last segment of the cached ring.
>
> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>

Thanks, nice catch.

I'm thinking about removing the whole endpoint ring caching instead.

We are using more memory by caching the rings instead of freeing them,
especially as we always try to allocate a new ring and only use the cached
ones when we fail to allocate a new ring.

Would you be willing to test a patch that removes the ring cache completely
with your setup?

Thanks
Mathias  

RE: [LINUX PATCH] usb: xhci: Add toggle cycle bit for the last seg trb when cached ring is used

[Anurag Kumar Vulisha]

HI Mathias,

>-----Original Message-----
>From: Mathias Nyman [mailto:mathias.nyman@linux.intel.com]
>Sent: Thursday, June 01, 2017 6:51 PM
>To: Anurag Kumar Vulisha <anuragku@xilinx.com>; mathias.nyman@intel.com;
>gregkh@linuxfoundation.org
>Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Anirudha Sarangi
><anirudh@xilinx.com>; Anurag Kumar Vulisha <anuragku@xilinx.com>
>Subject: Re: [LINUX PATCH] usb: xhci: Add toggle cycle bit for the last seg trb
>when cached ring is used
>
>On 31.05.2017 16:41, Anurag Kumar Vulisha wrote:
>> At present USB 2.0 cameras fail to play when run for 2nd time or greater.
>> This issue occurs only when previous endpoint cached ring is used
>> instead of allocating a new endpoint ring. The root cause of the
>> problem narrows down to the Toggle bit, which is not getting set on
>> the last trb of the cached ring last segment. When the controller
>> fetches the last link trb with no toggle bit set, abnormal behaviour is generated.
>> This patch solves that problem by adding the TOGGLE bit for the last
>> trb of the last segment of the cached ring.
>>
>> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
>
>Thanks, nice catch.
>
>I'm thinking about removing the whole endpoint ring caching instead.
>
>We are using more memory by caching the rings instead of freeing them,
>especially as we always try to allocate a new ring and only use the cached ones
>when we fail to allocate a new ring.
>
>Would you be willing to test a patch that removes the ring cache completely with
>your setup?
>

Thanks for reviewing the patch. I completely agree with you. But I think instead of
completely removing the cached ring , can we  modify the code to use cached ring
and try to allocate new ep ring on failure to get cached ring. I have previously sent
the patch for doing this https://lkml.org/lkml/2017/2/22/336
But anyways I am ready to test the changes with my platform. Please correct me
if I am wrong

Thanks,
Anurag Kumar Vulisha
 
>Thanks
>Mathias
>