[U-Boot] [PATCH 1/5] net: ll_temac: Add LL TEMAC driver to u-boot

[Stephan Linz]

Am Sonntag, den 15.01.2012, 13:47 -0500 schrieb Mike Frysinger: 
> On Sunday 15 January 2012 13:29:26 Stephan Linz wrote:
> > Am Sonntag, den 15.01.2012, 12:28 -0500 schrieb Mike Frysinger:
> > > On Sunday 15 January 2012 10:46:02 Stephan Linz wrote:
> > > > +/* Data buffer for LL TEMAC Rx and Tx direction */
> > > > +static unsigned char rx_buffer[PKTSIZE_ALIGN]
> > > > __attribute((aligned(DMAALIGN)));
> > > > +static unsigned char tx_buffer[PKTSIZE_ALIGN]
> > > > __attribute((aligned(DMAALIGN)));
> > > 
> > > come code already declares PktBuf ... can't you use that ?
> > 
> > Hm, what do you mean exactly here?
> > 
> > These are the two DMA transfer buffers. I have no idea if there are
> > buffers in the upper layer (NET) and how I can use it for DMA transfers.
> > Therfore I create my own rx/tx buffers and copy data. That reduce the
> > performance a little bit, but it's OK. Furthermore I have to use DMA
> > safe buffers here (no cache, 32 byte alignment).
> 
> from doc/README.drivers.eth:
> The recv function should process packets as long as the hardware has them 
> readily available before returning.  i.e. you should drain the hardware fifo. 
> For each packet you receive, you should call the NetReceive() function on it 
> along with the packet length.  The common code sets up packet buffers for you 
> already in the .bss (NetRxPackets), so there should be no need to allocate 
> your own.

OK, I'll try to change the buffer handling as soon as possible in one of
the next optimization stages of this driver. Looks interesting and
important to me ...

> 
> so you already have an array of packet buffers declared for you.  the first one 
> is &NetRxPackets[0], etc...  since you're just DMA-ing to random external 
> memory, i don't think the address matters.
> 
> the core code atm however doesn't align things at all, so we'll have to fix 
> that before this driver could use it.

The alignment is essential for the LL TEMAC driver, especially the 32
byte alignment. I look to continue to optimize this part after the patch
has been added.

> 
> > > > +static struct ll_temac_info ll_temac_info[] = {
> > > 
> > > this looks like a struct that should get allocated on the fly based on
> > > arguments given to the driver's registration func
> > 
> > OK, it wast a little bit RAM. We can optimize the code later. I want to
> > see more testing results on differnet Microblaze and PPC platforms.
> 
> the point wasn't optimization but to kill off defines influencing how the code 
> behaves.  drivers should operate based on on their per-device state 
> (addresses/settings/etc...) which is given to them via their registration 
> func.  that way multiple devices can be registered in different modes.
> 
> > > > +int xilinx_ll_temac_initialize(bd_t *bis, struct ll_temac_info
> > > > *devinf) ...
> > > > +	dev = calloc(1, sizeof(*dev));
> > > > ...
> > > > +	/* Tell u-boot to get the addr from the env */
> > > > +	for (i = 0; i < 6; i++)
> > > > +		dev->enetaddr[i] = 0;
> > > 
> > > the memory is already zero-ed by the call to calloc, so this for loop is
> > > useless (and if it wasn't, you'd still use memset())
> > 
> > Sure, we can remove this part in one of the next code optimization.
> 
> or do it now and re-submit a v2 ...

I have already done. Patch is coming up ... :-)


-- 
Best regards,
Stephan Linz
______________________________________________________________________________
MB-Ref: http://www.li-pro.de/xilinx_mb:mbref:start
OpenDCC: http://www.li-pro.net/opendcc.phtml
PC/M: http://www.li-pro.net/pcm.phtml
Sourceforge: http://sourceforge.net/users/slz
Gitorious: https://gitorious.org/~slz