I've also replied privately.

This applies to every part and will not work with the 82571 which requires an 
EEPROM. This is documented in the errata, which is publically available on 
http://www.intel.com.

I don't think adding flags to the kernel for your private driver is the way to 
go. So for those two reasons, I'm still saying NACK.

Todd Fujinaka
Software Application Engineer
Networking Division (ND)
Intel Corporation
[email protected]
(503) 712-4565


-----Original Message-----
From: Rojhalat Ibrahim [mailto:[email protected]] 
Sent: Tuesday, October 08, 2013 9:43 AM
To: Fujinaka, Todd
Cc: [email protected]
Subject: Re: [E1000-devel] [PATCH] e1000e: Permit operation without 
non-volatile memory

Disabling the NVM might not work with all of your parts. I can only test with 
what I have available.
It's definitely working with the 82574 here. And if it's working with some 
parts, why not have a config option that disables the NVM when it's not needed 
(and not present)?

   Rojhalat


On Tuesday 08 October 2013 15:47:14 Fujinaka, Todd wrote:
> Am I allowed to NACK things?
> 
> This would be a bad idea as eliminating the EEPROM only works with some of 
> our parts and I don't believe the 82574 is one of those parts. I would 
> suggest the i210 if you're looking to lower your BOM.
> 
> Todd Fujinaka
> Software Application Engineer
> Networking Division (ND)
> Intel Corporation
> [email protected]
> (503) 712-4565
> 
> 
> -----Original Message-----
> From: Rojhalat Ibrahim [mailto:[email protected]]
> Sent: Tuesday, October 08, 2013 12:04 AM
> To: [email protected]
> Subject: [E1000-devel] [PATCH] e1000e: Permit operation without 
> non-volatile memory
> 
> The e1000e driver expects to always have some kind of non-volatile memory 
> attached directly to the ethernet controller chip. This means that I would 
> have to add an additional separate flash chip to my custom board just to 
> store essentially the MAC address. Since I don't want to do that, this patch 
> introduces a new config option CONFIG_E1000E_NO_NVM. If defined it disables 
> all accesses to the NVM. I have tested the patch with a 82574 controller.
> 
> Signed-off-by: Rojhalat Ibrahim <[email protected]>
> ---
>  drivers/net/ethernet/intel/Kconfig      |    4 +++
>  drivers/net/ethernet/intel/e1000e/nvm.c |   38 
> +++++++++++++++++++++++++++++---
>  2 files changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/Kconfig 
> b/drivers/net/ethernet/intel/Kconfig
> index 149ac85..264a26f 100644
> --- a/drivers/net/ethernet/intel/Kconfig
> +++ b/drivers/net/ethernet/intel/Kconfig
> @@ -85,6 +85,10 @@ config E1000E
>         To compile this driver as a module, choose M here. The module
>         will be called e1000e.
>  
> +config E1000E_NO_NVM
> +     bool "Embedded PRO/1000 PCI-Express device without non-volatile memory"
> +     depends on E1000E && EMBEDDED
> +
>  config IGB
>       tristate "Intel(R) 82575/82576 PCI-Express Gigabit Ethernet support"
>       depends on PCI
> diff --git a/drivers/net/ethernet/intel/e1000e/nvm.c 
> b/drivers/net/ethernet/intel/e1000e/nvm.c
> index d70a039..24cb280 100644
> --- a/drivers/net/ethernet/intel/e1000e/nvm.c
> +++ b/drivers/net/ethernet/intel/e1000e/nvm.c
> @@ -28,6 +28,7 @@
>  
>  #include "e1000.h"
>  
> +#ifndef CONFIG_E1000E_NO_NVM
>  /**
>   *  e1000_raise_eec_clk - Raise EEPROM clock
>   *  @hw: pointer to the HW structure
> @@ -135,6 +136,7 @@ static u16 e1000_shift_in_eec_bits(struct e1000_hw 
> *hw, u16 count)
>  
>       return data;
>  }
> +#endif /* CONFIG_E1000E_NO_NVM */
>  
>  /**
>   *  e1000e_poll_eerd_eewr_done - Poll for EEPROM read/write completion @@ 
> -146,6 +148,7 @@ static u16 e1000_shift_in_eec_bits(struct e1000_hw *hw, u16 
> count)
>   **/
>  s32 e1000e_poll_eerd_eewr_done(struct e1000_hw *hw, int ee_reg)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       u32 attempts = 100000;
>       u32 i, reg = 0;
>  
> @@ -162,6 +165,9 @@ s32 e1000e_poll_eerd_eewr_done(struct e1000_hw *hw, int 
> ee_reg)
>       }
>  
>       return -E1000_ERR_NVM;
> +#else
> +     return 0;
> +#endif
>  }
>  
>  /**
> @@ -174,6 +180,7 @@ s32 e1000e_poll_eerd_eewr_done(struct e1000_hw *hw, int 
> ee_reg)
>   **/
>  s32 e1000e_acquire_nvm(struct e1000_hw *hw)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       u32 eecd = er32(EECD);
>       s32 timeout = E1000_NVM_GRANT_ATTEMPTS;
>  
> @@ -194,10 +201,11 @@ s32 e1000e_acquire_nvm(struct e1000_hw *hw)
>               e_dbg("Could not acquire NVM grant\n");
>               return -E1000_ERR_NVM;
>       }
> -
> +#endif
>       return 0;
>  }
>  
> +#ifndef CONFIG_E1000E_NO_NVM
>  /**
>   *  e1000_standby_nvm - Return EEPROM to standby state
>   *  @hw: pointer to the HW structure
> @@ -239,6 +247,7 @@ static void e1000_stop_nvm(struct e1000_hw *hw)
>               e1000_lower_eec_clk(hw, &eecd);
>       }
>  }
> +#endif /* CONFIG_E1000E_NO_NVM */
>  
>  /**
>   *  e1000e_release_nvm - Release exclusive access to EEPROM @@ -248,6 +257,7 
> @@ static void e1000_stop_nvm(struct e1000_hw *hw)
>   **/
>  void e1000e_release_nvm(struct e1000_hw *hw)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       u32 eecd;
>  
>       e1000_stop_nvm(hw);
> @@ -255,8 +265,10 @@ void e1000e_release_nvm(struct e1000_hw *hw)
>       eecd = er32(EECD);
>       eecd &= ~E1000_EECD_REQ;
>       ew32(EECD, eecd);
> +#endif
>  }
>  
> +#ifndef CONFIG_E1000E_NO_NVM
>  /**
>   *  e1000_ready_nvm_eeprom - Prepares EEPROM for read/write
>   *  @hw: pointer to the HW structure
> @@ -303,6 +315,7 @@ static s32 e1000_ready_nvm_eeprom(struct e1000_hw 
> *hw)
>  
>       return 0;
>  }
> +#endif
>  
>  /**
>   *  e1000e_read_nvm_eerd - Reads EEPROM using EERD register @@ -315,6 +328,7 
> @@ static s32 e1000_ready_nvm_eeprom(struct e1000_hw *hw)
>   **/
>  s32 e1000e_read_nvm_eerd(struct e1000_hw *hw, u16 offset, u16 words, 
> u16 *data)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       struct e1000_nvm_info *nvm = &hw->nvm;
>       u32 i, eerd = 0;
>       s32 ret_val = 0;
> @@ -341,6 +355,10 @@ s32 e1000e_read_nvm_eerd(struct e1000_hw *hw, u16 
> offset, u16 words, u16 *data)
>       }
>  
>       return ret_val;
> +#else
> +     memset(data, 0, sizeof(u16) * words);
> +     return 0;
> +#endif
>  }
>  
>  /**
> @@ -357,6 +375,7 @@ s32 e1000e_read_nvm_eerd(struct e1000_hw *hw, u16 offset, 
> u16 words, u16 *data)
>   **/
>  s32 e1000e_write_nvm_spi(struct e1000_hw *hw, u16 offset, u16 words, 
> u16 *data)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       struct e1000_nvm_info *nvm = &hw->nvm;
>       s32 ret_val = -E1000_ERR_NVM;
>       u16 widx = 0;
> @@ -419,6 +438,9 @@ s32 e1000e_write_nvm_spi(struct e1000_hw *hw, u16 offset, 
> u16 words, u16 *data)
>       }
>  
>       return ret_val;
> +#else
> +     return 0;
> +#endif
>  }
>  
>  /**
> @@ -433,6 +455,7 @@ s32 e1000e_write_nvm_spi(struct e1000_hw *hw, u16 
> offset, u16 words, u16 *data)
>  s32 e1000_read_pba_string_generic(struct e1000_hw *hw, u8 *pba_num,
>                                 u32 pba_num_size)
>  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       s32 ret_val;
>       u16 nvm_data;
>       u16 pba_ptr;
> @@ -525,7 +548,9 @@ s32 e1000_read_pba_string_generic(struct e1000_hw *hw, u8 
> *pba_num,
>               pba_num[(offset * 2) + 1] = (u8)(nvm_data & 0xFF);
>       }
>       pba_num[offset * 2] = '\0';
> -
> +#else
> +     memset(pba_num, 0, pba_num_size);
> +#endif
>       return 0;
>  }
>  
> @@ -567,6 +592,7 @@ s32 e1000_read_mac_addr_generic(struct e1000_hw *hw)
>   **/
>  s32 e1000e_validate_nvm_checksum_generic(struct e1000_hw *hw)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       s32 ret_val;
>       u16 checksum = 0;
>       u16 i, nvm_data;
> @@ -584,7 +610,7 @@ s32 e1000e_validate_nvm_checksum_generic(struct e1000_hw 
> *hw)
>               e_dbg("NVM Checksum Invalid\n");
>               return -E1000_ERR_NVM;
>       }
> -
> +#endif
>       return 0;
>  }
>  
> @@ -598,6 +624,7 @@ s32 e1000e_validate_nvm_checksum_generic(struct e1000_hw 
> *hw)
>   **/
>  s32 e1000e_update_nvm_checksum_generic(struct e1000_hw *hw)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       s32 ret_val;
>       u16 checksum = 0;
>       u16 i, nvm_data;
> @@ -616,6 +643,9 @@ s32 e1000e_update_nvm_checksum_generic(struct e1000_hw 
> *hw)
>               e_dbg("NVM Write Error while updating checksum.\n");
>  
>       return ret_val;
> +#else
> +     return 0;
> +#endif
>  }
>  
>  /**
> @@ -627,6 +657,7 @@ s32 e1000e_update_nvm_checksum_generic(struct e1000_hw 
> *hw)
>   **/
>  void e1000e_reload_nvm_generic(struct e1000_hw *hw)  {
> +#ifndef CONFIG_E1000E_NO_NVM
>       u32 ctrl_ext;
>  
>       usleep_range(10, 20);
> @@ -634,4 +665,5 @@ void e1000e_reload_nvm_generic(struct e1000_hw *hw)
>       ctrl_ext |= E1000_CTRL_EXT_EE_RST;
>       ew32(CTRL_EXT, ctrl_ext);
>       e1e_flush();
> +#endif
>  }
> --
> 1.8.1.5
> 
> 
> ----------------------------------------------------------------------
> -------- October Webinars: Code for Performance Free Intel webinars 
> can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the 
> most from the latest Intel processors and coprocessors. See abstracts 
> and register > 
> http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.c
> lktrk _______________________________________________
> E1000-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/e1000-devel
> To learn more about Intel&#174; Ethernet, visit 
> http://communities.intel.com/community/wired
> 

------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit 
http://communities.intel.com/community/wired

Reply via email to