public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	dan.j.williams@intel.com, Julius Werner <jwerner@chromium.org>
Subject: Re: [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint
Date: Tue, 24 Jun 2014 17:51:16 +0300	[thread overview]
Message-ID: <53A99064.5040505@linux.intel.com> (raw)
In-Reply-To: <20140624141028.GA2844@kroah.com>

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@chromium.org>
>>
>> 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@chromium.org>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>  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

  reply	other threads:[~2014-06-24 14:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-24 13:54 [PATCH 0/5] xhci fixes for 3.16-rc usb-linus Mathias Nyman
2014-06-24 14:14 ` [PATCH 1/5] xhci: Use correct SLOT ID when handling a reset device command Mathias Nyman
2014-06-24 14:06   ` David Laight
2014-06-24 14:14   ` [PATCH 2/5] xhci: correct burst count field for isoc transfers on 1.0 xhci hosts Mathias Nyman
2014-06-24 14:14   ` [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint Mathias Nyman
2014-06-24 14:10     ` Greg KH
2014-06-24 14:51       ` Mathias Nyman [this message]
2014-06-24 14:47         ` Greg KH
2014-06-24 14:14   ` [PATCH 4/5] xhci: clear root port wake on bits if controller isn't wake-up capable Mathias Nyman
2014-06-24 14:14   ` [PATCH 5/5] xhci: Fix runtime suspended xhci from blocking system suspend Mathias Nyman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53A99064.5040505@linux.intel.com \
    --to=mathias.nyman@linux.intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jwerner@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox