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 <[email protected]>
---
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));
--
2.17.1
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 <[email protected]>
> ---
> 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
<[email protected]> 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 <[email protected]>
> > ---
> > 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
> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
<[email protected]> wrote:
>
> On 10.3.2020 20.34, Alan Cooper wrote:
> > On Mon, Mar 9, 2020 at 8:19 AM Mathias Nyman
> > <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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