Subject: Re: [PATCH 10/21] e1000: force register write flushes to circumvent broken platforms

From: Auke Kok <[email-masked]>
Date: Tue, 27 Jun 2006 08:56:51 -0700

Auke Kok wrote:
> Jeff Garzik wrote:
>> Kok, Auke wrote:
>>> A certain AMD64 bridge (8132) has an option to turn on write combining
>>> which breaks our adapter. To circumvent this we need to flush every 
>>> write.
>>>
>>> Signed-off-by: Jesse Brandeburg <[email-masked]>
>>> Signed-off-by: Auke Kok <[email-masked]>
>>> ---
>>>
>>>  drivers/net/e1000/e1000_hw.c   |   24 ++++++++++++++++++++++--
>>>  drivers/net/e1000/e1000_main.c |   18 +++++++++++-------
>>>  2 files changed, 33 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
>>> index 3959039..749d621 100644
>>> --- a/drivers/net/e1000/e1000_hw.c
>>> +++ b/drivers/net/e1000/e1000_hw.c
>>> @@ -705,8 +705,12 @@ e1000_init_hw(struct e1000_hw *hw)
>>>      /* Zero out the Multicast HASH table */
>>>      DEBUGOUT("Zeroing the MTA\n");
>>>      mta_size = E1000_MC_TBL_SIZE;
>>> -    for(i = 0; i < mta_size; i++)
>>> +    for(i = 0; i < mta_size; i++) {
>>>          E1000_WRITE_REG_ARRAY(hw, MTA, i, 0);
>>> +        /* use write flush to prevent Memory Write Block (MWB) from
>>> +         * occuring when accessing our register space */
>>> +        E1000_WRITE_FLUSH(hw);
>>> +    }
>>
>> NAK.  We can't crap up every driver just for one weird piece of hardware.
>>
>> If this problem falls outside the grounds of mmiowb() [see 
>> Documentation/memory-barriers.txt], then other solutions need to be 
>> looked into... ioremap_nocache() ?  Tweaking the MTRR for this region? 
>> Chipset quirk?  ...
> 
> If there is an in-kernel fix for this that would make these flushes 
> obsolete I would certainly prefer it too, unfortunately we can't fix 
> already released kernels and our driver still needs to work there too, 
> so that's why I queued it. It can be dropped from this series if that's 
> preferred.

I have to get back on that - it's not as simple as "the platform is broken". 
e1000's don't support write-combining in hardware but the problem only shows 
itself during initialization. This patch makes it work on AMD platforms with 
write-combining enabled, the platform itself isn't broken - technically the 
NIC is (ixgb doesn't have this problem for instance).

The added write flushes are at initialization only, and fix the problem for 
any other case. I don't see much harm in that.

Auke