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

[Michal Simek <monstr@monstr.eu>]

Dear Wolfgang Denk,

Wolfgang Denk wrote:
> Dear Michal Simek,
> 
> In message <1314877154-14536-2-git-send-email-monstr@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@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.

Fixed.

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

Fixed.

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

Done

> 
>> +static int IsRxReady(struct eth_device *dev)
> 
> As requested before:  Please get rid of these CamelCaps identifiers!!!

Fixed this.

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

Done

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

I have fixed phywrite/phyread functions.

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

Because of miiphyutil code requires to return 0 and I am not aware about
doing anything useful for bus_reset.

Regards,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian