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

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

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.

> > > +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 ...
-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/20120115/3fd7cd57/attachment.pgp>