Re: [U-Boot] [PATCH 1/3] net: Adds Fast Ethernet Controller driver for Armada100

On Monday, August 22, 2011 06:02:26 PM Mike Frysinger wrote:

On Monday, August 22, 2011 01:11:57 Ajay Bhargav wrote:

...

• writel((u32) darmdfec->htpr, &regs->htpr);

do you really need to cast it yourself ? seems to show up a lot in this file.

[...]

make the reg base a parameter to armada100_fec_initialize()

...

• darmdfec = malloc(sizeof(struct armdfec_device));

sizeof(*darmdfec)

Why are you against sizeof(struct ...) ?

...

• while (!eth_getenv_enetaddr(s, dev->enetaddr)) {

• /* Generate Private MAC addr if not set */
  

• dev->enetaddr[0] = 0x00;
  

• dev->enetaddr[1] = 0x50;
  

• dev->enetaddr[2] = 0x43;
  

+#if defined(CONFIG_SKIP_LOCAL_MAC_RANDOMIZATION)

• /* Generate fixed lower MAC half */
  

• dev->enetaddr[3] = 0x11;
  

• dev->enetaddr[4] = 0x22;
  

• dev->enetaddr[5] = 0x33;
  

+#else

• /* Generate random lower MAC half */
  

• dev->enetaddr[3] = get_random_byte((u8)read_timer());
  

• dev->enetaddr[4] = get_random_byte(dev->enetaddr[3]);
  

• dev->enetaddr[5] = get_random_byte(dev->enetaddr[4]);
  

+#endif

• eth_setenv_enetaddr(s, dev->enetaddr);
  

• }

NAK on this whole thing. initialize dev->write_hwaddr and that is the only thing you should do. the higher eth layers will take care of calling that as necessary.

ACK on the comment here.