Message-ID: <1280177362.3721.7.camel@maxim-laptop>
Date:	Mon, 26 Jul 2010 23:49:22 +0300
From:	Maxim Levitsky <maximlevitsky@...il.com>
To:	"Luis R. Rodriguez" <lrodriguez@...eros.com>
Cc:	Bob Copeland <me@...copeland.com>,
	"Luis R. Rodriguez" <mcgrof@...badil.infradead.org>,
	Jussi Kivilinna <jussi.kivilinna@...et.fi>,
	"ath5k-devel@...ts.ath5k.org" <ath5k-devel@...ts.ath5k.org>,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	kernel-team@...ts.ubuntu.com, tim.gardner@...onical.com,
	Matthew Garrett <mjg59@...f.ucam.org>,
	David Quan <David.Quan@...eros.com>
Subject: Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM

On Mon, 2010-07-26 at 13:13 -0700, Luis R. Rodriguez wrote: 
> On Sat, Jun 19, 2010 at 08:32:44AM -0700, Maxim Levitsky wrote:
> > On Sat, 2010-06-19 at 16:02 +0300, Maxim Levitsky wrote: 
> > > On Sat, 2010-06-19 at 08:38 -0400, Bob Copeland wrote: 
> > > > On Sat, Jun 19, 2010 at 10:49:34AM +0300, Maxim Levitsky wrote:
> > > > > How this patch?
> > > > 
> > > > Looks fine to me.  Some nitpicking below but feel free to add my
> > > > 
> > > > Acked-by: Bob Copeland <me@...copeland.com>
> > > > 
> > Done.
> > 
> > Best regards,
> > Maxim Levitsky
> > 
> > --- 
> > 
> > commit 616afa397b3e843f2aba06be12a30e72dfff7740
> > Author: Maxim Levitsky <maximlevitsky@...il.com>
> > Date:   Thu Jun 17 23:21:42 2010 +0300
> > 
> >     ath5k: disable ASPM
> >     
> >     Atheros card on Acer Aspire One (AOA150, Atheros Communications Inc. AR5001
> >     Wireless Network Adapter [168c:001c] (rev 01)) doesn't work well with ASPM
> >     enabled. With ASPM ath5k will eventually stall on heavy traffic with often
> >     'unsupported jumbo' warnings appearing. Disabling ASPM L0s in ath5k fixes
> >     these problems.
> >     Also card sends a storm of RXORN interrupts even though medium is idle.
> >     
> >     Reproduced with pcie_aspm=force and by using 'nc < /dev/zero > /dev/null' at
> >     both ends (usually stalls within seconds).
> >     
> >     Unfortunately BIOS enables ASPM on this card by default on these machines
> >     This means that, problem shows up (less often) without pcie_aspm=force too. 
> >     Therefore to benefit from this fix you need to _enable_ CONFIG_PCIEASPM
> >     
> > 
> >     All credit for this patch goes to Jussi Kivilinna <jussi.kivilinna@...et.fi>
> >     for finding and fixing this bug.
> >     
> >     Based on patch that is
> >     From: Jussi Kivilinna <jussi.kivilinna@...et.fi>
> >     
> >     
> >     Signed-off-by: Maxim Levitsky <maximlevitsky@...il.com>
> >     Acked-by: Bob Copeland <me@...copeland.com>
> 
> Acked-by: Luis R. Rodriguez <lrodriguez@...eros.com>
> 
> But please resubmit and completley modify the commit log to indicate
> ath5k cards support ASPM but L0s must be disabled, only L1
> works correctly.
> 
> The comments about ASPM force should be removed as it would
> lead others to try to use the same and the fact of the matter is
> that ASPM force should never be used. As was clarified out of
> some of these discussions worth noting also is that in newer
> kernels CONFIG_PCIEASPM=y will always become the default, for
> older kernels this was never the default and some distributions
> (Ubunutu) do not have this enabled, the benefit of having it
> enabled is it will disable ASPM for these cases:
> 
> (a) the PCIE device is pre PCIE 1.1
> (b) the firmware has the FADT flag set to tell you not to and
> (c) the firmware doesn't grant control via _OSC. The powersave policy will
> enable ASPM even if the BIOS didn't, but only if something else doesn't
> tell us not to.
> 
> The last two checks were only recently added by Mathew and forcing
> CONFIG_PCIEASPM=y was also only recently made default.
> 
> In short, Linux distributions should also start enabling
> CONFIG_PCIEASPM=y on older kernels.


Just one note that since at least my ath5k device is pre 1.1, and you
say that L1 can be enabled, and should, I probably need to enable L1
explicitly in the driver.

OK?

Best regards,
Maxim Levitsky

> 
>   Luis
> 
> > 
> > diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> > index 3abbe75..4f6bd7c 100644
> > --- a/drivers/net/wireless/ath/ath5k/base.c
> > +++ b/drivers/net/wireless/ath/ath5k/base.c
> > @@ -48,6 +48,7 @@
> >  #include <linux/netdevice.h>
> >  #include <linux/cache.h>
> >  #include <linux/pci.h>
> > +#include <linux/pci-aspm.h>
> >  #include <linux/ethtool.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/slab.h>
> > @@ -469,6 +470,19 @@ ath5k_pci_probe(struct pci_dev *pdev,
> >  	int ret;
> >  	u8 csz;
> >  
> > +	/*
> > +	 * Disable PCIE ASPM L0S on the card.
> > +	 * ASPM triggers hardware bug, that makes card stall transmission
> > +	 * untill reset, and even that doesn't always help.
> > +	 * This happens on meduim to heavy transmit utilization.
> > +	 * In addition to stall, hardware usually gives a storm of
> > +	 * RXORN interrupts, despite idle channel, and otherwise doesn't work.
> > +	 * Windows driver also disables the L0s ASPM,
> > +	 * probably due to same reason
> > +	 * Note: to benefit from this fix, please _enable_ CONFIG_PCIEASPM
> > +	 */
> > +	pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S);
> > +
> >  	ret = pci_enable_device(pdev);
> >  	if (ret) {
> >  		dev_err(&pdev->dev, "can't enable device\n");
> > 
> > 
> > _______________________________________________
> > ath5k-devel mailing list
> > ath5k-devel@...ts.ath5k.org
> > https://lists.ath5k.org/mailman/listinfo/ath5k-devel


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

Powered by blists - more mailing lists