[PATCH] usb: xhci: Error enumerating USB TV Tuner

[PATCH] usb: xhci: Error enumerating USB TV Tuner

[Al Cooper]

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));
 
-- 
2.17.1

Re: [PATCH] usb: xhci: Error enumerating USB TV Tuner

[Mathias Nyman]

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 

Re: [PATCH] usb: xhci: Error enumerating USB TV Tuner

[Alan Cooper]

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

Re: [PATCH] usb: xhci: Error enumerating USB TV Tuner

[Mathias Nyman]

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

Re: [PATCH] usb: xhci: Error enumerating USB TV Tuner

[Alan Cooper]

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