[U-Boot] [PATCH v2] net: axi_ethernet: Add driver to u-boot

[Wolfgang Denk]

Dear Michal Simek,

In message <1314877154-14536-2-git-send-email-monstr at monstr.eu> you wrote:
> Add axi_ethernet driver for little-endian Microblaze.
> 
> RX/TX BDs and rxframe buffer are shared among all axi_ethernet MACs.
> Only one MAC can work in one time.
> 
> Signed-off-by: Michal Simek <monstr at monstr.eu>

> +/* Mask used to verify certain PHY features (or register contents)
> + * in the register above:
> + *  0x1000: 10Mbps full duplex support
> + *  0x0800: 10Mbps half duplex support
> + *  0x0008: Auto-negotiation support
> + */

Incorrect multiline comment style. Please fix globally.


> +	u32 timeout = 200;
> +	/* Wait till MDIO interface is ready to accept a new transaction. */
> +	while (timeout && (!(in_be32(&regs->mdio_mcr)
> +						& XAE_MDIO_MCR_READY_MASK)))
> +		timeout--;

Multiline statement - braches needed.

Also, add some udelay() or similar to have a defined length of the
timeout.  Please fix globally.


> +/* STOP DMA transfers */
> +static void axiemac_halt(struct eth_device *dev)
> +{
> +	struct axidma_priv *priv = dev->priv;
> +
> +	/* Stop the hardware */
> +	priv->dmatx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;
> +	priv->dmarx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;

Please ALWAYS use I/O accessors to access device registers.  Please
fix globally.

> +static int IsRxReady(struct eth_device *dev)

As requested before:  Please get rid of these CamelCaps identifiers!!!


> +static int axiemac_miiphy_read(const char *devname, uchar addr,
> +							uchar reg, ushort *val)
> +{
> +	struct eth_device *dev = eth_get_dev();
> +	*val = phyread(dev, addr, reg);
> +	debug("axiemac: Read MII 0x%x, 0x%x, 0x%x\n", addr, reg, *val);
> +	return 0;

Please separate declarations and code by a single blank line.  Please
fix globally.


Why does this function return int when you uncondsitionally always
return 0 only?

> +}
> +
> +static int axiemac_miiphy_write(const char *devname, uchar addr,
> +							uchar reg, ushort val)
> +{
> +	struct eth_device *dev = eth_get_dev();
> +	debug("axiemac: Write MII 0x%x, 0x%x, 0x%x\n", addr, reg, val);
> +	phywrite(dev, addr, reg, val);
> +	return 0;
> +}

Ditto.

> +static int axiemac_bus_reset(struct mii_dev *bus)
> +{
> +	debug("axiemac: Bus reset\n");
> +	return 0;
> +}

Ditto.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Marriage is like a cage; one sees the birds outside desperate to get
in, and those inside desperate to get out."               - Montaigne