From: Luis R. Rodriguez on
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(a)bobcopeland.com>
> > >
> Done.
>
> Best regards,
> Maxim Levitsky
>
> ---
>
> commit 616afa397b3e843f2aba06be12a30e72dfff7740
> Author: Maxim Levitsky <maximlevitsky(a)gmail.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(a)mbnet.fi>
> for finding and fixing this bug.
>
> Based on patch that is
> From: Jussi Kivilinna <jussi.kivilinna(a)mbnet.fi>
>
>
> Signed-off-by: Maxim Levitsky <maximlevitsky(a)gmail.com>
> Acked-by: Bob Copeland <me(a)bobcopeland.com>

Acked-by: Luis R. Rodriguez <lrodriguez(a)atheros.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.

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(a)lists.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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Maxim Levitsky on
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(a)bobcopeland.com>
> > > >
> > Done.
> >
> > Best regards,
> > Maxim Levitsky
> >
> > ---
> >
> > commit 616afa397b3e843f2aba06be12a30e72dfff7740
> > Author: Maxim Levitsky <maximlevitsky(a)gmail.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(a)mbnet.fi>
> > for finding and fixing this bug.
> >
> > Based on patch that is
> > From: Jussi Kivilinna <jussi.kivilinna(a)mbnet.fi>
> >
> >
> > Signed-off-by: Maxim Levitsky <maximlevitsky(a)gmail.com>
> > Acked-by: Bob Copeland <me(a)bobcopeland.com>
>
> Acked-by: Luis R. Rodriguez <lrodriguez(a)atheros.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(a)lists.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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Luis R. Rodriguez on
On Mon, Jul 26, 2010 at 01:49:22PM -0700, Maxim Levitsky wrote:
> 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(a)bobcopeland.com>
> > > > >
> > > Done.
> > >
> > > Best regards,
> > > Maxim Levitsky
> > >
> > > ---
> > >
> > > commit 616afa397b3e843f2aba06be12a30e72dfff7740
> > > Author: Maxim Levitsky <maximlevitsky(a)gmail.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(a)mbnet.fi>
> > > for finding and fixing this bug.
> > >
> > > Based on patch that is
> > > From: Jussi Kivilinna <jussi.kivilinna(a)mbnet.fi>
> > >
> > >
> > > Signed-off-by: Maxim Levitsky <maximlevitsky(a)gmail.com>
> > > Acked-by: Bob Copeland <me(a)bobcopeland.com>
> >
> > Acked-by: Luis R. Rodriguez <lrodriguez(a)atheros.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?

No, ASPM must be enabled by the Systems Integrator through the BIOS, there are
other settings that have to be taken care of like modifying some PCI entrance and
exit latency timers, the number of FTS packets we send to exit L0s, amongst
other things. If a user selectively enables L1 but the BIOS had it disabled on
the device it may not work correctly.

In other words leave the settings as-is on your card unless you developing
for a company to enable ASPM yourself or you are willing to take the risks.
Tweaks like force enabling a device to use ASPM should only then be done in
userspace and by an end user. This is not something Linux distributions should
let users tweak. Its a hacker task.

However, it is reasonable to force disable L0s, for example, completley on
the driver if it is known that L0s does not work for all chisets supported
on that driver.

Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Maxim Levitsky on
On Mon, 2010-07-26 at 14:06 -0700, Luis R. Rodriguez wrote:
> On Mon, Jul 26, 2010 at 01:49:22PM -0700, Maxim Levitsky wrote:
> > 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(a)bobcopeland.com>
> > > > > >
> > > > Done.
> > > >
> > > > Best regards,
> > > > Maxim Levitsky
> > > >
> > > > ---
> > > >
> > > > commit 616afa397b3e843f2aba06be12a30e72dfff7740
> > > > Author: Maxim Levitsky <maximlevitsky(a)gmail.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(a)mbnet.fi>
> > > > for finding and fixing this bug.
> > > >
> > > > Based on patch that is
> > > > From: Jussi Kivilinna <jussi.kivilinna(a)mbnet.fi>
> > > >
> > > >
> > > > Signed-off-by: Maxim Levitsky <maximlevitsky(a)gmail.com>
> > > > Acked-by: Bob Copeland <me(a)bobcopeland.com>
> > >
> > > Acked-by: Luis R. Rodriguez <lrodriguez(a)atheros.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?
>
> No, ASPM must be enabled by the Systems Integrator through the BIOS, there are
> other settings that have to be taken care of like modifying some PCI entrance and
> exit latency timers, the number of FTS packets we send to exit L0s, amongst
> other things. If a user selectively enables L1 but the BIOS had it disabled on
> the device it may not work correctly.
>
> In other words leave the settings as-is on your card unless you developing
> for a company to enable ASPM yourself or you are willing to take the risks.
> Tweaks like force enabling a device to use ASPM should only then be done in
> userspace and by an end user. This is not something Linux distributions should
> let users tweak. Its a hacker task.
>
> However, it is reasonable to force disable L0s, for example, completley on
> the driver if it is known that L0s does not work for all chisets supported
> on that driver.

You didn't understand me.


On my notebook, the AR5001 device is marked as pre 1.1 PCIE device.

ASPM *is* enabled for both L0s and L1 by BIOS.

Linux disables ASPM because the device is pre 1.1, and things work fine
with no more patching (assuming that CONFIG_PCIEASPM is set)


However, it is possible, (and that what I asked you) that some ath5k
devices aren't 'pre 1.1 pcie devices' so linux won't disable ASPM L0s
for them.
So indeed for 'good feeling' it is ok to disable L0s from ath5k
explicitly, but most of the time (or always) it will be no-op.

In *addition* to that, since you said that ASPM L1 *does* work, and is
enabled by BIOS, but linux disables it, that it might be worthy to
enable it again from ath5k driver explicitly.
As long as wireless works I don't really care if this done or not.


Best regards,
Maxim Levitsky

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Matthew Garrett on
On Mon, Jul 26, 2010 at 02:06:51PM -0700, Luis R. Rodriguez wrote:

> No, ASPM must be enabled by the Systems Integrator through the BIOS, there are
> other settings that have to be taken care of like modifying some PCI entrance and
> exit latency timers, the number of FTS packets we send to exit L0s, amongst
> other things. If a user selectively enables L1 but the BIOS had it disabled on
> the device it may not work correctly.

That's really the job of the driver. If the ASPM policy is set to
powersave, the fadt doesn't indicate that ASPM should be disabled and
the bus's _OSC method grants full control then the kernel will enable
whatever combination of L states meet the latency constraints. If the
hardware has additional constraints then the hardware-specific driver
needs to handle them.

We don't rely on the BIOS to set up ASPM states. Nor does Windows.

--
Matthew Garrett | mjg59(a)srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/