Sleep is wait mode
Deepsleep is very low power stop mode
During the interrupt that removes it from deepsleep it will run at low
speed until the PLL is automatically re-enabled after the interrupt.
- Linker script is based on LPC1768
number of NVIC is 16 (CORE) + 53 (M4 in LPC43xx) = 69,
therefor, reserve at the top of RAM0 (address:0x10000000)
to relocate NVIC vector table
- startup file is based on startup_ARMCM4.S in CMSIS V3.20
change NVIC name for cortex-M4 of LPC43xx
- add GCC_ARM for LPC4330_M4 in python scripts
- add some descriptions for GCC_ARM
The INITIAL_SP value used by RTX was originally at the 4k limit instead
of at the 8k limit. The HEAP_START macro was defined to __HeapLimit
but the heap really starts at __end__. __HeapLimit is just used for
some link time sanity checks to verify that the heap and stack can
grow a bit without colliding.
Updated CMSIS DSP to latest version (CMSIS-SP-00300-r3p2-00rel1.zip)
Build system changes to be able to preprocess assembler sources before compiling them:
- GCC: use gcc '-x assembler-with-cpp'
- ARM: preprocess first, then assemble (two separate commands)
- IAR: added macro definitions and include directories to the assembler command line
Removed CORTEX_ARM_SUPPORT restriction for the DSP libraries.
Tested: LPC1768 with ARM, GCC_ARM and IAR, LPC11U24 with ARM.
If lwIP placed more than 2 pbufs in a TCP segment, the ethernet driver
would fail to send it as it didn't have enough Tx descriptors. The
maximum number of pbufs outstanding for transmit that lwIP keeps is
defined by the TCP_SND_QUEUELEN macro. I modifed the value of
LPC_NUM_BUFF_TXDESCS to take advantage of this lwIP value. The +1
takes into account that LPC_EMAC->TxProduceIndex ==
LPC->TxConsumeIndex is reserved for indicating that the queue is empty
so a full queue uses one less than the maximum count.
tcp_write() would incorrectly byte swap the checksum 1 too many times
when concatenating a pbuf to an existing TCP segment if the number of
bytes in the concatenated data was odd. I hit this issue when I tried
to reproduce a lost segment issue reported by a mbed user in this forum
thread: http://mbed.org/forum/mbed/topic/4354/?page=2#comment-22657
If don't know if this is an issue that anyone cares about. I am also
not sure what the best way to solve it is either. I just thought I
would issue a pull request with this commit to bring the issue to light
and show a possible solution that I have tested on my mbed-1768 device.
Previously the serial_putc() API didn't make any use of the Tx FIFO
since the serial_writable() API it utilizes only returns true when the
FIFO is completely empty. This is due to the fact that the THRE bit of
the UART's LSR (Line Status Register) only goes high when the whole
FIFO is empty.
I noticed this when doing some performance testing with the network
stack. I went from calling printf() to output 3 bytes every 10 seconds
(with packet drop stats) to instead output 4 bytes every 10 seconds.
I thought these should easily fit in the 16 byte FIFO but outputting
one extra byte caused an additional three 550 byte UDP packets to be
dropped. This should only happen if the additional character being
sent to the UART was taking away extra CPU cycles from the network
stack.
My solution is to keep track of the number of bytes that have been
placed in the Tx FIFO since it was last detected as being completely
empty (via the THRE bit). Only once this count hits 16 does the code
then block, waiting for the THRE bit to go high. Each time the THRE
bit does go high, the count is reset to 0 again and it is incremented
for each byte that is loaded into the THR.
Because the LPC81X HAL implementation calls error(), which in turn calls
fprintf(), quite a bit of code is added to the image, which is not a good
idea on such resource constrained targets.
The new RawSerial class is a simple wrapper over the serial HAL that can
be safely used from an interrupt handler.
Interrupt chaining code was removed from InterruptIn, Serial and Ticker
because it caused lots of issues with the RTOS. Interrupt chaining is
still possible using the InterruptManager class.
The test is not automated now, so it can be synced with the official
repository.
Also changed it to look more like the original VodafoneUSBModem test by
adding another thread in the code.
In the future, USBHostConf.h should really reside in the project that
imports USBHost, not inside the USBHost library itself. Doing that now
though might break compatibility with projects that currently import
USBHost, so we need to figure out a better solution.
Because CT32B1 (P1_1, P1_2 and P1_3) is used by us_ticker.c for wait and ticker function. Since wait/ticker is commonly used by mbed code and I decided CT32B1 of LPC11XX should only be used for this function, not for PwmOut.
P1_6 and P1_7 are used by UART (USBTX/USBRX) and I think they should not be assigned to other function.
DEVICE_STDIO_MESSAGES is the only #define in device.h that is being used in the library as #ifdef rather than #if. This is misleading since setting it to 0 will not disable including the <stdio.h> libraries.
A recent commit, 43acaa4166, to get _sbrk() to build successfully for
LPC2368 broke the Cortex-M implementation. __get_MSP() isn't ever
defined as a macro, it is an inline function. This means that the
code would always be compiled to use SP instead of MSP on Cortex-M
parts. I switched the code to instead use the TARGET_ARM7 define
to choose which stack pointer to utilize.
I tested this fix by making sure that the LPC2368 version of the mbed
SDK would still build successfully with the Python scripts and that the
NET1 test still built and ran successfully on my mbed-LPC1768 device.
Fault is triggered by trying to read LPC_CAN1->IER when the peripheral is powered off. Fixed by checking the power control register before checking the IER register.
While fixing this issue in the various LPC* ports, I noticed a comment
pointing to this mbed forum post which summarizes this bug quite well:
https://mbed.org/forum/bugs-suggestions/topic/4473/
This bug was introduced in the following commit:
2662e105c4
The following code was added to serial_putc() as part of this commit:
uint32_t lsr = obj->uart->LSR;
lsr = lsr;
uint32_t thr = obj->uart->THR;
thr = thr;
As the forum post indicates, this causes the serial_putc() routine to
actually eat an inbound received byte if it exists since reading THR is
really reading the RBR, the Receiver Buffer Register. This code looks
like code that was probably added so that the developer could take a
snapshot of these registers and look at them in the debugger. It
probably got committed in error.
LPC11U24/LPC11U24_301/LPC11U35_401 shared the same startup file for ARM
and uARM toolchains, which is wrong, because the initial SP value is
different for LPC11U24_301. This commit fixes this issue by giving each
target its own startup file.
There were lots of overlaps in the code for LPC810 and LPC812, including
duplicated source files. This commit adds a TARGET_LPC81X_COMMON folder in
both HAL and CMSIS, this folder keeps common code for the targets.
If the FileBase::lookup operation in the constructor of FilePath returns
NULL, subsequent operations (such as isFile()/isFileSystem()) will call
methods on a NULL 'fb' pointer. This commit fixes this issue by adding
explicit NULL checks and a new method in FilePath (exists()).
Asm versions of netstack memcpy() and lwip_standard_chksum()
[Note] I'm generally a bit reluctant when including optimizations like this (from an architectural standpoint), because they tend to be a bit too specific (for example, this one works only with lwIP+GCC+Cortex-M3 or M4), but for now it looks as this is the right place for them, although the optimized memcpy should ideally be in libc (or even better replaced with a DMA transfer in this particular case). But this will be both a nice optimization and a reminder of what we need to implement/change in the future.
Based on great feedback from Martin Kojtal on my previous commit, I
have modified my USBHost::fileControlBuf() change to be more portable.
ddb3fbe826 (commitcomment-3988044)
The code now fills in the setupPacket byte array a byte at a time,
using bit shifts to extract lower and upper bytes from the 16-bit
values so that the code should work on big endian or little endian
machines. I also removed the 2-byte alignment attribute from the
setupPacket array as it is no longer required.
1. Added: GCC_CR toolchain ID for LPC2368. (targets.py)
2. Modified: Startup codes for GCC_ARM and GCC_CR toolchain.
3. Verified: "ticker" and "basic" test program work well, so far.
(Fixed typo.)
1. Added: GCC_CR toolchain ID for LPC2368. (targets.py)
2. Modified: Startup codes for GCC_ARM and GCC_CR toolchain.
3. Verified: "ticker" and "basic" test program works well, so far.
I verified that the hang issue I was seeing when building and running
the mbed official networking tests with GCC_ARM was related to this
issue reported on the mbed forums:
http://mbed.org/forum/mbed/topic/3803/?page=1#comment-18934
If you are using the 4.7 2013q1 update of GCC_ARM or newer then it
will have a _sbrk() implementation which checks the new top of heap
pointer against the current thread SP, stack pointer.
See this GCC_ARM related thread for more information:
https://answers.launchpad.net/gcc-arm-embedded/+question/218972
When using RTX RTOS threads, the thread's stack pointer can easily
point to an address which is below the current top of heap so this
check will incorrectly fail the allocation.
I have added a _sbrk() implementation to the mbed SDK which checks the
heap pointer against the MSP instead of the current thread SP. I have
only enabled this for TOOLCHAIN_GCC_ARM as this is the only GCC based
toolchain that I am sure requires this.
For tests such as TCPEchoServer
(http://mbed.org/users/emilmont/notebook/networking-libraries-benchmark/)
this change showed a 28% improvement (14Mbps to 18Mbps) when the echo
test was modified to instead use 1K data buffers.
I targetted these two functions based on manual profiling samples which
showed that a great deal of time was being spent in these two functions
when the network stack was being slammed with UDP packets.
A new hooks mechanism (hooks.py) allows various targets to customize
part(s) of the build process. This was implemented to allow generation of
custom binary images for the EA LPC4088 target, but it should be generic
enough to allow other such customizations in the future. For now, only the
'binary' step is hooked in toolchains/arm.py.
I changed the following initialization from:
uint8_t cmd[6] = {0x12, (lun << 5) | evpd, page_code, 0, 36, 0};
to:
uint8_t cmd[6] = {0x12, uint8_t((lun << 5) | evpd), page_code, 0, 36, 0};
This makes it clear to the compiler that we are Ok with the 32-bit
integer result from the shift and logical OR operation (after integral
promotions) being truncated down to a 8-bit unsigned value. This is
safe as long as lun only has a value of 7 or lower.
USBHostHub.cpp made a few calls to the USBHALHost::deviceDisconnected()
virtual method passing in NULL for the addr parameter which is actually
declared as uint32_t and not a pointer type. Switching these calls
to pass in a 0 for this parameter silences GCC warnings about
incompatible types.
I removed the initialization of some variables which were never used to
silence GCC warnings.
One of them had me removing the following line from
USBHostMouse::rxHandler():
int len = int_in->getLengthTransferred();
The variable len is never used again in this method and it doesn't
appear that the int_in->getLengthTransferred() call has any side
effects which would require this extraneous call to remain in the code.
There are a few warnings thrown by GCC for this source file that I have
attempted to correct. I would definitely appreciate feedback from
others on these changes:
* USBHost::usb_process() would attempt to write past the end of the
deviceInited[] array in the following code snippet:
if (i == MAX_DEVICE_CONNECTED) {
USB_ERR("Too many device connected!!\r\n");
deviceInited[i] = false;
The i variable is guaranteed to index 1 item past then end of this
array since it only contains MAX_DEVICE_CONNECTED elements. I have
removed the line which sets deviceInited[i] to false. Two questions
result though:
1) What was the intent of this line of code and is it Ok that I
just removed it or should it be replaced with something else?
2) I see no where else that elements in the deviceInited array are
set to false except when all are set to false in the usbThread()
method. Should there be code in DEVICE_DISCONNECTED_EVENT to
do this as well?
* USBHost::transferCompleted(volatile uint32_t addr) was comparing addr
to NULL, which is of pointer type. GCC issues a warning on this
since the types are different (void* being compared to uint32_t). I
switched it to just compare with 0 instead.
* There is a switch statement in USBHost::unqueueEndpoint() which
is conditional on a ENDPOINT_TYPE enumeration but doesn't contain
cases for all values in the enumeration. I just added a default
case to simply break on other values to silence this GCC warning and
allow the code to continue working as it did before. Is it Ok that
this particular piece of code only handles these two particular
cases?
* USBHost::fillControlBuf() was generating a warning about possible
alignment issues when accessing the setupPacket byte array as 16-bit
half words instead. I changed the casting to silence the warnings
and modified the declaration of the setupPacket field to make sure
that it is at least 2-byte aligned for these 16-bit accesses.
The two following memcpy() calls in USBEndpoint::init() are passing in
a NULL pointer as the copy source location.
memcpy(td_list_[0], 0, sizeof(HCTD));
memcpy(td_list_[1], 0, sizeof(HCTD));
I suspect that these were meant to be memset() calls instead so I
switched them. In one of the places that I found where this method is
called, USBHost::newEndpoint(), its passes in an array of HCTD objects
which have already been cleared with similar memset() calls. I am
therefore pretty certain that these were meant to be memset() calls but
if all callers already guarantee that they are zeroed out then maybe
the memset()s can be removed from USBEndpoint::init() anyway.
I updated a few things in the USBHost source code to get it to compile
with GCC:
* In USBHALHost.cpp, the usb_buf global variable was defined with two
different aligmnent specifiers:
static volatile __align(256) uint8_t usb_buf[TOTAL_SIZE] __attribute((section("AHBSRAM1"),aligned)); //256 bytes aligned!
The first one was not accepted by GCC. I removed the duplicate
alignment specifier and updated the one in the existing __attribute
property to force the desired 256 byte alignment.
* Removed the explicit use of the __packed property from structures
and instead used the PACKED macro defined in the mbed SDK's
toolchain.h header file. This macro does the right thing for the
various compilers supported by the mbed team.
* Updated USB_* message macros in dbg.h to place spaces around
references to the 'x' macro parameter. Without this, C++ 11
compilation thought the x character was a string literal operator but
it wasn't one it recognized. Adding the spaces makes it easier to
see the use of the parameter, fixes this compile time error, and
doesn't add any extra space to the final output string since the
compiler only concatenates the contents within the double quotes of
the strings.
By the way, I build with the gnu++11 standard since I have had requests
for its support from gcc4mbed. Some of the items fixed here might not
be errors when using older standards with GCC.
I built and tested a USBHostMSD sample with these updates using both
GCC and the online compiler. I will test more of the host interfaces
before issuing a pull request containing this commit.
I now use a signal to communicate when a packet has been received by
the ethernet hardware and should be processed by the packet_rx thread.
Previously the change to make the lwIP stack thread safe introduced
enough delay in packet_rx that the semaphore count could lag behind
the processed packets and overflow its maximum token count. Now the
ISR uses the signal to indicate that >= 1 packet has been received
since the last time packet_rx() was awaken.
Previously the ethernet driver used generic sys_arch* APIs exposed from
lwIP to manipulate the semaphores. I now call CMSIS RTOS APIs
directly when using the signals. I think this is acceptable since that
same driver source file already contains similar os* calls that talk
directly to the RTOS.
This reverts commit acb35785c9.
It turns out that this commit actually causes problems if an ethernet
interrupt is dropped because a higher privilege task is running, such
as LocalFileSystem accesses. If this happens, the semaphore count isn't
incremented enough times and the packet_rx() thread will fall behind and
end up running as though it had only one ethernet receive buffer. This
causes even more lost packets.
I plan to fix this by switching the semaphore to be a signal so that
the syncronization object is more boolean. It simply indicates if an
interrupt has arrived since the last time packet_rx() was awaken to
process inbound packets.
I recently pulled a NXP crash fix for their ethernet driver which will
requeue a pbuf to the ethernet driver rather than sending it to the
lwip stack if it can't allocate a new pbuf to keep the ethernet
hardware primed with available packet buffers. While recently
reviewing this code I noticed that the full size of the pbuf wasn't
used on this re-queueing operation but the size of the last received
packet. I now reset the pbuf size back to its originally allocated
size before doing this requeue operation.
Previously the packet_rx() function would wait on the RxSem and when
signalled it would process all available inbound packets. This used to
cause no problem but once the thread synchronization was turned
on via SYS_LIGHTWEIGHT_PROT, the semaphore actually started to overflow
its maximum token count of 65535. This caused the mbed_die() flashing
LEDs of death. The old code was really breaking the producer/consumer
pattern that I typically see with a semaphore since the consumer was
written to consume more than 1 produced object per semaphore wait.
Before the thread synchronization was enabled, the packet_rx() thread
could use a single time slice to process all of these packets and then
loop back around a few more times to decrement the semaphore count
while skipping the packet processing since it had all been done.
Now the packet processing code would cause the thread to give up its
time slice as it hit newly enabled critical sections. In the end it
was possible for the code to leak 2 semaphore signals for every 1 by
which the thread was awaken. After about 10 seconds of load, this
would cause a leak of 65535 signals.
NOTE: Two potential issues with this change:
1) The LPC_EMAC->RxConsumeIndex != LPC_EMAC->RxProduceIndex check was
removed from packet_rx(). I believe that this is Ok since the same
condition is later checked in lpc_low_level_input() anyway so it
won't now try to process more packets than what exist.
2) What if ENET_IRQHandler(void) ends up not signalling the RxSem for
every packet received? When would that happen? I could see it
happening if the ethernet hardware would try to pend more than 1
interrupt when the priority was too elevated to process the
pending requests. Putting the consumer loop back in packet_rx()
and using a Signal instead of a Semaphore might be a better
solution?
This option actually enables the use of the lwip_sys_mutex for
protecting concurrent access to such important lwIP resources as:
select_cb_list (this is the one which orig flagged problem)
sockets array
mem stats (if enabled)
heap (if LWIP_ALLOW_MEM_FREE_FROM_OTHER_CONTEXT was non-zero)
memp pool allocs/frees
netif->loop_last pbuf linked list
pbuf reference counts
...
I first noticed this issue when I hit a crash while slamming the net
stack with a large number of TCP packets (I was actually sending 1k
data buffers from the TCPEchoServer mbed sample.) It crashed in the
last line of this code snippet from event_callback:
for (scb = select_cb_list; scb != NULL; scb = scb->next) {
if (scb->sem_signalled == 0) {
It was crashing because scb had an invalid address so it generated a
bus fault. I figured that memory was either corrupted or there was
some kind of concurrency issue. In trying to determine which, I wanted
to walk through the select_cb_list linked list and see where it was
corrupted:
(gdb) p scb
$1 = (struct lwip_select_cb *) 0x85100080
(gdb) p select_cb_list
$2 = (struct lwip_select_cb *) 0x0
That was interesting, the head of the linked list was now NULL but it
must have had a non-NULL value when this loop started running or we
would have never gotten to the point where we hit this crash.
This was starting to look like a concurrency issue since the linked
list was modified out from underneath this thread. Looking through the
source code for this function, I saw use of macros like
SYS_ARCH_PROTECT and SYS_ARCH_UNPROTECT which looked like they should
be providing the thead synchronization. I disassembled the
event_callback() function in the debugger and saw no signs of the
usage of synchronizition APIs that I expected. A search
through the code for the definition of these SYS_ARCH_UN/PROTECT
macros led me to discovering that they were actually ignored unless an
implementation defined them itself (the mbed version doesn't do so) or
the SYS_LIGHTWEIGHT_PROT macro is set to non-zero (the mbed version
didn't do this either). Flipping the SYS_LIGHTWEIGHT_PROT macro on in
lwipopts.h fixed the crash I kept hitting, increased the size of the
code a bit, and unfortunately slows things down a bit since it now
actually serializes access to these data structures by making calls
to the RTOS sync APIs.
After making my previous commit to completely disable LWIP_ASSERT
macro invocations, I ended up with a warning in pbuf.c where an
err variable was set but only checked for success in an assert. I
added a "(void)err;" reference to silence this warning.
I was doing some debugging that had me looking at the disassembly of
lpc_rx_queue() from within the debugger. I was looking for the call to
pbuf_alloc() that we see in the following code snippet:
p = pbuf_alloc(PBUF_RAW, (u16_t) EMAC_ETH_MAX_FLEN, PBUF_RAM);
if (p == NULL) {
LWIP_DEBUGF(UDP_LPC_EMAC | LWIP_DBG_TRACE,
("lpc_rx_queue: could not allocate RX pbuf (free desc=%d)\n",
lpc_enetif->rx_free_descs));
return queued;
}
/* pbufs allocated from the RAM pool should be non-chained. */
LWIP_ASSERT("lpc_rx_queue: pbuf is not contiguous (chained)",
pbuf_clen(p) <= 1);
When I was looking through the disassembly for this code I noticed a
call to pbuf_clen() in the actual machine code.
=> 0x0000bab0 <+24>: bl 0x44c0 <pbuf_clen>
0x0000bab4 <+28>: ldr r3, [r4, #112] ; 0x70
0x0000bab6 <+30>: ldrh.w r12, [r5, #10]
0x0000baba <+34>: add.w r2, r3, #9
0x0000babe <+38>: add.w r0, r12, #4294967295 ; 0xffffffff
The only call to pbuf_clean made from this function is made from
within the LWIP_ASSERT. When I looked more closely at how this macro
was defined, I saw that the mbed version of the stack had disabled the
LWIP_PLATFORM_ASSERT macro when LWIP_DEBUG was false which means that
no action will be taken if the assert is false but it still allows the
LWIP_ASSERT macro to potentially evaluate the assert expression.
Defining the LWIP_NOASSERT macro will fully disable the LWIP_ASSERT
macro.
I saw one of my TCP/IP samples shrink about 0.5K when I made this
change.
The following line in USBHAL_KL25Z.cpp would generate a warning in GCC
because of a potential operator precendence issue:
return((USB0->FRMNUML | (USB0->FRMNUMH << 8) & 0x07FF));
This would have been interpreted as:
return((USB0->FRMNUML | ((USB0->FRMNUMH << 8) & 0x07FF)));
since & has higher precedence than |
I switched it to be:
return((USB0->FRMNUML | (USB0->FRMNUMH << 8)) & 0x07FF);
Since it makes more sense to & with 0x7FF after having merged the lower
and upper bytes together rather than just the upper byte. It should
have resulted in the same value either way.
The SDFileSystem class contained a few routines which compared a signed
integer loop index to an unsigned integer length/size. I switched the
loop index to be uint32_t as well.
The code in netif_set_ipaddr would read the memory pointed to by its
ipaddr parameter, even if it was NULL on this line:
if ((ip_addr_cmp(ipaddr, &(netif->ip_addr))) == 0) {
On the Cortex-M3, it is typically OK to read from address 0 so this
code will actually compare the reset stack pointer value to the
current value in netif->ip_addr.
Later in the code, this same pointer will be used for a second read:
ip_addr_set(&(netif->ip_addr), ipaddr);
The ip_addr_set call will first check to see if the ipaddr is NULL and
if so, treats it like IP_ADDR_ANY (4 bytes of 0).
/** Safely copy one IP address to another (src may be NULL) */
#define ip_addr_set(dest, src) ((dest)->addr = \
((src) == NULL ? 0 : \
(src)->addr))
The issue here is that when GCC optimizes this code, it assumes that
the first dereference of ipaddr would have thrown an invalid memory
access exception and execution would never make it to this second
dereference. Therefore it optimizes out the NULL check in ip_addr_set.
The -fno-delete-null-pointer-checks will disable this optimization and
is a good thing to use with GCC in general on Cortex-M parts. I will
let the mbed guys make that change to their build system.
I have however corrected the code so that the intent of how to handle a
NULL ipaddr is more obvious and gets rid of the potential NULL
dereference.
By the way, this bug caused connect() to fail in obtaining an
address from DHCP. If I recall correctly from when I first debugged
this issue (late last year), I actually saw the initial value of the
stack pointer being used in the DHCP request as an IP address which
caused it to be rejected.
Peter's and my changes to LPC1768.ld ended up adding the same AHBSRAM0
and AHBSRAM1 section clauses to the script twice. I removed one copy.
I also pulled Peter's define of the ETHMEM_SECTION macro up into the
previous nested #if so that the preprocessor wouldn't spit out a
redefined macro warning.
I verified that building the code clean before and after these changes
still results in the same .bin file but now without warnings and/or
duplicate code.
I started out looking at some UDP receive code that was only able to
handle 3 inbound 550 byte datagrams out of 16 when sent in quick
succession. I stepped through the ethernet driver code and it
seemed to work as expected but it just couldn't queue up more than
3 PBUFs for each burst. It was almost like it was being starved of
CPU cycles. Based on that observation, I looked up the thread
priorities for the receive ethernet thread and found the following
close to the top of the lpc17_emac.c source file:
#define RX_PRIORITY (osPriorityNormal)
This got me to thinking, what is the priority of the tcp thead? It
turns out that it gets its priority from the following line in
lwipopts.h:
#define TCPIP_THREAD_PRIO 1
Interesting! What priority is 1? It turns out that it corresponds
to osPriorityAboveNormal. This means that while the tcp thread is
handling one packet that has been posted to its mailbox from the
ethernet receive thread, the receive thread is starved from processing
any more inbound ethernet packets.
What happens if we set TCP_IP_THREAD_PRIO to osPriorityNormal? Crash!
The ethernet driver ends up crashing in lpc_low_level_input() when
it tries to set p->len on a NULL p pointer. The p pointer ended up
being NULL because an earlier call to pbuf_alloc() in lpc_rx_queue()
failed its allocation (I will have more to say about this failed
allocation later since that is caused by yet another bug). I pulled a
fix from http://lpcware.com/content/bugtrackerissue/lpc17xx-mac-bugs to
remedy this issue. When the pbuf allocation fails, it discards the
inbound packet in the pbuf and just puts it back into the rx queue.
This means we never end up with a NULL pointer in that queue to
dereference and crash on.
With that bug fixed, the application would just appear to hang after
receiving and processing a few datagrams. I could place breakpoints in
the packet_rx() thread function and found that it was being signalled
by the ethernet ISR but it was always failing to allocate new PBUFs,
which is what led to our previous crash. This means that the new
crash prevention code was just discarding every packet that arrived.
Why are these allocations failing? In my opinion, this was the most
interesting bug to track down. Is there a memory leak somewhere in
the code which maybe only triggers in low memory situations? I
figured the easiest way to determine that would be to learn a bit
about the format of the lwIP heap from which the PBUF was failing to
be allocated. I started by just stepping into the failing lwIP memory
allocator, mem_malloc(). The loop which search the free list starts
with this code:
for (ptr = (mem_size_t)((u8_t *)lfree - ram);
This loop didn't even go through one iteration and when I looked at the
initial ptr value it contained a really large value. It turns out that
lfree was actually lower than ram. At this point I figured that lfree
had probably been corrupted during a free operation after one of the
heap allocations had been underflowed/overflowed to cause the metadata
for an allocation to be corrupted. As I started thinking about how to
track that kind of bug down, I noticed that the ram variable might be
too large (0x20080a68). I restarted the debugger and looked at the
initial value. It was at a nice even address (0x2007c000) and
certainly nothing like what I saw when the allocations were failing.
This global variable shouldn't change at all during the execution of
the program. I placed a memory access watchpoint on this ram variable
and it fired very quickly inside of the rt_mbx_send() function. The
ram variable was being changed by this line in rt_mbx_send():
p_MCB->msg[p_MCB->first] = p_msg;
What the what? Why does writing to the mailbox queue overwrite the
ram global variable? Let's start by looking at the data structure used
in the lwIP port to target RTX (defined in sys_arch.h):
// === MAIL BOX ===
typedef struct {
osMessageQId id;
osMessageQDef_t def;
uint32_t queue[MB_SIZE];
} sys_mbox_t;
Compare that to the utility macro that RTX defines to help setup one of
these mailboxes with queue:
#define osMessageQDef(name, queue_sz, type) \
uint32_t os_messageQ_q_##name[4+(queue_sz)]; \
osMessageQDef_t os_messageQ_def_##name = \
{ (queue_sz), (os_messageQ_q_##name) }
Note the 4+(queue_sz) used in the definition of the message queue
array. What a hack! The RTX OS requires an extra 16 bytes to contain
its OS_MCB header and this is how it adds it in. Obviously the
sys_mbox_t structure used in the lwIP OS targetting code doesn't have
this. Without it, the RTX mailbox routines end up scribbling on
memory following the structure in memory. Adding 4 in that structure
fixes the memory allocation failure that I was seeing and now the network
stack can handle between 7 and 10 datagrams within a burst.
The phy_speed_100mbs, phy_full_duplex, and phy_link_active fields of
PHY_STATUS_TYPE are 1 bit wide but lpc_phy_init() attempted to
initialize them to a value of 2. I switched the initializations to
be 0 instead and it still generated the same .bin image.
The first was a potential out of range index read in dhcp_handle_ack().
The (n < DNS_MAX_SERVERS) check should occur first. There is also a
documented lwIP bug for this issue here:
http://savannah.nongnu.org/bugs/?36170
In dhcp_bind() there is no need to perform the NULL check in
ip_addr_isany() for &gw_addr. Just check (gw_addr.addr == IPADDR_ANY)
instead.
I refactored the chaddr[] copy in dhcp_create_msg() to first copy all
of the valid bytes in hwaddr and then pad the rest of the bytes with 0.
Before it used to check on every destination byte if it should copy or
pad. GCC originally complained about an index out of range read from
the hwaddr[] array even though it was protected by a conditional
operator. The refactor makes the intent a bit clearer and saves the
extra comparison per loop iteration. It also stops GCC from
complaining :)
GCC will issue a warning when the ip_addr_isany() macro is used on
a pointer which can never be NULL since the macros NULL check will
always be false:
#define ip_addr_isany(addr1) ((addr1) == NULL || \
(addr1)->addr == IPADDR_ANY)
In these cases, it is probably clearer to just perform the
x.addr == IPADDR_ANY check inline.
The dn variable in lpc_low_level_output() was originally defined as a
u32_t but it is later compared to the s32_t return value from
lpc_tx_ready(). Since it is intialized to pbuf_clean() which returns
a u8_t, a s32_t type can safely hold the initial value and remains
consistent with the signed lpc_tx_ready() comparison.
I also modifed writtenLen in TCPSocketConnection::send_all() and
readLen in TCPSocketConnection::recieve_all() to be of type int instead
of size_t. This is more consistent with their usage within these
methods (they accumulate int ret values and are compared to the int
length value) and their use as a signed integer return values.
The original script assigned memory ranges to USB_RAM and ETH_RAM but
it never placed any section data in those regions. I added clauses
towards the bottom of the script to place data that the programmer
has marked for the AHBSRAM0 and AHBSRAM1 sections into these regions
of RAM. Previously the data destined for these sections was being
placed in the lower 32K RAM bank and overflowing it at link time.
I also added a few Image$$ linker symbols to mimic those used by the
online compiler. I have had samples in the past which took advantage
of these to display static memory statistics for each SRAM region.
I also changed LENGTH=0x7F38 to LENGTH=(32K - 0xC8) to make it more
consistent with the sizing of the other regions in this script which
use human readable K sizing information. The 0xC8 subtraction reflects
the starting offset of 0xC8 for this region.
i2c_frequency() compares a uint32_t ref variable to the int hz
function parameter passed in by the caller. I forced this to be an
uint32_t comparison.
i2c_slave_write() declared i and count variables to be of type uint32_t
but used them as int type throughout the code (in comparisons and
returns) so I switched them to be of signed int type.
spi_frequency() contains a change similar to that made in
i2c_frequency().
This commit targets the KL25Z code, whereas previous ones targetted
similar issues in the LPC1768 and LPC11U24 mbed HAL code.
These changes were made to silence GCC warnings and fix potential bugs
where they would never be equal when the enumeration wasn't a 32-bit
type.
For example, pinmap.c used to contain this code:
if (pin == (uint32_t)NC) return;
I switched it to:
if (pin == (PinName)NC) return;
I wonder why this casting to uint32_t was done in the first place?
Maybe another supported compiler requires it?
This commit targets the LPC11U24 code, whereas a previous one
targetted similar issues in the LPC1768 mbed HAL code.
These changes were made to silence GCC warnings and fix potential bugs
where they would never be equal when the enumeration wasn't a 32-bit
type.
For example, pinmap.c used to contain this code:
if (pin == (uint32_t)NC) return;
I switched it to:
if (pin == (PinName)NC) return;
I wonder why this casting to uint32_t was done in the first place?
Maybe another supported compiler requires it?
The original code was:
if(LPC_CAN1->IER | LPC_CAN2->IER != 0) {
This would actually be interpreted as:
if(LPC_CAN1->IER | (LPC_CAN2->IER != 0)) {
I simplified it to:
if(LPC_CAN1->IER | LPC_CAN2->IER) {
With the comparison removed, the GCC warning no longer fires since the
user's intent is no longer unclear. However, the end result should be
the same.
These were done to silence GCC warnings and fix potential bugs where
they would never be equal when the enumeration wasn't a 32-bit type.
For example, common/pinmap_common.c used to contain this code:
if (pin == (uint32_t)NC)
I switched it to:
if (pin == (PinName)NC)
I wonder why this casting to uint32_t was done in the first place?
Maybe another supported compiler requires it?
- rename stdio.cpp to retarget.cpp, since it doesn't contain only stdio-related
code
- move __cxa_pure_virtual from exit.c to retarget.cpp where it belongs.
This commit fixes a number of issues with invalid/wrong pin assignments for the
KL25Z mbed target:
- analog: PTC1 doesn't seem to be exported at all, replaced with PTC2.
- digitalin/out, interruptin, portin/out: these tests used PTA1 and PTA2, which
are the console UART pins for the KL25Z mbed. The tests now use PTA4<->PTC5 and
PTA5<->PTC6.
Don't set a clock higher than the one requested for the I2C and SPI interfaces,
even if this setting is the closest to the requested one. This was causing
some issues with the EEPROM test and possibly other issues with the SPI SD test.
- send {{message}} instead of {message}. This is neede to overcome a serial
interface issue on the Freedom board.
- don't blink the passed/failed LEDs if they are not actually connected.
Changed pwm test code for LPC1114 using P1_1 (CT32B1_MAT0) and P1_2
(CT32B1_MAT1).
[PWM test (MBED_5) result]
Initialize PWM on pin 13 with duty cycle: 0.75
Initialize PWM on pin 14 with duty cycle: 0.50
{success}
{end}
In between the last two commits, I added debugging lines using serial to
try and locate the line of code that was causing the issues with GPIO.
However, didn't get anywhere with this because the gpio_write function
is defined in a header file, rather than in an implementation file, so
the printf function can't go there. As a result, it's just refactoring.
Now the I2C send/receive functions in the mbed HAL return the number of
bytes actually transferred or an error code (a negative value). The
public API remains unchanged.
bcostm
Sissors
0xc0170
Michael Moon
tkuyucu
Joe Turner
Bogdan Marinescu
Emilio Monti
mazgch
hototogi
Adam Green
Joris Aerts
Bogdan Marinescu
Bogdan Marinescu
Yihui Xiong
Andreas Rebert
pbrier
Michael Ammann
Bogdan Marinescu
Michael Conners
Toyomasa Watarai
dinau
ytsuboi
unknown
Toyomasa Watarai
Matthew Else
jesusalvarez
samux
unknown