Re: [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint

[Mathias Nyman]

On 06/24/2014 05:10 PM, Greg KH wrote:
> On Tue, Jun 24, 2014 at 05:14:42PM +0300, Mathias Nyman wrote:
>> From: Julius Werner <jwerner@xxxxxxxxxxxx>
>>
>> The current XHCI driver recalculates the Context Entries field in the
>> Slot Context on every add_endpoint() and drop_endpoint() call. In the
>> case of drop_endpoint(), it seems to assume that the add_flags will
>> always contain every endpoint for the new configuration, which is not
>> necessarily correct if you don't make assumptions about how the USB core
>> uses the add_endpoint/drop_endpoint interface (add_flags only contains
>> endpoints that are new additions in the new configuration).
>>
>> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
>> the lifetime of a device. This means that when all endpoints are
>> dropped, the Context Entries field can be set to 0 (which is invalid and
>> may cause a Parameter Error) or -1 (which is interpreted as 31 and
>> causes the driver to keep using the old, incorrect value).
>>
>> The only surefire way to set this field right is to also take all
>> existing endpoints into account, and to force the value to 1 (meaning
>> only EP0 is active) if no other endpoint is found. This patch implements
>> that as a single step in the final check_bandwidth() call and removes
>> the intermediary calculations from add_endpoint() and drop_endpoint().
>>
>> Signed-off-by: Julius Werner <jwerner@xxxxxxxxxxxx>
>> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
>>  1 file changed, 18 insertions(+), 33 deletions(-)
> 
> Is this something for older (i.e. stable) kernels?
> 

This was intentionally left out of stable, reasoning in a previous mail was:

"We discussed with Sarah that we should try this out and queue it for 
usb-linus. This clearly fixes the generation of a invalid slot context 
if all endpoints are dropped.

But because there hasn't been any reported issue about the other case 
this changes (always setting context entries to last valid ep in target 
configuration), and spec still has this tiny contradiction in this case, 
we should keep this out of stable. At least for now, until it gets some 
real world testing coverage."

The mail:
http://marc.info/?l=linux-usb&m=139879120000807&w=2

Or if you prefer this patch could go to usb-next instead.

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/