Hi Xenia, On Mon, Oct 07, 2013 at 06:52:39PM +0300, Xenia Ragiadakou wrote: > The previous patch on the endpoint reset uses the already implemented function > xhci_drop_endpoint() to reduce code duplication. However, the way that xhci > updates the last valid endpoint in the Input Slot Context, when an endpoint > is dropped, can lead to incosistent value for the last valid endpoint. > That can happen when the endpoint to be reset has index smaller than the > current last valid endpoint. In that case the last valid endpoint will end up > being updated with a smaller value and if there are valid endpoints with index > higher than the index of the reset endpoint, the xHC will consider them as > invalid and the transactions to these endpoints will break. > > This patch updates the last valid endpoint with the index of the first not > disabled endpoint, starting from the current last valid endpoint and skipping > the dropped endpoint. I notice that in section 4.6.6 of the 08/2012 errata to the xHCI 1.0 spec, that when the host executes the Configure Endpoint command, it must: "Set the Context Entries field in the Output Slot Context to the index of the last valid Endpoint Context in its Output Device Context structure, which shall be greater to or equal to the value of the Context Entries field in the Input Slot Context." There was a discussion about a similar patch to this one in June: http://marc.info/?l=linux-usb&m=137158978503741&w=2 The end result was that I didn't take the patch, and VMWare fixed their virtual xHCI host. Is this patch necessary for your first patch to work? What happens if you don't have this patch applied? Did you actually experience the transactions to those endpoints breaking, or do you just theorize that it could happen? Sarah Sharp > Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx> > --- > drivers/usb/host/xhci.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index e6300b5..97670f5 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1547,7 +1547,7 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev, > 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 last_ep_index; > unsigned int ep_index; > struct xhci_ep_ctx *ep_ctx; > u32 drop_flag; > @@ -1598,13 +1598,21 @@ 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, out_ctx); > + last_ep_index = LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info)); > 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)); > + for (; last_ep_index >= 0; --last_ep_index) { > + /* skip the dropped endpoint's index */ > + if (last_ep_index == ep_index) > + continue; > + ep_ctx = xhci_get_ep_ctx(xhci, out_ctx, last_ep_index); > + if ((ep_ctx->ep_info & cpu_to_le32(EP_STATE_MASK)) != > + cpu_to_le32(EP_STATE_DISABLED)) { > + slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK); > + slot_ctx->dev_info |= > + cpu_to_le32(LAST_CTX(last_ep_index + 1)); > + break; > + } > } > new_slot_info = le32_to_cpu(slot_ctx->dev_info); > > -- > 1.8.3.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
