Improve heap_useNewlib_ST.c documentation

[ApiumJacob]

The following two symbols are defined at the top of heap_useNewlib_ST.c that seem like they need some configuration (will not compile without doing so) but its not clear how to do this. Below is my best guess from spending a few minutes in the file.

#define STM_VERSION // Replace sane LD symbols with STM CubeMX's poor standard exported LD symbols

Seems like this should be on if using .LD file generated by running code generation in the .ioc file.

#define ISR_STACK_LENGTH_BYTES (configISR_STACK_SIZE_WORDS*4) // bytes to reserve for ISR (MSP) stack

ISR_STACK_LENGTH_BYTES relies on configISR_STACK_SIZE_WORDS being defined and it is not by default. It seems pretty self explanatory what configISR_STACK_SIZE_WORDS is for (allocate some heap for ISR's) but it not clear where this should be set. I'm not sure so I'm defining it above STM_VERISION. I'm also just using a SWAG at the number and assuming that the only malloc going on in an IRS is in the USB code (as described in Dave's article on the topic).

I was not successful at getting this code to replace my heap_4.c file.

Jacob

[leech001]

Add this code to heap_useNewlib_ST.c

#define configISR_STACK_SIZE_WORDS (0x100)

or use my fork lib https://github.com/leech001/STM32-FreeRTOS-float

[ApiumJacob]

(so I'm documenting this here for my future self and for anyone else having problems with the STM32, FreeRTOS and USB CDC ACM)

@leech001 Thanks! Yes I had added 0x500 for this parameter because I wasn't sure how much memory is being malloced in the ISR for the USB code. It seems to work at either 0x100 or 0x500.

This open question for me was how big to make this value. This lead me down the following path:

Because I am using the ST USB stack and as Dave points out this stack is calling malloc in an ISR. This caused an assert to be fired in __malloc_lock in Dave's code which is wrapped in a #if defined(MALLOCS_INSIDE_ISRs) #else. The assert produced this call stack in the debugger:

__malloc_lock() at heap_useNewlib_ST.c:218 0x8026a7a
_malloc_r() at 0x80659da
USBD_CDC_Init() at usbd_cdc.c:509 0x801d94c
USBD_SetClassConfig() at usbd_core.c:231 0x801df56
USBD_SetConfig() at usbd_ctlreq.c:584 0x801eb10
USBD_StdDevReq() at usbd_ctlreq.c:136 0x801e474
USBD_LL_SetupStage() at usbd_core.c:272 0x801dfe4
HAL_PCD_SetupStageCallback() at usbd_conf.c:137 0x803b5c8
PCD_EP_OutSetupPacket_int() at stm32f4xx_hal_pcd.c:2,196 0x8016c5e
HAL_PCD_IRQHandler() at stm32f4xx_hal_pcd.c:1,083 0x8015d62
OTG_FS_IRQHandler() at stm32f4xx_it.c:321 0x8012652
() at 0xfffffffd
prvPortStartFirstTask() at port.c:267 0x8026608
xPortStartScheduler() at port.c:379 0x8026716

So now when add this define at the top of the file to keep the assert from firing:

#define MALLOCS_INSIDE_ISRs

I also did some research to try to figure out how much memory is actually needed by the USB function and it seems as if the only USB function calling malloc is USBD_CDC_Init() and this is only about 24 bytes for the USBD_CDC_HandleTypeDef struct. So is did some experimenting to see how small I could get the ISR stack before things broke. I did this by halving the value until it broke. 0x40 was the smallest stack that worked and 0x20 broke. I didn't test anything in between. I decided to leave this at 0x100 for the time being. Here is my rational:

1. I seems to work and we have sufficient memory to leave it at this number.

2. The call stack depth was 10 function (in this instance) and when walk down the stack I can see about 40 bytes worth of auto variables being created so total memory needed (down this path) is:

   (return address * 10) + 40 bytes worth of auto variable + 24 bytes for malloc ~= 104 bytes.

3. The amount if stack allocated is 4x configISR_STACK_SIZE_WORDS chosen so this give a bit of safety margin.

I'm going to have to agree with Dave, if there is only 24 bytes that are allocated and you need allocate and deallocate in an ISR why not just make this a global? This saves a bunch of time, and doesn't really affect the scheme of thing, if you need USB then you are going to need those 24 bytes sometime, and why not when the program starts?

Jacob

P.S. I'm not sure if this code has solved our problem yet, as Dave points out in his article, because we now do not trust malloc we are developing a suite of unit functions to exercise memory management. But I suspect our hat is off to Dave for saving us maybe months of heartache.

[leech001]

@ApiumJacob Try to generate your project from scratch in the new STM32CubeMX. The latest version seems to have solved the problem with FreeRTOS and no crutches are needed.

[ApiumJacob]

@leech001 I am using the latest version of STM32CubeMX and it broke other things that used to work! Thank goodness for git or it would have never found the issue. It was still hard to find due to ST changing white space in about a gazillion places.

I have no idea if this will fix our problem but is smells like a possible fix. Basically our project broke after upgrading, but we were making a lot of changes at the the same time because of know reentrancy issues with malloc.

[DRNadler]

@leech001 Last I looked ST FreeRTOS/newlib free storage was still very buggy - beware. @ApiumJacob Put the define in FreeRTOSconfig.h. Rather than guessing/trial and error, use my port.c which tells you required ISR stack depth used. In any case neither approach will catch all possible nested ISR stack depth so calculate carefully.

[ApiumJacob]

@DRNadler thanks for the heads up... Do you know if the USB drops incoming (rx) our outgoing (tx) packets? I've stress tested outgoing and haven't seen any issues, but not so much receiving.

Edit...

I digress. About as soon as I wrote this I started seeing dropped packets while transmitting on USB. It is happening at the start right after enumeration. The PC is also doing things like changing port baud rate at this time too. As soon as this is all settled out USB tx seems to be working, but alas this is ugly.

[DRNadler]

@ApiumJacob - In my stress testing USB CDC occasionally dropped packets from ST device to host (after I modified the code to send/receive using FreeRTOS queues). ST USB stack is horrendous.

[ApiumJacob]

@DRNadler Do you think the hardware is sound? I have written USB stack from the ground up before and I can fix software but if there is a hardware issue I'm sol. I would hate to find out there is a hardware issue after sinking a lot of time into fixing software.

I appreciate your opinion having walked this path before me.