[U-Boot] [PATCH 1/2] net: add Calxeda xgmac driver

Mike Frysinger
2011-12-02T21:30:33Z

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
-------------- 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/20111202/c795e902/attachment.pgp>