Subject: Re: [PATCH 11/21] e1000: disable CRC stripping workaround

From: Auke Kok <[email-masked]>
Date: Tue, 27 Jun 2006 07:29:03 -0700

Jeff Garzik wrote:
> Kok, Auke wrote:
>> CRC stripping is breaking SMBUS-connected BMC's. We disable this
>> feature to make it work. This fixes related bugs regarding SOL.
>>
>> Signed-off-by: Jesse Brandeburg <[email-masked]>
>> Signed-off-by: Auke Kok <[email-masked]>
>> ---
>>
>>  drivers/net/e1000/e1000_main.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/e1000/e1000_main.c 
>> b/drivers/net/e1000/e1000_main.c
>> index c44ed6f..7787299 100644
>> --- a/drivers/net/e1000/e1000_main.c
>> +++ b/drivers/net/e1000/e1000_main.c
>> @@ -1628,8 +1628,11 @@ e1000_setup_rctl(struct e1000_adapter *a
>>          E1000_RCTL_LBM_NO | E1000_RCTL_RDMTS_HALF |
>>          (adapter->hw.mc_filter_type << E1000_RCTL_MO_SHIFT);
>>  
>> +    /* disable hardware stripping of CRC because it breaks
>> +     * BMC firmware connected over SMBUS
>>      if (adapter->hw.mac_type > e1000_82543)
>>          rctl |= E1000_RCTL_SECRC;
>> +     */
>>  
>>      if (adapter->hw.tbi_compatibility_on == 1)
>>          rctl |= E1000_RCTL_SBP;
>> @@ -1696,7 +1699,9 @@ e1000_setup_rctl(struct e1000_adapter *a
>>          rfctl |= E1000_RFCTL_IPV6_DIS;
>>          E1000_WRITE_REG(&adapter->hw, RFCTL, rfctl);
>>  
>> -        rctl |= E1000_RCTL_DTYP_PS | E1000_RCTL_SECRC;
>> +        /* disable the stripping of CRC because it breaks
>> +         * BMC firmware connected over SMBUS */
>> +        rctl |= E1000_RCTL_DTYP_PS /* | E1000_RCTL_SECRC */;
> 
> This is quite ugly.  You are basically bloating the code with historic, 
> dead code, no different than filling the e1000/*.c with '#if 0' regions.
> 
> Just delete it, and explain why in the patch description.

Adjusted the patch on our git server to this:

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index c44ed6f..a9e55dc 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1628,9 +1628,6 @@ e1000_setup_rctl(struct e1000_adapter *a
                 E1000_RCTL_LBM_NO | E1000_RCTL_RDMTS_HALF |
                 (adapter->hw.mc_filter_type << E1000_RCTL_MO_SHIFT);

-       if (adapter->hw.mac_type > e1000_82543)
-               rctl |= E1000_RCTL_SECRC;
-
         if (adapter->hw.tbi_compatibility_on == 1)
                 rctl |= E1000_RCTL_SBP;
         else
@@ -1696,7 +1693,7 @@ e1000_setup_rctl(struct e1000_adapter *a
                 rfctl |= E1000_RFCTL_IPV6_DIS;
                 E1000_WRITE_REG(&adapter->hw, RFCTL, rfctl);

-               rctl |= E1000_RCTL_DTYP_PS | E1000_RCTL_SECRC;
+               rctl |= E1000_RCTL_DTYP_PS;

                 psrctl |= adapter->rx_ps_bsize0 >>
                         E1000_PSRCTL_BSIZE0_SHIFT;

Cheers,

Auke