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

[Mike Frysinger]

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.

> +#ifdef ETH_DUMP_REGS
> +	eth_dump_regs(dev);
> +#endif

use #ifdef DEBUG

> +	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()

> +	darmdfec = malloc(sizeof(struct armdfec_device));

sizeof(*darmdfec)


> +	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.

> +	dev->init = (void *) armdfec_init;
> +	dev->halt = (void *) armdfec_halt;
> +	dev->send = (void *) armdfec_send;
> +	dev->recv = (void *) armdfec_recv;

drop the (void*) casts.  either you dont need them, or your funcs here are 
wrong and need fixing.

> +#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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110822/06a8031c/attachment.pgp