[PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Julius Werner]

The current XHCI driver recalculates the Context Entries field in the
Slot Context on every add_endpoint() and drop_endpoint() call. In the
case of drop_endpoint(), it seems to assume that the add_flags will
always contain every endpoint for the new configuration, which is not
necessarily correct if you don't make assumptions about how the USB core
uses the add_endpoint/drop_endpoint interface (add_flags only contains
endpoints that are new additions in the new configuration).

Furthermore, EP0_FLAG is not consistently set in add_flags throughout
the lifetime of a device. This means that when all endpoints are
dropped, the Context Entries field can be set to 0 (which is invalid and
may cause a Parameter Error) or -1 (which is interpreted as 31 and
causes the driver to keep using the old, incorrect value).

The only surefire way to set this field right is to also take all
existing endpoints into account, and to force the value to 1 (meaning
only EP0 is active) if no other endpoint is found. This patch implements
that as a single step in the final check_bandwidth() call and removes
the intermediary calculations from add_endpoint() and drop_endpoint().

This patch should be backported to kernels as old as 2.6.31 that contain
the commit f94e0186312b0fc39f41eed4e21836ed74b7efe1 "USB: xhci:
Bandwidth allocation support".

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 924a6cc..e7d9dfa 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1562,12 +1562,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	struct xhci_hcd *xhci;
 	struct xhci_container_ctx *in_ctx, *out_ctx;
 	struct xhci_input_control_ctx *ctrl_ctx;
-	struct xhci_slot_ctx *slot_ctx;
-	unsigned int last_ctx;
 	unsigned int ep_index;
 	struct xhci_ep_ctx *ep_ctx;
 	u32 drop_flag;
-	u32 new_add_flags, new_drop_flags, new_slot_info;
+	u32 new_add_flags, new_drop_flags;
 	int ret;
 
 	ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1614,24 +1612,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
 	new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
 
-	last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
-	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-	/* Update the last valid endpoint context, if we deleted the last one */
-	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
-	    LAST_CTX(last_ctx)) {
-		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-	}
-	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
 	xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
 
-	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
+	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
 			(unsigned int) ep->desc.bEndpointAddress,
 			udev->slot_id,
 			(unsigned int) new_drop_flags,
-			(unsigned int) new_add_flags,
-			(unsigned int) new_slot_info);
+			(unsigned int) new_add_flags);
 	return 0;
 }
 
@@ -1654,11 +1641,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	struct xhci_hcd *xhci;
 	struct xhci_container_ctx *in_ctx, *out_ctx;
 	unsigned int ep_index;
-	struct xhci_slot_ctx *slot_ctx;
 	struct xhci_input_control_ctx *ctrl_ctx;
 	u32 added_ctxs;
-	unsigned int last_ctx;
-	u32 new_add_flags, new_drop_flags, new_slot_info;
+	u32 new_add_flags, new_drop_flags;
 	struct xhci_virt_device *virt_dev;
 	int ret = 0;
 
@@ -1673,7 +1658,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 		return -ENODEV;
 
 	added_ctxs = xhci_get_endpoint_flag(&ep->desc);
-	last_ctx = xhci_last_valid_endpoint(added_ctxs);
 	if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
 		/* FIXME when we have to issue an evaluate endpoint command to
 		 * deal with ep0 max packet size changing once we get the
@@ -1739,24 +1723,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	 */
 	new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
 
-	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-	/* Update the last valid endpoint context, if we just added one past */
-	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
-	    LAST_CTX(last_ctx)) {
-		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-	}
-	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
 	/* Store the usb_device pointer for later use */
 	ep->hcpriv = udev;
 
-	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
+	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
 			(unsigned int) ep->desc.bEndpointAddress,
 			udev->slot_id,
 			(unsigned int) new_drop_flags,
-			(unsigned int) new_add_flags,
-			(unsigned int) new_slot_info);
+			(unsigned int) new_add_flags);
 	return 0;
 }
 
@@ -2723,8 +2697,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 			ctrl_ctx->drop_flags == 0)
 		return 0;
 
-	xhci_dbg(xhci, "New Input Control Context:\n");
+	/* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
 	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
+	for (i = 31; i >= 1; i--) {
+		u32 le32 = cpu_to_le32(BIT(i));
+		if ((virt_dev->eps[i-1].ring && !(ctrl_ctx->drop_flags & le32))
+				|| (ctrl_ctx->add_flags & le32) || i == 1) {
+			slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
+			slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(i));
+			break;
+		}
+	}
+
+	xhci_dbg(xhci, "New Input Control Context:\n");
 	xhci_dbg_ctx(xhci, virt_dev->in_ctx,
 		     LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info)));
 
-- 
1.8.3.2

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 588 bytes --]

Hi,

On Tue, Mar 25, 2014 at 11:42:43AM -0700, Julius Werner wrote:
> @@ -2723,8 +2697,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
>  			ctrl_ctx->drop_flags == 0)
>  		return 0;
>  
> -	xhci_dbg(xhci, "New Input Control Context:\n");
> +	/* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
>  	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
> +	for (i = 31; i >= 1; i--) {
> +		u32 le32 = cpu_to_le32(BIT(i));

can you use __le32 instead of u32 here, just to make sure we can make
good use of sparse ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Mathias Nyman]

On 03/25/2014 08:42 PM, Julius Werner wrote:
> The current XHCI driver recalculates the Context Entries field in the
> Slot Context on every add_endpoint() and drop_endpoint() call. In the
> case of drop_endpoint(), it seems to assume that the add_flags will
> always contain every endpoint for the new configuration, which is not
> necessarily correct if you don't make assumptions about how the USB core
> uses the add_endpoint/drop_endpoint interface (add_flags only contains
> endpoints that are new additions in the new configuration).
>
> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
> the lifetime of a device. This means that when all endpoints are
> dropped, the Context Entries field can be set to 0 (which is invalid and
> may cause a Parameter Error) or -1 (which is interpreted as 31 and
> causes the driver to keep using the old, incorrect value).
>
> The only surefire way to set this field right is to also take all
> existing endpoints into account, and to force the value to 1 (meaning
> only EP0 is active) if no other endpoint is found. This patch implements
> that as a single step in the final check_bandwidth() call and removes
> the intermediary calculations from add_endpoint() and drop_endpoint().
>

Hi

I think this looks good in general, but before I really dig into it I'd 
like to know if this has triggered any problems so far? (e.g. slot 
context's context entries getting zeroed, or set to a smaller value than 
the last active endpoint context,  causing trouble)

-Mathias

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Sarah Sharp]

On Tue, Mar 25, 2014 at 11:42:43AM -0700, Julius Werner wrote:
> The current XHCI driver recalculates the Context Entries field in the
> Slot Context on every add_endpoint() and drop_endpoint() call. In the
> case of drop_endpoint(), it seems to assume that the add_flags will
> always contain every endpoint for the new configuration, which is not
> necessarily correct if you don't make assumptions about how the USB core
> uses the add_endpoint/drop_endpoint interface (add_flags only contains
> endpoints that are new additions in the new configuration).

The last valid endpoint context has been discussed before:

http://marc.info/?l=linux-usb&m=137158978503741&w=2

There's an xHCI spec ambiguity:  Does the last valid context entry refer
to the last valid endpoint context in the *input* device context or the
*output* device context?

The code currently assumes it refers to the input device context, namely
the endpoints we're adding or changing.  If hardware needs the last
valid endpoint context for the re-calculated *output* device context,
then yes, this needs to be changed.  However, based on spec errata, I
believe that's not the intent of the spec authors:

http://marc.info/?l=linux-kernel&m=137208958411696&w=2

What is the impact if we calculate the valid last valid endpoint context
for the input context?  Do you have evidence of hardware misbehaving?
If so, which hardware?

> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
> the lifetime of a device. This means that when all endpoints are
> dropped, the Context Entries field can be set to 0 (which is invalid and
> may cause a Parameter Error) or -1 (which is interpreted as 31 and
> causes the driver to keep using the old, incorrect value).

That should be fixed in a separate patch.

Sarah Sharp

> The only surefire way to set this field right is to also take all
> existing endpoints into account, and to force the value to 1 (meaning
> only EP0 is active) if no other endpoint is found. This patch implements
> that as a single step in the final check_bandwidth() call and removes
> the intermediary calculations from add_endpoint() and drop_endpoint().
> 
> This patch should be backported to kernels as old as 2.6.31 that contain
> the commit f94e0186312b0fc39f41eed4e21836ed74b7efe1 "USB: xhci:
> Bandwidth allocation support".
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
>  1 file changed, 18 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 924a6cc..e7d9dfa 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1562,12 +1562,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	struct xhci_hcd *xhci;
>  	struct xhci_container_ctx *in_ctx, *out_ctx;
>  	struct xhci_input_control_ctx *ctrl_ctx;
> -	struct xhci_slot_ctx *slot_ctx;
> -	unsigned int last_ctx;
>  	unsigned int ep_index;
>  	struct xhci_ep_ctx *ep_ctx;
>  	u32 drop_flag;
> -	u32 new_add_flags, new_drop_flags, new_slot_info;
> +	u32 new_add_flags, new_drop_flags;
>  	int ret;
>  
>  	ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
> @@ -1614,24 +1612,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
>  	new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
>  
> -	last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
> -	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
> -	/* Update the last valid endpoint context, if we deleted the last one */
> -	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
> -	    LAST_CTX(last_ctx)) {
> -		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> -		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
> -	}
> -	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
> -
>  	xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
>  
> -	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
> +	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
>  			(unsigned int) ep->desc.bEndpointAddress,
>  			udev->slot_id,
>  			(unsigned int) new_drop_flags,
> -			(unsigned int) new_add_flags,
> -			(unsigned int) new_slot_info);
> +			(unsigned int) new_add_flags);
>  	return 0;
>  }
>  
> @@ -1654,11 +1641,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	struct xhci_hcd *xhci;
>  	struct xhci_container_ctx *in_ctx, *out_ctx;
>  	unsigned int ep_index;
> -	struct xhci_slot_ctx *slot_ctx;
>  	struct xhci_input_control_ctx *ctrl_ctx;
>  	u32 added_ctxs;
> -	unsigned int last_ctx;
> -	u32 new_add_flags, new_drop_flags, new_slot_info;
> +	u32 new_add_flags, new_drop_flags;
>  	struct xhci_virt_device *virt_dev;
>  	int ret = 0;
>  
> @@ -1673,7 +1658,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  		return -ENODEV;
>  
>  	added_ctxs = xhci_get_endpoint_flag(&ep->desc);
> -	last_ctx = xhci_last_valid_endpoint(added_ctxs);
>  	if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
>  		/* FIXME when we have to issue an evaluate endpoint command to
>  		 * deal with ep0 max packet size changing once we get the
> @@ -1739,24 +1723,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	 */
>  	new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
>  
> -	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
> -	/* Update the last valid endpoint context, if we just added one past */
> -	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
> -	    LAST_CTX(last_ctx)) {
> -		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> -		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
> -	}
> -	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
> -
>  	/* Store the usb_device pointer for later use */
>  	ep->hcpriv = udev;
>  
> -	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
> +	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
>  			(unsigned int) ep->desc.bEndpointAddress,
>  			udev->slot_id,
>  			(unsigned int) new_drop_flags,
> -			(unsigned int) new_add_flags,
> -			(unsigned int) new_slot_info);
> +			(unsigned int) new_add_flags);
>  	return 0;
>  }
>  
> @@ -2723,8 +2697,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
>  			ctrl_ctx->drop_flags == 0)
>  		return 0;
>  
> -	xhci_dbg(xhci, "New Input Control Context:\n");
> +	/* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
>  	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
> +	for (i = 31; i >= 1; i--) {
> +		u32 le32 = cpu_to_le32(BIT(i));
> +		if ((virt_dev->eps[i-1].ring && !(ctrl_ctx->drop_flags & le32))
> +				|| (ctrl_ctx->add_flags & le32) || i == 1) {
> +			slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> +			slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(i));
> +			break;
> +		}
> +	}
> +
> +	xhci_dbg(xhci, "New Input Control Context:\n");
>  	xhci_dbg_ctx(xhci, virt_dev->in_ctx,
>  		     LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info)));
>  
> -- 
> 1.8.3.2
> 

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Julius Werner]

> http://marc.info/?l=linux-usb&m=137158978503741&w=2
>
> There's an xHCI spec ambiguity:  Does the last valid context entry refer
> to the last valid endpoint context in the *input* device context or the
> *output* device context?
>
> The code currently assumes it refers to the input device context, namely
> the endpoints we're adding or changing.  If hardware needs the last
> valid endpoint context for the re-calculated *output* device context,
> then yes, this needs to be changed.  However, based on spec errata, I
> believe that's not the intent of the spec authors:
>
> http://marc.info/?l=linux-kernel&m=137208958411696&w=2

Oh, okay, it didn't even occur to me to interpret it that way. It
seems very odd since then Context Entries is essentially redundant
with the information already provided by the Add Context flags.

> What is the impact if we calculate the valid last valid endpoint context
> for the input context?  Do you have evidence of hardware misbehaving?
> If so, which hardware?

I haven't actually seen a problem from this, it just seemed like the
right thing to do for me when looking at it. The only real error we
had was when the command fails due to Context Entries being 0.

However, the question remains: What is the right value for Context
Entries when we have no Add Context flags, or only the SLOT_FLAG? It
should be perfectly legal to just drop a bunch of endpoints without
adding/changing anything else, such as when you switch a UVC interface
back to alternate setting 0 (which has no endpoints). Then the Input
Context really ends at the Slot Context (DCI = 0), but Context Entries
= 0 is very clearly forbidden in the spec. I guess we could just force
it to 1 there and it would probably work, but that would technically
be incorrect since the EP0 context is not updated and not part of the
Add Context flags.

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Alan Stern]

On Tue, 1 Apr 2014, Julius Werner wrote:

> > http://marc.info/?l=linux-usb&m=137158978503741&w=2
> >
> > There's an xHCI spec ambiguity:  Does the last valid context entry refer
> > to the last valid endpoint context in the *input* device context or the
> > *output* device context?

It's not ambiguous at all.  6.2.2.2 clearly states:

	A 'valid' Input Slot Context for a Configure Endpoint Command
	requires the Context Entries field to be initialized to the
	index of the last valid Endpoint Context that is defined by the
	_target configuration_.

(my emphasis).

Thus, if the current config has ep0, ep1, and ep2 and the command drops
ep1, the Context Entries field should be set to the index of ep2 since
that is the last valid Endpoint Context in the target config (which
has only ep0 and ep2).

Alan Stern

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Julius Werner]

*bump*

Sarah, Mathias, can we decide how to proceed with this? I think the
section Alan quoted is a pretty good argument in favor of my
interpretation (although of course this would not be the first time
that two sections of a spec contradict each other). But more
importantly, we have a case that just generates a clearly wrong Slot
Context right now (the one that only drops endpoints), and I see no
way how you could construct a correct Slot Context for that case with
Sarah's interpretation. We need to resolve that somehow.

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Mathias Nyman]

On 04/29/2014 06:11 AM, Julius Werner wrote:
> *bump*
>
> Sarah, Mathias, can we decide how to proceed with this? I think the
> section Alan quoted is a pretty good argument in favor of my
> interpretation (although of course this would not be the first time
> that two sections of a spec contradict each other). But more
> importantly, we have a case that just generates a clearly wrong Slot
> Context right now (the one that only drops endpoints), and I see no
> way how you could construct a correct Slot Context for that case with
> Sarah's interpretation. We need to resolve that somehow.

We discussed with Sarah that we should try this out and queue it for 
usb-linus. This clearly fixes the generation of a invalid slot context 
if all endpoints are dropped.

But because there hasn't been any reported issue about the other case 
this changes (always setting context entries to last valid ep in target 
configuration), and spec still has this tiny contradiction in this case, 
we should keep this out of stable. At least for now, until it gets some 
real world testing coverage.

-Mathias

Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Julius Werner]

Okay, thanks, sounds good. I personally don't care that much about the
stable backport. Let me respin this with a fixed commit message and
the type change Felipe suggested.

[PATCH v2] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Julius Werner]

The current XHCI driver recalculates the Context Entries field in the
Slot Context on every add_endpoint() and drop_endpoint() call. In the
case of drop_endpoint(), it seems to assume that the add_flags will
always contain every endpoint for the new configuration, which is not
necessarily correct if you don't make assumptions about how the USB core
uses the add_endpoint/drop_endpoint interface (add_flags only contains
endpoints that are new additions in the new configuration).

Furthermore, EP0_FLAG is not consistently set in add_flags throughout
the lifetime of a device. This means that when all endpoints are
dropped, the Context Entries field can be set to 0 (which is invalid and
may cause a Parameter Error) or -1 (which is interpreted as 31 and
causes the driver to keep using the old, incorrect value).

The only surefire way to set this field right is to also take all
existing endpoints into account, and to force the value to 1 (meaning
only EP0 is active) if no other endpoint is found. This patch implements
that as a single step in the final check_bandwidth() call and removes
the intermediary calculations from add_endpoint() and drop_endpoint().

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 924a6cc..fec6423 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1562,12 +1562,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	struct xhci_hcd *xhci;
 	struct xhci_container_ctx *in_ctx, *out_ctx;
 	struct xhci_input_control_ctx *ctrl_ctx;
-	struct xhci_slot_ctx *slot_ctx;
-	unsigned int last_ctx;
 	unsigned int ep_index;
 	struct xhci_ep_ctx *ep_ctx;
 	u32 drop_flag;
-	u32 new_add_flags, new_drop_flags, new_slot_info;
+	u32 new_add_flags, new_drop_flags;
 	int ret;
 
 	ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1614,24 +1612,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
 	new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
 
-	last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
-	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-	/* Update the last valid endpoint context, if we deleted the last one */
-	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
-	    LAST_CTX(last_ctx)) {
-		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-	}
-	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
 	xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
 
-	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
+	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
 			(unsigned int) ep->desc.bEndpointAddress,
 			udev->slot_id,
 			(unsigned int) new_drop_flags,
-			(unsigned int) new_add_flags,
-			(unsigned int) new_slot_info);
+			(unsigned int) new_add_flags);
 	return 0;
 }
 
@@ -1654,11 +1641,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	struct xhci_hcd *xhci;
 	struct xhci_container_ctx *in_ctx, *out_ctx;
 	unsigned int ep_index;
-	struct xhci_slot_ctx *slot_ctx;
 	struct xhci_input_control_ctx *ctrl_ctx;
 	u32 added_ctxs;
-	unsigned int last_ctx;
-	u32 new_add_flags, new_drop_flags, new_slot_info;
+	u32 new_add_flags, new_drop_flags;
 	struct xhci_virt_device *virt_dev;
 	int ret = 0;
 
@@ -1673,7 +1658,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 		return -ENODEV;
 
 	added_ctxs = xhci_get_endpoint_flag(&ep->desc);
-	last_ctx = xhci_last_valid_endpoint(added_ctxs);
 	if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
 		/* FIXME when we have to issue an evaluate endpoint command to
 		 * deal with ep0 max packet size changing once we get the
@@ -1739,24 +1723,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 	 */
 	new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
 
-	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-	/* Update the last valid endpoint context, if we just added one past */
-	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
-	    LAST_CTX(last_ctx)) {
-		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-	}
-	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
 	/* Store the usb_device pointer for later use */
 	ep->hcpriv = udev;
 
-	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
+	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
 			(unsigned int) ep->desc.bEndpointAddress,
 			udev->slot_id,
 			(unsigned int) new_drop_flags,
-			(unsigned int) new_add_flags,
-			(unsigned int) new_slot_info);
+			(unsigned int) new_add_flags);
 	return 0;
 }
 
@@ -2723,8 +2697,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 			ctrl_ctx->drop_flags == 0)
 		return 0;
 
-	xhci_dbg(xhci, "New Input Control Context:\n");
+	/* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
 	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
+	for (i = 31; i >= 1; i--) {
+		__le32 le32 = cpu_to_le32(BIT(i));
+		if ((virt_dev->eps[i-1].ring && !(ctrl_ctx->drop_flags & le32))
+				|| (ctrl_ctx->add_flags & le32) || i == 1) {
+			slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
+			slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(i));
+			break;
+		}
+	}
+
+	xhci_dbg(xhci, "New Input Control Context:\n");
 	xhci_dbg_ctx(xhci, virt_dev->in_ctx,
 		     LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info)));
 
-- 
1.8.3.2

Re: [PATCH v2] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Sarah Sharp]

Hi Mathias,

I tested both this patch and your global command queue patches on top of
your for-usb-linus branch.  After reverting commit 400362f1d8dc "ALSA:
usb-audio: Resume mixer values properly", I was able to get my USB
webcam working. [1]

I wrote a small shell script (attached) to start and kill guvcview over
and over, so that I could test the xHCI driver issuing Configure
Endpoint commands, and proceeded to plug and unplug a VIA USB 3.0 hub in
over and over again.  I got occasional descriptor fetch errors and once
saw a Set Address timeout, and everything seemed to work as expected.

In short, I think it's fine to merge Julius' patch to usb-linus and your
command queue patches to usb-next.

Sarah Sharp

[1] https://lkml.org/lkml/2014/4/19/117

On Tue, Apr 29, 2014 at 10:38:17AM -0700, Julius Werner wrote:
> The current XHCI driver recalculates the Context Entries field in the
> Slot Context on every add_endpoint() and drop_endpoint() call. In the
> case of drop_endpoint(), it seems to assume that the add_flags will
> always contain every endpoint for the new configuration, which is not
> necessarily correct if you don't make assumptions about how the USB core
> uses the add_endpoint/drop_endpoint interface (add_flags only contains
> endpoints that are new additions in the new configuration).
> 
> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
> the lifetime of a device. This means that when all endpoints are
> dropped, the Context Entries field can be set to 0 (which is invalid and
> may cause a Parameter Error) or -1 (which is interpreted as 31 and
> causes the driver to keep using the old, incorrect value).
> 
> The only surefire way to set this field right is to also take all
> existing endpoints into account, and to force the value to 1 (meaning
> only EP0 is active) if no other endpoint is found. This patch implements
> that as a single step in the final check_bandwidth() call and removes
> the intermediary calculations from add_endpoint() and drop_endpoint().
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
>  1 file changed, 18 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 924a6cc..fec6423 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1562,12 +1562,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	struct xhci_hcd *xhci;
>  	struct xhci_container_ctx *in_ctx, *out_ctx;
>  	struct xhci_input_control_ctx *ctrl_ctx;
> -	struct xhci_slot_ctx *slot_ctx;
> -	unsigned int last_ctx;
>  	unsigned int ep_index;
>  	struct xhci_ep_ctx *ep_ctx;
>  	u32 drop_flag;
> -	u32 new_add_flags, new_drop_flags, new_slot_info;
> +	u32 new_add_flags, new_drop_flags;
>  	int ret;
>  
>  	ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
> @@ -1614,24 +1612,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
>  	new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
>  
> -	last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
> -	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
> -	/* Update the last valid endpoint context, if we deleted the last one */
> -	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
> -	    LAST_CTX(last_ctx)) {
> -		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> -		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
> -	}
> -	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
> -
>  	xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
>  
> -	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
> +	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
>  			(unsigned int) ep->desc.bEndpointAddress,
>  			udev->slot_id,
>  			(unsigned int) new_drop_flags,
> -			(unsigned int) new_add_flags,
> -			(unsigned int) new_slot_info);
> +			(unsigned int) new_add_flags);
>  	return 0;
>  }
>  
> @@ -1654,11 +1641,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	struct xhci_hcd *xhci;
>  	struct xhci_container_ctx *in_ctx, *out_ctx;
>  	unsigned int ep_index;
> -	struct xhci_slot_ctx *slot_ctx;
>  	struct xhci_input_control_ctx *ctrl_ctx;
>  	u32 added_ctxs;
> -	unsigned int last_ctx;
> -	u32 new_add_flags, new_drop_flags, new_slot_info;
> +	u32 new_add_flags, new_drop_flags;
>  	struct xhci_virt_device *virt_dev;
>  	int ret = 0;
>  
> @@ -1673,7 +1658,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  		return -ENODEV;
>  
>  	added_ctxs = xhci_get_endpoint_flag(&ep->desc);
> -	last_ctx = xhci_last_valid_endpoint(added_ctxs);
>  	if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
>  		/* FIXME when we have to issue an evaluate endpoint command to
>  		 * deal with ep0 max packet size changing once we get the
> @@ -1739,24 +1723,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
>  	 */
>  	new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
>  
> -	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
> -	/* Update the last valid endpoint context, if we just added one past */
> -	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
> -	    LAST_CTX(last_ctx)) {
> -		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> -		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
> -	}
> -	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
> -
>  	/* Store the usb_device pointer for later use */
>  	ep->hcpriv = udev;
>  
> -	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
> +	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
>  			(unsigned int) ep->desc.bEndpointAddress,
>  			udev->slot_id,
>  			(unsigned int) new_drop_flags,
> -			(unsigned int) new_add_flags,
> -			(unsigned int) new_slot_info);
> +			(unsigned int) new_add_flags);
>  	return 0;
>  }
>  
> @@ -2723,8 +2697,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
>  			ctrl_ctx->drop_flags == 0)
>  		return 0;
>  
> -	xhci_dbg(xhci, "New Input Control Context:\n");
> +	/* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
>  	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
> +	for (i = 31; i >= 1; i--) {
> +		__le32 le32 = cpu_to_le32(BIT(i));
> +		if ((virt_dev->eps[i-1].ring && !(ctrl_ctx->drop_flags & le32))
> +				|| (ctrl_ctx->add_flags & le32) || i == 1) {
> +			slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> +			slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(i));
> +			break;
> +		}
> +	}
> +
> +	xhci_dbg(xhci, "New Input Control Context:\n");
>  	xhci_dbg_ctx(xhci, virt_dev->in_ctx,
>  		     LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info)));
>  
> -- 
> 1.8.3.2
> 

Re: [PATCH v2] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

Hi,

On Wed, Apr 30, 2014 at 04:04:24PM -0700, Sarah Sharp wrote:
> Hi Mathias,
> 
> I tested both this patch and your global command queue patches on top of
> your for-usb-linus branch.  After reverting commit 400362f1d8dc "ALSA:
> usb-audio: Resume mixer values properly", I was able to get my USB
> webcam working. [1]
> 
> I wrote a small shell script (attached) to start and kill guvcview over

-ENOATTACHMENT :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Sarah Sharp]

[-- Attachment #1: Type: text/plain, Size: 7535 bytes --]

Script is attached now.

Sarah

On Wed, Apr 30, 2014 at 04:04:24PM -0700, Sarah Sharp wrote:
> Hi Mathias,
> 
> I tested both this patch and your global command queue patches on top of
> your for-usb-linus branch.  After reverting commit 400362f1d8dc "ALSA:
> usb-audio: Resume mixer values properly", I was able to get my USB
> webcam working. [1]
> 
> I wrote a small shell script (attached) to start and kill guvcview over
> and over, so that I could test the xHCI driver issuing Configure
> Endpoint commands, and proceeded to plug and unplug a VIA USB 3.0 hub in
> over and over again.  I got occasional descriptor fetch errors and once
> saw a Set Address timeout, and everything seemed to work as expected.
> 
> In short, I think it's fine to merge Julius' patch to usb-linus and your
> command queue patches to usb-next.
> 
> Sarah Sharp
> 
> [1] https://lkml.org/lkml/2014/4/19/117
> 
> On Tue, Apr 29, 2014 at 10:38:17AM -0700, Julius Werner wrote:
> > The current XHCI driver recalculates the Context Entries field in the
> > Slot Context on every add_endpoint() and drop_endpoint() call. In the
> > case of drop_endpoint(), it seems to assume that the add_flags will
> > always contain every endpoint for the new configuration, which is not
> > necessarily correct if you don't make assumptions about how the USB core
> > uses the add_endpoint/drop_endpoint interface (add_flags only contains
> > endpoints that are new additions in the new configuration).
> > 
> > Furthermore, EP0_FLAG is not consistently set in add_flags throughout
> > the lifetime of a device. This means that when all endpoints are
> > dropped, the Context Entries field can be set to 0 (which is invalid and
> > may cause a Parameter Error) or -1 (which is interpreted as 31 and
> > causes the driver to keep using the old, incorrect value).
> > 
> > The only surefire way to set this field right is to also take all
> > existing endpoints into account, and to force the value to 1 (meaning
> > only EP0 is active) if no other endpoint is found. This patch implements
> > that as a single step in the final check_bandwidth() call and removes
> > the intermediary calculations from add_endpoint() and drop_endpoint().
> > 
> > Signed-off-by: Julius Werner <jwerner@chromium.org>
> > ---
> >  drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
> >  1 file changed, 18 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 924a6cc..fec6423 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -1562,12 +1562,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
> >  	struct xhci_hcd *xhci;
> >  	struct xhci_container_ctx *in_ctx, *out_ctx;
> >  	struct xhci_input_control_ctx *ctrl_ctx;
> > -	struct xhci_slot_ctx *slot_ctx;
> > -	unsigned int last_ctx;
> >  	unsigned int ep_index;
> >  	struct xhci_ep_ctx *ep_ctx;
> >  	u32 drop_flag;
> > -	u32 new_add_flags, new_drop_flags, new_slot_info;
> > +	u32 new_add_flags, new_drop_flags;
> >  	int ret;
> >  
> >  	ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
> > @@ -1614,24 +1612,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
> >  	ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
> >  	new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
> >  
> > -	last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
> > -	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
> > -	/* Update the last valid endpoint context, if we deleted the last one */
> > -	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
> > -	    LAST_CTX(last_ctx)) {
> > -		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> > -		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
> > -	}
> > -	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
> > -
> >  	xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
> >  
> > -	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
> > +	xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
> >  			(unsigned int) ep->desc.bEndpointAddress,
> >  			udev->slot_id,
> >  			(unsigned int) new_drop_flags,
> > -			(unsigned int) new_add_flags,
> > -			(unsigned int) new_slot_info);
> > +			(unsigned int) new_add_flags);
> >  	return 0;
> >  }
> >  
> > @@ -1654,11 +1641,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
> >  	struct xhci_hcd *xhci;
> >  	struct xhci_container_ctx *in_ctx, *out_ctx;
> >  	unsigned int ep_index;
> > -	struct xhci_slot_ctx *slot_ctx;
> >  	struct xhci_input_control_ctx *ctrl_ctx;
> >  	u32 added_ctxs;
> > -	unsigned int last_ctx;
> > -	u32 new_add_flags, new_drop_flags, new_slot_info;
> > +	u32 new_add_flags, new_drop_flags;
> >  	struct xhci_virt_device *virt_dev;
> >  	int ret = 0;
> >  
> > @@ -1673,7 +1658,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
> >  		return -ENODEV;
> >  
> >  	added_ctxs = xhci_get_endpoint_flag(&ep->desc);
> > -	last_ctx = xhci_last_valid_endpoint(added_ctxs);
> >  	if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
> >  		/* FIXME when we have to issue an evaluate endpoint command to
> >  		 * deal with ep0 max packet size changing once we get the
> > @@ -1739,24 +1723,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
> >  	 */
> >  	new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
> >  
> > -	slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
> > -	/* Update the last valid endpoint context, if we just added one past */
> > -	if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
> > -	    LAST_CTX(last_ctx)) {
> > -		slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> > -		slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
> > -	}
> > -	new_slot_info = le32_to_cpu(slot_ctx->dev_info);
> > -
> >  	/* Store the usb_device pointer for later use */
> >  	ep->hcpriv = udev;
> >  
> > -	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x, new slot info = %#x\n",
> > +	xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
> >  			(unsigned int) ep->desc.bEndpointAddress,
> >  			udev->slot_id,
> >  			(unsigned int) new_drop_flags,
> > -			(unsigned int) new_add_flags,
> > -			(unsigned int) new_slot_info);
> > +			(unsigned int) new_add_flags);
> >  	return 0;
> >  }
> >  
> > @@ -2723,8 +2697,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
> >  			ctrl_ctx->drop_flags == 0)
> >  		return 0;
> >  
> > -	xhci_dbg(xhci, "New Input Control Context:\n");
> > +	/* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
> >  	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
> > +	for (i = 31; i >= 1; i--) {
> > +		__le32 le32 = cpu_to_le32(BIT(i));
> > +		if ((virt_dev->eps[i-1].ring && !(ctrl_ctx->drop_flags & le32))
> > +				|| (ctrl_ctx->add_flags & le32) || i == 1) {
> > +			slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> > +			slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(i));
> > +			break;
> > +		}
> > +	}
> > +
> > +	xhci_dbg(xhci, "New Input Control Context:\n");
> >  	xhci_dbg_ctx(xhci, virt_dev->in_ctx,
> >  		     LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info)));
> >  
> > -- 
> > 1.8.3.2
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: endless-guvcview.sh --]
[-- Type: application/x-sh, Size: 90 bytes --]