On 09/15/2014 10:50 PM, Alan Stern wrote:
> On Mon, 15 Sep 2014, Mathias Nyman wrote:
>
>> I haven't had time to dig into the usbmon traces, but I just got another
>> report from Gunter K�ningsmann about similar scanner problem, and I just
>> noticed
>> that in both cases we round the interval for high speed bulk _IN_ endpoint,
>> which should not have interval set at all.
>> The interval value should be ignored by host, but maybe setting it still
>> could cause some issues.
>>
>> I don't have high hopes for this patch, but it's worth a shot.
>> Can you try this out and see if it makes any difference?
>>
>> -Mathias
>>
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 8936211..7f21b77 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -1291,7 +1291,8 @@ static unsigned int xhci_get_endpoint_interval(struct
>> usb_device *udev,
>> /* Max NAK rate */
>> if (usb_endpoint_xfer_control(&ep->desc) ||
>> usb_endpoint_xfer_bulk(&ep->desc)) {
>> - interval = xhci_parse_microframe_interval(udev, ep);
>> + if (!usb_endpoint_dir_in(&ep->desc))
>> + interval =
>> xhci_parse_microframe_interval(udev, ep);
>
> It's not a good idea to test the direction of control endpoints, since
> they are bi-directional.
Thats right, thanks, I only wanted the bulk endpoints checked here, I need to
change that.
>
> Also, xhci_parse_microframe_interval assumes the value is stored with
> an exponential encoding, but the NAK rate value for control and
> bulk-OUT endpoints is stored in the endpoint descriptor directly as a
> number of microframes (not exponential).
>
> It's not fully clear how xHCI controllers use the Interval field in the
> Endpoint Context. Table 65 notably fails to include a line for HS
> bulk-OUT/control endpoints, and the text isn't clear as to whether the
> field is interpreted using exponential coding for non-periodic
> endpoints. As near as I can tell, it is -- which means you should call
> xhci_microframes_to_exponent() here, not
> xhci_parse_microframe_interval().
>
> Alan Stern
>
I think this is correct now as it is, assuming xhci Interval field for HS
bulk-OUT/control endpoint context uses the same exponential coding as for all
other endpoints.
flow goes:
if (HS bulk or control ep) //endpoints bInterval is in
number of microframes here,
xhci_parse_microframe_interval()
xhci_microframes_to_exponent(bInterval, ...)
interval = fls(bInterval) - 1; // interval is now in
exponent form
-Mathias
--
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
