2009-12-20 05:01:08

by Larry Finger

[permalink] [raw]
Subject: [PATCH] b43: Clear PCI configuration reg. 0x41 to avoid interference with C3 processor state

In exploring the cause of DMA errors for BCM4312 devices on Atom
processors, other drivers that work write to PCI configuration
register 0x40. The code fragment below was found in the open-code
portion of the Broadcom hybrid wl driver. It is also used in the
ipw2100 and ath9k drivers. In the latter case, it had been removed
and later restored to prevent fatal interrupt errors.

Signed-off-by: Larry Finger <[email protected]>
---

John,

I think this is 2.6.34 material. By itself, it does not affect the
DMA errors in BCM4312 devices. If it is found to help in the resolution
of that long-standing problem, I will request a change in the status.

Larry
---

Index: wireless-testing/drivers/net/wireless/b43/main.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/main.c
+++ wireless-testing/drivers/net/wireless/b43/main.c
@@ -4292,6 +4292,15 @@ static int b43_wireless_core_init(struct
setup_struct_wldev_for_init(dev);
phy->ops->prepare_structs(dev);

+ /* We disable the RETRY_TIMEOUT register (0x41) to keep
+ * PCI Tx retries from interfering with C3 CPU state
+ */
+ if (bus->bustype == SSB_BUSTYPE_PCI) {
+ pci_read_config_dword(bus->host_pci, 0x40, &tmp);
+ pci_write_config_dword(bus->host_pci, 0x40, tmp &
+ ~(0x0000ff00));
+ }
+
/* Enable IRQ routing to this device. */
ssb_pcicore_dev_irqvecs_enable(&bus->pcicore, dev->dev);

Index: wireless-testing/drivers/ssb/pcihost_wrapper.c
===================================================================
--- wireless-testing.orig/drivers/ssb/pcihost_wrapper.c
+++ wireless-testing/drivers/ssb/pcihost_wrapper.c
@@ -35,6 +35,7 @@ static int ssb_pcihost_resume(struct pci
{
struct ssb_bus *ssb = pci_get_drvdata(dev);
int err;
+ u32 val;

pci_set_power_state(dev, 0);
err = pci_enable_device(dev);


2009-12-20 21:54:04

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] b43: Clear PCI configuration reg. 0x41 to avoid interference with C3 processor state

On Sat, Dec 19, 2009 at 10:51 PM, Luis R. Rodriguez <[email protected]> wrote:
> On Sat, Dec 19, 2009 at 9:01 PM, Larry Finger <[email protected]> wrote:
>> In exploring the cause of DMA errors for BCM4312 devices on Atom
>> processors, other drivers that work write to PCI configuration
>> register 0x40. The code fragment below was found in the open-code
>> portion of the Broadcom hybrid wl driver. It is also used in the
>> ipw2100 and ath9k drivers. In the latter case, it had been removed
>> and later restored to prevent fatal interrupt errors.
>
> I should note 0x40 starts with vendor specific PCI config space so you
> cannot guarantee different PCI devices use 0x41 will be used the same
> for different devices. The documentation for the ath9k PCI-E devices
> used that entry for something completely different but what I did not
> do is try to very and ensure PCI devices do not use it it for the
> same. I am told though that although this is PCI vendor space some
> devices may still use similar private PCI config spaces on different
> devices which just follows a practice. At this point we now have not
> only b43, ipw and ath9k follow this but also prism54 and I think p54
> uses this. I'll note I *highly* doubt this is used for the same thing
> on all these devices and was just code copied from other Linux
> drivers. In the case of Atheros Linux drivers I know it was copied
> form Intel drivers, which is why I started questioning it all.
>
> Anyway, if it helps, that's great :) but it cannot be concluded its
> all for the same thing unless you have proper documentation as this is
> in PCI vendor space which *can* vary depending on device and vendor.

Larry, does this actually fix something or is this code purely speculative?

Luis

2009-12-20 22:23:53

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Clear PCI configuration reg. 0x41 to avoid interference with C3 processor state

On 12/20/2009 03:53 PM, Luis R. Rodriguez wrote:
>> I should note 0x40 starts with vendor specific PCI config space so you
>> cannot guarantee different PCI devices use 0x41 will be used the same
>> for different devices. The documentation for the ath9k PCI-E devices
>> used that entry for something completely different but what I did not
>> do is try to very and ensure PCI devices do not use it it for the
>> same. I am told though that although this is PCI vendor space some
>> devices may still use similar private PCI config spaces on different
>> devices which just follows a practice. At this point we now have not
>> only b43, ipw and ath9k follow this but also prism54 and I think p54
>> uses this. I'll note I *highly* doubt this is used for the same thing
>> on all these devices and was just code copied from other Linux
>> drivers. In the case of Atheros Linux drivers I know it was copied
>> form Intel drivers, which is why I started questioning it all.
>>
>> Anyway, if it helps, that's great :) but it cannot be concluded its
>> all for the same thing unless you have proper documentation as this is
>> in PCI vendor space which *can* vary depending on device and vendor.
>
> Larry, does this actually fix something or is this code purely speculative?

I got on this idea because I logged all PCI configuration read/write
values and discovered that the Broadcom wl driver did a read/write on
this location. A later examination of the open-source part of that
driver showed that they copied it from ipw2100. If anyone knows the
layout of the private area of the configuration space, they should;
however, they may be just propagating the code as you say.

This change does not fix the DMA error problem with Atom processors in
Netbooks. We have had one report of DMA errors in a Core Duo system -
AFAIK, the only non-Atom processor with the problem. That user has not
tested this patch.

Larry

2009-12-20 22:25:34

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Clear PCI configuration reg. 0x41 to avoid interference with C3 processor state

On 12/19/2009 11:01 PM, Larry Finger wrote:
> In exploring the cause of DMA errors for BCM4312 devices on Atom
> processors, other drivers that work write to PCI configuration
> register 0x40. The code fragment below was found in the open-code
> portion of the Broadcom hybrid wl driver. It is also used in the
> ipw2100 and ath9k drivers. In the latter case, it had been removed
> and later restored to prevent fatal interrupt errors.
>
> Signed-off-by: Larry Finger <[email protected]>
> ---
>
> John,
>
> I think this is 2.6.34 material. By itself, it does not affect the
> DMA errors in BCM4312 devices. If it is found to help in the resolution
> of that long-standing problem, I will request a change in the status.
>
> Larry
> ---

John,

Please consider this patch retracted. If I actually find that it helps
some system, I'll resubmit it then.

Larry


2009-12-20 06:51:38

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] b43: Clear PCI configuration reg. 0x41 to avoid interference with C3 processor state

On Sat, Dec 19, 2009 at 9:01 PM, Larry Finger <[email protected]> wrote:
> In exploring the cause of DMA errors for BCM4312 devices on Atom
> processors, other drivers that work write to PCI configuration
> register 0x40. The code fragment below was found in the open-code
> portion of the Broadcom hybrid wl driver. It is also used in the
> ipw2100 and ath9k drivers. In the latter case, it had been removed
> and later restored to prevent fatal interrupt errors.

I should note 0x40 starts with vendor specific PCI config space so you
cannot guarantee different PCI devices use 0x41 will be used the same
for different devices. The documentation for the ath9k PCI-E devices
used that entry for something completely different but what I did not
do is try to very and ensure PCI devices do not use it it for the
same. I am told though that although this is PCI vendor space some
devices may still use similar private PCI config spaces on different
devices which just follows a practice. At this point we now have not
only b43, ipw and ath9k follow this but also prism54 and I think p54
uses this. I'll note I *highly* doubt this is used for the same thing
on all these devices and was just code copied from other Linux
drivers. In the case of Atheros Linux drivers I know it was copied
form Intel drivers, which is why I started questioning it all.

Anyway, if it helps, that's great :) but it cannot be concluded its
all for the same thing unless you have proper documentation as this is
in PCI vendor space which *can* vary depending on device and vendor.

Luis