[U-Boot] [PATCH v2 04/17] SPEAr : Placing ethaddr write and read within CONFIG_CMD_NET

[Vipin KUMAR]

On 5/3/2010 5:06 AM, Tom Rix wrote:
> Vipin KUMAR wrote:
>> ethaddr can be optionally read from i2c memory. So, chip_config
>> command supports
>> reading/writing hw mac id into i2c memory. Placing this code within
>> CONFIG_CMD_NET as this would only be needed when network interface is
>> configured
>>
>> Signed-off-by: Vipin Kumar <vipin.kumar at st.com>
>> ---
>>  board/spear/common/spr_misc.c |   23 ++++++++++++++++++++---
>>  1 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/board/spear/common/spr_misc.c
>> b/board/spear/common/spr_misc.c
>> index e356912..4af9436 100644
>> --- a/board/spear/common/spr_misc.c
>> +++ b/board/spear/common/spr_misc.c
>> @@ -38,6 +38,10 @@
>>  DECLARE_GLOBAL_DATA_PTR;
>>  static struct chip_data chip_data;
>>  
>> +#if defined(CONFIG_CMD_NET)
>> +static int i2c_read_mac(uchar *buffer);
>> +#endif
>> +
>>  int dram_init(void)
>>  {
>>      struct xloader_table *xloader_tb =
>> @@ -166,6 +170,7 @@ int spear_board_init(ulong mach_type)
>>      return 0;
>>  }
>>  
>> +#if defined(CONFIG_CMD_NET)
>>  static int i2c_read_mac(uchar *buffer)
>>  {
>>      u8 buf[2];
>> @@ -205,15 +210,20 @@ static int write_mac(uchar *mac)
>>      puts("I2C EEPROM writing failed \n");
>>      return -1;
>>  }
>> +#endif
>>  
>>  int do_chip_config(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>>  {
>>      void (*sram_setfreq) (unsigned int, unsigned int);
>>      struct chip_data *chip = &chip_data;
>> -    unsigned char mac[6];
>> -    unsigned int reg, frequency;
>> +    unsigned int frequency;
>> +
>> +#if defined(CONFIG_CMD_NET)
>> +    unsigned int reg;
>>      char *s, *e;
>>      char i2c_mac[20];
>> +    unsigned char mac[6];
>> +#endif
> 
> These variable should be moved to "etheraddr" if-check scope
> This will clean up having multiple #if defined(CONFIG_CMD_NET) checks

unsigned char mac[] has a larger scope so it is not moved. 
Other variable definitions moved to "etheraddr" if-check scope as suggested

>>  
>>      if ((argc > 3) || (argc < 2)) {
>>          cmd_usage(cmdtp);
>> @@ -244,6 +254,8 @@ int do_chip_config(cmd_tbl_t *cmdtp, int flag, int
>> argc, char *argv[])
>>          }
>>  
>>          return 0;
>> +
>> +#if defined(CONFIG_CMD_NET)
>>      } else if (!strcmp(argv[1], "ethaddr")) {
>>  
>>          s = argv[2];
>> @@ -255,6 +267,7 @@ int do_chip_config(cmd_tbl_t *cmdtp, int flag, int
>> argc, char *argv[])
>>          write_mac(mac);
>>  
>>          return 0;
>> +#endif
>>      } else if (!strcmp(argv[1], "print")) {
>>  
>>          if (chip->cpufreq == -1)
>> @@ -274,13 +287,14 @@ int do_chip_config(cmd_tbl_t *cmdtp, int flag,
>> int argc, char *argv[])
>>          else
>>              printf("DDR Type    = Not Known\n");
>>  
>> +#if defined(CONFIG_CMD_NET)
>>          if (!i2c_read_mac(mac)) {
>>              sprintf(i2c_mac, "%pM", mac);
>>              printf("Ethaddr (from i2c mem) = %s\n", i2c_mac);
>>          } else {
>>              printf("Ethaddr (from i2c mem) = Not set\n");
>>          }
> 
> From misc_init_r, the mac_id is stored in the enviromement.
> Because the enviroment has precendence over the i2c.
> Reporting the i2c mac address may be inconsistent or wrong.
> 
The print in the chip_config print clearly says that this mac address 
is set in i2c memory and in fact this is also the intention.

Probably, I should add a comment saying ethaddr in environment variable 
may be different and it is the one used as mac id in network interface
OR should I add a print itself that this is the mac id stored in i2c 
memory and may be stale ?

> Tom
> 
> 
>> -
>> +#endif
>>          printf("Xloader Rev = %s\n", chip->version);
>>  
>>          return 0;
>> @@ -293,4 +307,7 @@ int do_chip_config(cmd_tbl_t *cmdtp, int flag, int
>> argc, char *argv[])
>>  U_BOOT_CMD(chip_config, 3, 1, do_chip_config,
>>         "configure chip",
>>         "chip_config cpufreq/ddrfreq frequency\n"
>> +#if defined(CONFIG_CMD_NET)
>> +       "chip_config ethaddr XX:XX:XX:XX:XX:XX\n"
>> +#endif
>>         "chip_config print");
> 
>