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