On Friday 02 December 2011 15:21:48 Rob Herring wrote:
> --- /dev/null
> +++ b/drivers/net/calxedaxgmac.c
>
> +     writel(value, dev->iobase + XGMAC_CORE_CONFIG);

you should declare a C struct that represents the hardware's register layout, 
and then use that rather than iobase+register_offset

> +static int xgmac_init(struct eth_device *dev, bd_t * bis)
> +{
> ...
> +     /* check that there is a valid MAC address */
> +     if (memcmp(dev->enetaddr, "\0\0\0\0\0\0", 6) == 0) {
> +             printf("ERROR: ethaddr not set!\n");
> +             return -1;
> +     }

i'd just omit this

> +int calxedaxgmac_initialize(u32 id, ulong base_addr)
> +{
> ...
> +     priv = (struct calxeda_eth_dev *)memalign(32, sizeof(*priv));

no need for the cast

> +     if (!priv) {
> +             free(dev);
> +             return -ENOMEM;

return 0

> +     if (readl(dev->iobase + XGMAC_CORE_VERSION) != 0x1012)
> +             return -EINVAL;

return -1

> +     macaddr[1] = readl(dev->iobase + XGMAC_CORE_MACADDR0HI);
> +     macaddr[0] = readl(dev->iobase + XGMAC_CORE_MACADDR0LO);
> +     memcpy(dev->enetaddr, macaddr, 6);

does the initial mac regs really start off with useful info ?

> +     sprintf(enetvar, id ? "eth%daddr" : "ethaddr", id);
> +     eth_setenv_enetaddr(enetvar, dev->enetaddr);

NAK: delete this
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to