> 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.
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
*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.
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
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.
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
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
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 <[email protected]>
> > ---
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html