| Message ID | 20200306160659.16319-1-alcooperx@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | a73d9d9cfc3cfceabd91fb0b0c13e4062b6dbcd7 |
| Headers | show |
| Series | usb: xhci: Error enumerating USB TV Tuner | expand |
On 6.3.2020 18.06, Al Cooper wrote: > Unable to complete the enumeration of a USB TV Tuner device. > > Per XHCI spec (4.6.5), the EP state field of the input context shall > be cleared for a set address command. In the special case of an FS > device that has "MaxPacketSize0 = 8", the Linux XHCI driver does > not do this before evaluating the context. With an XHCI controller > that checks the EP state field for parameter context error this > causes a problem in cases such as the device getting reset again > after enumeration. > > When that field is cleared, the problem does not occur. > > This was found and fixed by Sasi Kumar. > > Signed-off-by: Al Cooper <alcooperx@gmail.com> > --- > drivers/usb/host/xhci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index dbac0fa9748d..5f034e143082 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1428,6 +1428,7 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, > xhci->devs[slot_id]->out_ctx, ep_index); > > ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index); > + ep_ctx->ep_info &= cpu_to_le32(~EP_STATE_MASK);/* must clear */ > ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK); > ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size)); > > Thanks, nice catch. If you agree I'd like to change the the subject of this patch to something like: "xhci: Fix enumeration issue when setting actual max packet size for FS devices" While looking at this it seems that the current EP_STATE_MASK is not correct either. It should be 0x7 instead of 0xf. -Mathias
Agree on both points. Should I re-send a v2? Thanks Al On Mon, Mar 9, 2020 at 8:19 AM Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > > On 6.3.2020 18.06, Al Cooper wrote: > > Unable to complete the enumeration of a USB TV Tuner device. > > > > Per XHCI spec (4.6.5), the EP state field of the input context shall > > be cleared for a set address command. In the special case of an FS > > device that has "MaxPacketSize0 = 8", the Linux XHCI driver does > > not do this before evaluating the context. With an XHCI controller > > that checks the EP state field for parameter context error this > > causes a problem in cases such as the device getting reset again > > after enumeration. > > > > When that field is cleared, the problem does not occur. > > > > This was found and fixed by Sasi Kumar. > > > > Signed-off-by: Al Cooper <alcooperx@gmail.com> > > --- > > drivers/usb/host/xhci.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index dbac0fa9748d..5f034e143082 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -1428,6 +1428,7 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, > > xhci->devs[slot_id]->out_ctx, ep_index); > > > > ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index); > > + ep_ctx->ep_info &= cpu_to_le32(~EP_STATE_MASK);/* must clear */ > > ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK); > > ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size)); > > > > > > Thanks, nice catch. > > If you agree I'd like to change the the subject of this patch to something like: > "xhci: Fix enumeration issue when setting actual max packet size for FS devices" > > While looking at this it seems that the current EP_STATE_MASK is not correct either. > It should be 0x7 instead of 0xf. > > -Mathias
On 10.3.2020 20.34, Alan Cooper wrote: > On Mon, Mar 9, 2020 at 8:19 AM Mathias Nyman > <mathias.nyman@linux.intel.com> wrote: >> >> On 6.3.2020 18.06, Al Cooper wrote: >>> Unable to complete the enumeration of a USB TV Tuner device. >>> >>> Per XHCI spec (4.6.5), the EP state field of the input context shall >>> be cleared for a set address command. In the special case of an FS >>> device that has "MaxPacketSize0 = 8", the Linux XHCI driver does >>> not do this before evaluating the context. With an XHCI controller >>> that checks the EP state field for parameter context error this >>> causes a problem in cases such as the device getting reset again >>> after enumeration. >>> >>> When that field is cleared, the problem does not occur. >>> >>> This was found and fixed by Sasi Kumar. >>> >>> Signed-off-by: Al Cooper <alcooperx@gmail.com> >>> --- >>> drivers/usb/host/xhci.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >>> index dbac0fa9748d..5f034e143082 100644 >>> --- a/drivers/usb/host/xhci.c >>> +++ b/drivers/usb/host/xhci.c >>> @@ -1428,6 +1428,7 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, >>> xhci->devs[slot_id]->out_ctx, ep_index); >>> >>> ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index); >>> + ep_ctx->ep_info &= cpu_to_le32(~EP_STATE_MASK);/* must clear */ >>> ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK); >>> ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size)); >>> >>> >> >> Thanks, nice catch. >> >> If you agree I'd like to change the the subject of this patch to something like: >> "xhci: Fix enumeration issue when setting actual max packet size for FS devices" >> >> While looking at this it seems that the current EP_STATE_MASK is not correct either. >> It should be 0x7 instead of 0xf. >> > > Agree on both points. > Should I re-send a v2? > No need, I can make those changes -Mathias
Thanks! Al On Wed, Mar 11, 2020 at 3:28 AM Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > > On 10.3.2020 20.34, Alan Cooper wrote: > > On Mon, Mar 9, 2020 at 8:19 AM Mathias Nyman > > <mathias.nyman@linux.intel.com> wrote: > >> > >> On 6.3.2020 18.06, Al Cooper wrote: > >>> Unable to complete the enumeration of a USB TV Tuner device. > >>> > >>> Per XHCI spec (4.6.5), the EP state field of the input context shall > >>> be cleared for a set address command. In the special case of an FS > >>> device that has "MaxPacketSize0 = 8", the Linux XHCI driver does > >>> not do this before evaluating the context. With an XHCI controller > >>> that checks the EP state field for parameter context error this > >>> causes a problem in cases such as the device getting reset again > >>> after enumeration. > >>> > >>> When that field is cleared, the problem does not occur. > >>> > >>> This was found and fixed by Sasi Kumar. > >>> > >>> Signed-off-by: Al Cooper <alcooperx@gmail.com> > >>> --- > >>> drivers/usb/host/xhci.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > >>> index dbac0fa9748d..5f034e143082 100644 > >>> --- a/drivers/usb/host/xhci.c > >>> +++ b/drivers/usb/host/xhci.c > >>> @@ -1428,6 +1428,7 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, > >>> xhci->devs[slot_id]->out_ctx, ep_index); > >>> > >>> ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index); > >>> + ep_ctx->ep_info &= cpu_to_le32(~EP_STATE_MASK);/* must clear */ > >>> ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK); > >>> ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size)); > >>> > >>> > >> > >> Thanks, nice catch. > >> > >> If you agree I'd like to change the the subject of this patch to something like: > >> "xhci: Fix enumeration issue when setting actual max packet size for FS devices" > >> > >> While looking at this it seems that the current EP_STATE_MASK is not correct either. > >> It should be 0x7 instead of 0xf. > >> > > > > Agree on both points. > > Should I re-send a v2? > > > > No need, I can make those changes > > -Mathias
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dbac0fa9748d..5f034e143082 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1428,6 +1428,7 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, xhci->devs[slot_id]->out_ctx, ep_index); ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index); + ep_ctx->ep_info &= cpu_to_le32(~EP_STATE_MASK);/* must clear */ ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK); ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
Unable to complete the enumeration of a USB TV Tuner device. Per XHCI spec (4.6.5), the EP state field of the input context shall be cleared for a set address command. In the special case of an FS device that has "MaxPacketSize0 = 8", the Linux XHCI driver does not do this before evaluating the context. With an XHCI controller that checks the EP state field for parameter context error this causes a problem in cases such as the device getting reset again after enumeration. When that field is cleared, the problem does not occur. This was found and fixed by Sasi Kumar. Signed-off-by: Al Cooper <alcooperx@gmail.com> --- drivers/usb/host/xhci.c | 1 + 1 file changed, 1 insertion(+)