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