Add CompositeEMAC support for Nuvoton M480 #459
Conversation
| // Maximum frame length. | ||
| // Don't know exactly why, but need to set this 4 bytes higher than the MTU of 1514 bytes | ||
| // or 1514 byte packets get rejected | ||
| base->MRFL = 1518; |
There was a problem hiding this comment.
@ccli8 This was the one mystery I couldn't really figure out. Any idea what's up here?
| @@ -116,11 +116,11 @@ | |||
| }, | |||
| "tcpip-thread-priority": { | |||
| "help": "Priority of lwip TCPIP thread", | |||
| "value": "osPriorityNormal" | |||
| "value": "osPriorityHigh" | |||
There was a problem hiding this comment.
Rolling in this fix from arduino/ArduinoCore-mbed#955 (comment)
| "help": "Size of heap (bytes) - used for outgoing packets, and also used by some drivers for reception, see LWIP's opt.h for more information. Current default is 1600.", | ||
| "value": 1600 | ||
| "help": "Size of heap (bytes) - used for outgoing packets, and also used by some drivers for reception, see LWIP's opt.h for more information.", | ||
| "value": 4000 |
There was a problem hiding this comment.
We need to increase this since, unlike before CompositeEMAC, Tx DMA memory is now allocated off of the LwIP heap instead of being statically allocated.
| @@ -62,7 +62,7 @@ emac_mem_buf_t *NanostackMemoryManager::alloc_pool(uint32_t size, uint32_t align | |||
|
|
|||
| uint32_t NanostackMemoryManager::get_pool_alloc_unit(uint32_t align) const | |||
| { | |||
| return MBED_CONF_NSAPI_EMAC_RX_POOL_BUF_SIZE; | |||
| return MBED_CONF_NSAPI_EMAC_RX_POOL_BUF_SIZE - align; | |||
There was a problem hiding this comment.
Bringing this in line with the other memory manager implementations -- seems like it was wrong before
| // Normally that would be 1518 (1514 byte MTU plus 4 bytes for alignment overhead), but this EMAC also always | ||
| // receives the CRC (plus other stuff??) at the end of the packet, with no way to turn it off. So, we need an extra 6 | ||
| // bytes padding that can be clobbered here. | ||
| "emac-rx-pool-buf-size": 1524, |
There was a problem hiding this comment.
@ccli8 Another mystery. I am seeing that 6 bytes after the end of the packet always get written with stuff. Couldn't really find info about this in the datasheet.
There was a problem hiding this comment.
6 bytes should be 2 bytes of length and 4 bytes of CRC.
| @@ -105,3 +105,42 @@ void mbed_sdk_init(void) | |||
| SYS_LockReg(); | |||
| } | |||
| } | |||
|
|
|||
| // Override mbed_mac_address of mbed_interface.c to provide ethernet devices with a semi-unique MAC address | |||
| void mbed_mac_address(char *mac) | |||
There was a problem hiding this comment.
Moved this here from the old ethernet code
There was a problem hiding this comment.
What's the purpose to move mbed_mac_address() here instead of keeping in emac driver ?
There was a problem hiding this comment.
Oh, I deleted the file where this function previously lived, so I had to move it somewhere.
Also, putting it here is better than putting it in the Ethernet driver, because now it's available to the code even if you don't link the ethernet library. Example: if I connect an external Wi-Fi or bluetooth module to my M480, the wifi and bluetooth stacks also need to get the MAC address, and this allows them to!
There was a problem hiding this comment.
OK, I see. Most WiFi or BLE module should be within eeprom to store mac address.
Just remind, mbed could support dual connection interface, such like as ETH + WiFi. So, it's risky to share mbed_mac_address() for other connection interfaces.
There was a problem hiding this comment.
That's a fair point. To be honest, I kinda feel like no one thought that far ahead -- it's hard enough to get one of the two things working :/.
I did a check of the codebase and right now, every non-Nuvoton or Giga Devices implementation does put this in mbed_overrides.c, so I think this is the accepted place to put it. But I'd be happy to accept a PR down the line to improve this situation.
Summary of changes
Now that I have an M480 board, I figured I would try and upgrade its Ethernet implementation to use the new CompositeEMAC structure. This lets the target-specific part of the code be as small as possible, as most of the work is done in the generic EMAC, PHY, and DMA drivers.
The old driver worked fine, and passed nearly all of the network tests. As far as I could tell, the only omissions were power-down support and multicast support. However, using CompositeEMAC brings new features, such as zero-copy Rx, which should help the code run a little faster. (though sadly zero copy Tx isn't really possible on this device since the hardware doesn't support sending multiple buffers at a time) Also, it just removes duplicated code and helps more of the Ethernet drivers use the same common core.
Impact of changes
Nuvoton M480 Ethernet support now implemented using CompositeEMAC.
Migration actions required
None
Documentation
Pull request type
Test results