Re: [ath5k-devel] [PATCH v2] ath5k: disable ASPM

From: Maxim Levitsky
Date: Sun Jun 20 2010 - 14:04:28 EST

On Sun, 2010-06-20 at 14:18 +0300, Maxim Levitsky wrote:
> On Sun, 2010-06-20 at 01:13 -0700, Luis R. Rodriguez wrote:
> > Note: this e-mail is on a public mailing list.
> >
> > On Sat, Jun 19, 2010 at 12:49 AM, Maxim Levitsky
> > <maximlevitsky@xxxxxxxxx> wrote:
> > > On Fri, 2010-06-18 at 17:11 +0300, Maxim Levitsky wrote:
> > >> On Fri, 2010-06-18 at 09:59 -0400, Bob Copeland wrote:
> > >> > On Fri, Jun 18, 2010 at 7:05 AM, Maxim Levitsky <maximlevitsky@xxxxxxxxx> wrote:
> > >> > >> Patch I made uses GPL code from e1000e, but since ath5k is
> > >> > >> dual-licensed so patch can't be accepted. So if I got it right, patch
> > >> > >> has to be remade from scratch by someone who really knows about pci
> > >> > >> registers etc. I don't, and learning this to fix something that is
> > >> > >> already fixed in my point of view is waste of (my) time.
> > >> > > Sure, regardless of licensing, this patch has to be redone (and e1000
> > >> > > with it)
> > >> >
> > >> > At any rate, Jussi, thanks a bundle for tracking it down. I owe you a
> > >> > beer, lots of bugs have been reported on these devices.
> > >> >
> > >> > Maxim, this device was always broken in the same way, right? Just
> > >> > curious if anything made it worse recently.
> > >>
> > >> Always was broken, of course even with madwifi.
> > >>
> > >> Recently I think driver stopped doing reset on RXORN, which sometimes
> > >> helped. This did made things a bit worse.
> > >>
> > >> Anyway, just disable L0S, and card works perfectly.
> > >
> > > How this patch?
> > > Its same patch but without open coded ASPM disabler.
> > > Of course to work therefore you need CONFIG_PCIEASPM.
> > > Without it, this reduces to noop.
> > > However I asked at linux-pci, and they said that its not a bad idea to
> > > just remove CONFIG_PCIEASPM and make it default.
> > >
> > > I hope there won't be a silly GPL vs BSD debate over one line of code...
> >
> > When you use the SOB you respect the license of the file you are
> > editing, please see Documentation/SubmittingPatches Developer's
> > Certificate of Origin 1.1.
> This is ok of course.
> The debate was about Jussi Kivilinna copying ASPM disabler code from
> e1000 which is GPL.
>
> >
> > > commit ac5de416f822917b927958b21186a82141550da7
> > > Author: Maxim Levitsky <maximlevitsky@xxxxxxxxx>
> > > Date: Thu Jun 17 23:21:42 2010 +0300
> > >
> > > ath5k: disable ASPM
> >
> > You are not disabling ASPM, you are disabling L0s. ASPM can work with
> > L1, for example.
> This is left over from original patch.
> with open coded code I was able to disable just L0s and get stable
> operation.
> Note however that with this patch which implies CONFIG_PCIEASPM, pci
> core disables both L0s and L1
> (I still need to test and see if I need that patch at all. Maybe just
> enabling CONFIG_PCIEASPM is enough...)
Yep, the patch isn't needed, because of this:

pci 0000:03:00.0: disabling ASPM on pre-1.1 PCIe device. You can enable it with 'pcie_aspm=force'


03:00.0 Ethernet controller: Atheros Communications Inc. AR5001 Wireless Network Adapter (rev 01)
Subsystem: Foxconn International, Inc. Device e008
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 18
Region 0: Memory at 55200000 (64-bit, non-prefetchable) [size=64K]
Capabilities: [40] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
Status: D0 PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] Message Signalled Interrupts: Mask- 64bit- Queue=0/0 Enable-
Address: 00000000 Data: 0000
Capabilities: [60] Express (v1) Legacy Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 512 bytes
DevSta: CorrErr+ UncorrErr+ FatalErr- UnsuppReq+ AuxPwr- TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <512ns, L1 <64us
ClockPM- Suprise- LLActRep- BwNot-
LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
Capabilities: [90] MSI-X: Enable- Mask- TabSize=1
Vector table: BAR=0 offset=00000000
PBA: BAR=0 offset=00000000
Capabilities: [100] Advanced Error Reporting <?>
Capabilities: [140] Virtual Channel <?>
Kernel driver in use: ath5k
Kernel modules: ath5k


static int pcie_aspm_sanity_check(struct pci_dev *pdev)
{
struct pci_dev *child;
int pos;
u32 reg32;
/*
* Some functions in a slot might not all be PCIe functions,
* very strange. Disable ASPM for the whole slot
*/
list_for_each_entry(child, &pdev->subordinate->devices, bus_list) {
pos = pci_pcie_cap(child);
if (!pos)
return -EINVAL;
/*
* Disable ASPM for pre-1.1 PCIe device, we follow MS to use
* RBER bit to determine if a function is 1.1 version device
*/
pci_read_config_dword(child, pos + PCI_EXP_DEVCAP, &reg32);
if (!(reg32 & PCI_EXP_DEVCAP_RBER) && !aspm_force) {
dev_printk(KERN_INFO, &child->dev, "disabling ASPM"
" on pre-1.1 PCIe device. You can enable it"
" with 'pcie_aspm=force'\n");
return -EINVAL;
}
}
return 0;
}


If MS=Microsoft, that explains it, so windows just disables ASPM on pre 1.1 PCIE devices....

It still is a hardware bug that Atheros device advertises ASPM support, but it is broken.

Best regards,
Maxim Levitsky

>
>
> >
> > > 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.
> > >
> > > Reproduced with pcie_aspm=force and by using 'nc < /dev/zero > /dev/null' at
> > > both ends (usually stalls within seconds).
> >
> > I *highly* discourage the use of pcie_aspm=force, in fact I'm inclined
> > to just remove this junk code from the kernel. What you should do to
> > test ASPM on a device is to use setpci on the config space. I have
> > documented how you can do this here:
>
> >
> > http://wireless.kernel.org/en/users/Documentation/ASPM
> >
> > Reason for discouraging this is when you use this you enable ASPM on
> > *all* root complexes and *all* devices which do support ASPM. If you
> > have another device which is capable of ASPM but has it disabled for
> > some reason you will run into other issues.
> >
> > I should also note that loading a module already has an effect on
> > devices for ASPM. An example today is ath9k's ath9k_hw_init() which
> > runs simply during module load, this has some ASPM related code which
> > for example disables the PLL for ASPM for AR9003. I don't recall
> > exactly what we do with ath5k but just giving you an idea. To truly
> > test ASPM well I recommend to do something similar as with this script
> > or you can just give it a shot.
> >
> > http://kernel.org/pub/linux/kernel/people/mcgrof/aspm/enable-aspm
> >
> > Not like I expect very different results but just wanted to clarify
> > the details on force aspm.
> >
> > Why are you disabling L0s for all devices though? Why not just for the
> > reported device? Granted, L0s won't save you much more power but
> > still, why remove it completely, your commit log does not address that
> > in any way. It only states you have issues with L0s on one chipset but
> > what the patch really implies is you are disabling L0s completely for
> > all ath5k chipsets.
> First of all there aren't many PCIE ath5k based devices.
> Two of them are known to be broken.
> Also Jussi Kivilinna said that he found that in windows .inf file there
> are some instructions to enable L1 but not L0s.
>
>
> Note that I tested that again, and card works very stable.
> I didn't see a single drop to 0 bytes/s. In fact throughput never drops
> below 1 Mb/s. (usually about 2.4 Mb/s, with rare drops for few seconds
> to ~1Mb/s)
>
>
> Best regards,
> Maxim Levitsky
>
> >
> > David, are you aware of recent L0s issues with some legacy cards?
> >
> > > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@xxxxxxxx>
> > > Signed-off-by: Maxim Levitsky <maximlevitsky@xxxxxxxxx>
> > >
> > >
> > > diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> > > index 3abbe75..e7a189a 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,9 @@ ath5k_pci_probe(struct pci_dev *pdev,
> > > int ret;
> > > u8 csz;
> > >
> > > + /* Disable PCIE ASPM L0S. It is never enabled by windows driver */
> > > + 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");
> > > @@ -722,6 +726,8 @@ static int ath5k_pci_resume(struct device *dev)
> > > struct ieee80211_hw *hw = pci_get_drvdata(pdev);
> > > struct ath5k_softc *sc = hw->priv;
> > >
> > > + pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S);
> > > +
> > > /*
> > > * Suspend/Resume resets the PCI configuration space, so we have to
> > > * re-disable the RETRY_TIMEOUT register (0x41) to keep
> > >
> > >
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > ath5k-devel mailing list
> > > ath5k-devel@xxxxxxxxxxxxxxx
> > > 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/