Re: [34-longterm 077/196] USB: xhci - fix math in xhci_get_endpoint_interval()

Re: [34-longterm 077/196] USB: xhci - fix math in xhci_get_endpoint_interval()

[Paul Gortmaker]

On 12-03-13 12:20 PM, Sarah Sharp wrote:
> This commit introduced a bug, so you will need this fix as well:

Thanks.

> 
> commit 340a3504fd39dad753ba908fb6f894ee81fc3ae2
> Author: Sarah Sharp <sarah.a.sharp@linux.intel.com>
> Date:   Mon Feb 13 14:42:11 2012 -0800
> 
>     xhci: Fix encoding for HS bulk/control NAK rate.
>     
>     The xHCI 0.96 spec says that HS bulk and control endpoint NAK rate must
>     be encoded as an exponent of two number of microframes.  The endpoint
>     descriptor has the NAK rate encoded in number of microframes.  We were
>     just copying the value from the endpoint descriptor into the endpoint
>     context interval field, which was not correct.  This lead to the VIA
>     host rejecting the add of a bulk OUT endpoint from any USB 2.0 mass
>     storage device.
>     
>     The fix is to use the correct encoding.  Refactor the code to convert
>     number of frames to an exponential number of microframes, and make sure
>     we convert the number of microframes in HS bulk and control endpoints to
>     an exponent.
>     
>     This should be back ported to kernels as old as 2.6.31, that contain the
>     commit dfa49c4ad120a784ef1ff0717168aa79f55a483a "USB: xhci - fix math
>     in xhci_get_endpoint_interval"
>     
>     Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
>     Tested-by: Felipe Contreras <felipe.contreras@gmail.com>
>     Suggested-by: Andiry Xu <andiry.xu@amd.com>
>     Cc: stable@vger.kernel.org
> 
> Why are you hand-picking which of the xHCI commits marked for stable to
> apply to this new 2.6.34 stable tree?  On top of this patch, you're

I've been using the commits applied to 2.6.32.x versions as a reference.
I can't do a release with 1000 commits, so I have to chunkify things
somewhat, and doing roughly parallels of the individual 2.6.32.x releases
is a good starting point.  Most times the commits for stable are independent
bugfixes chosen for high value and low risk of regression, so this typically
works fine.  If there are dependencies and regressions in the xHCI stable
content that I should be trying to avoid, I'm fine with taking that input.

Paul.

> probably missing twenty or so other required bug fix patches.  I'm very
> careful about saying which patches should apply to which stable kernels,
> so it should be easy for you to go through the current -rc git log and
> grep for changes to the xHCI driver that are marked for stable.
> 
> Sarah Sharp
> 
> On Mon, Mar 12, 2012 at 08:19:50PM -0400, Paul Gortmaker wrote:
>> From: Dmitry Torokhov <dtor@vmware.com>
>>
>>                    -------------------
>>     This is a commit scheduled for the next v2.6.34 longterm release.
>>     If you see a problem with using this for longterm, please comment.
>>                    -------------------
>>
>> commit dfa49c4ad120a784ef1ff0717168aa79f55a483a upstream.
>>
>> When parsing exponent-expressed intervals we subtract 1 from the
>> value and then expect it to match with original + 1, which is
>> highly unlikely, and we end with frequent spew:
>>
>> 	usb 3-4: ep 0x83 - rounding interval to 512 microframes
>>
>> Also, parsing interval for fullspeed isochronous endpoints was
>> incorrect - according to USB spec they use exponent-based
>> intervals (but xHCI spec claims frame-based intervals). I trust
>> USB spec more, especially since USB core agrees with it.
>>
>> This should be queued for stable kernels back to 2.6.31.
>>
>> Reviewed-by: Micah Elizabeth Scott <micah@vmware.com>
>> Signed-off-by: Dmitry Torokhov <dtor@vmware.com>
>> Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>  drivers/usb/host/xhci-mem.c |   90 +++++++++++++++++++++++++++++--------------
>>  1 file changed, 62 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index e560dd4..e1dbcc2 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -520,6 +520,47 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *ud
>>  	return 0;
>>  }
>>  
>> +/*
>> + * Convert interval expressed as 2^(bInterval - 1) == interval into
>> + * straight exponent value 2^n == interval.
>> + *
>> + */
>> +static unsigned int xhci_parse_exponent_interval(struct usb_device *udev,
>> +		struct usb_host_endpoint *ep)
>> +{
>> +	unsigned int interval;
>> +
>> +	interval = clamp_val(ep->desc.bInterval, 1, 16) - 1;
>> +	if (interval != ep->desc.bInterval - 1)
>> +		dev_warn(&udev->dev,
>> +			 "ep %#x - rounding interval to %d microframes\n",
>> +			 ep->desc.bEndpointAddress,
>> +			 1 << interval);
>> +
>> +	return interval;
>> +}
>> +
>> +/*
>> + * Convert bInterval expressed in frames (in 1-255 range) to exponent of
>> + * microframes, rounded down to nearest power of 2.
>> + */
>> +static unsigned int xhci_parse_frame_interval(struct usb_device *udev,
>> +		struct usb_host_endpoint *ep)
>> +{
>> +	unsigned int interval;
>> +
>> +	interval = fls(8 * ep->desc.bInterval) - 1;
>> +	interval = clamp_val(interval, 3, 10);
>> +	if ((1 << interval) != 8 * ep->desc.bInterval)
>> +		dev_warn(&udev->dev,
>> +			 "ep %#x - rounding interval to %d microframes, ep desc says %d microframes\n",
>> +			 ep->desc.bEndpointAddress,
>> +			 1 << interval,
>> +			 8 * ep->desc.bInterval);
>> +
>> +	return interval;
>> +}
>> +
>>  /* Return the polling or NAK interval.
>>   *
>>   * The polling interval is expressed in "microframes".  If xHCI's Interval field
>> @@ -537,45 +578,38 @@ static inline unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
>>  	case USB_SPEED_HIGH:
>>  		/* Max NAK rate */
>>  		if (usb_endpoint_xfer_control(&ep->desc) ||
>> -				usb_endpoint_xfer_bulk(&ep->desc))
>> +		    usb_endpoint_xfer_bulk(&ep->desc)) {
>>  			interval = ep->desc.bInterval;
>> +			break;
>> +		}
>>  		/* Fall through - SS and HS isoc/int have same decoding */
>> +
>>  	case USB_SPEED_SUPER:
>>  		if (usb_endpoint_xfer_int(&ep->desc) ||
>> -				usb_endpoint_xfer_isoc(&ep->desc)) {
>> -			if (ep->desc.bInterval == 0)
>> -				interval = 0;
>> -			else
>> -				interval = ep->desc.bInterval - 1;
>> -			if (interval > 15)
>> -				interval = 15;
>> -			if (interval != ep->desc.bInterval + 1)
>> -				dev_warn(&udev->dev, "ep %#x - rounding interval to %d microframes\n",
>> -						ep->desc.bEndpointAddress, 1 << interval);
>> +		    usb_endpoint_xfer_isoc(&ep->desc)) {
>> +			interval = xhci_parse_exponent_interval(udev, ep);
>>  		}
>>  		break;
>> -	/* Convert bInterval (in 1-255 frames) to microframes and round down to
>> -	 * nearest power of 2.
>> -	 */
>> +
>>  	case USB_SPEED_FULL:
>> +		if (usb_endpoint_xfer_int(&ep->desc)) {
>> +			interval = xhci_parse_exponent_interval(udev, ep);
>> +			break;
>> +		}
>> +		/*
>> +		 * Fall through for isochronous endpoint interval decoding
>> +		 * since it uses the same rules as low speed interrupt
>> +		 * endpoints.
>> +		 */
>> +
>>  	case USB_SPEED_LOW:
>>  		if (usb_endpoint_xfer_int(&ep->desc) ||
>> -				usb_endpoint_xfer_isoc(&ep->desc)) {
>> -			interval = fls(8*ep->desc.bInterval) - 1;
>> -			if (interval > 10)
>> -				interval = 10;
>> -			if (interval < 3)
>> -				interval = 3;
>> -			if ((1 << interval) != 8*ep->desc.bInterval)
>> -				dev_warn(&udev->dev,
>> -						"ep %#x - rounding interval"
>> -						" to %d microframes, "
>> -						"ep desc says %d microframes\n",
>> -						ep->desc.bEndpointAddress,
>> -						1 << interval,
>> -						8*ep->desc.bInterval);
>> +		    usb_endpoint_xfer_isoc(&ep->desc)) {
>> +
>> +			interval = xhci_parse_frame_interval(udev, ep);
>>  		}
>>  		break;
>> +
>>  	default:
>>  		BUG();
>>  	}
>> -- 
>> 1.7.9.3
>>

Re: [34-longterm 098/196] iwlagn: Support new 5000 microcode.

[Paul Gortmaker]

On 12-03-13 12:24 PM, Bjørn Mork wrote:
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> 
>> From: "Fry, Donald H" <donald.h.fry@intel.com>
>>
>>                    -------------------
>>     This is a commit scheduled for the next v2.6.34 longterm release.
>>     If you see a problem with using this for longterm, please comment.
>>                    -------------------
>>
>> commit 41504cce240f791f1e16561db95728c5537fbad9 upstream.
>>
>> New iwlwifi-5000 microcode requires driver support for API version 5.
>>
>> Signed-off-by: Don Fry <donald.h.fry@intel.com>
>> Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>  drivers/net/wireless/iwlwifi/iwl-5000.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
>> index d05fad4..dc191b8 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl-5000.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
>> @@ -48,7 +48,7 @@
>>  #include "iwl-6000-hw.h"
>>  
>>  /* Highest firmware API version supported */
>> -#define IWL5000_UCODE_API_MAX 2
>> +#define IWL5000_UCODE_API_MAX 5
>>  #define IWL5150_UCODE_API_MAX 2
> 
> This looks like the patch which broke the driver in stable-2.6.32.y and
> was later reverted by:

Thanks for the heads up. I'll be sure to drop this.

Paul.
--

> 
> 
> commit 2a96ae707525ef00a85bf374e56f0219c07eac5a
> Author: Greg Kroah-Hartman <gregkh@suse.de>
> Date:   Wed Jun 15 13:12:35 2011 -0700
> 
>     Revert "iwlagn: Support new 5000 microcode."
>     
>     This reverts commit 6f63415fc1b690cb50c2ad48ba6e9e6e88e271b4.
>     
>     It turns out this is not what we want to have happen for the .32 and
>     .33-longterm kernels as it does not work properly at all.
>     
>     This was reported by Gentoo, Arch, and Canonical developers as causing
>     problems for their users:
>         https://bugs.archlinux.org/task/24302
>         http://bugs.gentoo.org/show_bug.cgi?id=359445
>         https://bugs.launchpad.net/ubuntu/+source/linux/+bug/796336
>     
>     Cc: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
>     Cc: Gordon Malm <gengor@gentoo.org>
>     Cc: Don Fry <donald.h.fry@intel.com>
>     Cc: Wey-Yi Guy <wey-yi.w.guy@intel.com>
>     Cc: Stanislaw Gruszka <sgruszka@redhat.com>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> 
> 
> 
> Bjørn

Re: [34-longterm 181/196] x86, AMD: Fix ARAT feature setting again

[Paul Gortmaker]

On 12-03-13 09:55 AM, Boris Ostrovsky wrote:
> On 03/12/12 20:21, Paul Gortmaker wrote:
>> From: Borislav Petkov<borislav.petkov@amd.com>
>>
>>                     -------------------
>>      This is a commit scheduled for the next v2.6.34 longterm release.
>>      If you see a problem with using this for longterm, please comment.
> 
> 
> Paul, please use e9cdd343a5e42c43bcda01e609fa23089e026470 instead of the 
> one below.

Thanks, I'll make the exchange.

P.

> 
> -boris
> 
>>                     -------------------
>>
>> commit 14fb57dccb6e1defe9f89a66f548fcb24c374c1d upstream.
>>
>> Trying to enable the local APIC timer on early K8 revisions
>> uncovers a number of other issues with it, in conjunction with
>> the C1E enter path on AMD. Fixing those causes much more churn
>> and troubles than the benefit of using that timer brings so
>> don't enable it on K8 at all, falling back to the original
>> functionality the kernel had wrt to that.
>>
>> Reported-and-bisected-by: Nick Bowler<nbowler@elliptictech.com>
>> Cc: Boris Ostrovsky<Boris.Ostrovsky@amd.com>
>> Cc: Andreas Herrmann<andreas.herrmann3@amd.com>
>> Cc: Greg Kroah-Hartman<greg@kroah.com>
>> Cc: Hans Rosenfeld<hans.rosenfeld@amd.com>
>> Cc: Nick Bowler<nbowler@elliptictech.com>
>> Cc: Joerg-Volker-Peetz<jvpeetz@web.de>
>> Signed-off-by: Borislav Petkov<borislav.petkov@amd.com>
>> Link: http://lkml.kernel.org/r/1305636919-31165-3-git-send-email-bp@amd64.org
>> Signed-off-by: Ingo Molnar<mingo@elte.hu>
>> Signed-off-by: Paul Gortmaker<paul.gortmaker@windriver.com>
>> ---
>>   arch/x86/kernel/cpu/amd.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index d618dc1..76a7f76 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -567,7 +567,7 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
>>   #endif
>>
>>   	/* As a rule processors have APIC timer running in deep C states */
>> -	if (c->x86>= 0xf&&  !cpu_has_amd_erratum(amd_erratum_400))
>> +	if (c->x86>  0xf&&  !cpu_has_amd_erratum(amd_erratum_400))
>>   		set_cpu_cap(c, X86_FEATURE_ARAT);
>>
>>   	/*
> 
> 

Re: [34-longterm 181/196] x86, AMD: Fix ARAT feature setting again

[Paul Gortmaker]

On 12-03-13 09:55 AM, Boris Ostrovsky wrote:
> On 03/12/12 20:21, Paul Gortmaker wrote:
>> From: Borislav Petkov<borislav.petkov@amd.com>
>>
>>                     -------------------
>>      This is a commit scheduled for the next v2.6.34 longterm release.
>>      If you see a problem with using this for longterm, please comment.
> 
> 
> Paul, please use e9cdd343a5e42c43bcda01e609fa23089e026470 instead of the 
> one below.

It turns out that the e9cdd wants to apply on top of (vs instead of) the
14fb57dccb commit.

So I will queue it directly after the 14fb57dcc commit.

Thanks,
Paul.

> 
> -boris
> 
>>                     -------------------
>>
>> commit 14fb57dccb6e1defe9f89a66f548fcb24c374c1d upstream.
>>
>> Trying to enable the local APIC timer on early K8 revisions
>> uncovers a number of other issues with it, in conjunction with
>> the C1E enter path on AMD. Fixing those causes much more churn
>> and troubles than the benefit of using that timer brings so
>> don't enable it on K8 at all, falling back to the original
>> functionality the kernel had wrt to that.
>>
>> Reported-and-bisected-by: Nick Bowler<nbowler@elliptictech.com>
>> Cc: Boris Ostrovsky<Boris.Ostrovsky@amd.com>
>> Cc: Andreas Herrmann<andreas.herrmann3@amd.com>
>> Cc: Greg Kroah-Hartman<greg@kroah.com>
>> Cc: Hans Rosenfeld<hans.rosenfeld@amd.com>
>> Cc: Nick Bowler<nbowler@elliptictech.com>
>> Cc: Joerg-Volker-Peetz<jvpeetz@web.de>
>> Signed-off-by: Borislav Petkov<borislav.petkov@amd.com>
>> Link: http://lkml.kernel.org/r/1305636919-31165-3-git-send-email-bp@amd64.org
>> Signed-off-by: Ingo Molnar<mingo@elte.hu>
>> Signed-off-by: Paul Gortmaker<paul.gortmaker@windriver.com>
>> ---
>>   arch/x86/kernel/cpu/amd.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index d618dc1..76a7f76 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -567,7 +567,7 @@ static void __cpuinit init_amd(struct cpuinfo_x86 *c)
>>   #endif
>>
>>   	/* As a rule processors have APIC timer running in deep C states */
>> -	if (c->x86>= 0xf&&  !cpu_has_amd_erratum(amd_erratum_400))
>> +	if (c->x86>  0xf&&  !cpu_has_amd_erratum(amd_erratum_400))
>>   		set_cpu_cap(c, X86_FEATURE_ARAT);
>>
>>   	/*
> 
>