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

On Wednesday, August 24, 2011 09:07:18 Ajay Bhargav wrote:

•   darmdfec->p_rxdesc = (struct rx_desc *) memalign(PKTALIGN,
  

•                   ARMDFEC_RXQ_DESC_ALIGNED_SIZE * RINGSZ + 1);
  

memalign() returns a void*, so you shouldnt need to cast its return value (you do this a couple of times)

• /* Read mac from env if available */
• eth_getenv_enetaddr("ethaddr", dev->enetaddr);

you shouldnt need to do this. the higher layers will take care of this for you when you set write_hwaddr

also, it seems like some of my previous feedback wasnt addressed ?

• while (cmd_sts & BUF_OWNED_BY_DMA) {
  

...

• };
  

no semi-colon needed

+int armada100_fec_initialize() +{ ...

• darmdfec->regs = (void *) ARMD1_FEC_BASE;
  

make the reg base a parameter to armada100_fec_initialize()

+#if defined(CONFIG_PHY_BASE_ADR)

• miiphy_write(dev->name, PHY_ADR_REQ, PHY_ADR_REQ,
  

•              (u16) CONFIG_PHY_BASE_ADR);
  

+#else

• /* Search phy address from range 0-31 */
  

• phy_adr = ethernet_phy_detect(dev);
  

• if (phy_adr < 0) {
  

•         printf("Error: PHY not detected at address range 0-31\n");
  

•         return -1;
  

• } else {
  

•         debug("PHY detected at addr %d\n", phy_adr);
  

•         miiphy_write(dev->name, PHY_ADR_REQ, PHY_ADR_REQ,
  

•                      (u16) phy_adr);
  

• }
  

+#endif

this should be done in the armdfec_init() func, not the initialize func -mike