Update WHD wi-fi driver to use Mbed network stack memory manager #502
Conversation
…boot up the wi-fi module OK but cannot bring up the actual interface.
| osStatus_t osMutexAcquire (osMutexId_t mutex_id, uint32_t timeout) { | ||
| osStatus_t status; | ||
| | ||
| // a null mutex handle is always invalid |
There was a problem hiding this comment.
I ran into a case where I got a very difficult to debug kernel-mode error due to passing a null mutex handle, and adding this check here turns it into a much-easier-to-debug user-mode error. So this should be a decent usability improvement.
| #endif | ||
| #endif | ||
| | ||
| // If present, set the bits to enable memory / usage / bus faults. |
There was a problem hiding this comment.
This is something I've wanted to do for a while. By default, ARM cores don't enable the more specific fault types, so everything gets escalated to hard fault. Somehow, Mbed never set this option to on, at least not for 99% of targets. Doing that now.
| * | ||
| * @return MTU in bytes | ||
| */ | ||
| virtual uint32_t get_mtu_size() const; |
There was a problem hiding this comment.
As usual, removing doc comments from child classes because these are documented in the parent class and I don't want the docs here to get out of date
connectivity/drivers/wifi/COMPONENT_WHD/wifi-host-driver/inc/whd_network_types.h Show resolved Hide resolved
| * Current number of header bytes that have been removed (skipped) from the start of the buffer. | ||
| * Added by Mbed to implement memory manager functionality. | ||
| */ | ||
| u16_t header_bytes_removed; |
There was a problem hiding this comment.
Also had to patch LwIP to track the current amount of header bytes added/removed
There was a problem hiding this comment.
TODO can we remove this and just do math based on the pointer?
| hostname = user_network_interface->get_hostname(); | ||
| if (hostname) { | ||
| netif_set_hostname(&interface->netif, hostname); | ||
| } |
There was a problem hiding this comment.
Found a bug in this change that was submitted a while back. If the user network interface is null (and it can be), you get a hard fault.
| * @param to_buf Memory buffer chain to copy to | ||
| * @param from_buf Memory buffer chain to copy from | ||
| */ | ||
| virtual void copy(net_stack_mem_buf_t *to_buf, const net_stack_mem_buf_t *from_buf) = 0; |
There was a problem hiding this comment.
I am removing this function as it's complex to implement, is not really properly specified in terms of how it works, and is (wait for it!) totally unused in the entire Mbed codebase
| } | ||
| time_t seconds; | ||
| if (!_rtc_maketime(&rtc_time, &seconds, RTC_FULL_LEAP_YEAR_SUPPORT)) { | ||
| if (!_rtc_maketime(&rtc_time, &seconds, RTC_4_YEAR_LEAP_YEAR_SUPPORT)) { |
There was a problem hiding this comment.
Looking at the datasheet, it is quite clear that PSoC 62 only has 4 year leap year support, not full leap year support. Rather surprised there's no test that will detect if this is set incorrectly...
1 participant
Summary of changes
This MR turned into a bit of a bigger project then I originally intended, but... I have noticed for a while that devices with Cypress WHD wi-fi modules hard fault when running the nanostack and EMAC tests. Recently I investigated why, and it turns out that the memory manager code for the wi-fi driver is hardcoded to use LwIP, so it will segfault if used before LwIP has been initialized. This is not the way! Network drivers should use the memory manager interface so that they are agnostic of the network stack in use and so that they can be run with the emac tests.
This MR rewrites the WHD driver's memory manager interface code to talk to a NetStackMemoryManager instance. It also adds new code to the net stack memory manager to provide an equivalent of
pbuf_header(), which is a required interface for the network driver.I also ran into a failure in the TLS tests caused by the RTC not initializing properly to a representable time. I fixed this and added a new test to check for it (though the test only actually catches the issue if it's the first RTC-related test run after a power cycle, sadly).
Impact of changes
Migration actions required
Documentation
None (this is more focused on making something behave in the way it's supposed to according to the docs...)
Pull request type
Test results