[U-Boot] [PATCH v2 1/3] net: Adds Fast Ethernet Controller driver for Armada100
Ajay Bhargav
ajay.bhargav at einfochips.com
Thu Aug 25 07:10:30 CEST 2011
----- "Mike Frysinger" <vapier at gentoo.org> wrote:
> 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
>
I do not have a hardware storage for MAC on my controller. write_hwaddr
is not needed for me.
> also, it seems like some of my previous feedback wasnt addressed ?
>
I might have missed some points. My apologies.
> > + 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()
>
This driver is for Armada100 series and base address is same for
the whole series, so i did not feel passing it as a parameter. Can
you please tell me if there is any specific reason for the same?
> > +#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
Okay.. will move it there.
Thanks & Regards,
Ajay Bhargav
More information about the U-Boot
mailing list