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

Michal Simek
2011-02-28T18:50:55Z

Mike Frysinger wrote:
> 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 ?

it could/should be.

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

ok. I haven't know about this possibility. Will fix it.

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

For transmit I decided to setup BD directly in function axiemac_send.
For recv func BD must be setup before RX is enable that's why I have done it.
I haven't tried to setup BD and enable RX in axiemac_recv function.
Anyway axiemac_recv is called after axiemac_send and if I setup BD in that it will take some time to
get that packet that's why I think it is better to setup BD and enable RX in init function.

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

init function is called from eth.c:eth_init(). From the code below you see that return can be >=0.

if (eth_current->init(eth_current,bis) >= 0) {
eth_current->state = ETH_STATE_ACTIVE;

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

dev contains:
iobase - axi emac baseaddr
init, halt, send, recv pointers to functions
and pointer priv
+ others

I need to clear it that's why memset.

dma controller is special IP that's why I use priv for storing pointer to axidma_priv structure
which contains two pointers to dma for master to slave and slave to master directions (rx and tx if
you like). As I wrote dma controller is different IP that's why there are different addresses.
axi_dma has offset between channels which is 0x30.
I used structures for hw access.

Thanks for your comments,
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