In message 1212760465-16024-1-git-send-email-hugo.villeneuve@lyrtech.com you wrote:
ARM: This patch adds support for the Lyrtech SFF-SDR board, based on the TI DaVinci architecture (ARM926EJS).
...
+int read_mac_address(uint8_t *buf) +{
- u_int32_t value, mac[2], address;
- /* Read Integrity data structure checkword. */
- if (i2c_read(CFG_I2C_EEPROM_ADDR, INTEGRITY_CHECKWORD_OFFSET,
CFG_I2C_EEPROM_ADDR_LEN, (uint8_t *) &value, 4)) {printf("Read from EEPROM @ 0x%02x failed\n",CFG_I2C_EEPROM_ADDR);return 1;
# 1
- }
- if (value != INTEGRITY_CHECKWORD_VALUE)
return 1;- /* Read SYSCFG structure offset. */
- if (i2c_read(CFG_I2C_EEPROM_ADDR, INTEGRITY_SYSCFG_OFFSET,
CFG_I2C_EEPROM_ADDR_LEN, (uint8_t *) &value, 4)) {printf("Read from EEPROM @ 0x%02x failed\n",CFG_I2C_EEPROM_ADDR);return 1;
# 2
- }
- address = 0x800 + (int) value; /* Address where SYSCFG structure
* is located. */- /* Read NET CONFIG structure offset. */
- if (i2c_read(CFG_I2C_EEPROM_ADDR, address,
CFG_I2C_EEPROM_ADDR_LEN, (uint8_t *) &value, 4)) {printf("Read from EEPROM @ 0x%02x failed\n",CFG_I2C_EEPROM_ADDR);return 1;
# 3
- }
- address = 0x800 + (int) value; /* Address where NET CONFIG structure
* is located. */- address += 12; /* Address where NET INTERFACE CONFIG structure
* is located. */- /* Read NET INTERFACE CONFIG 2 structure offset. */
- if (i2c_read(CFG_I2C_EEPROM_ADDR, address,
CFG_I2C_EEPROM_ADDR_LEN, (uint8_t *) &value, 4)) {printf("Read from EEPROM @ 0x%02x failed\n",CFG_I2C_EEPROM_ADDR);return 1;
# 4
- }
- address = 0x800 + 16 + (int) value;/* Address where NET INTERFACE
* CONFIG 2 structure is located. */- /* Read MAC address. */
- if (i2c_read(CFG_I2C_EEPROM_ADDR, address,
CFG_I2C_EEPROM_ADDR_LEN, (uint8_t *) &mac[0], 8)) {printf("Read from EEPROM @ 0x%02x failed\n",CFG_I2C_EEPROM_ADDR);return 1;
# 5
- }
- buf[0] = mac[0] >> 24;
- buf[1] = mac[0] >> 16;
- buf[2] = mac[0] >> 8;
- buf[3] = mac[0];
- buf[4] = mac[1] >> 24;
- buf[5] = mac[1] >> 16;
- return 0;
Instead of repeating the same code 5 times, replace it by a "goto err;" and add here:
err: printf("Read from EEPROM @ 0x%02x failed\n", CFG_I2C_EEPROM_ADDR); return 1;
That's MUCH easier to read, and especially easier to maintain.
+int misc_init_r(void) +{
- u_int8_t tmp[20], buf[10];
- int clk = 0;
- /* EMIF-A CS3 configuration for FPGA. */
- REG(DAVINCI_A3CR) = DAVINCI_A3CR_VAL;
- clk = ((REG(PLL2_PLLM) + 1) * 27) / ((REG(PLL2_DIV2) & 0x1f) + 1);
- printf("ARM Clock: %dMHz\n", ((REG(PLL1_PLLM) + 1) * 27) / 2);
- printf("DDR Clock: %dMHz\n", (clk / 2));
- /* Configure I2C switch (PCA9543) to enable channel 0. */
- tmp[0] = CFG_I2C_PCA9543_ENABLE_CH0;
- if (i2c_write(CFG_I2C_PCA9543_ADDR, 0,
CFG_I2C_PCA9543_ADDR_LEN, tmp, 1))printf("Write to MUX @ 0x%02x failed\n", CFG_I2C_PCA9543_ADDR);
And this is not an error that needs to be handled?
- /* Set Ethernet MAC address from EEPROM if available. */
- if (read_mac_address(buf) == 0) {
if ((buf[0] != 0xff) && (getenv("ethaddr") == NULL)) {sprintf((char *)&tmp[0],"%02x:%02x:%02x:%02x:%02x:%02x",buf[0], buf[1], buf[2], buf[3],buf[4], buf[5]);setenv("ethaddr", (char *)&tmp[0]);}- }
Hm. I always thought this was documented somewhere (probably it's in the CHANGELOG ?), but I didn't find it right now. The intended "standard" behaviour should be:
If there is a MAC address in the environment, and if it is not identical to the MAC address in the ROM, then a warning shall be printed, and the MAC address from the environment be used.
If there is no MAC address in the environment, then it shall be initialized (silently) from the value in the ROM.
See for example drivers/net/cs8900.c (which is the first driver where we implemented this, IIRC).
- /* For most DaVinci boards, U-Boot is copied in RAM by the UBL,
* so we are always in the relocated state. This is* necessary for command history to work. */- gd->flags |= GD_FLG_RELOC;
"most" != "all". What happens for the N (N = all - most) remaining boards?
Best regards,
Wolfgang Denk