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

[Mike Frysinger <vapier@gentoo.org>]

On Monday, February 28, 2011 13:50:55 Michal Simek wrote:
> Mike Frysinger wrote:
> > On Monday, February 28, 2011 04:40:33 Michal Simek wrote:
> >> +	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.

funny enough, that func in your patch is returning 0 when it should be 1.  it 
doesnt explain why your recv/send are returning 1 when it should be 0.

the init func is a special beast -- it returns the # of devices registered, 
not "1 is success".

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

ok, but i think you missed my point.  let me use this example:
dev->priv = 0;	/* the memset */
dma = dev->priv;
dma->dmatx = dma_addr;

you're doing a NULL pointer deref here.
-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/d2db302c/attachment.pgp