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

[Mike Frysinger <vapier@gentoo.org>]

On Monday, February 28, 2011 04:40:33 Michal Simek wrote:
> --- /dev/null
> +++ b/drivers/net/xilinx_axi_emac.c
> +static void axi_ethernet_init(struct eth_device *dev)
> +{
> ...
> +	u32 timeout = 200;
> +	while (timeout &&
> +		(!(in_be32(dev->iobase + XAE_IS_OFFSET) & XAE_INT_MGTRDY_MASK)))
> +		timeout--;
> ...
> +static void axi_dma_init(struct eth_device *dev)
> +{
> ...
> +	/* At the initialization time, hardware should finish reset quickly */
> +	u32 timeout = 500;
> +	while (timeout--) {
> +		/* Check transmit/receive channel */
> +		/* Reset is done when the reset bit is low */
> +		if (!(dma->dmatx->control | dma->dmarx->control)
> +						& XAXIDMA_CR_RESET_MASK)
> +			break;
> +		timeout -= 1;
> +	}

shouldnt this be using a timer rather than some constant that will change the 
actual timeout length based on the speed of the core ?

> +static void setup_mac(struct eth_device *dev)
> +{
> +	/* Set the MAC address */
> +	int val = ((dev->enetaddr[3] << 24) | (dev->enetaddr[2] << 16) |
> +		(dev->enetaddr[1] << 8) | (dev->enetaddr[0]));
> +	out_be32(dev->iobase + XAE_UAW0_OFFSET, val);
> +
> +	val = (dev->enetaddr[5] << 8) | dev->enetaddr[4] ;
> +	val |= in_be32(dev->iobase + XAE_UAW1_OFFSET)
> +						& ~XAE_UAW1_UNICASTADDR_MASK;
> +	out_be32(dev->iobase + XAE_UAW1_OFFSET, val);
> +}
> ...
> +static int axiemac_init(struct eth_device *dev, bd_t * bis)
> +{
> ...
> +	setup_mac(dev);

this should be moved to eth_device.write_hwaddr in the initialize function.  
then the common layers will call setup_mac for you as needed.

> +	/* Setup the BD. */
> +	memset((void *) &rx_bd, 0, sizeof(axidma_bd));
> +	rx_bd.next = (u32)&rx_bd;
> +	rx_bd.phys = (u32)&RxFrame;
> +	rx_bd.cntrl = sizeof(RxFrame);
> +	/* Flush the last BD so DMA core could see the updates */
> +	flush_cache((u32)&rx_bd, sizeof(axidma_bd));
> +
> +	/* it is necessary to flush RxFrame because if you don't do it
> +	 * then cache can contain uninitialized data */
> +	flush_cache((u32)&RxFrame, sizeof(RxFrame));
> +
> +	/* Rx BD is ready - start */
> +	dma->dmarx->tail = (u32)&rx_bd;

hmm, shouldnt this be done only in the recv func ?

> +	return 1;

a bunch of these funcs return 1 when i'm pretty sure they should be 0

> +int xilinx_axiemac_initialize(bd_t *bis, int base_addr, int dma_addr)
> +{
> +	struct eth_device *dev;
> +	struct axidma_priv *dma;
> +
> +	dev = malloc(sizeof(*dev));
> +	if (dev == NULL)
> +		hang();
> +
> +	memset(dev, 0, sizeof(*dev));
> +	sprintf(dev->name, "Xilinx_AxiEmac");
> +
> +	dev->iobase = base_addr;
> +	dma = dev->priv;
> +	dma->dmatx = dma_addr;
> +	dma->dmarx = (u32)dma->dmatx + 0x30; /* rx channel offset */

hmm, this is weird.  you just memset(dev) to 0, and then used dev->priv 
without assigning it storage.  so you're scribbling on top of address 0 with 
your dma struct here arent you ?
-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/20110228/b68f2e0f/attachment.pgp